On Wed, Feb 02, 2022 at 03:11:03PM +0100, Michal Prívozník wrote:
> On 2/2/22 14:34, Andrea Bolognani wrote:
> > On Wed, Feb 02, 2022 at 02:26:23PM +0100, Michal Prívozník wrote:
> >> On 2/2/22 13:44, Andrea Bolognani wrote:
> >>> Maybe we should add a test case where the memory device is not on the
> >>> root bus? We can't catch the QEMU error of course, but that would at
> >>> least serve as some sort of implicit documentation of the fact that
> >>> we expect that scenario to work.
> >>
> >> Sure, I can do that. I'm not that convinced on its value, but I can
> >> alter an existing test case.
> >
> > If you don't think it's going to be useful, then just don't do it :)
>
> Yeah, thing is, this bug depends on how QEMU behaves (namely order in
> which it parses arguments). Libvirt produced "correct" output (in sense
> that devices that need to be there are there). So unless we are starting
> QEMU we won't notice the QEMU behavior.
>
> Think of this in a different way, if QEMU started parsing arguments in
> different order (highly improbable, but let's assume that for a while).
> Even if I added test case as you suggest, our test suite would not
> notice anything different, and yet - users would be unable to start
> their guests.
>
> But maybe I'm missing something and we might get something valuable from
> such test? What was your thinking?
I was imagining some change happening in the future under the
assumption that memory devices will only ever be plugged into
pci(e).0: the current test suite would not catch that, but if we had
at least one case in which a memory device is plugged into a
different bus then we'd have a chance of noticing.
Maybe that's a bit far-fetched, but even though the potential gain
would be small the effort needed to obtain it would also be small
(something along the lines of the diff below), so overall it seems
like a decent deal to me :)
I agree completely with you that we can't have a proper test for the
issue that your patch fixed without actually running QEMU.
diff --git a/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args
b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args
index dba2452ccf..5aa8110aeb 100644
--- a/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args
@@ -28,11 +28,12 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
-no-shutdown \
-no-acpi \
-boot strict=on \
+-device '{"driver":"pci-bridge","chassis_nr":1,"id":"pci.1","bus":"pci.0","addr":"0x3"}'
\
-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}'
\
-object '{"qom-type":"memory-backend-ram","id":"memvirtiomem0","reserve":false,"size":1073741824}'
\
-device '{"driver":"virtio-mem-pci","node":0,"block-size":2097152,"requested-size":536870912,"memdev":"memvirtiomem0","id":"virtiomem0","bus":"pci.0","addr":"0x2"}'
\
-object '{"qom-type":"memory-backend-file","id":"memvirtiomem1","mem-path":"/dev/hugepages2M/libvirt/qemu/-1-QEMUGuest1","reserve":false,"size":2147483648,"host-nodes":[1,2,3],"policy":"bind"}'
\
--device '{"driver":"virtio-mem-pci","node":0,"block-size":2097152,"requested-size":1073741824,"memdev":"memvirtiomem1","prealloc":true,"id":"virtiomem1","bus":"pci.0","addr":"0x3"}'
\
+-device '{"driver":"virtio-mem-pci","node":0,"block-size":2097152,"requested-size":1073741824,"memdev":"memvirtiomem1","prealloc":true,"id":"virtiomem1","bus":"pci.1","addr":"0x1"}'
\
-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}'
\
-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}'
\
-device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-format","id":"ide0-0-0","bootindex":1}'
\
diff --git a/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.xml
b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.xml
index ea9f5e8765..73036d8602 100644
--- a/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.xml
+++ b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.xml
@@ -35,6 +35,11 @@
<address type='pci' domain='0x0000' bus='0x00' slot='0x01'
function='0x2'/>
</controller>
<controller type='pci' index='0' model='pci-root'/>
+ <controller type='pci' index='1' model='pci-bridge'>
+ <model name='pci-bridge'/>
+ <target chassisNr='1'/>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x03'
function='0x0'/>
+ </controller>
<input type='mouse' bus='ps2'/>
<input type='keyboard' bus='ps2'/>
<audio id='1' type='none'/>
@@ -61,7 +66,7 @@
<block unit='KiB'>2048</block>
<requested unit='KiB'>1048576</requested>
</target>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x03'
function='0x0'/>
+ <address type='pci' domain='0x0000' bus='0x01' slot='0x01'
function='0x0'/>
</memory>
</devices>
</domain>
--
Andrea Bolognani / Red Hat / Virtualization
