Re: [libvirt] [PATCH v3] pci address conflict when virtio disk with drive type

On 10/23/2011 05:31 AM, Xu He Jie wrote: > When using the xml as below: > ------------------------------------------------------ > <devices> > <emulator>/home/soulxu/data/work-code/qemu-kvm/x86_64-softmmu/qemu-system-x86_64</emulator> > > <disk type='file' device='disk'> > <driver name='qemu' type='qcow2'/> > <source file='/home/soulxu/data/VM/images/linux.img'/> > <target dev='vda' bus='virtio'/> > <address type='drive' controller='0' bus='0' unit='0'/> > </disk> > <input type='mouse' bus='ps2'/> > <graphics type='vnc' port='-1' autoport='yes'/> > <video> > <model type='cirrus' vram='9216' heads='1'/> > <address type='pci' domain='0x0000' bus='0x00' slot='0x02' > function='0x0'/> > </video> > <memballoon model='virtio'> > <address type='pci' domain='0x0000' bus='0x00' slot='0x03' > function='0x0'/> > </memballoon> > </devices> Lack of indentation made this harder to read. > ------------------------------------------------------ > > Then can't startup qemu, the error message as below: > virsh # start test-vm > error: Failed to start domain test-vm > error: internal error process exited while connecting to monitor: > qemu-system-x86_64: -device > virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3: PCI: slot 3 > function 0 not available for virtio-balloon-pci, in use by > virtio-blk-pci > qemu-system-x86_64: -device > virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3: Device > 'virtio-balloon-pci' could not be initialized > > So adding check for bus type and address type. Only the address of > pci type support by virtio bus. > > Signed-off-by: Xu He Jie&lt;xuhj(a)linux.vnet.ibm.com&gt; > --- > src/qemu/qemu_command.c | 10 +++++++--- > 1 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 30c0be6..7c4bc0a 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -1321,13 +1321,17 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, > qemuDomainPCIAddressSetPtr addrs) > > /* Disks (VirtIO only for now */ While we're here, add the closing ). > for (i = 0; i< def->ndisks ; i++) { > - if (def->disks[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) > - continue; > - > /* Only VirtIO disks use PCI addrs */ > if (def->disks[i]->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) > continue; > > + if ((def->disks[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)&& > + (def->disks[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)) { > + qemuReportError(VIR_ERR_INTERNAL_ERROR, This is user-visible, so VIR_ERR_CONFIG_UNSUPPORTED is probably better. > + _("virtio only supports device address type 'PCI' ")); no trailing space in the message > + goto error; > + } Your patch fails 'make check', so it needs another respin that resolves this ('make check -C tests TESTS=qemuxml2argvtest VIR_TEST_DEBUG=1'): 58) QEMU XML-2-ARGV disk-ioeventfd ... Offset 387 Expect [4,drive=drive-virtio-disk0,id=virtio-disk0 -drive file=/var/lib/libvirt/Fedora-14-x86_64-Live-KDE.iso,if=none,media=cdrom,id=drive-ide0-1-0 -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 -device virtio-net-pci,tx=bh,ioeventfd=off,vlan=0,id=net0,mac=52:54:00:e5:48:58,bus=pci.0,addr=0x3 -net user,vlan=0,name=hostnet0 -serial pty -usb -vnc 127.0.0.1:-809 -std-vga -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5] Actual [5,drive=drive-virtio-disk0,id=virtio-disk0 -drive file=/var/lib/libvirt/Fedora-14-x86_64-Live-KDE.iso,if=none,media=cdrom,id=drive-ide0-1-0 -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 -device virtio-net-pci,tx=bh,ioeventfd=off,vlan=0,id=net0,mac=52:54:00:e5:48:58,bus=pci.0,addr=0x3 -net user,vlan=0,name=hostnet0 -serial pty -usb -vnc 127.0.0.1:-809 -std-vga -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x7] ... FAILED In other words, your patch silently caused pci devices to be renumbered. If it was a bug in our tests for using the wrong addresses in the first place, then let's fix our tests; but more likely, this means your patch was incorrect and would break guest ABI stability if it were applied.