scsi_device: add new case of hostdev device for scsi commands testing #5986
Conversation
meinaLi
force-pushed
the
hostdev_device
branch
from
82ee2b4 to
6dfaa9b
Compare
meinaLi
force-pushed
the
hostdev_device
branch
2 times, most recently
from
ca52f4b to
8a24ae2
Compare
|
|
@chunfuwen @smitterl This case is in scsi device plan. The test scenarios is coming from customer. Could you help review it when you have time? Thanks. |
| script_path = params.get("script_path") | ||
| vmxml = vm_xml.VMXML.new_from_dumpxml(vm_name) | ||
| backup_xml = vmxml.copy() | ||
| get_scsi_cmd = "lsscsi | grep LIO | wc -l" |
There was a problem hiding this comment.
I'm not a fan of using pipes in bash commands because in case the output is not as expected, it's harder to debug.
How about you do the following?
def get_scsi_count(...):
...
status, output = cmd_status_output("lsscsi")
scsi_lio_count = len(re.findall("LIO", output))
...
There was a problem hiding this comment.
Thanks for your suggestion. I moved it to a function now.
| "EOF" | ||
| ).format(script_path, mpath_dev) | ||
| vm_session.cmd(script_in_guest) | ||
| run_script_in_guest = "/bin/bash %s" % script_path |
There was a problem hiding this comment.
If you use "/bin/bash" you do not need the shebang "#!/bin/bash".
In any case, why do you create a bash script and then execute those commands, instead of something like the following?
for cmd in [
"mpathpersist --out ...",
"mpathpersist --out ...",
...
"mpathpersist --out --register ..."
]:
s, o = vm_session.cmd_status_output(cmd)
if s:
test.error(f"Error running '{cmd}' in VM: '{o}'")Again, I think it might be easier later to debug any issues.
There was a problem hiding this comment.
Yes, Using list but not script can be more easy to debug in our test. So I've changed it.
| if vm.is_alive(): | ||
| vm.destroy() | ||
| backup_xml.sync() | ||
| # Delete the first iscsi target |
There was a problem hiding this comment.
IMO, the comments are unnecessary.
| def create_second_target_by_iscsi(emulated_image, second_target): | ||
| """ | ||
| This test needs to have two LUNs with same image but not same target name. | ||
| The current libvirt.setup_or_cleanup_iscsi() can't cover it. |
There was a problem hiding this comment.
This looks like duplicated code. Couldn't you use iscsi.Iscsi.create_iSCSI(second_params) instead? It allows for setting the target name, second_params['target']='tgp1'.
This is what setup_or_cleanup_iscsi does, too: https://github.com/avocado-framework/avocado-vt/blob/master/virttest/utils_test/libvirt.py#L587
There was a problem hiding this comment.
I considered this method before. But in this case we want to use a same backend storage to create two different iscsi target. It can't be created in the above function.
Automate case: VIRT-302167 - [Host device assignment][SCSI] SCSI command passthrough test for hostdev scsi device Signed-off-by: Meina Li <[email protected]>
meinaLi
force-pushed
the
hostdev_device
branch
from
8a24ae2 to
bb7d772
Compare
Automate case:
VIRT-302167 - [Host device assignment][SCSI] SCSI command passthrough test for hostdev scsi device