[PATCH 0/2] Fix wrong parameter "scsi" for vhost-user-blk-pci device
From: shenjiatong <yshxxsjt715@gmail.com> Co-authored-by: Peter Krempa Peter Krempa (1): qemuxml2argvtest: disk-vhostuser: Add invocation for qemu-4.2 shenjiatong (1): Remove scsi parameter for vhost-user-blk for qemu 4.2 src/qemu/qemu_command.c | 6 ++- .../disk-vhostuser-numa.x86_64-4.2.0.args | 39 +++++++++++++++++++ .../disk-vhostuser-numa.x86_64-latest.args | 39 +++++++++++++++++++ .../qemuxml2argvdata/disk-vhostuser-numa.xml | 32 +++++++++++++++ tests/qemuxml2argvtest.c | 2 + 5 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/disk-vhostuser-numa.x86_64-4.2.0.args create mode 100644 tests/qemuxml2argvdata/disk-vhostuser-numa.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-vhostuser-numa.xml -- 2.25.1
From: Peter Krempa <pkrempa@redhat.com> With qemu versions prior to qemu-5.0 we'll format 'scsi=off' for virtio-blk disks, but also for vhost-user-blk. This is a bug as it's not supported. Add a test case to show that wrong configuration is generated by adding running 'disk-vhostuser' test case on capabilities from qemu-4.2. For this to be possible it's required to enable shared memory via NUMA configuration as old QEMU's don't allow configuration of the default memory backend. This is achieved by adding a copy of the 'disk-vhostuser' XML with NUMA enabled. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../disk-vhostuser-numa.x86_64-4.2.0.args | 39 +++++++++++++++++++ .../disk-vhostuser-numa.x86_64-latest.args | 39 +++++++++++++++++++ .../qemuxml2argvdata/disk-vhostuser-numa.xml | 32 +++++++++++++++ tests/qemuxml2argvtest.c | 2 + 4 files changed, 112 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-vhostuser-numa.x86_64-4.2.0.args create mode 100644 tests/qemuxml2argvdata/disk-vhostuser-numa.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-vhostuser-numa.xml diff --git a/tests/qemuxml2argvdata/disk-vhostuser-numa.x86_64-4.2.0.args b/tests/qemuxml2argvdata/disk-vhostuser-numa.x86_64-4.2.0.args new file mode 100644 index 0000000000..6c5d6de46b --- /dev/null +++ b/tests/qemuxml2argvdata/disk-vhostuser-numa.x86_64-4.2.0.args @@ -0,0 +1,39 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc-i440fx-4.2,usb=off,dump-guest-core=off \ +-accel tcg \ +-cpu qemu64 \ +-m 14336 \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram/-1-QEMUGuest1/ram-node0,share=on,size=15032385536 \ +-numa node,nodeid=0,cpus=0,memdev=ram-node0 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-chardev socket,id=chr-vu-virtio-disk0,path=/tmp/vhost1.sock \ +-device vhost-user-blk-pci,scsi=off,bus=pci.0,addr=0x2,chardev=chr-vu-virtio-disk0,id=virtio-disk0,bootindex=1 \ +-chardev socket,id=chr-vu-virtio-disk1,path=/tmp/vhost1.sock,reconnect=10 \ +-device vhost-user-blk-pci,iommu_platform=on,ats=on,packed=on,scsi=off,bus=pci.0,addr=0x3,chardev=chr-vu-virtio-disk1,id=virtio-disk1 \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/disk-vhostuser-numa.x86_64-latest.args b/tests/qemuxml2argvdata/disk-vhostuser-numa.x86_64-latest.args new file mode 100644 index 0000000000..75b3232dad --- /dev/null +++ b/tests/qemuxml2argvdata/disk-vhostuser-numa.x86_64-latest.args @@ -0,0 +1,39 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off \ +-accel tcg \ +-cpu qemu64 \ +-m 14336 \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-object '{"qom-type":"memory-backend-file","id":"ram-node0","mem-path":"/var/lib/libvirt/qemu/ram/-1-QEMUGuest1/ram-node0","share":true,"size":15032385536}' \ +-numa node,nodeid=0,cpus=0,memdev=ram-node0 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-chardev socket,id=chr-vu-virtio-disk0,path=/tmp/vhost1.sock \ +-device '{"driver":"vhost-user-blk-pci","bus":"pci.0","addr":"0x2","chardev":"chr-vu-virtio-disk0","id":"virtio-disk0","bootindex":1}' \ +-chardev socket,id=chr-vu-virtio-disk1,path=/tmp/vhost1.sock,reconnect=10 \ +-device '{"driver":"vhost-user-blk-pci","iommu_platform":true,"ats":true,"packed":true,"bus":"pci.0","addr":"0x3","chardev":"chr-vu-virtio-disk1","id":"virtio-disk1"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x4"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/disk-vhostuser-numa.xml b/tests/qemuxml2argvdata/disk-vhostuser-numa.xml new file mode 100644 index 0000000000..49efbae0a2 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-vhostuser-numa.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + <numa> + <cell id='0' cpus='0' memory='14680064' unit='KiB' memAccess='shared'/> + </numa> + </cpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='vhostuser' device='disk'> + <driver name='qemu' type='raw'/> + <source type='unix' path='/tmp/vhost1.sock'/> + <target dev='vda' bus='virtio'/> + <boot order='1'/> + </disk> + <disk type='vhostuser' device='disk'> + <driver name='qemu' type='raw' iommu='on' ats='on' packed='on'/> + <source type='unix' path='/tmp/vhost1.sock'> + <reconnect enabled='yes' timeout='10'/> + </source> + <target dev='vdb' bus='virtio'/> + </disk> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 9d2de2a569..afb1cb76e0 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1442,6 +1442,8 @@ mymain(void) VIR_FREE(driver.config->vxhsTLSx509certdir); DO_TEST_CAPS_LATEST("disk-no-boot"); DO_TEST_CAPS_LATEST("disk-nvme"); + DO_TEST_CAPS_VER("disk-vhostuser-numa", "4.2.0"); + DO_TEST_CAPS_LATEST("disk-vhostuser-numa"); DO_TEST_CAPS_LATEST("disk-vhostuser"); DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-device-lun-type-invalid"); DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-attaching-partition-nosupport"); -- 2.25.1
From: shenjiatong <yshxxsjt715@gmail.com> When trying to attach vhost-user-blk device to virtual machine using libvirt 7.8 and qemu 4.2. Libvirt mistakenly add a scsi=off parameter which is not supported by Qemu Signed-off-by: shenjiatong <yshxxsjt715@gmail.com> --- src/qemu/qemu_command.c | 6 +++++- .../qemuxml2argvdata/disk-vhostuser-numa.x86_64-4.2.0.args | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 662d15e28e..cdf89fc74c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2253,11 +2253,15 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, "S:iothread", iothread, "T:ioeventfd", disk->ioeventfd, "T:event_idx", disk->event_idx, - "T:scsi", scsi, "p:num-queues", disk->queues, "p:queue-size", disk->queue_size, NULL) < 0) return NULL; + + if (virStorageSourceGetActualType(disk->src) != VIR_STORAGE_TYPE_VHOST_USER) { + if (virJSONValueObjectAdd(&props, "T:scsi", scsi, NULL) < 0) + return NULL; + } } break; diff --git a/tests/qemuxml2argvdata/disk-vhostuser-numa.x86_64-4.2.0.args b/tests/qemuxml2argvdata/disk-vhostuser-numa.x86_64-4.2.0.args index 6c5d6de46b..4480266e75 100644 --- a/tests/qemuxml2argvdata/disk-vhostuser-numa.x86_64-4.2.0.args +++ b/tests/qemuxml2argvdata/disk-vhostuser-numa.x86_64-4.2.0.args @@ -30,9 +30,9 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -boot strict=on \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ -chardev socket,id=chr-vu-virtio-disk0,path=/tmp/vhost1.sock \ --device vhost-user-blk-pci,scsi=off,bus=pci.0,addr=0x2,chardev=chr-vu-virtio-disk0,id=virtio-disk0,bootindex=1 \ +-device vhost-user-blk-pci,bus=pci.0,addr=0x2,chardev=chr-vu-virtio-disk0,id=virtio-disk0,bootindex=1 \ -chardev socket,id=chr-vu-virtio-disk1,path=/tmp/vhost1.sock,reconnect=10 \ --device vhost-user-blk-pci,iommu_platform=on,ats=on,packed=on,scsi=off,bus=pci.0,addr=0x3,chardev=chr-vu-virtio-disk1,id=virtio-disk1 \ +-device vhost-user-blk-pci,iommu_platform=on,ats=on,packed=on,bus=pci.0,addr=0x3,chardev=chr-vu-virtio-disk1,id=virtio-disk1 \ -audiodev '{"id":"audio1","driver":"none"}' \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ -- 2.25.1
On Sat, Jan 22, 2022 at 12:10:35 +0800, yshxxsjt715@gmail.com wrote:
From: shenjiatong <yshxxsjt715@gmail.com>
When trying to attach vhost-user-blk device to virtual machine using libvirt 7.8 and qemu 4.2. Libvirt mistakenly add a scsi=off parameter which is not supported by Qemu
Signed-off-by: shenjiatong <yshxxsjt715@gmail.com> --- src/qemu/qemu_command.c | 6 +++++- .../qemuxml2argvdata/disk-vhostuser-numa.x86_64-4.2.0.args | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 662d15e28e..cdf89fc74c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2253,11 +2253,15 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, "S:iothread", iothread, "T:ioeventfd", disk->ioeventfd, "T:event_idx", disk->event_idx, - "T:scsi", scsi,
Rather than moving this line out of the formatter,
"p:num-queues", disk->queues, "p:queue-size", disk->queue_size, NULL) < 0) return NULL; + + if (virStorageSourceGetActualType(disk->src) != VIR_STORAGE_TYPE_VHOST_USER) { + if (virJSONValueObjectAdd(&props, "T:scsi", scsi, NULL) < 0)
you can simply skip the code that assigns VIR_TRISTATE_SWITCH_ON/OFF to the 'scsi' value. By default the formatter doesn't print anything for the _ABSENT case. If you don't mind I can do that before pushing so you don't have to send another round.
Ok, thanks. On Sat, Jan 22, 2022 at 4:24 PM Peter Krempa <pkrempa@redhat.com> wrote:
On Sat, Jan 22, 2022 at 12:10:35 +0800, yshxxsjt715@gmail.com wrote:
From: shenjiatong <yshxxsjt715@gmail.com>
When trying to attach vhost-user-blk device to virtual machine using libvirt 7.8 and qemu 4.2. Libvirt mistakenly add a scsi=off parameter which is not supported by Qemu
Signed-off-by: shenjiatong <yshxxsjt715@gmail.com> --- src/qemu/qemu_command.c | 6 +++++- .../qemuxml2argvdata/disk-vhostuser-numa.x86_64-4.2.0.args | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 662d15e28e..cdf89fc74c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2253,11 +2253,15 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, "S:iothread", iothread, "T:ioeventfd", disk->ioeventfd, "T:event_idx", disk->event_idx, - "T:scsi", scsi,
Rather than moving this line out of the formatter,
"p:num-queues", disk->queues, "p:queue-size", disk->queue_size, NULL) < 0) return NULL; + + if (virStorageSourceGetActualType(disk->src) !=
VIR_STORAGE_TYPE_VHOST_USER) {
+ if (virJSONValueObjectAdd(&props, "T:scsi", scsi, NULL) < 0)
you can simply skip the code that assigns VIR_TRISTATE_SWITCH_ON/OFF to the 'scsi' value. By default the formatter doesn't print anything for the _ABSENT case.
If you don't mind I can do that before pushing so you don't have to send another round.
-- Best Regards, Jiatong Shen
participants (3)
-
Jiatong Shen -
Peter Krempa -
yshxxsjt715＠gmail.com