Perf config #832

PraveenPenguin · 2024-04-27T07:50:20Z

1-Added a library function which perf collection attribute set in HMC
2- Added Library which help to set value of perf collection via HMC
3-Added support in Machine config as perf collection data can be set via dynamic profile
4-Remove non-relevant function docstring

common/OpTestHMC.py

PraveenPenguin · 2024-04-29T12:00:35Z

@abdhaleegit addressed review comment, please have a review

abdhaleegit

few more comments, rest all looks good

abdhaleegit · 2024-05-03T05:28:52Z

common/OpTestHMC.py

+        if  perf_collection:
+            return True
+        else:
+            return False


no need of else here and also short name for variable perf_collection like rc

if rc: return True return False

Looks good will adopt

abdhaleegit · 2024-05-03T05:30:41Z

testcases/MachineConfig.py

+                if self.cv_HMC.is_perfcollection_enabled:
+                    log.info("System is already booted with perf collection profile enabled")
+                else:
+                    return "Failed to boot perf collection profile enabled"


return a string ? or this should be log.info ? or raise error

+1. Also the message string should be
"Failed to enable Performance Information collection"

Thanks Sachin, I will reword , @abdhaleegit @sacsant as we need to return statement with string as in caller we are making test fails with return string

sacsant · 2024-05-03T10:08:54Z

testcases/MachineConfig.py

+                log.info("System is already booted with perf collection profile enabled")
+            else:
+                self.cv_HMC.hmc_perfcollect_configure()
+                if self.cv_HMC.is_perfcollection_enabled:


since hmc_perfcollect_configure() returns a value, why not add a check for the same and avoid calling is_percollection_enabled() twice?

Thanks sachin , earlier logic was not up to mark in OptestHMC.py to return so removed return statement from hmc_perfcollect_configure() , now we are good that calling is_percollection_enabled() return exact check

sacsant · 2024-05-03T10:11:37Z

testcases/MachineConfig.py

+        if "perf=1" in self.machine_config:
+            conf = OpTestConfiguration.conf
+            if self.cv_HMC.is_perfcollection_enabled():
+                log.info("System is already booted with perf collection profile enabled")


Better message will be
"Performance Information collection is already enabled"
or
"Performance Information Collection is active"

sacsant · 2024-05-03T10:13:15Z

testcases/MachineConfig.py

+                if self.cv_HMC.is_perfcollection_enabled:
+                    log.info("System is already booted with perf collection profile enabled")
+                else:
+                    return "Failed to boot perf collection profile enabled"


+1. Also the message string should be
"Failed to enable Performance Information collection"

PraveenPenguin · 2024-05-03T13:07:58Z

@abdhaleegit @sacsant addressed review comment please have look

abdhaleegit

last comments

abdhaleegit · 2024-05-06T05:43:06Z

common/OpTestHMC.py

+
+    def is_perfcollection_enabled(self):
+        '''
+        Get pef collection allowed in hmc profile


abdhaleegit · 2024-05-06T05:44:06Z

common/OpTestHMC.py

+        Enable/Disable perfcollection from HMC
+        '''
+        # Set perf collection profile  value using HMC command
+        cmd = ('chsyscfg -r lpar -m %s -i "name=%s, allow_perf_collection=' %


there is only one double qoute .. is this a typo ?

yes that is correct

2024-05-08 14:52:29,484:op-test.common.OpTestHMC:wait_lpar_state:INFO:Current state: Shutting Down [console-expect]#lssyscfg -m system -r lpar --filter lpar_names=system-lp13-CR -F state lssyscfg -m system -r lpar --filter lpar_names=system-lp13-CR -F state Not Activated [console-expect]#echo $? echo $? 0 [console-expect]#lsrefcode -m system -r lpar --filter lpar_names=system-lp13-CR -F refcode lsrefcode -m system -r lpar --filter lpar_names=system-lp13-CR -F refcode 00000000 [console-expect]#echo $? echo $? 0 2024-05-08 14:52:44,765:op-test.common.OpTestHMC:wait_lpar_state:INFO:Current state: Not Activated [console-expect]#lssyscfg -m system -r lpar --filter lpar_names=system-lp13-CR -F allow_perf_collection lssyscfg -m system -r lpar --filter lpar_names=system-lp13-CR -F allow_perf_collection 1 [console-expect]#echo $? echo $? 0 2024-05-08 14:52:59,910:op-test.testcases.MachineConfig:LparSetup:INFO:System is already booted with perf collection profile enabled [console-expect]#lssyscfg -m system -r lpar --filter lpar_names=system-lp13-CR -F state lssyscfg -m system -r lpar --filter lpar_names=system-lp13-CR -F state Not Activated [console-expect]#echo $? echo $? 0 [console-expect]#lsrefcode -m system -r lpar --filter lpar_names=system-lp13-CR -F refcode lsrefcode -m system -r lpar --filter lpar_names=system-lp13-CR -F refcode 00000000 [console-expect]#echo $? echo $? 0 [console-expect]#lssyscfg -m system -r lpar --filter lpar_names=system-lp13-CR -F state lssyscfg -m system -r lpar --filter lpar_names=system-lp13-CR -F state Not Activated [console-expect]#echo $?

abdhaleegit · 2024-05-06T05:46:56Z

common/OpTestHMC.py

+               (self.mg_system, self.lpar_name))
+        if enable: # Value '1' to enable perfcollection
+            cmd = '%s1"' % cmd
+        else: # Value '0' to disable perfcollection


also documentation can be moved to method doc string

…on attribute set to LPAR profile Added library function which retun True/False if performance collection attribute set to LPAR profile Signed-off-by: Praveen K Pandey <[email protected]>

…to LPAR using HMC command Added a library function which help to set perf collection attribute to LPAR using HMC command Signed-off-by: Praveen K Pandey <[email protected]>

…a dynamic profile Added support in Machine config as perf collection data can be set via dynamic profile Signed-off-by: Praveen K Pandey <[email protected]>

Remove non relevant function docstring Signed-off-by: Praveen K Pandey <[email protected]>

PraveenPenguin · 2024-05-08T09:31:19Z

@abdhaleegit thank you for taking time to review code changes , pushed changes , can you please review again

sacsant · 2024-05-08T17:32:44Z

common/OpTestHMC.py

@@ -927,20 +927,20 @@ def is_perfcollection_enabled(self):
        if  rc:
            return True
 	return False
-


Do we need this additional commit? It changes the code(cosmetic changes) added by previous commit? Can we fold this into previous one?

as i was looking to add changes align with logging as changes , let me see if some thing messed up while , let me resend

yeah.. you can just ammend the changes to same previous commit

yes closing this PR as have logic separation of commit as some while addressing review comment bank line goes as diffrent commit

PraveenPenguin · 2024-05-13T02:50:37Z

Closing this PR raised another PR #839

PraveenPenguin requested a review from abdhaleegit April 29, 2024 06:38

abdhaleegit requested changes Apr 29, 2024

View reviewed changes

common/OpTestHMC.py Show resolved Hide resolved

common/OpTestHMC.py Outdated Show resolved Hide resolved

common/OpTestHMC.py Outdated Show resolved Hide resolved

PraveenPenguin force-pushed the perf_config branch from a4e88bd to b455bc3 Compare April 29, 2024 11:59

abdhaleegit self-requested a review May 2, 2024 04:47

abdhaleegit requested changes May 3, 2024

View reviewed changes

sacsant suggested changes May 3, 2024

View reviewed changes

PraveenPenguin force-pushed the perf_config branch 3 times, most recently from b80b3d2 to c3fd861 Compare May 3, 2024 13:05

PraveenPenguin requested review from sacsant and abdhaleegit May 3, 2024 13:08

abdhaleegit self-assigned this May 6, 2024

abdhaleegit reviewed May 6, 2024

View reviewed changes

PraveenPenguin added 4 commits May 8, 2024 14:52

Added library function which retun True/False if performance collecti…

493e28c

…on attribute set to LPAR profile Added library function which retun True/False if performance collection attribute set to LPAR profile Signed-off-by: Praveen K Pandey <[email protected]>

Added a library function which help to set perf collection attribute …

03e25d2

…to LPAR using HMC command Added a library function which help to set perf collection attribute to LPAR using HMC command Signed-off-by: Praveen K Pandey <[email protected]>

Added support in Machine config as perf collection data can be set vi…

b532165

…a dynamic profile Added support in Machine config as perf collection data can be set via dynamic profile Signed-off-by: Praveen K Pandey <[email protected]>

Remove non relevant function docstring

f595b2f

Remove non relevant function docstring Signed-off-by: Praveen K Pandey <[email protected]>

PraveenPenguin force-pushed the perf_config branch from c3fd861 to f595b2f Compare May 8, 2024 09:27

PraveenPenguin requested a review from abdhaleegit May 8, 2024 10:04

sacsant reviewed May 8, 2024

View reviewed changes

PraveenPenguin closed this May 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Perf config #832

Perf config #832

PraveenPenguin commented Apr 27, 2024

PraveenPenguin commented Apr 29, 2024

abdhaleegit left a comment

abdhaleegit May 3, 2024

PraveenPenguin May 3, 2024

abdhaleegit May 3, 2024

sacsant May 3, 2024

PraveenPenguin May 3, 2024 •

edited

Loading

sacsant May 3, 2024

PraveenPenguin May 3, 2024

sacsant May 3, 2024

sacsant May 3, 2024

PraveenPenguin commented May 3, 2024

abdhaleegit left a comment

abdhaleegit May 6, 2024

abdhaleegit May 6, 2024

PraveenPenguin May 8, 2024

abdhaleegit May 6, 2024

PraveenPenguin commented May 8, 2024

sacsant May 8, 2024

PraveenPenguin May 9, 2024

abdhaleegit May 9, 2024

PraveenPenguin May 9, 2024

PraveenPenguin commented May 13, 2024

Perf config #832

Perf config #832

Conversation

PraveenPenguin commented Apr 27, 2024

PraveenPenguin commented Apr 29, 2024

abdhaleegit left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PraveenPenguin May 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PraveenPenguin commented May 3, 2024

abdhaleegit left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PraveenPenguin commented May 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PraveenPenguin commented May 13, 2024

PraveenPenguin May 3, 2024 •

edited

Loading