[libvirt] [PATCH 1/2] qemu: switch PCI address alocation to use virDevicePCIAddress

Laine Stump laine at laine.org
Tue Feb 19 03:02:23 UTC 2013


On 02/15/2013 03:22 AM, Ján Tomko wrote:
> Some functions were using virDomainDeviceInfo where virDevicePCIAddress
> would suffice. Some were only using integers for slots and functions,
> assuming the bus numbers are always 0.
>
> Switch from virDomainDeviceInfoPtr to virDevicePCIAddressPtr:
> qemuPCIAddressAsString
> qemuDomainPCIAddressCheckSlot
> qemuDomainPCIAddressReserveAddr
> qemuDomainPCIAddressReleaseAddr
>
> Switch from int slot to virDevicePCIAddressPtr:
> qemuDomainPCIAddressReserveSlot
> qemuDomainPCIAddressReleaseSlot
> qemuDomainPCIAddressGetNextSlot
>
> Deleted functions (they would take the same parameters
> as ReserveAddr/ReleaseAddr do now.)
> qemuDomainPCIAddressReserveFunction
> qemuDomainPCIAddressReleaseFunction
> ---
>  src/qemu/qemu_command.c | 267 +++++++++++++++++++++---------------------------
>  src/qemu/qemu_command.h |  13 +--
>  src/qemu/qemu_hotplug.c |  16 +--
>  3 files changed, 127 insertions(+), 169 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 6c28123..b50e779 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -953,30 +953,30 @@ cleanup:
>  #define QEMU_PCI_ADDRESS_LAST_FUNCTION 8
>  struct _qemuDomainPCIAddressSet {
>      virHashTablePtr used;
> -    int nextslot;
> +    virDevicePCIAddress lastaddr;
>  };
>  
>  
> -static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev)
> +static char *qemuPCIAddressAsString(virDevicePCIAddressPtr addr)
>  {
> -    char *addr;
> +    char *str;
>  
> -    if (dev->addr.pci.domain != 0 ||
> -        dev->addr.pci.bus != 0) {
> +    if (addr->domain != 0 ||
> +        addr->bus != 0) {
>          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                         _("Only PCI domain 0 and bus 0 are available"));
>          return NULL;
>      }
>  
> -    if (virAsprintf(&addr, "%d:%d:%d.%d",
> -                    dev->addr.pci.domain,
> -                    dev->addr.pci.bus,
> -                    dev->addr.pci.slot,
> -                    dev->addr.pci.function) < 0) {
> +    if (virAsprintf(&str, "%d:%d:%d.%d",
> +                    addr->domain,
> +                    addr->bus,
> +                    addr->slot,
> +                    addr->function) < 0) {
>          virReportOOMError();
>          return NULL;
>      }
> -    return addr;
> +    return str;
>  }
>  
>  
> @@ -999,7 +999,7 @@ static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
>          return 0;
>      }
>  
> -    addr = qemuPCIAddressAsString(info);
> +    addr = qemuPCIAddressAsString(&info->addr.pci);
>      if (!addr)
>          goto cleanup;
>  
> @@ -1024,12 +1024,11 @@ static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
>      if ((info->addr.pci.function == 0) &&
>          (info->addr.pci.multi != VIR_DEVICE_ADDRESS_PCI_MULTI_ON)) {
>          /* a function 0 w/o multifunction=on must reserve the entire slot */
> -        int function;
> -        virDomainDeviceInfo temp_info = *info;
> +        virDevicePCIAddress tmp_addr = info->addr.pci;
> +        unsigned int *func = &tmp_addr.function;
>  
> -        for (function = 1; function < QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) {
> -            temp_info.addr.pci.function = function;
> -            addr = qemuPCIAddressAsString(&temp_info);
> +        for (*func = 1; *func < QEMU_PCI_ADDRESS_LAST_FUNCTION; (*func)++) {
> +            addr = qemuPCIAddressAsString(&tmp_addr);
>              if (!addr)
>                  goto cleanup;
>  
> @@ -1142,90 +1141,75 @@ error:
>   * is used by the other device.
>   */
>  static int qemuDomainPCIAddressCheckSlot(qemuDomainPCIAddressSetPtr addrs,
> -                                         virDomainDeviceInfoPtr dev)
> +                                         virDevicePCIAddressPtr addr)
>  {
> -    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)
> +    char *str;
> +    virDevicePCIAddress tmp_addr = *addr;
> +    unsigned int *func = &(tmp_addr.function);
> +
> +    for (*func = 0; *func < QEMU_PCI_ADDRESS_LAST_FUNCTION; (*func)++) {
> +        str = qemuPCIAddressAsString(&tmp_addr);
> +        if (!str)
>              return -1;
>  
> -        if (virHashLookup(addrs->used, addr)) {
> -            VIR_FREE(addr);
> +        if (virHashLookup(addrs->used, str)) {
> +            VIR_FREE(str);
>              return -1;
>          }
>  
> -        VIR_FREE(addr);
> +        VIR_FREE(str);
>      }
>  
>      return 0;
>  }
>  
>  int qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs,
> -                                    virDomainDeviceInfoPtr dev)
> +                                    virDevicePCIAddressPtr addr)
>  {
> -    char *addr;
> +    char *str;
>  
> -    addr = qemuPCIAddressAsString(dev);
> -    if (!addr)
> +    str = qemuPCIAddressAsString(addr);
> +    if (!str)
>          return -1;
>  
> -    VIR_DEBUG("Reserving PCI addr %s", addr);
> +    VIR_DEBUG("Reserving PCI addr %s", str);
>  
> -    if (virHashLookup(addrs->used, addr)) {
> +    if (virHashLookup(addrs->used, str)) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("unable to reserve PCI address %s"), addr);
> -        VIR_FREE(addr);
> +                       _("unable to reserve PCI address %s"), str);
> +        VIR_FREE(str);
>          return -1;
>      }
>  
> -    if (virHashAddEntry(addrs->used, addr, addr)) {
> -        VIR_FREE(addr);
> +    if (virHashAddEntry(addrs->used, str, str)) {
> +        VIR_FREE(str);
>          return -1;
>      }
>  
> -    if (dev->addr.pci.slot > addrs->nextslot) {
> -        addrs->nextslot = dev->addr.pci.slot + 1;
> -        if (QEMU_PCI_ADDRESS_LAST_SLOT < addrs->nextslot)
> -            addrs->nextslot = 0;
> -    }
> -
> +    addrs->lastaddr = *addr;
> +    addrs->lastaddr.function = 0;
> +    addrs->lastaddr.multi = 0;
>      return 0;
>  }
>  
> -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 = function;
> -
> -    return qemuDomainPCIAddressReserveAddr(addrs, &dev);
> -}
> -
>  int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs,
> -                                    int slot)
> +                                    virDevicePCIAddressPtr addr)
>  {
> -    int function;
> +    virDevicePCIAddress tmp_addr = *addr;
> +    unsigned int *func = &tmp_addr.function;
> +    int i;
>  
> -    for (function = 0; function < QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) {
> -        if (qemuDomainPCIAddressReserveFunction(addrs, slot, function) < 0)
> +    for (*func = 0; *func < QEMU_PCI_ADDRESS_LAST_FUNCTION; (*func)++) {
> +        if (qemuDomainPCIAddressReserveAddr(addrs, &tmp_addr) < 0)
>              goto cleanup;
>      }
>  
>      return 0;
>  
>  cleanup:
> -    for (function--; function >= 0; function--) {
> -        qemuDomainPCIAddressReleaseFunction(addrs, slot, function);
> +    for (i = *func; i >= 0; i--) {
> +        *func = i;
> +        qemuDomainPCIAddressReleaseAddr(addrs, &tmp_addr);

The original loop started at function - 1 and continued while function
>= 0. The new loop starts at function and continues while function >= 0
- it does one extra call at the beginning.

>      }
>      return -1;
>  }
> @@ -1245,7 +1229,7 @@ int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs,
>              return -1;
>          }
>  
> -        ret = qemuDomainPCIAddressReserveSlot(addrs, dev->addr.pci.slot);
> +        ret = qemuDomainPCIAddressReserveSlot(addrs, &dev->addr.pci);
>      } else {
>          ret = qemuDomainPCIAddressSetNextAddr(addrs, dev);
>      }
> @@ -1254,59 +1238,43 @@ int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs,
>  
>  
>  int qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs,
> -                                    virDomainDeviceInfoPtr dev)
> +                                    virDevicePCIAddressPtr addr)
>  {
> -    char *addr;
> +    char *str;
>      int ret;
>  
> -    addr = qemuPCIAddressAsString(dev);
> -    if (!addr)
> +    str = qemuPCIAddressAsString(addr);
> +    if (!str)
>          return -1;
>  
> -    ret = virHashRemoveEntry(addrs->used, addr);
> +    ret = virHashRemoveEntry(addrs->used, str);
>  
> -    VIR_FREE(addr);
> +    VIR_FREE(str);
>  
>      return ret;
>  }
>  
> -int qemuDomainPCIAddressReleaseFunction(qemuDomainPCIAddressSetPtr addrs,
> -                                        int slot, int function)
> +int qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs,
> +                                    virDevicePCIAddressPtr addr)
>  {
> -    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);
> -}
> -
> -int qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs, int slot)
> -{
> -    virDomainDeviceInfo dev;
> -    char *addr;
> +    char *str;
>      int ret = 0;
> -    unsigned int *function = &dev.addr.pci.function;
> -
> -    dev.addr.pci.domain = 0;
> -    dev.addr.pci.bus = 0;
> -    dev.addr.pci.slot = slot;
> +    virDevicePCIAddress tmp_addr = *addr;
> +    unsigned int *func = &tmp_addr.function;
>  
> -    for (*function = 0; *function < QEMU_PCI_ADDRESS_LAST_FUNCTION; (*function)++) {
> -        addr = qemuPCIAddressAsString(&dev);
> -        if (!addr)
> +    for (*func = 0; *func < QEMU_PCI_ADDRESS_LAST_FUNCTION; (*func)++) {
> +        str = qemuPCIAddressAsString(&tmp_addr);
> +        if (!str)
>              return -1;
>  
> -        if (!virHashLookup(addrs->used, addr)) {
> -            VIR_FREE(addr);
> +        if (!virHashLookup(addrs->used, str)) {
> +            VIR_FREE(str);
>              continue;
>          }
>  
> -        VIR_FREE(addr);
> +        VIR_FREE(str);
>  
> -        if (qemuDomainPCIAddressReleaseFunction(addrs, slot, *function) < 0)
> +        if (qemuDomainPCIAddressReleaseAddr(addrs, &tmp_addr) < 0)
>              ret = -1;
>      }
>  
> @@ -1323,28 +1291,24 @@ void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs)
>  }
>  
>  
> -static int qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs)
> +static int
> +qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs,
> +                                virDevicePCIAddressPtr next_addr)
>  {
> +    virDevicePCIAddress tmp_addr = addrs->lastaddr;
>      int i;
> -    int iteration;
> -
> -    for (i = addrs->nextslot, iteration = 0;
> -         iteration <= QEMU_PCI_ADDRESS_LAST_SLOT; i++, iteration++) {
> -        virDomainDeviceInfo maybe;
> -        char *addr;
> +    char *addr;
>  
> -        if (QEMU_PCI_ADDRESS_LAST_SLOT < i)
> -            i = 0;
> -        memset(&maybe, 0, sizeof(maybe));
> -        maybe.addr.pci.domain = 0;
> -        maybe.addr.pci.bus = 0;
> -        maybe.addr.pci.slot = i;
> -        maybe.addr.pci.function = 0;
> +    tmp_addr.slot++;
> +    for (i = 0; i <= QEMU_PCI_ADDRESS_LAST_SLOT; i++, tmp_addr.slot++) {
> +        if (QEMU_PCI_ADDRESS_LAST_SLOT < tmp_addr.slot) {
> +            tmp_addr.slot = 0;
> +        }
>  
> -        if (!(addr = qemuPCIAddressAsString(&maybe)))
> +        if (!(addr = qemuPCIAddressAsString(&tmp_addr)))
>              return -1;
>  
> -        if (qemuDomainPCIAddressCheckSlot(addrs, &maybe) < 0) {
> +        if (qemuDomainPCIAddressCheckSlot(addrs, &tmp_addr) < 0) {
>              VIR_DEBUG("PCI addr %s already in use", addr);
>              VIR_FREE(addr);
>              continue;
> @@ -1353,7 +1317,9 @@ static int qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs)
>          VIR_DEBUG("Found free PCI addr %s", addr);
>          VIR_FREE(addr);
>  
> -        return i;
> +        addrs->lastaddr = tmp_addr;
> +        *next_addr = tmp_addr;
> +        return 0;
>      }
>  
>      virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -1364,24 +1330,17 @@ static int qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs)
>  int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
>                                      virDomainDeviceInfoPtr dev)
>  {
> -    int slot = qemuDomainPCIAddressGetNextSlot(addrs);
> -
> -    if (slot < 0)
> +    virDevicePCIAddress addr;
> +    if (qemuDomainPCIAddressGetNextSlot(addrs, &addr) < 0)
>          return -1;
>  
> -    if (qemuDomainPCIAddressReserveSlot(addrs, slot) < 0)
> +    if (qemuDomainPCIAddressReserveSlot(addrs, &addr) < 0)
>          return -1;
>  
>      dev->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
> -    dev->addr.pci.bus = 0;
> -    dev->addr.pci.domain = 0;
> -    dev->addr.pci.slot = slot;
> -    dev->addr.pci.function = 0;
> -
> -    addrs->nextslot = slot + 1;
> -    if (QEMU_PCI_ADDRESS_LAST_SLOT < addrs->nextslot)
> -        addrs->nextslot = 0;
> +    dev->addr.pci = addr;
>  
> +    addrs->lastaddr = addr;
>      return 0;
>  }
>  
> @@ -1433,11 +1392,15 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
>      size_t i, j;
>      bool reservedIDE = false;
>      bool reservedUSB = false;
> -    int function;
>      bool qemuDeviceVideoUsable = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
> +    virDevicePCIAddress tmp_addr;
> +    virDevicePCIAddressPtr addrptr;
> +    unsigned int *func = &tmp_addr.function;
> +
>  
> -    /* Host bridge */
> -    if (qemuDomainPCIAddressReserveSlot(addrs, 0) < 0)
> +    /* Reserve slot 0 for the host bridge */
> +    memset(&tmp_addr, 0, sizeof(tmp_addr));
> +    if (qemuDomainPCIAddressReserveSlot(addrs, &tmp_addr) < 0)
>          goto error;
>  
>      /* Verify that first IDE and USB controllers (if any) is on the PIIX3, fn 1 */
> @@ -1491,13 +1454,15 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
>      /* PIIX3 (ISA bridge, IDE controller, something else unknown, USB controller)
>       * hardcoded slot=1, multifunction device
>       */
> -    for (function = 0; function < QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) {
> -        if ((function == 1 && reservedIDE) ||
> -            (function == 2 && reservedUSB))
> +    memset(&tmp_addr, 0, sizeof(tmp_addr));
> +    tmp_addr.slot = 1;
> +    for (*func = 0; *func < QEMU_PCI_ADDRESS_LAST_FUNCTION; (*func)++) {
> +        if ((*func == 1 && reservedIDE) ||
> +            (*func == 2 && reservedUSB))
>              /* we have reserved this pci address */
>              continue;
>  
> -        if (qemuDomainPCIAddressReserveFunction(addrs, 1, function) < 0)
> +        if (qemuDomainPCIAddressReserveAddr(addrs, &tmp_addr) < 0)
>              goto error;
>      }
>  
> @@ -1509,11 +1474,12 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
>              primaryVideo->info.addr.pci.bus = 0;
>              primaryVideo->info.addr.pci.slot = 2;
>              primaryVideo->info.addr.pci.function = 0;
> +            addrptr = &primaryVideo->info.addr.pci;
>  
> -            if (qemuDomainPCIAddressCheckSlot(addrs, &primaryVideo->info) < 0) {
> +            if (qemuDomainPCIAddressCheckSlot(addrs, addrptr) < 0) {
>                  if (qemuDeviceVideoUsable) {
>                      virResetLastError();
> -                    if (qemuDomainPCIAddressSetNextAddr(addrs, &primaryVideo->info) < 0)
> +                    if (qemuDomainPCIAddressSetNextAddr(addrs,&primaryVideo->info) < 0)

You just removed a space between args in the line above ^^^


>                          goto error;
>                  } else {
>                      virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> @@ -1521,7 +1487,7 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
>                                       "QEMU needs it for primary video"));
>                      goto error;
>                  }
> -            } else if (qemuDomainPCIAddressReserveSlot(addrs, 2) < 0) {
> +            } else if (qemuDomainPCIAddressReserveSlot(addrs, addrptr) < 0) {
>                  goto error;
>              }
>          } else if (!qemuDeviceVideoUsable) {
> @@ -1537,16 +1503,15 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
>               * has already reserved the address, so we must skip */
>          }
>      } else if (!qemuDeviceVideoUsable) {
> -        virDomainDeviceInfo dev;
> -        memset(&dev, 0, sizeof(dev));
> -        dev.addr.pci.slot = 2;
> +        memset(&tmp_addr, 0, sizeof(tmp_addr));
> +        tmp_addr.slot = 2;
>  
> -        if (qemuDomainPCIAddressCheckSlot(addrs, &dev) < 0) {
> +        if (qemuDomainPCIAddressCheckSlot(addrs, &tmp_addr) < 0) {
>              VIR_DEBUG("PCI address 0:0:2.0 in use, future addition of a video"
>                        " device will not be possible without manual"
>                        " intervention");
>              virResetLastError();
> -        } else if (qemuDomainPCIAddressReserveSlot(addrs, 2) < 0) {
> +        } else if (qemuDomainPCIAddressReserveSlot(addrs, &tmp_addr) < 0) {
>              goto error;
>          }
>      }
> @@ -1609,6 +1574,7 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
>          /* USB2 needs special handling to put all companions in the same slot */
>          if (IS_USB2_CONTROLLER(def->controllers[i])) {
>              virDevicePCIAddress addr = { 0, 0, 0, 0, false };
> +            memset(&tmp_addr, 0, sizeof(tmp_addr));
>              for (j = 0 ; j < i ; j++) {
>                  if (IS_USB2_CONTROLLER(def->controllers[j]) &&
>                      def->controllers[j]->idx == def->controllers[i]->idx) {
> @@ -1636,19 +1602,14 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
>              if (addr.slot == 0) {
>                  /* This is the first part of the controller, so need
>                   * to find a free slot & then reserve a function */
> -                int slot = qemuDomainPCIAddressGetNextSlot(addrs);
> -                if (slot < 0)
> +                if (qemuDomainPCIAddressGetNextSlot(addrs, &tmp_addr) < 0)
>                      goto error;
>  
> -                addr.slot = slot;
> -                addrs->nextslot = addr.slot + 1;
> -                if (QEMU_PCI_ADDRESS_LAST_SLOT < addrs->nextslot)
> -                    addrs->nextslot = 0;
> +                addr.bus = tmp_addr.bus;
> +                addr.slot = tmp_addr.slot;
>              }
>              /* Finally we can reserve the slot+function */
> -            if (qemuDomainPCIAddressReserveFunction(addrs,
> -                                                    addr.slot,
> -                                                    addr.function) < 0)
> +            if (qemuDomainPCIAddressReserveAddr(addrs, &addr) < 0)
>                  goto error;
>  
>              def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index 7e52c5d..e4db000 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -194,21 +194,18 @@ int qemuDomainAssignPCIAddresses(virDomainDefPtr def,
>                                   virQEMUCapsPtr qemuCaps,
>                                   virDomainObjPtr obj);
>  qemuDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def);
> -int qemuDomainPCIAddressReserveFunction(qemuDomainPCIAddressSetPtr addrs,
> -                                        int slot, int function);
>  int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs,
> -                                    int slot);
> +                                    virDevicePCIAddressPtr addr);
>  int qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs,
> -                                    virDomainDeviceInfoPtr dev);
> +                                    virDevicePCIAddressPtr addr);
>  int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
>                                      virDomainDeviceInfoPtr dev);
>  int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs,
>                                     virDomainDeviceInfoPtr dev);
>  int qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs,
> -                                    virDomainDeviceInfoPtr dev);
> -int qemuDomainPCIAddressReleaseFunction(qemuDomainPCIAddressSetPtr addrs,
> -                                        int slot, int function);
> -int qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs, int slot);
> +                                    virDevicePCIAddressPtr addr);
> +int qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs,
> +                                    virDevicePCIAddressPtr addr);
>  
>  void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs);
>  int  qemuAssignDevicePCISlots(virDomainDefPtr def,
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 488a440..4b34122 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -325,7 +325,7 @@ error:
>          (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
>          releaseaddr &&
>          qemuDomainPCIAddressReleaseSlot(priv->pciaddrs,
> -                                        disk->info.addr.pci.slot) < 0)
> +                                        &disk->info.addr.pci) < 0)
>          VIR_WARN("Unable to release PCI address on %s", disk->src);
>  
>      if (virSecurityManagerRestoreImageLabel(driver->securityManager,
> @@ -402,7 +402,7 @@ cleanup:
>          (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
>          releaseaddr &&
>          qemuDomainPCIAddressReleaseSlot(priv->pciaddrs,
> -                                        controller->info.addr.pci.slot) < 0)
> +                                        &controller->info.addr.pci) < 0)
>          VIR_WARN("Unable to release PCI address on controller");
>  
>      VIR_FREE(devstr);
> @@ -899,7 +899,7 @@ cleanup:
>              (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
>              releaseaddr &&
>              qemuDomainPCIAddressReleaseSlot(priv->pciaddrs,
> -                                            net->info.addr.pci.slot) < 0)
> +                                            &net->info.addr.pci) < 0)
>              VIR_WARN("Unable to release PCI address on NIC");
>  
>          if (iface_connected) {
> @@ -1042,7 +1042,7 @@ error:
>          (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
>          releaseaddr &&
>          qemuDomainPCIAddressReleaseSlot(priv->pciaddrs,
> -                                        hostdev->info->addr.pci.slot) < 0)
> +                                        &hostdev->info->addr.pci) < 0)
>          VIR_WARN("Unable to release PCI address on host device");
>  
>      qemuDomainReAttachHostdevDevices(driver, vm->def->name, &hostdev, 1);
> @@ -2084,7 +2084,7 @@ int qemuDomainDetachPciDiskDevice(virQEMUDriverPtr driver,
>  
>      if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) &&
>          qemuDomainPCIAddressReleaseSlot(priv->pciaddrs,
> -                                        detach->info.addr.pci.slot) < 0)
> +                                        &detach->info.addr.pci) < 0)
>          VIR_WARN("Unable to release PCI address on %s", dev->data.disk->src);
>  
>      virDomainDiskRemove(vm->def, i);
> @@ -2321,7 +2321,7 @@ int qemuDomainDetachPciControllerDevice(virQEMUDriverPtr driver,
>  
>      if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) &&
>          qemuDomainPCIAddressReleaseSlot(priv->pciaddrs,
> -                                        detach->info.addr.pci.slot) < 0)
> +                                        &detach->info.addr.pci) < 0)
>          VIR_WARN("Unable to release PCI address on controller");
>  
>      ret = 0;
> @@ -2397,7 +2397,7 @@ qemuDomainDetachHostPciDevice(virQEMUDriverPtr driver,
>  
>      if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) &&
>          qemuDomainPCIAddressReleaseSlot(priv->pciaddrs,
> -                                        detach->info->addr.pci.slot) < 0)
> +                                        &detach->info->addr.pci) < 0)
>          VIR_WARN("Unable to release PCI address on host device");
>  
>  cleanup:
> @@ -2649,7 +2649,7 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver,
>  
>      if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) &&
>          qemuDomainPCIAddressReleaseSlot(priv->pciaddrs,
> -                                        detach->info.addr.pci.slot) < 0)
> +                                        &detach->info.addr.pci) < 0)
>          VIR_WARN("Unable to release PCI address on NIC");
>  
>      virDomainConfNWFilterTeardown(detach);

There were a lot of mechanical changes in this. Assuming that you've run
make syntax-check, and tried out several cases of defining new domains
with devices that have no pci address specified (with satisfactory
results!), and that you fix the two small nits I found above, then ACK.




More information about the libvir-list mailing list