[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 6/7] the hotplugged PCI device should use the whole slot



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.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]