OpTestInterrupt.py: This patch validates interrupt controller mechanism #580
Conversation
|
Can one of the admins verify this patch? |
shirishaganta
force-pushed
the
interrupt_controller
branch
from
2d61665 to
99a130a
Compare
testcases/OpTestInterrupt.py
Outdated
| # | ||
| # OpenPOWER Automated Test Project | ||
| # | ||
| # Contributors Listed Below - COPYRIGHT 2017 |
| from common.OpTestSSH import ConsoleState as SSHConnectionState | ||
| from common.OpTestConstants import OpTestConstants as BMC_CONST | ||
| from common.Exceptions import KernelOOPS, KernelKdump | ||
|
|
There was a problem hiding this comment.
DO you really need to import all these imports?
testcases/OpTestInterrupt.py
Outdated
| This module contain testcases related to XIVE. | ||
|
|
||
| 1.Enable kernel logging, basically generate traffic | ||
| 2.Enable console traffic by printing the |
There was a problem hiding this comment.
Unnecessary new line. Generally sentence can go up to 80 chars.
Also I see unecessary spaces, trailing spaces in multiple places . Please fix them.
testcases/OpTestInterrupt.py
Outdated
|
|
||
| This module contain testcases related to XIVE. | ||
|
|
||
| 1.Enable kernel logging, basically generate traffic |
There was a problem hiding this comment.
Generate what traffic ?
Also may be its worth to expand description little bit . .so that its easy for review.
| self.util = self.cv_SYSTEM.util | ||
| self.c = self.cv_SYSTEM.console | ||
| self.con = self.cv_SYSTEM.cv_HOST.get_ssh_connection() | ||
|
|
There was a problem hiding this comment.
Looks like some of the variables are just assigned but not used.
| ''' | ||
| This function will validate interrupts handled by CPU's. | ||
| Make sure while running this function xive should be on | ||
| ''' |
There was a problem hiding this comment.
Where are you making sure xive is on?
testcases/OpTestInterrupt.py
Outdated
| ''' | ||
| self.con = self.cv_SYSTEM.cv_HOST.get_ssh_connection() | ||
| res = self.con.run_command("ip route list | grep default | awk '{print $5}'") | ||
| inter = "cat /proc/interrupts | grep %s | head -1" % res[0] |
There was a problem hiding this comment.
replace inter with some name. which reflects usage (may be interrupt)?
testcases/OpTestInterrupt.py
Outdated
| self.interrupt_cpu_check() | ||
|
|
||
|
|
||
| def crash_suite(): |
There was a problem hiding this comment.
Why do you need crash_suite here? Also name doesn't reflect what you are doing here.
IIUC _suite for meant for grouping multiple tests into one test bucket and run.
testcases/OpTestInterrupt.py
Outdated
| smp = "%s | awk '{print $1}'" % inter | ||
| result = self.con.run_command(smp) | ||
| self.con.run_command("cd /proc/irq/%s" % result[0].lstrip().split(':')[0]) | ||
| self.con.run_command("cat smp_affinity_list") |
There was a problem hiding this comment.
Can't you use absolute path and run cat instead of switching to that directory?
| result = self.con.run_command(smp) | ||
| self.con.run_command("cd /proc/irq/%s" % result[0].lstrip().split(':')[0]) | ||
| self.con.run_command("cat smp_affinity_list") | ||
| log.info("setting smp_affinity_list to CPU2") |
There was a problem hiding this comment.
Are you sure it will be always CPU2?
There was a problem hiding this comment.
randomly we selected the cpu number as atleast 4 cpus we will have in smt8
It checks system reboot with xive on and off and it also validates interrupts handled by CPU's Signed-off-by: shirisha Ganta <[email protected]>
|
After addressing the above comments, Changed the testcase according to that |
shirishaganta
force-pushed
the
interrupt_controller
branch
from
99a130a to
8160ec1
Compare
PraveenPenguin
force-pushed
the
master
branch
2 times, most recently
from
4d0cb14 to
b976629
Compare
It checks system reboot with xive on and off and
it also validates interrupts handled by CPU's
Signed-off-by: shirisha Ganta [email protected]