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

[Eric Blake]

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<xuhj at linux.vnet.ibm.com>
> ---
>   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.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org