qemu_vm: create qemu command in json style phase II

[nickzhq]

ID: 2131121 Signed-off-by: Houqi (Nick) Zuo [email protected]

[nickzhq]

(1/2) Host_RHEL.m9.u1.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.1.0.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35: STARTED (1/2) Host_RHEL.m9.u1.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.1.0.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35: PASS (916.79 s) (2/2) Host_RHEL.m9.u1.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.1.0.x86_64.io-github-autotest-qemu.virtio_mode.with_balloon.with_modern.q35: STARTED (2/2) Host_RHEL.m9.u1.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.1.0.x86_64.io-github-autotest-qemu.virtio_mode.with_balloon.with_modern.q35: PASS (61.53 s) RESULTS : PASS 2 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0

[lgtm-com[bot]]

This pull request introduces 1 alert when merging d769c236382e165dcb70f071463f601975e051f3 into cd8ddb260583c479e4cc9fd358bde42a8126e3af - view on LGTM.com

new alerts:

• 1 for Wrong name for an argument in a class instantiation

[lgtm-com[bot]]

This pull request introduces 1 alert when merging db55cd302cf194c504f93fa1bc84c131573a8011 into cd8ddb260583c479e4cc9fd358bde42a8126e3af - view on LGTM.com

new alerts:

• 1 for Wrong name for an argument in a class instantiation

[jingzhao84]

Ack : [email protected] based on feature test result https://issues.redhat.com/browse/XKVMNINE-1387

[nickzhq]

@luckyh @YongxueHong @zhencliu Please take a look, if you guys want a meeting for some issues, just let me know! Thanks!

[nickzhq]

Hello @vivianQizhu , Please take a look on following file: virttest/qemu_devices/qcontainer.py There's only line changed in this file. Based on the docstring, we guess there's a bug at here: On ARM platform, the return type should be list instead of the single device. If we are wrong, please correct us, thanks!

[zhencliu]

one more comment, as we changed the device object, it's better to do a basic verification against those devices @nickzhq

[nickzhq]

one more comment, as we changed the device object, it's better to do a basic verification against those devices @nickzhq

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

(1/3) Host_RHEL.m9.u2.ovmf.virtio_rng.rng_builtin.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.2.0.x86_64.io-github-autotest-qemu.rng_bat.q35: STARTED (1/3) Host_RHEL.m9.u2.ovmf.virtio_rng.rng_builtin.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.2.0.x86_64.io-github-autotest-qemu.rng_bat.q35: PASS (53.53 s)

(1/1) Host_RHEL.m9.u2.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.2.0.x86_64.io-github-autotest-qemu.verify_panic_status_with_pvpanic.trigger_kernel_panic.q35: STARTED (1/1) Host_RHEL.m9.u2.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.2.0.x86_64.io-github-autotest-qemu.verify_panic_status_with_pvpanic.trigger_kernel_panic.q35: PASS (88.09 s) RESULTS : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0

[luckyh]

The issue of CI failures is addressed by https://github.com/avocado-framework/avocado-vt/pull/3581.

[YongxueHong]

Please add more detail of the commit message to the commit qemu_vm: create qemu command in json style phase II.

[nickzhq]

Please add more detail of the commit message to the commit qemu_vm: create qemu command in json style phase II.

Done

[nickzhq]

@zhencliu @luckyh @YongxueHong Please check again, thanks!

[PaulYuuu]

I want to point out 1 thing, the file qdevices.py only has 1 def _cmdline_raw method, so it can handle all kinds of devices, currently it has 5 def _cmdline_json methods, but unfortunately it can't handle all the devices smartly like def _cmdline_raw, such as "NO_EQUAL_STRING" https://github.com/avocado-framework/avocado-vt/blob/a8c6b9e7e8e080bd0370d6e2cf8e9749b3b82706/virttest/qemu_devices/qdevices.py#L435-L451

[stdlog]     -device '{"driver": "usb-storage", "id": "stg", "drive": "drive_stg", "write-cache": "on", "rerror": "stop", "werror": "stop", "serial": "NO_EQUAL_STRING", "bus": "usbtest.0", "port": "2"}' \

In my opinion, cmdline_json should have a base method to convert common options, and then handle some specific options case by case, the base method can be a wrapper or a parent method in QCustomDevice, and call it in subclasses via super.

[nickzhq]

I want to point out 1 thing, the file qdevices.py only has 1 def _cmdline_raw method, so it can handle all kinds of devices, currently it has 5 def _cmdline_json methods, but unfortunately it can't handle all the devices smartly like def _cmdline_raw, such as "NO_EQUAL_STRING"

https://github.com/avocado-framework/avocado-vt/blob/a8c6b9e7e8e080bd0370d6e2cf8e9749b3b82706/virttest/qemu_devices/qdevices.py#L435-L451

[stdlog]     -device '{"driver": "usb-storage", "id": "stg", "drive": "drive_stg", "write-cache": "on", "rerror": "stop", "werror": "stop", "serial": "NO_EQUAL_STRING", "bus": "usbtest.0", "port": "2"}' \

In my opinion, cmdline_json should have a base method to convert common options, and then handle some specific options case by case, the base method can be a wrapper or a parent method in QCustomDevice, and call it in subclasses via super.

Hello @PaulYuuu , the code has been supported the "serial": "NO_EQUAL_STRING".

[nickzhq]

I don't suggest creating such API as the standard interface in the framework since this is just for the qume perspective, but is not available for others. And this function may be blurry, it is hard to understand what it does, I mean I can not get the purpose of this function. And would you like to elaborate on the purpose of this module? Thanks.

"""
Virtualization test utility functions.
"""

@YongxueHong please take a look, thanks!

[nickzhq]

@zhencliu @YongxueHong Please take a look, thanks!

[YongxueHong]

Hi @PaulYuuu Would you like to help review it? Thanks.

[zhencliu]

hi @nickzhq , would you please run the related 'NO_EQUAL_STRING' case and paste the snippet of the qemu cmdline here? Then we can see the json format after code update, cc @PaulYuuu

[nickzhq]

hi @nickzhq , would you please run the related 'NO_EQUAL_STRING' case and paste the snippet of the qemu cmdline here? Then we can see the json format after code update, cc @PaulYuuu

For the related cases I found are the usb_storage testcases( the usb_storage cases were passed), here are some logs including "serial": [stdlog] -device '{"driver": "usb-storage", "id": "stg", "drive": "drive_stg", "write-cache": "on", "rerror": "stop", "werror": "stop", "serial": "", "bus": "usbtest.0", "port": "2"}'
........ [stdlog] -device '{"driver": "usb-storage", "id": "stg", "drive": "drive_stg", "write-cache": "on", "rerror": "stop", "werror": "stop", "serial": "on", "bus": "usbtest.0", "port": "2"}' ........ [stdlog] -device '{"driver": "usb-storage", "removable": true, "id": "stg", "drive": "drive_stg", "write-cache": "on", "rerror": "stop", "werror": "stop", "serial": "on", "bus": "usbtest.0", "port": "2"}' ........ [stdlog] -device '{"driver": "usb-storage", "removable": false, "id": "stg", "drive": "drive_stg", "min_io_size": 0, "opt_io_size": 0, "write-cache": "on", "rerror": "stop", "werror": "stop", "serial": "on", "bus": "usbtest.0", "port": "2"}' ........ [stdlog] -device '{"driver": "usb-storage", "id": "stg", "drive": "drive_stg", "write-cache": "on", "rerror": "stop", "werror": "stop", "serial": "TARGET_DISK0", "bus": "usbtest.0", "port": "2"}' ........ [stdlog] -device '{"driver": "usb-storage", "id": "stg", "drive": "drive_stg", "write-cache": "on", "rerror": "stop", "werror": "stop", "serial": "0bc686af96c04f988b9da74fc568dfda", "bus": "usbtest.0", "port": "2"}'

cc @zhencliu @YongxueHong @PaulYuuu

[nickzhq]

Hello @YongxueHong , is this pr ready to merge? Thanks!

[nickzhq]

@YongxueHong Please check again, thanks!

[nickzhq]

@YongxueHong Please check again, thanks!

[YongxueHong]

Hi all Since this PR has got two approvals. Let's merge it first, please leave a comment if you have any issues. Thanks.