Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hotplug_virtio_mem: forbid virtio_mem unplug for QEMU < 8.1.0 #4109

Merged
merged 1 commit into from
Jul 22, 2024

Conversation

mcasquer
Copy link
Contributor

@mcasquer mcasquer commented Jul 17, 2024

hotplug_virtio_mem: forbid virtio_mem unplug for QEMU < 8.1.0

The unplug support for virtio_mem was introduced in RHEL 9.4
with QEMU 8.1, corrects the test to only do this action when
the proper QEMU is used. Also applies the black code formatter.

Signed-off-by: mcasquer [email protected]
ID: 2626

@mcasquer
Copy link
Contributor Author

QEMU 8.2.0

 (1/2) Host_RHEL.m9.u4.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.hotplug_virtio_mem.q35: STARTED
 (1/2) Host_RHEL.m9.u4.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.hotplug_virtio_mem.q35: PASS (172.40 s)
 (2/2) Host_RHEL.m9.u4.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.hotplug_virtio_mem.with_unplug.q35: STARTED
 (2/2) Host_RHEL.m9.u4.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.4.0.x86_64.io-github-autotest-qemu.hotplug_virtio_mem.with_unplug.q35: PASS (166.20 s)
RESULTS    : PASS 2 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0

QEMU 8.0.0

 (1/2) Host_RHEL.m9.u3.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.3.0.x86_64.io-github-autotest-qemu.hotplug_virtio_mem.q35: STARTED
 (1/2) Host_RHEL.m9.u3.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.3.0.x86_64.io-github-autotest-qemu.hotplug_virtio_mem.q35: PASS (166.33 s)
 (2/2) Host_RHEL.m9.u3.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.3.0.x86_64.io-github-autotest-qemu.hotplug_virtio_mem.with_unplug.q35: STARTED
 (2/2) Host_RHEL.m9.u3.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.3.0.x86_64.io-github-autotest-qemu.hotplug_virtio_mem.with_unplug.q35: CANCEL: Got host qemu version:8.0.0-16, which is not in [8.1.0,) (2.59 s)
RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 1

@mcasquer mcasquer marked this pull request as ready for review July 17, 2024 12:16
@mcasquer
Copy link
Contributor Author

@zhenyzha @menli820 @YongxueHong @PaulYuuu please, could you review this PR? Thanks !

@menli820
Copy link
Contributor

LGTM.

(1/3) Host_RHEL.m9.u5.qcow2.virtio_blk.up.virtio_net.Guest.RHEL.9.5.0.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35: STARTED
(1/3) Host_RHEL.m9.u5.qcow2.virtio_blk.up.virtio_net.Guest.RHEL.9.5.0.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35: PASS (724.16 s)
(2/3) Host_RHEL.m9.u5.qcow2.virtio_blk.up.virtio_net.Guest.RHEL.9.5.0.x86_64.io-github-autotest-qemu.hotplug_virtio_mem.q35: STARTED
(2/3) Host_RHEL.m9.u5.qcow2.virtio_blk.up.virtio_net.Guest.RHEL.9.5.0.x86_64.io-github-autotest-qemu.hotplug_virtio_mem.q35: PASS (140.92 s)
(3/3) Host_RHEL.m9.u5.qcow2.virtio_blk.up.virtio_net.Guest.RHEL.9.5.0.x86_64.io-github-autotest-qemu.hotplug_virtio_mem.with_unplug.q35: STARTED
(3/3) Host_RHEL.m9.u5.qcow2.virtio_blk.up.virtio_net.Guest.RHEL.9.5.0.x86_64.io-github-autotest-qemu.hotplug_virtio_mem.with_unplug.q35: PASS (110.45 s)
RESULTS : PASS 3 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML : /root/avocado/job-results/job-2024-07-18T10.17-7ecd7df/results.html
JOB TIME : 977.90 s

PaulYuuu
PaulYuuu previously approved these changes Jul 18, 2024
@yanan-fu
Copy link
Contributor

IMO, it is redundant to add a new variant. For QEMU >= 8.1.0, the case with_unplug can cover all the test scenarios for case default . Inefficient to run both of these 2 cases.

I would like to suggest handle it in the script, filter different test steps based on qemu version.

    qemu_path = utils_misc.get_qemu_binary(params)
    qemu_version = env_process._get_qemu_version(qemu_path)
    match = re.search(r"[0-9]+\.[0-9]+\.[0-9]+(\-[0-9]+)?", qemu_version)
    host_qemu = match.group(0)
    if host_qemu in VersionInterval('[8.1.0,)'):
        **** hot unplug ****

@zhenyzha
Copy link
Contributor

LGTM.Acked-by: Zhenyu Zhang [email protected]

 (1/2) Host_RHEL.m9.u5.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.5.0.aarch64.page_4k.io-github-autotest-qemu.hotplug_virtio_mem.arm64-pci: STARTED
 (1/2) Host_RHEL.m9.u5.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.5.0.aarch64.page_4k.io-github-autotest-qemu.hotplug_virtio_mem.arm64-pci: PASS (142.26 s)
 (2/2) Host_RHEL.m9.u5.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.5.0.aarch64.page_4k.io-github-autotest-qemu.hotplug_virtio_mem.with_unplug.arm64-pci: STARTED
 (2/2) Host_RHEL.m9.u5.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.5.0.aarch64.page_4k.io-github-autotest-qemu.hotplug_virtio_mem.with_unplug.arm64-pci: PASS (129.31 s)

@mcasquer
Copy link
Contributor Author

IMO, it is redundant to add a new variant. For QEMU >= 8.1.0, the case with_unplug can cover all the test scenarios for case default . Inefficient to run both of these 2 cases.

I would like to suggest handle it in the script, filter different test steps based on qemu version.

    qemu_path = utils_misc.get_qemu_binary(params)
    qemu_version = env_process._get_qemu_version(qemu_path)
    match = re.search(r"[0-9]+\.[0-9]+\.[0-9]+(\-[0-9]+)?", qemu_version)
    host_qemu = match.group(0)
    if host_qemu in VersionInterval('[8.1.0,)'):
        **** hot unplug ****

@yanan-fu thanks for the suggestion, code updated !

@yanan-fu
Copy link
Contributor

Could you please update the doc string to show the test steps correctly ? Thanks!

@mcasquer
Copy link
Contributor Author

Could you please update the doc string to show the test steps correctly ? Thanks!

Done !

@mcasquer
Copy link
Contributor Author

 (1/1) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.5.0.x86_64.io-github-autotest-qemu.hotplug_virtio_mem.q35: STARTED
 (1/1) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.5.0.x86_64.io-github-autotest-qemu.hotplug_virtio_mem.q35: PASS (174.90 s)
RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0

yanan-fu
yanan-fu previously approved these changes Jul 18, 2024
Copy link
Contributor

@yanan-fu yanan-fu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acked-by: Yanan Fu [email protected]

YongxueHong
YongxueHong previously approved these changes Jul 19, 2024
Copy link
Contributor

@YongxueHong YongxueHong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Comment on lines 39 to 41
qemu_version = env_process._get_qemu_version(qemu_path)
match = re.search(r"[0-9]+\.[0-9]+\.[0-9]+(\-[0-9]+)?", qemu_version)
host_qemu = match.group(0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend calling a public function to get the qemu version: virttest.utils_qemu.get_qemu_version instead of the private function _get_qemu_version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@YongxueHong Thanks for the suggestion, code updated !

@mcasquer mcasquer dismissed stale reviews from YongxueHong and yanan-fu via 0b7a85c July 19, 2024 09:39
@mcasquer mcasquer force-pushed the 2626_limit_unplug branch 2 times, most recently from 0b7a85c to 578e582 Compare July 19, 2024 10:10
@mcasquer
Copy link
Contributor Author

 (1/1) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.5.0.x86_64.io-github-autotest-qemu.hotplug_virtio_mem.q35: STARTED
 (1/1) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.5.0.x86_64.io-github-autotest-qemu.hotplug_virtio_mem.q35: PASS (137.17 s)
RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0

Comment on lines 38 to 41
qemu_path = utils_misc.get_qemu_binary(params)
qemu_version = utils_qemu.get_qemu_version(qemu_path)
match = re.search(r"[0-9]+\.[0-9]+\.[0-9]+(\-[0-9]+)?", str(qemu_version))
host_qemu = match.group(0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
qemu_path = utils_misc.get_qemu_binary(params)
qemu_version = utils_qemu.get_qemu_version(qemu_path)
match = re.search(r"[0-9]+\.[0-9]+\.[0-9]+(\-[0-9]+)?", str(qemu_version))
host_qemu = match.group(0)
qemu_path = utils_misc.get_qemu_binary(params)
qemu_version = utils_qemu.get_qemu_version(qemu_path)[0]

HIi @mcasquer
I think that we could get the host_qemu(qemu_version) by qemu_version[0] directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@YongxueHong thanks, code updated!

The unplug support for virtio_mem was introduced in RHEL 9.4
with QEMU 8.1, corrects the test to only do this action when
the proper QEMU is used. Also applies the black code formatter.

Signed-off-by: mcasquer <[email protected]>
@mcasquer
Copy link
Contributor Author

 (1/1) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.5.0.x86_64.io-github-autotest-qemu.hotplug_virtio_mem.q35: STARTED
 (1/1) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.5.0.x86_64.io-github-autotest-qemu.hotplug_virtio_mem.q35: PASS (140.41 s)
RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0

Copy link
Contributor

@YongxueHong YongxueHong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Contributor

@yanan-fu yanan-fu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack

@yanan-fu yanan-fu merged commit e9fa1ee into autotest:master Jul 22, 2024
6 checks passed
@mcasquer mcasquer deleted the 2626_limit_unplug branch July 22, 2024 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants