[libvirt] [PATCH V2 0/7] support multi function PCI device

We want to use more than 200+ device. Libvirt does not use multi function PCI device and PCI-to-PCI bridge. So we can not use more than 200+ device if it's a PCI device or it's controller is a PCI device. This patchset adds the support of multi function PCI device. It does not support to hot plug/unplug multi function PCI device. Change log: v1 -> v2: 1. split the patch, and it will be easily to review 2. Auto assign a slot not a function if user does not specify the address. Wen Congyang (7): check whether qemu supports multi function PCI device prevent hot unplugging multi function PCI device the key of hash table should include the function value Reimplement qemuDomainPCIAddressReserveSlot(): reserve all functions in the slot assign the whole slot to the PCI device that has no address the hotplugged PCI device should use the whole slot support multifunction PCI device src/conf/domain_conf.c | 3 + src/qemu/qemu_capabilities.c | 5 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 151 +++++++++++++++++++++++++++++++++++++----- src/qemu/qemu_command.h | 5 ++ src/qemu/qemu_hotplug.c | 76 +++++++++++++++++++-- tests/qemuhelptest.c | 3 +- 7 files changed, 218 insertions(+), 26 deletions(-)

qemu supports multi function PCI device after version 0.13.0. --- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + tests/qemuhelptest.c | 3 ++- 3 files changed, 8 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c69cf1d..28c89b5 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -119,6 +119,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "device-spicevmc", "virtio-tx-alg", "device-qxl-vga", + + "pci-multifunction", /* 60 */ ); struct qemu_feature_flags { @@ -1024,6 +1026,9 @@ qemuCapsComputeCmdFlags(const char *help, */ if (version >= 13000) qemuCapsSet(flags, QEMU_CAPS_MONITOR_JSON); + + if (version >= 13000) + qemuCapsSet(flags, QEMU_CAPS_PCI_MULTIFUNCTION); } /* We parse the output of 'qemu -help' to get the QEMU diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 4c1ad63..e6d2fa3 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -95,6 +95,7 @@ enum qemuCapsFlags { QEMU_CAPS_DEVICE_SPICEVMC = 57, /* older -device spicevmc*/ QEMU_CAPS_VIRTIO_TX_ALG = 58, /* -device virtio-net-pci,tx=string */ QEMU_CAPS_DEVICE_QXL_VGA = 59, /* Is the primary and vga campatible qxl device named qxl-vga? */ + QEMU_CAPS_PCI_MULTIFUNCTION = 60, /* -device multifunction=on|off */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 71780d8..327a0c7 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -430,7 +430,8 @@ mymain(void) QEMU_CAPS_VGA_NONE, QEMU_CAPS_MIGRATE_QEMU_FD, QEMU_CAPS_DRIVE_AIO, - QEMU_CAPS_DEVICE_SPICEVMC); + QEMU_CAPS_DEVICE_SPICEVMC, + QEMU_CAPS_PCI_MULTIFUNCTION); DO_TEST("qemu-kvm-0.12.1.2-rhel61", 12001, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, -- 1.7.1

On Fri, May 27, 2011 at 06:19:42PM +0800, Wen Congyang wrote:
qemu supports multi function PCI device after version 0.13.0. --- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + tests/qemuhelptest.c | 3 ++- 3 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c69cf1d..28c89b5 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -119,6 +119,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "device-spicevmc", "virtio-tx-alg", "device-qxl-vga", + + "pci-multifunction", /* 60 */ );
struct qemu_feature_flags { @@ -1024,6 +1026,9 @@ qemuCapsComputeCmdFlags(const char *help, */ if (version >= 13000) qemuCapsSet(flags, QEMU_CAPS_MONITOR_JSON); + + if (version >= 13000) + qemuCapsSet(flags, QEMU_CAPS_PCI_MULTIFUNCTION); }
/* We parse the output of 'qemu -help' to get the QEMU diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 4c1ad63..e6d2fa3 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -95,6 +95,7 @@ enum qemuCapsFlags { QEMU_CAPS_DEVICE_SPICEVMC = 57, /* older -device spicevmc*/ QEMU_CAPS_VIRTIO_TX_ALG = 58, /* -device virtio-net-pci,tx=string */ QEMU_CAPS_DEVICE_QXL_VGA = 59, /* Is the primary and vga campatible qxl device named qxl-vga? */ + QEMU_CAPS_PCI_MULTIFUNCTION = 60, /* -device multifunction=on|off */
QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 71780d8..327a0c7 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -430,7 +430,8 @@ mymain(void) QEMU_CAPS_VGA_NONE, QEMU_CAPS_MIGRATE_QEMU_FD, QEMU_CAPS_DRIVE_AIO, - QEMU_CAPS_DEVICE_SPICEVMC); + QEMU_CAPS_DEVICE_SPICEVMC, + QEMU_CAPS_PCI_MULTIFUNCTION); DO_TEST("qemu-kvm-0.12.1.2-rhel61", 12001, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT,
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 05/27/2011 04:19 AM, Wen Congyang wrote:
qemu supports multi function PCI device after version 0.13.0.
After (> 0.13.0) or at least (>= 0.13.0)?
@@ -1024,6 +1026,9 @@ qemuCapsComputeCmdFlags(const char *help, */ if (version >= 13000) qemuCapsSet(flags, QEMU_CAPS_MONITOR_JSON); + + if (version >= 13000) + qemuCapsSet(flags, QEMU_CAPS_PCI_MULTIFUNCTION);
This is a rather bad test. We should be avoiding version-based tests where possible, and instead favor -help parsing tests. Is there any device xxx where 'qemu -device xxx,?' will list multifunction? If so, qemuCapsExtractDeviceStr is the better place to modify to probe for this capability. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

At 06/03/2011 10:07 PM, Eric Blake Write:
On 05/27/2011 04:19 AM, Wen Congyang wrote:
qemu supports multi function PCI device after version 0.13.0.
After (> 0.13.0) or at least (>= 0.13.0)?
at least.
@@ -1024,6 +1026,9 @@ qemuCapsComputeCmdFlags(const char *help, */ if (version >= 13000) qemuCapsSet(flags, QEMU_CAPS_MONITOR_JSON); + + if (version >= 13000) + qemuCapsSet(flags, QEMU_CAPS_PCI_MULTIFUNCTION);
This is a rather bad test. We should be avoiding version-based tests
Yes, it is a bad test. But I do not find a better way.
where possible, and instead favor -help parsing tests. Is there any device xxx where 'qemu -device xxx,?' will list multifunction? If so,
No, for example: # /usr/local2/bin/qemu-system-x86_64 -device rtl8139,? rtl8139.mac=macaddr rtl8139.vlan=vlan rtl8139.netdev=netdev rtl8139.bootindex=int32
qemuCapsExtractDeviceStr is the better place to modify to probe for this capability.

We do not support to hot unplug multi function PCI device now. If the device is one function of multi function PCI device, we shoul not allow to hot unplugg it. --- src/qemu/qemu_hotplug.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 52 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 3cf7d35..e98c677 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1100,6 +1100,30 @@ static inline int qemuFindDisk(virDomainDefPtr def, const char *dst) return -1; } +static int qemuComparePCIDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceInfoPtr dev1, + void *opaque) +{ + virDomainDeviceInfoPtr dev2 = opaque; + + if (dev1->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI || + dev2->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + return 0; + + if (dev1->addr.pci.slot == dev2->addr.pci.slot && + dev1->addr.pci.function != dev2->addr.pci.function) + return -1; + return 0; +} + +static bool qemuIsMultiFunctionDevice(virDomainDefPtr def, + virDomainDeviceInfoPtr dev) +{ + if (virDomainDeviceInfoIterate(def, qemuComparePCIDevice, dev) < 0) + return true; + return false; +} + int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, virDomainObjPtr vm, @@ -1121,6 +1145,13 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, detach = vm->def->disks[i]; + if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot hot unplug multifunction PCI device: %s"), + dev->data.disk->dst); + goto cleanup; + } + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, @@ -1351,6 +1382,13 @@ int qemuDomainDetachPciControllerDevice(struct qemud_driver *driver, goto cleanup; } + if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot hot unplug multifunction PCI device: %s"), + dev->data.disk->dst); + goto cleanup; + } + if (qemuDomainControllerIsBusy(vm, detach)) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("device cannot be detached: device is busy")); @@ -1438,6 +1476,13 @@ int qemuDomainDetachNetDevice(struct qemud_driver *driver, goto cleanup; } + if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot hot unplug multifunction PCI device :%s"), + dev->data.disk->dst); + goto cleanup; + } + if ((vlan = qemuDomainNetVLAN(detach)) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("unable to determine original VLAN")); @@ -1567,6 +1612,13 @@ int qemuDomainDetachHostPciDevice(struct qemud_driver *driver, return -1; } + if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot hot unplug multifunction PCI device: %s"), + dev->data.disk->dst); + return -1; + } + if (!virDomainDeviceAddressIsValid(&detach->info, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { qemuReportError(VIR_ERR_OPERATION_FAILED, -- 1.7.1

On Fri, May 27, 2011 at 06:20:10PM +0800, Wen Congyang wrote:
We do not support to hot unplug multi function PCI device now. If the device is one function of multi function PCI device, we shoul not allow to hot unplugg it. --- src/qemu/qemu_hotplug.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 52 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 3cf7d35..e98c677 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1100,6 +1100,30 @@ static inline int qemuFindDisk(virDomainDefPtr def, const char *dst) return -1; }
+static int qemuComparePCIDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceInfoPtr dev1, + void *opaque) +{ + virDomainDeviceInfoPtr dev2 = opaque; + + if (dev1->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI || + dev2->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + return 0; + + if (dev1->addr.pci.slot == dev2->addr.pci.slot && + dev1->addr.pci.function != dev2->addr.pci.function) + return -1; + return 0; +} + +static bool qemuIsMultiFunctionDevice(virDomainDefPtr def, + virDomainDeviceInfoPtr dev) +{ + if (virDomainDeviceInfoIterate(def, qemuComparePCIDevice, dev) < 0) + return true; + return false; +} +
int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, virDomainObjPtr vm, @@ -1121,6 +1145,13 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver,
detach = vm->def->disks[i];
+ if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot hot unplug multifunction PCI device: %s"), + dev->data.disk->dst); + goto cleanup; + } + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, @@ -1351,6 +1382,13 @@ int qemuDomainDetachPciControllerDevice(struct qemud_driver *driver, goto cleanup; }
+ if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot hot unplug multifunction PCI device: %s"), + dev->data.disk->dst); + goto cleanup; + } + if (qemuDomainControllerIsBusy(vm, detach)) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("device cannot be detached: device is busy")); @@ -1438,6 +1476,13 @@ int qemuDomainDetachNetDevice(struct qemud_driver *driver, goto cleanup; }
+ if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot hot unplug multifunction PCI device :%s"), + dev->data.disk->dst); + goto cleanup; + } + if ((vlan = qemuDomainNetVLAN(detach)) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("unable to determine original VLAN")); @@ -1567,6 +1612,13 @@ int qemuDomainDetachHostPciDevice(struct qemud_driver *driver, return -1; }
+ if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot hot unplug multifunction PCI device: %s"), + dev->data.disk->dst); + return -1; + } + if (!virDomainDeviceAddressIsValid(&detach->info, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { qemuReportError(VIR_ERR_OPERATION_FAILED,
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Hi I applied these pacches to libvirt tree 8077d64f964705c1034555abeea38773532b762f And built a qemu-kvm from the upstream qemu-kvm tree version 0.14. I tested these new binaries on my Redhat Enterprise Linux 6.1 GA. Now I can have multiple function devices after I changed the VM XML config file manually, please see the 'lspci' output below. Also, I can hotplug one device, but it seems that unplugging it failed. ------------------------------------------------------------------------------------- [root@kvm-rhel-6 qemu]# virsh qemu-monitor-command kvm-rhel-6.1-multifunction-1 --hmp 'device_add lsi,id=scsi10,bus=pci.0,addr=0x7.0x00' VM: **In the VM, 'lspci" can see one line added: VM: **00:07.0 SCSI storage controller: LSI logic / Symbios Logic 5sc895a [root@kvm-rhel-6 qemu]# virsh qemu-monitor-command kvm-rhel-6.1-multifunction-1 --hmp 'device_del lsi,id=scsi10,bus=pci.0,addr=0x7.0x00' Device 'lsi,id=scsi10,bus=pci.0,addr=0x7.0x00' not found --------------------------------------------------------------------------------------------------------------- #lspci 00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma] (rev 02) 00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II] 00:01.1 IDE interface: Intel Coporation 82371SB PIIX3 IDE [Natoma/Triton II] 00:01.2 USB Controller: Intel Corporation 82371SB PIIX3 USB Natoma/Triton II] 00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03) 00:02.0 VGA compatible controller: Cirrus Logic GD 5446 00:03.0 Ethernet controller: Red Hat, Inc Virtio network device ---> function 0 00:03.1 Ethernet controller: Red Hat, Inc Virtio network device 00:03.2 Ethernet controller: Red Hat, Inc Virtio network device 00:03.3 Ethernet controller: Red Hat, Inc Virtio network device 00:03.4 Ethernet controller: Red Hat, Inc Virtio network device 00:03.5 Ethernet controller: Red Hat, Inc Virtio network device 00:03.6 Ethernet controller: Red Hat, Inc Virtio network device 00:03.7 Ethernet controller: Red Hat, Inc Virtio network device ---> function 7 00:04.0 Audio device: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) High 00:05.0 SCSI storage controller: Red Hat, Inc Virtio bloc device 00:06.0 RAM memory: Red Hat, Inc Virtio memory balloon Daniel P. Berrange:
On Fri, May 27, 2011 at 06:20:10PM +0800, Wen Congyang wrote:
We do not support to hot unplug multi function PCI device now. If the device is one function of multi function PCI device, we shoul not allow to hot unplugg it. --- src/qemu/qemu_hotplug.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 52 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 3cf7d35..e98c677 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1100,6 +1100,30 @@ static inline int qemuFindDisk(virDomainDefPtr def, const char *dst) return -1; }
+static int qemuComparePCIDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceInfoPtr dev1, + void *opaque) +{ + virDomainDeviceInfoPtr dev2 = opaque; + + if (dev1->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI || + dev2->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + return 0; + + if (dev1->addr.pci.slot == dev2->addr.pci.slot&& + dev1->addr.pci.function != dev2->addr.pci.function) + return -1; + return 0; +} + +static bool qemuIsMultiFunctionDevice(virDomainDefPtr def, + virDomainDeviceInfoPtr dev) +{ + if (virDomainDeviceInfoIterate(def, qemuComparePCIDevice, dev)< 0) + return true; + return false; +} +
int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, virDomainObjPtr vm, @@ -1121,6 +1145,13 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver,
detach = vm->def->disks[i];
+ if (qemuIsMultiFunctionDevice(vm->def,&detach->info)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot hot unplug multifunction PCI device: %s"), + dev->data.disk->dst); + goto cleanup; + } + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { if (virCgroupForDomain(driver->cgroup, vm->def->name,&cgroup, 0) != 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, @@ -1351,6 +1382,13 @@ int qemuDomainDetachPciControllerDevice(struct qemud_driver *driver, goto cleanup; }
+ if (qemuIsMultiFunctionDevice(vm->def,&detach->info)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot hot unplug multifunction PCI device: %s"), + dev->data.disk->dst); + goto cleanup; + } + if (qemuDomainControllerIsBusy(vm, detach)) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("device cannot be detached: device is busy")); @@ -1438,6 +1476,13 @@ int qemuDomainDetachNetDevice(struct qemud_driver *driver, goto cleanup; }
+ if (qemuIsMultiFunctionDevice(vm->def,&detach->info)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot hot unplug multifunction PCI device :%s"), + dev->data.disk->dst); + goto cleanup; + } + if ((vlan = qemuDomainNetVLAN(detach))< 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("unable to determine original VLAN")); @@ -1567,6 +1612,13 @@ int qemuDomainDetachHostPciDevice(struct qemud_driver *driver, return -1; }
+ if (qemuIsMultiFunctionDevice(vm->def,&detach->info)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot hot unplug multifunction PCI device: %s"), + dev->data.disk->dst); + return -1; + } + if (!virDomainDeviceAddressIsValid(&detach->info, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { qemuReportError(VIR_ERR_OPERATION_FAILED, ACK
Daniel

At 06/09/2011 04:37 PM, shu ming Write:
Hi I applied these pacches to libvirt tree
8077d64f964705c1034555abeea38773532b762f
And built a qemu-kvm from the upstream qemu-kvm tree version 0.14. I tested these new binaries on my Redhat Enterprise Linux 6.1 GA.
Now I can have multiple function devices after I changed the VM XML config file manually, please see the 'lspci' output below. Also, I can hotplug one device, but it seems that unplugging it failed. -------------------------------------------------------------------------------------
[root@kvm-rhel-6 qemu]# virsh qemu-monitor-command kvm-rhel-6.1-multifunction-1 --hmp 'device_add lsi,id=scsi10,bus=pci.0,addr=0x7.0x00'
VM: **In the VM, 'lspci" can see one line added:
VM: **00:07.0 SCSI storage controller: LSI logic / Symbios Logic 5sc895a
[root@kvm-rhel-6 qemu]# virsh qemu-monitor-command kvm-rhel-6.1-multifunction-1 --hmp 'device_del lsi,id=scsi10,bus=pci.0,addr=0x7.0x00'
Oh, the command is wrong. The parameters of device_del is id. Please try: 'device_del scsi10'. Thanks. Wen Congyang
Device 'lsi,id=scsi10,bus=pci.0,addr=0x7.0x00' not found
---------------------------------------------------------------------------------------------------------------
#lspci 00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma] (rev 02) 00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II] 00:01.1 IDE interface: Intel Coporation 82371SB PIIX3 IDE [Natoma/Triton II] 00:01.2 USB Controller: Intel Corporation 82371SB PIIX3 USB Natoma/Triton II] 00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03) 00:02.0 VGA compatible controller: Cirrus Logic GD 5446 00:03.0 Ethernet controller: Red Hat, Inc Virtio network device ---> function 0 00:03.1 Ethernet controller: Red Hat, Inc Virtio network device 00:03.2 Ethernet controller: Red Hat, Inc Virtio network device 00:03.3 Ethernet controller: Red Hat, Inc Virtio network device 00:03.4 Ethernet controller: Red Hat, Inc Virtio network device 00:03.5 Ethernet controller: Red Hat, Inc Virtio network device 00:03.6 Ethernet controller: Red Hat, Inc Virtio network device 00:03.7 Ethernet controller: Red Hat, Inc Virtio network device ---> function 7 00:04.0 Audio device: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) High 00:05.0 SCSI storage controller: Red Hat, Inc Virtio bloc device 00:06.0 RAM memory: Red Hat, Inc Virtio memory balloon
Daniel P. Berrange:
On Fri, May 27, 2011 at 06:20:10PM +0800, Wen Congyang wrote:
We do not support to hot unplug multi function PCI device now. If the device is one function of multi function PCI device, we shoul not allow to hot unplugg it. --- src/qemu/qemu_hotplug.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 52 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 3cf7d35..e98c677 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1100,6 +1100,30 @@ static inline int qemuFindDisk(virDomainDefPtr def, const char *dst) return -1; }
+static int qemuComparePCIDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceInfoPtr dev1, + void *opaque) +{ + virDomainDeviceInfoPtr dev2 = opaque; + + if (dev1->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI || + dev2->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + return 0; + + if (dev1->addr.pci.slot == dev2->addr.pci.slot&& + dev1->addr.pci.function != dev2->addr.pci.function) + return -1; + return 0; +} + +static bool qemuIsMultiFunctionDevice(virDomainDefPtr def, + virDomainDeviceInfoPtr dev) +{ + if (virDomainDeviceInfoIterate(def, qemuComparePCIDevice, dev)< 0) + return true; + return false; +} +
int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, virDomainObjPtr vm, @@ -1121,6 +1145,13 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver,
detach = vm->def->disks[i];
+ if (qemuIsMultiFunctionDevice(vm->def,&detach->info)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot hot unplug multifunction PCI device: %s"), + dev->data.disk->dst); + goto cleanup; + } + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { if (virCgroupForDomain(driver->cgroup, vm->def->name,&cgroup, 0) != 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, @@ -1351,6 +1382,13 @@ int qemuDomainDetachPciControllerDevice(struct qemud_driver *driver, goto cleanup; }
+ if (qemuIsMultiFunctionDevice(vm->def,&detach->info)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot hot unplug multifunction PCI device: %s"), + dev->data.disk->dst); + goto cleanup; + } + if (qemuDomainControllerIsBusy(vm, detach)) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("device cannot be detached: device is busy")); @@ -1438,6 +1476,13 @@ int qemuDomainDetachNetDevice(struct qemud_driver *driver, goto cleanup; }
+ if (qemuIsMultiFunctionDevice(vm->def,&detach->info)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot hot unplug multifunction PCI device :%s"), + dev->data.disk->dst); + goto cleanup; + } + if ((vlan = qemuDomainNetVLAN(detach))< 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("unable to determine original VLAN")); @@ -1567,6 +1612,13 @@ int qemuDomainDetachHostPciDevice(struct qemud_driver *driver, return -1; }
+ if (qemuIsMultiFunctionDevice(vm->def,&detach->info)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot hot unplug multifunction PCI device: %s"), + dev->data.disk->dst); + return -1; + } + if (!virDomainDeviceAddressIsValid(&detach->info,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { qemuReportError(VIR_ERR_OPERATION_FAILED, ACK
Daniel

Wen Congyang:
At 06/09/2011 04:37 PM, shu ming Write:
Hi I applied these pacches to libvirt tree
8077d64f964705c1034555abeea38773532b762f
And built a qemu-kvm from the upstream qemu-kvm tree version 0.14. I tested these new binaries on my Redhat Enterprise Linux 6.1 GA.
Now I can have multiple function devices after I changed the VM XML config file manually, please see the 'lspci' output below. Also, I can hotplug one device, but it seems that unplugging it failed. -------------------------------------------------------------------------------------
[root@kvm-rhel-6 qemu]# virsh qemu-monitor-command kvm-rhel-6.1-multifunction-1 --hmp 'device_add lsi,id=scsi10,bus=pci.0,addr=0x7.0x00'
VM: **In the VM, 'lspci" can see one line added:
VM: **00:07.0 SCSI storage controller: LSI logic / Symbios Logic 5sc895a
[root@kvm-rhel-6 qemu]# virsh qemu-monitor-command kvm-rhel-6.1-multifunction-1 --hmp 'device_del lsi,id=scsi10,bus=pci.0,addr=0x7.0x00' Oh, the command is wrong. The parameters of device_del is id. Please try: 'device_del scsi10'.
Thanks. Wen Congyang Yeah, unplugging it works.

We save all used PCI address in the hash table. The key is generated by domain, bus and slot now. We will support multi function PCI device, so the key should be generated by domain, bus, slot and function. --- src/qemu/qemu_command.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ef2d002..4ca6fe3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -684,10 +684,11 @@ static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev) return NULL; } - if (virAsprintf(&addr, "%d:%d:%d", + if (virAsprintf(&addr, "%d:%d:%d.%d", dev->addr.pci.domain, dev->addr.pci.bus, - dev->addr.pci.slot) < 0) { + dev->addr.pci.slot, + dev->addr.pci.function) < 0) { virReportOOMError(); return NULL; } @@ -817,6 +818,7 @@ int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs, dev.addr.pci.domain = 0; dev.addr.pci.bus = 0; dev.addr.pci.slot = slot; + dev.addr.pci.function = 0; return qemuDomainPCIAddressReserveAddr(addrs, &dev); } @@ -879,6 +881,7 @@ int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs, maybe.addr.pci.domain = 0; maybe.addr.pci.bus = 0; maybe.addr.pci.slot = i; + maybe.addr.pci.function = 0; if (!(addr = qemuPCIAddressAsString(&maybe))) return -1; -- 1.7.1

On Fri, May 27, 2011 at 06:20:41PM +0800, Wen Congyang wrote:
We save all used PCI address in the hash table. The key is generated by domain, bus and slot now. We will support multi function PCI device, so the key should be generated by domain, bus, slot and function. --- src/qemu/qemu_command.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ef2d002..4ca6fe3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -684,10 +684,11 @@ static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev) return NULL; }
- if (virAsprintf(&addr, "%d:%d:%d", + if (virAsprintf(&addr, "%d:%d:%d.%d", dev->addr.pci.domain, dev->addr.pci.bus, - dev->addr.pci.slot) < 0) { + dev->addr.pci.slot, + dev->addr.pci.function) < 0) { virReportOOMError(); return NULL; } @@ -817,6 +818,7 @@ int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs, dev.addr.pci.domain = 0; dev.addr.pci.bus = 0; dev.addr.pci.slot = slot; + dev.addr.pci.function = 0;
return qemuDomainPCIAddressReserveAddr(addrs, &dev); } @@ -879,6 +881,7 @@ int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs, maybe.addr.pci.domain = 0; maybe.addr.pci.bus = 0; maybe.addr.pci.slot = i; + maybe.addr.pci.function = 0;
if (!(addr = qemuPCIAddressAsString(&maybe))) return -1;
ACK, If we want to be really pedantic, this would be merged with patch 4, so we don't have a functionality regression at this point in the bisect, but it isn't really too important. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

We will support multi function PCI device. So we should reserve all functions in the slot if we want to reserve a slot. --- src/qemu/qemu_command.c | 37 ++++++++++++++++++++++++++++++++++--- src/qemu/qemu_command.h | 4 ++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4ca6fe3..48834f1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -667,6 +667,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virBitmapPtr qemuCaps) #define QEMU_PCI_ADDRESS_LAST_SLOT 31 +#define QEMU_PCI_ADDRESS_LAST_FUNCTION 8 struct _qemuDomainPCIAddressSet { virHashTablePtr used; int nextslot; @@ -810,19 +811,37 @@ int qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs, return 0; } -int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs, - int slot) +int qemuDomainPCIAddressReserveFunction(qemuDomainPCIAddressSetPtr addrs, + int slot, int function) { virDomainDeviceInfo dev; dev.addr.pci.domain = 0; dev.addr.pci.bus = 0; dev.addr.pci.slot = slot; - dev.addr.pci.function = 0; + dev.addr.pci.function = function; return qemuDomainPCIAddressReserveAddr(addrs, &dev); } +int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs, + int slot) +{ + int function; + + for (function = 0; function <= QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) { + if (qemuDomainPCIAddressReserveFunction(addrs, slot, function) < 0) + goto cleanup; + } + + return 0; + +cleanup: + for (function--; function >= 0; function--) { + qemuDomainPCIAddressReleaseFunction(addrs, slot, function); + } + return -1; +} int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev) @@ -853,6 +872,18 @@ int qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs, return ret; } +int qemuDomainPCIAddressReleaseFunction(qemuDomainPCIAddressSetPtr addrs, + int slot, int function) +{ + virDomainDeviceInfo dev; + + dev.addr.pci.domain = 0; + dev.addr.pci.bus = 0; + dev.addr.pci.slot = slot; + dev.addr.pci.function = function; + + return qemuDomainPCIAddressReleaseAddr(addrs, &dev); +} void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs) { diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 528031d..4c83182 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -146,6 +146,8 @@ virDomainDefPtr qemuParseCommandLineString(virCapsPtr caps, int qemuDomainAssignPCIAddresses(virDomainDefPtr def); qemuDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def); +int qemuDomainPCIAddressReserveFunction(qemuDomainPCIAddressSetPtr addrs, + int slot, int function); int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs, int slot); int qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs, @@ -156,6 +158,8 @@ int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev); int qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev); +int qemuDomainPCIAddressReleaseFunction(qemuDomainPCIAddressSetPtr addrs, + int slot, int function); void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs); int qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs); -- 1.7.1

On Fri, May 27, 2011 at 06:21:43PM +0800, Wen Congyang wrote:
We will support multi function PCI device. So we should reserve all functions in the slot if we want to reserve a slot. --- src/qemu/qemu_command.c | 37 ++++++++++++++++++++++++++++++++++--- src/qemu/qemu_command.h | 4 ++++ 2 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4ca6fe3..48834f1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -667,6 +667,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virBitmapPtr qemuCaps)
#define QEMU_PCI_ADDRESS_LAST_SLOT 31 +#define QEMU_PCI_ADDRESS_LAST_FUNCTION 8 struct _qemuDomainPCIAddressSet { virHashTablePtr used; int nextslot; @@ -810,19 +811,37 @@ int qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs, return 0; }
-int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs, - int slot) +int qemuDomainPCIAddressReserveFunction(qemuDomainPCIAddressSetPtr addrs, + int slot, int function) { virDomainDeviceInfo dev;
dev.addr.pci.domain = 0; dev.addr.pci.bus = 0; dev.addr.pci.slot = slot; - dev.addr.pci.function = 0; + dev.addr.pci.function = function;
return qemuDomainPCIAddressReserveAddr(addrs, &dev); }
+int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs, + int slot) +{ + int function; + + for (function = 0; function <= QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) { + if (qemuDomainPCIAddressReserveFunction(addrs, slot, function) < 0) + goto cleanup; + } + + return 0; + +cleanup: + for (function--; function >= 0; function--) { + qemuDomainPCIAddressReleaseFunction(addrs, slot, function); + } + return -1; +}
int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev) @@ -853,6 +872,18 @@ int qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs, return ret; }
+int qemuDomainPCIAddressReleaseFunction(qemuDomainPCIAddressSetPtr addrs, + int slot, int function) +{ + virDomainDeviceInfo dev; + + dev.addr.pci.domain = 0; + dev.addr.pci.bus = 0; + dev.addr.pci.slot = slot; + dev.addr.pci.function = function; + + return qemuDomainPCIAddressReleaseAddr(addrs, &dev); +}
void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs) { diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 528031d..4c83182 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -146,6 +146,8 @@ virDomainDefPtr qemuParseCommandLineString(virCapsPtr caps,
int qemuDomainAssignPCIAddresses(virDomainDefPtr def); qemuDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def); +int qemuDomainPCIAddressReserveFunction(qemuDomainPCIAddressSetPtr addrs, + int slot, int function); int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs, int slot); int qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs, @@ -156,6 +158,8 @@ int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev); int qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev); +int qemuDomainPCIAddressReleaseFunction(qemuDomainPCIAddressSetPtr addrs, + int slot, int function);
void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs); int qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs);
ACK, much simpler than I was fearing this would be Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

If user does not specify the PCI address, we should auto assign an unused slot. --- src/qemu/qemu_command.c | 36 ++++++++++++++++++++++++++++++++---- 1 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 48834f1..6f9540c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -779,6 +779,35 @@ error: return NULL; } +/* check whether the slot is used by the other device + * Return 0 if the slot is not used by the other device, or -1 if the slot + * is used by the other device. + */ +static int qemuDomainPCIAddressCheckSlot(qemuDomainPCIAddressSetPtr addrs, + virDomainDeviceInfoPtr dev) +{ + char *addr; + virDomainDeviceInfo temp_dev; + int function; + + temp_dev = *dev; + for (function = 0; function < QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) { + temp_dev.addr.pci.function = function; + addr = qemuPCIAddressAsString(&temp_dev); + if (!addr) + return -1; + + if (virHashLookup(addrs->used, addr)) { + VIR_FREE(addr); + return -1; + } + + VIR_FREE(addr); + } + + return 0; +} + int qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev) { @@ -917,18 +946,17 @@ int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs, if (!(addr = qemuPCIAddressAsString(&maybe))) return -1; - if (virHashLookup(addrs->used, addr)) { + if (qemuDomainPCIAddressCheckSlot(addrs, &maybe) < 0) { VIR_DEBUG("PCI addr %s already in use", addr); VIR_FREE(addr); continue; } VIR_DEBUG("Allocating PCI addr %s", addr); + VIR_FREE(addr); - if (virHashAddEntry(addrs->used, addr, addr) < 0) { - VIR_FREE(addr); + if (qemuDomainPCIAddressReserveSlot(addrs, i) < 0) return -1; - } dev->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; dev->addr.pci = maybe.addr.pci; -- 1.7.1

On Fri, May 27, 2011 at 06:22:08PM +0800, Wen Congyang wrote:
If user does not specify the PCI address, we should auto assign an unused slot. --- src/qemu/qemu_command.c | 36 ++++++++++++++++++++++++++++++++---- 1 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 48834f1..6f9540c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -779,6 +779,35 @@ error: return NULL; }
+/* check whether the slot is used by the other device + * Return 0 if the slot is not used by the other device, or -1 if the slot + * is used by the other device. + */ +static int qemuDomainPCIAddressCheckSlot(qemuDomainPCIAddressSetPtr addrs, + virDomainDeviceInfoPtr dev) +{ + char *addr; + virDomainDeviceInfo temp_dev; + int function; + + temp_dev = *dev; + for (function = 0; function < QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) { + temp_dev.addr.pci.function = function; + addr = qemuPCIAddressAsString(&temp_dev); + if (!addr) + return -1; + + if (virHashLookup(addrs->used, addr)) { + VIR_FREE(addr); + return -1; + } + + VIR_FREE(addr); + } + + return 0; +} + int qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev) { @@ -917,18 +946,17 @@ int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs, if (!(addr = qemuPCIAddressAsString(&maybe))) return -1;
- if (virHashLookup(addrs->used, addr)) { + if (qemuDomainPCIAddressCheckSlot(addrs, &maybe) < 0) { VIR_DEBUG("PCI addr %s already in use", addr); VIR_FREE(addr); continue; }
VIR_DEBUG("Allocating PCI addr %s", addr); + VIR_FREE(addr);
- if (virHashAddEntry(addrs->used, addr, addr) < 0) { - VIR_FREE(addr); + if (qemuDomainPCIAddressReserveSlot(addrs, i) < 0) return -1; - }
dev->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; dev->addr.pci = maybe.addr.pci;
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Hot pluging/unpluging multi PCI device is not supported now. So the function of hotplugged PCI device must be 0. When we hot unplug it, we should set release all functions in the slot. --- src/qemu/qemu_command.c | 46 +++++++++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_command.h | 1 + src/qemu/qemu_hotplug.c | 24 ++++++++++++++++-------- 3 files changed, 60 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6f9540c..da18719 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -876,9 +876,19 @@ int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev) { int ret = 0; - if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) - ret = qemuDomainPCIAddressReserveAddr(addrs, dev); - else + if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + /* We do not support hotplug multi-function PCI device now, so we should + * reserve the whole slot. The function of the PCI device must be 0. + */ + if (dev->addr.pci.function != 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only PCI device addresses with function=0" + " are supported")); + return -1; + } + + ret = qemuDomainPCIAddressReserveSlot(addrs, dev->addr.pci.slot); + } else ret = qemuDomainPCIAddressSetNextAddr(addrs, dev); return ret; } @@ -914,6 +924,36 @@ int qemuDomainPCIAddressReleaseFunction(qemuDomainPCIAddressSetPtr addrs, return qemuDomainPCIAddressReleaseAddr(addrs, &dev); } +int qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs, int slot) +{ + virDomainDeviceInfo dev; + char *addr; + int function; + int ret = 0; + + dev.addr.pci.domain = 0; + dev.addr.pci.bus = 0; + dev.addr.pci.slot = slot; + + for (function = 0; function <= QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) { + addr = qemuPCIAddressAsString(&dev); + if (!addr) + return -1; + + if (!virHashLookup(addrs->used, addr)) { + VIR_FREE(addr); + continue; + } + + VIR_FREE(addr); + + if (qemuDomainPCIAddressReleaseFunction(addrs, slot, function) < 0) + ret = -1; + } + + return ret; +} + void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs) { if (!addrs) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 4c83182..d7673ad 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -160,6 +160,7 @@ int qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev); int qemuDomainPCIAddressReleaseFunction(qemuDomainPCIAddressSetPtr addrs, int slot, int function); +int qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs, int slot); void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs); int qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e98c677..9f0ec06 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -221,7 +221,8 @@ error: if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && releaseaddr && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &disk->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + disk->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on %s", disk->src); if (virSecurityManagerRestoreImageLabel(driver->securityManager, @@ -290,7 +291,8 @@ cleanup: qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && releaseaddr && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &controller->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + controller->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on controller"); VIR_FREE(devstr); @@ -697,7 +699,8 @@ cleanup: qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && releaseaddr && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &net->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + net->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on NIC"); if (ret != 0) @@ -828,7 +831,8 @@ error: if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && (hostdev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && releaseaddr && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &hostdev->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + hostdev->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on host device"); qemuDomainReAttachHostdevDevices(driver, &hostdev, 1); @@ -1198,7 +1202,8 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, qemuAuditDisk(vm, detach, NULL, "detach", ret >= 0); if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + detach->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on %s", dev->data.disk->src); virDomainDiskRemove(vm->def, i); @@ -1430,7 +1435,8 @@ int qemuDomainDetachPciControllerDevice(struct qemud_driver *driver, } if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + detach->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on controller"); virDomainControllerDefFree(detach); @@ -1529,7 +1535,8 @@ int qemuDomainDetachNetDevice(struct qemud_driver *driver, qemuAuditNet(vm, detach, NULL, "detach", true); if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + detach->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on NIC"); virDomainConfNWFilterTeardown(detach); @@ -1653,7 +1660,8 @@ int qemuDomainDetachHostPciDevice(struct qemud_driver *driver, } if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + detach->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on host device"); if (vm->def->nhostdevs > 1) { -- 1.7.1

On Fri, May 27, 2011 at 06:22:34PM +0800, Wen Congyang wrote:
Hot pluging/unpluging multi PCI device is not supported now. So the function of hotplugged PCI device must be 0. When we hot unplug it, we should set release all functions in the slot. --- src/qemu/qemu_command.c | 46 +++++++++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_command.h | 1 + src/qemu/qemu_hotplug.c | 24 ++++++++++++++++-------- 3 files changed, 60 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6f9540c..da18719 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -876,9 +876,19 @@ int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev) { int ret = 0; - if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) - ret = qemuDomainPCIAddressReserveAddr(addrs, dev); - else + if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + /* We do not support hotplug multi-function PCI device now, so we should + * reserve the whole slot. The function of the PCI device must be 0. + */ + if (dev->addr.pci.function != 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only PCI device addresses with function=0" + " are supported")); + return -1; + } + + ret = qemuDomainPCIAddressReserveSlot(addrs, dev->addr.pci.slot); + } else ret = qemuDomainPCIAddressSetNextAddr(addrs, dev);
Since the if() uses {}, you should add {} around the else clause too.
return ret; } @@ -914,6 +924,36 @@ int qemuDomainPCIAddressReleaseFunction(qemuDomainPCIAddressSetPtr addrs, return qemuDomainPCIAddressReleaseAddr(addrs, &dev); }
+int qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs, int slot) +{ + virDomainDeviceInfo dev; + char *addr; + int function; + int ret = 0; + + dev.addr.pci.domain = 0; + dev.addr.pci.bus = 0; + dev.addr.pci.slot = slot; + + for (function = 0; function <= QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) { + addr = qemuPCIAddressAsString(&dev); + if (!addr) + return -1; + + if (!virHashLookup(addrs->used, addr)) { + VIR_FREE(addr); + continue; + } + + VIR_FREE(addr); + + if (qemuDomainPCIAddressReleaseFunction(addrs, slot, function) < 0) + ret = -1; + } + + return ret; +} + void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs) { if (!addrs) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 4c83182..d7673ad 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -160,6 +160,7 @@ int qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev); int qemuDomainPCIAddressReleaseFunction(qemuDomainPCIAddressSetPtr addrs, int slot, int function); +int qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs, int slot);
void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs); int qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e98c677..9f0ec06 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -221,7 +221,8 @@ error: if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && releaseaddr && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &disk->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + disk->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on %s", disk->src);
if (virSecurityManagerRestoreImageLabel(driver->securityManager, @@ -290,7 +291,8 @@ cleanup: qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && releaseaddr && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &controller->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + controller->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on controller");
VIR_FREE(devstr); @@ -697,7 +699,8 @@ cleanup: qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && releaseaddr && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &net->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + net->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on NIC");
if (ret != 0) @@ -828,7 +831,8 @@ error: if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && (hostdev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && releaseaddr && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &hostdev->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + hostdev->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on host device");
qemuDomainReAttachHostdevDevices(driver, &hostdev, 1); @@ -1198,7 +1202,8 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, qemuAuditDisk(vm, detach, NULL, "detach", ret >= 0);
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + detach->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on %s", dev->data.disk->src);
virDomainDiskRemove(vm->def, i); @@ -1430,7 +1435,8 @@ int qemuDomainDetachPciControllerDevice(struct qemud_driver *driver, }
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + detach->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on controller");
virDomainControllerDefFree(detach); @@ -1529,7 +1535,8 @@ int qemuDomainDetachNetDevice(struct qemud_driver *driver, qemuAuditNet(vm, detach, NULL, "detach", true);
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + detach->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on NIC");
virDomainConfNWFilterTeardown(detach); @@ -1653,7 +1660,8 @@ int qemuDomainDetachHostPciDevice(struct qemud_driver *driver, }
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + detach->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on host device");
if (vm->def->nhostdevs > 1) {
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

At 06/03/2011 09:28 PM, Daniel P. Berrange Write:
On Fri, May 27, 2011 at 06:22:34PM +0800, Wen Congyang wrote:
Hot pluging/unpluging multi PCI device is not supported now. So the function of hotplugged PCI device must be 0. When we hot unplug it, we should set release all functions in the slot. --- src/qemu/qemu_command.c | 46 +++++++++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_command.h | 1 + src/qemu/qemu_hotplug.c | 24 ++++++++++++++++-------- 3 files changed, 60 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6f9540c..da18719 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -876,9 +876,19 @@ int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev) { int ret = 0; - if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) - ret = qemuDomainPCIAddressReserveAddr(addrs, dev); - else + if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + /* We do not support hotplug multi-function PCI device now, so we should + * reserve the whole slot. The function of the PCI device must be 0. + */ + if (dev->addr.pci.function != 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only PCI device addresses with function=0" + " are supported")); + return -1; + } + + ret = qemuDomainPCIAddressReserveSlot(addrs, dev->addr.pci.slot); + } else ret = qemuDomainPCIAddressSetNextAddr(addrs, dev);
Since the if() uses {}, you should add {} around the else clause too.
return ret; } @@ -914,6 +924,36 @@ int qemuDomainPCIAddressReleaseFunction(qemuDomainPCIAddressSetPtr addrs, return qemuDomainPCIAddressReleaseAddr(addrs, &dev); }
+int qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs, int slot) +{ + virDomainDeviceInfo dev; + char *addr; + int function; + int ret = 0; + + dev.addr.pci.domain = 0; + dev.addr.pci.bus = 0; + dev.addr.pci.slot = slot; + + for (function = 0; function <= QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) { + addr = qemuPCIAddressAsString(&dev); + if (!addr) + return -1; + + if (!virHashLookup(addrs->used, addr)) { + VIR_FREE(addr); + continue; + } + + VIR_FREE(addr); + + if (qemuDomainPCIAddressReleaseFunction(addrs, slot, function) < 0) + ret = -1; + } + + return ret; +} + void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs) { if (!addrs) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 4c83182..d7673ad 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -160,6 +160,7 @@ int qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev); int qemuDomainPCIAddressReleaseFunction(qemuDomainPCIAddressSetPtr addrs, int slot, int function); +int qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs, int slot);
void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs); int qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e98c677..9f0ec06 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -221,7 +221,8 @@ error: if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && releaseaddr && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &disk->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + disk->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on %s", disk->src);
if (virSecurityManagerRestoreImageLabel(driver->securityManager, @@ -290,7 +291,8 @@ cleanup: qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && releaseaddr && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &controller->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + controller->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on controller");
VIR_FREE(devstr); @@ -697,7 +699,8 @@ cleanup: qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && releaseaddr && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &net->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + net->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on NIC");
if (ret != 0) @@ -828,7 +831,8 @@ error: if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && (hostdev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && releaseaddr && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &hostdev->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + hostdev->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on host device");
qemuDomainReAttachHostdevDevices(driver, &hostdev, 1); @@ -1198,7 +1202,8 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, qemuAuditDisk(vm, detach, NULL, "detach", ret >= 0);
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + detach->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on %s", dev->data.disk->src);
virDomainDiskRemove(vm->def, i); @@ -1430,7 +1435,8 @@ int qemuDomainDetachPciControllerDevice(struct qemud_driver *driver, }
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + detach->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on controller");
virDomainControllerDefFree(detach); @@ -1529,7 +1535,8 @@ int qemuDomainDetachNetDevice(struct qemud_driver *driver, qemuAuditNet(vm, detach, NULL, "detach", true);
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + detach->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on NIC");
virDomainConfNWFilterTeardown(detach); @@ -1653,7 +1660,8 @@ int qemuDomainDetachHostPciDevice(struct qemud_driver *driver, }
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0) + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + detach->info.addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on host device");
if (vm->def->nhostdevs > 1) {
ACK
Daniel
I addressed this comment, rebased this patch and pushed this series patch. Thanks for reviewing it.

If qemu supports multi function PCI device, the format of the PCI address passed to qemu is "bus=pci.0,multifunction=on,addr=slot.function". If qemu does not support multi function PCI device, the format of the PCI address passed to qemu is "bus=pci.0,addr=slot". --- src/conf/domain_conf.c | 3 +++ src/qemu/qemu_command.c | 27 +++++++++++++++++++++------ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8ff155b..6d70efd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1296,6 +1296,9 @@ int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr) { + /* PCI bus has 32 slots and 8 functions per slot */ + if (addr->slot >= 32 || addr->function >= 8) + return 0; return addr->domain || addr->bus || addr->slot; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index da18719..21c7cdb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1251,10 +1251,20 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, _("Only PCI device addresses with bus=0 are supported")); return -1; } - if (info->addr.pci.function != 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Only PCI device addresses with function=0 are supported")); - return -1; + if (qemuCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIFUNCTION)) { + if (info->addr.pci.function > 7) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("The function of PCI device addresses must " + "less than 8")); + return -1; + } + } else { + if (info->addr.pci.function != 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only PCI device addresses with function=0 " + "are supported")); + return -1; + } } /* XXX @@ -1264,9 +1274,14 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, * to pciNN.0 where NN is the domain number */ if (qemuCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS)) - virBufferAsprintf(buf, ",bus=pci.0,addr=0x%x", info->addr.pci.slot); + virBufferAsprintf(buf, ",bus=pci.0"); + else + virBufferAsprintf(buf, ",bus=pci"); + if (qemuCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIFUNCTION)) + virBufferAsprintf(buf, ",multifunction=on,addr=0x%x.0x%x", + info->addr.pci.slot, info->addr.pci.function); else - virBufferAsprintf(buf, ",bus=pci,addr=0x%x", info->addr.pci.slot); + virBufferAsprintf(buf, ",addr=0x%x", info->addr.pci.slot); } return 0; } -- 1.7.1

On Fri, May 27, 2011 at 06:23:16PM +0800, Wen Congyang wrote:
If qemu supports multi function PCI device, the format of the PCI address passed to qemu is "bus=pci.0,multifunction=on,addr=slot.function".
If qemu does not support multi function PCI device, the format of the PCI address passed to qemu is "bus=pci.0,addr=slot". --- src/conf/domain_conf.c | 3 +++ src/qemu/qemu_command.c | 27 +++++++++++++++++++++------ 2 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8ff155b..6d70efd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1296,6 +1296,9 @@ int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info,
int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr) { + /* PCI bus has 32 slots and 8 functions per slot */ + if (addr->slot >= 32 || addr->function >= 8) + return 0; return addr->domain || addr->bus || addr->slot; }
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index da18719..21c7cdb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1251,10 +1251,20 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, _("Only PCI device addresses with bus=0 are supported")); return -1; } - if (info->addr.pci.function != 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Only PCI device addresses with function=0 are supported")); - return -1; + if (qemuCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIFUNCTION)) { + if (info->addr.pci.function > 7) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("The function of PCI device addresses must " + "less than 8")); + return -1; + } + } else { + if (info->addr.pci.function != 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only PCI device addresses with function=0 " + "are supported")); + return -1; + } }
/* XXX @@ -1264,9 +1274,14 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, * to pciNN.0 where NN is the domain number */ if (qemuCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS)) - virBufferAsprintf(buf, ",bus=pci.0,addr=0x%x", info->addr.pci.slot); + virBufferAsprintf(buf, ",bus=pci.0"); + else + virBufferAsprintf(buf, ",bus=pci"); + if (qemuCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIFUNCTION)) + virBufferAsprintf(buf, ",multifunction=on,addr=0x%x.0x%x", + info->addr.pci.slot, info->addr.pci.function); else - virBufferAsprintf(buf, ",bus=pci,addr=0x%x", info->addr.pci.slot); + virBufferAsprintf(buf, ",addr=0x%x", info->addr.pci.slot); } return 0; }
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 05/27/2011 04:23 AM, Wen Congyang wrote:
If qemu supports multi function PCI device, the format of the PCI address passed to qemu is "bus=pci.0,multifunction=on,addr=slot.function".
If qemu does not support multi function PCI device, the format of the PCI address passed to qemu is "bus=pci.0,addr=slot". --- src/conf/domain_conf.c | 3 +++ src/qemu/qemu_command.c | 27 +++++++++++++++++++++------ 2 files changed, 24 insertions(+), 6 deletions(-)
Missing a testsuite addition. Please add a .xml and .args file in tests/qemuxml2argvdata, along with a line in tests/qemuxml2argvtest.c to run the test, where you prove that the new multifunction=on phrase appears in the .args file. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Fri, May 27, 2011 at 06:17:21PM +0800, Wen Congyang wrote:
We want to use more than 200+ device. Libvirt does not use multi function PCI device and PCI-to-PCI bridge. So we can not use more than 200+ device if it's a PCI device or it's controller is a PCI device.
This patchset adds the support of multi function PCI device. It does not support to hot plug/unplug multi function PCI device.
Change log: v1 -> v2: 1. split the patch, and it will be easily to review 2. Auto assign a slot not a function if user does not specify the address.
This series looks good to me, but please wait until after this current release is done before pushing to GIT Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

--- .../qemuxml2argv-multifunction-pci-device.args | 15 ++++++ .../qemuxml2argv-multifunction-pci-device.xml | 51 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 4 ++ 3 files changed, 70 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.args b/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.args new file mode 100644 index 0000000..ff229f2 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.args @@ -0,0 +1,15 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ +-device lsi,id=scsi0,bus=pci.0,multifunction=on,addr=0x3.0x0 \ +-device lsi,id=scsi1,bus=pci.0,multifunction=on,addr=0x4.0x0 \ +-device lsi,id=scsi2,bus=pci.0,multifunction=on,addr=0x4.0x1 \ +-device lsi,id=scsi3,bus=pci.0,multifunction=on,addr=0x4.0x2 \ +-device lsi,id=scsi4,bus=pci.0,multifunction=on,addr=0x4.0x3 \ +-device lsi,id=scsi5,bus=pci.0,multifunction=on,addr=0x4.0x4 \ +-device lsi,id=scsi6,bus=pci.0,multifunction=on,addr=0x4.0x5 \ +-device lsi,id=scsi7,bus=pci.0,multifunction=on,addr=0x4.0x6 \ +-device lsi,id=scsi8,bus=pci.0,multifunction=on,addr=0x4.0x7 \ +-drive file=/tmp/scsidisk.img,if=none,id=drive-scsi0-0-0 \ +-device scsi-disk,bus=scsi0.0,scsi-id=0,drive=drive-scsi0-0-0,id=scsi0-0-0 \ +-usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,multifunction=on,addr=0x5.0x0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.xml b/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.xml new file mode 100644 index 0000000..672fb61 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.xml @@ -0,0 +1,51 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219136</memory> + <currentMemory>219136</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='file' device='disk'> + <source file='/tmp/scsidisk.img'/> + <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='scsi' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </controller> + <controller type='scsi' index='1'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </controller> + <controller type='scsi' index='2'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x1'/> + </controller> + <controller type='scsi' index='3'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x2'/> + </controller> + <controller type='scsi' index='4'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x3'/> + </controller> + <controller type='scsi' index='5'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x4'/> + </controller> + <controller type='scsi' index='6'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x5'/> + </controller> + <controller type='scsi' index='7'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x6'/> + </controller> + <controller type='scsi' index='8'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x7'/> + </controller> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index cbd9a5f..b8fd468 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -503,6 +503,10 @@ mymain(void) DO_TEST("blkiotune", false, QEMU_CAPS_NAME); DO_TEST("cputune", false, QEMU_CAPS_NAME); + DO_TEST("multifunction-pci-device", false, + QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_PCI_MULTIFUNCTION); + free(driver.stateDir); virCapabilitiesFree(driver.caps); free(map); -- 1.7.1
participants (4)
-
Daniel P. Berrange
-
Eric Blake
-
shu ming
-
Wen Congyang