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

qdevices: bug fixing #3710

Merged
merged 1 commit into from
Jun 27, 2023
Merged

Conversation

nickzhq
Copy link
Contributor

@nickzhq nickzhq commented Jun 20, 2023

Do not change the type of "detect-zeroes" in -blockdev for json format

ID: 2216076

@nickzhq
Copy link
Contributor Author

nickzhq commented Jun 20, 2023

@YongxueHong @zhencliu @qingwangrh please take a look! Thanks!

@zhencliu
Copy link
Contributor

it's better to revise the commit header, e.g.
qdevices: type of detect-zeroes should be a string

You may paste the valid values from man page in the commit body

@qingwangrh
Copy link
Contributor

(1/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.off.q35: STARTED
(1/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.off.q35: PASS (48.95 s)
(2/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.on.q35: STARTED
(2/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.on.q35: PASS (47.65 s)
(3/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.unmap.q35: STARTED
(3/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.unmap.q35: PASS (47.60 s)
RESULTS : PASS 3 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCE

LGTM

@nickzhq
Copy link
Contributor Author

nickzhq commented Jun 20, 2023

@YongxueHong @zhencliu please take a look! Thanks!

@YongxueHong
Copy link
Contributor

(1/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.off.q35: STARTED (1/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.off.q35: PASS (48.95 s) (2/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.on.q35: STARTED (2/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.on.q35: PASS (47.65 s) (3/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.unmap.q35: STARTED (3/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.unmap.q35: PASS (47.60 s) RESULTS : PASS 3 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCE

LGTM

Hi @qingwangrh
Could you help to paste the related command line?
Thanks.

@zhencliu
Copy link
Contributor

(1/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.off.q35: STARTED (1/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.off.q35: PASS (48.95 s) (2/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.on.q35: STARTED (2/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.on.q35: PASS (47.65 s) (3/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.unmap.q35: STARTED (3/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.unmap.q35: PASS (47.60 s) RESULTS : PASS 3 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCE

LGTM

Hi @qingwangrh , do we plan to add a hotplug case with the option 'detect-zeroes'? It looks block_detect_zeroes is a new case which is not in upstream yet. If it is, talked with @YongxueHong , we may also need to run the hotplug case, as Nick's code change can also affect the hotplug function cc @nickzhq , thanks.

@qingwangrh
Copy link
Contributor

qingwangrh commented Jun 20, 2023

(1/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.off.q35: STARTED (1/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.off.q35: PASS (48.95 s) (2/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.on.q35: STARTED (2/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.on.q35: PASS (47.65 s) (3/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.unmap.q35: STARTED (3/3) Host_RHEL.m8.u5.product_av.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.4.0.x86_64.io-github-autotest-qemu.block_detect_zeroes.unmap.q35: PASS (47.60 s) RESULTS : PASS 3 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCE
LGTM

Hi @qingwangrh , do we plan to add a hotplug case with the option 'detect-zeroes'? It looks block_detect_zeroes is a new case which is not in upstream yet. If it is, talked with @YongxueHong , we may also need to run the hotplug case, as Nick's code change can also affect the hotplug function cc @nickzhq , thanks.

@YongxueHong the command line
python ConfigTest.py --testcase=block_detect_zeroes --guestname=RHEL.8.4.0 --clone=no --firmware=default_bios
It is a new case not exist upstream.
@zhencliu it is a new case boot vm with the disk has detect-zeroes option, it does not include hotplug operation. maybe it is another story, i may develop a new case to hotplug .

@qingwangrh
Copy link
Contributor

Test hotplug with this patch, it looks like run the correct QMP command :
(monitor avocado-vt-vm1.qmpmonitor1) Sending command 'blockdev-add'
[stdlog] 2023-06-20 06:04:50,206 qemu_monitor L2007 DEBUG| Send command: {'execute': 'blockdev-add', 'arguments': {'node-name': 'file_stg1', 'driver': 'file', 'auto-read-only': True, 'discard': 'unmap', 'aio': 'threads', 'filename': '/home/kvm_autotest_root/images/stg1.qcow2', 'cache': {'direct': True, 'no-flush': False}, 'detect-zeroes': 'on'}, 'id': 'pr7cbRsh'}
[stdlog] 2023-06-20 06:04:50,287 qemu_monitor L0443 DEBUG| (monitor avocado-vt-vm1.qmpmonitor1) Sending command 'blockdev-add'
[stdlog] 2023-06-20 06:04:50,287 qemu_monitor L2007 DEBUG| Send command: {'execute': 'blockdev-add', 'arguments': {'node-name': 'drive_stg1', 'driver': 'qcow2', 'read-only': False, 'cache': {'direct': True, 'no-flush': False}, 'file': 'file_stg1', 'discard': 'unmap', 'detect-zeroes': 'on'}, 'id': 'TCNLalBV'}
[stdlog] 2023-06-20 06:04:50,319 qemu_monitor L0443 DEBUG| (monitor avocado-vt-vm1.qmpmonitor1) Sending command 'device_add'
[stdlog] 2023-06-20 06:04:50,319 qemu_monitor L2007 DEBUG| Send command: {'execute': 'device_add', 'arguments': OrderedDict([('driver', 'scsi-hd'), ('id', 'stg1'), ('drive', 'drive_stg1'), ('write-cache', 'on'), ('serial', 'stg1'), ('bus', 'virtio_scsi_pci0.0')]), 'id': 'axaxDwKb'}

@zhencliu
Copy link
Contributor

Test hotplug with this patch, it looks like run the correct QMP command : (monitor avocado-vt-vm1.qmpmonitor1) Sending command 'blockdev-add' [stdlog] 2023-06-20 06:04:50,206 qemu_monitor L2007 DEBUG| Send command: {'execute': 'blockdev-add', 'arguments': {'node-name': 'file_stg1', 'driver': 'file', 'auto-read-only': True, 'discard': 'unmap', 'aio': 'threads', 'filename': '/home/kvm_autotest_root/images/stg1.qcow2', 'cache': {'direct': True, 'no-flush': False}, 'detect-zeroes': 'on'}, 'id': 'pr7cbRsh'} [stdlog] 2023-06-20 06:04:50,287 qemu_monitor L0443 DEBUG| (monitor avocado-vt-vm1.qmpmonitor1) Sending command 'blockdev-add' [stdlog] 2023-06-20 06:04:50,287 qemu_monitor L2007 DEBUG| Send command: {'execute': 'blockdev-add', 'arguments': {'node-name': 'drive_stg1', 'driver': 'qcow2', 'read-only': False, 'cache': {'direct': True, 'no-flush': False}, 'file': 'file_stg1', 'discard': 'unmap', 'detect-zeroes': 'on'}, 'id': 'TCNLalBV'} [stdlog] 2023-06-20 06:04:50,319 qemu_monitor L0443 DEBUG| (monitor avocado-vt-vm1.qmpmonitor1) Sending command 'device_add' [stdlog] 2023-06-20 06:04:50,319 qemu_monitor L2007 DEBUG| Send command: {'execute': 'device_add', 'arguments': OrderedDict([('driver', 'scsi-hd'), ('id', 'stg1'), ('drive', 'drive_stg1'), ('write-cache', 'on'), ('serial', 'stg1'), ('bus', 'virtio_scsi_pci0.0')]), 'id': 'axaxDwKb'}

Thanks for the information.

values of "detect-zeroes" should be one of ["on", "off", "unmap"]

Signed-off-by: Houqi (Nick) Zuo <[email protected]>
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.

@YongxueHong YongxueHong merged commit fdcfdde into avocado-framework:master Jun 27, 2023
49 checks passed
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.

4 participants