Re: [PATCH RESEND 04/20] virpci.c: simplify virPCIDeviceNew() signature

On 1/18/21 2:53 PM, Daniel Henrique Barboza wrote: > The current virPCIDeviceNew() signature, receiving 4 uints in sequence > (domain, bus, slot, function), is not neat. > > We already have a way to represent a PCI address in virPCIDeviceAddress > that is used in the code. Aside from the test files, most of > virPCIDeviceNew() callers have access to a virPCIDeviceAddress reference, > but then we need to retrieve the 4 required uints (addr.domain, addr.bus, > addr.slot, addr.function) to satisfy virPCIDeviceNew(). The result is > that we have extra verbosity/boilerplate to retrieve an information that > is already available in virPCIDeviceAddress. > > A better way is presented by virNVMEDeviceNew(), where the caller just > supplies a virPCIDeviceAddress pointer and the function handles the > details internally. > > This patch changes virPCIDeviceNew() to receive a virPCIDeviceAddress > pointer instead of 4 uints. > > Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; > --- >   src/hypervisor/virhostdev.c        |  3 +-- >   src/libxl/libxl_driver.c           |  6 ++--- >   src/node_device/node_device_udev.c | 11 +++++--- >   src/qemu/qemu_domain_address.c     |  5 +--- >   src/qemu/qemu_driver.c             |  6 ++--- >   src/security/security_apparmor.c   |  3 +-- >   src/security/security_dac.c        |  6 ++--- >   src/security/security_selinux.c    |  6 ++--- >   src/security/virt-aa-helper.c      |  6 +---- >   src/util/virnetdev.c               |  3 +-- >   src/util/virnvme.c                 |  5 +--- >   src/util/virpci.c                  | 40 +++++++++--------------------- >   src/util/virpci.h                  |  5 +--- >   tests/virhostdevtest.c             |  3 ++- >   tests/virpcitest.c                 | 35 +++++++++++++++++++------- >   15 files changed, 64 insertions(+), 79 deletions(-) > > diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c > index be32a26164..bd35397f2c 100644 > --- a/src/hypervisor/virhostdev.c > +++ b/src/hypervisor/virhostdev.c > @@ -235,8 +235,7 @@ virHostdevGetPCIHostDevice(const virDomainHostdevDef *hostdev, >           hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) >           return 0; > -    actual = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, > -                             pcisrc->addr.slot, pcisrc->addr.function); > +    actual = virPCIDeviceNew(&pcisrc->addr); >       if (!actual) >           return -1; > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > index 360d553a22..3eaf106006 100644 > --- a/src/libxl/libxl_driver.c > +++ b/src/libxl/libxl_driver.c > @@ -5818,7 +5818,7 @@ libxlNodeDeviceDetachFlags(virNodeDevicePtr dev, >       if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0) >           goto cleanup; > -    pci = virPCIDeviceNew(devAddr.domain, devAddr.bus, devAddr.slot, devAddr.function); > +    pci = virPCIDeviceNew(&devAddr); >       if (!pci) >           goto cleanup; > @@ -5889,7 +5889,7 @@ libxlNodeDeviceReAttach(virNodeDevicePtr dev) >       if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0) >           goto cleanup; > -    pci = virPCIDeviceNew(devAddr.domain, devAddr.bus, devAddr.slot, devAddr.function); > +    pci = virPCIDeviceNew(&devAddr); >       if (!pci) >           goto cleanup; > @@ -5947,7 +5947,7 @@ libxlNodeDeviceReset(virNodeDevicePtr dev) >       if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0) >           goto cleanup; > -    pci = virPCIDeviceNew(devAddr.domain, devAddr.bus, devAddr.slot, devAddr.function); > +    pci = virPCIDeviceNew(&devAddr); >       if (!pci) >           goto cleanup; > diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c > index 55a2731681..fceb135aa5 100644 > --- a/src/node_device/node_device_udev.c > +++ b/src/node_device/node_device_udev.c > @@ -367,6 +367,7 @@ udevProcessPCI(struct udev_device *device, >       virNodeDevCapPCIDevPtr pci_dev = &def->caps->data.pci_dev; >       virPCIEDeviceInfoPtr pci_express = NULL; >       virPCIDevicePtr pciDev = NULL; > +    virPCIDeviceAddress devAddr; >       int ret = -1; >       char *p; >       bool privileged; > @@ -416,10 +417,12 @@ udevProcessPCI(struct udev_device *device, >       if (virNodeDeviceGetPCIDynamicCaps(def->sysfs_path, pci_dev) < 0) >           goto cleanup; > -    if (!(pciDev = virPCIDeviceNew(pci_dev->domain, > -                                   pci_dev->bus, > -                                   pci_dev->slot, > -                                   pci_dev->function))) > +    devAddr.domain = pci_dev->domain; > +    devAddr.bus = pci_dev->bus; > +    devAddr.slot = pci_dev->slot; > +    devAddr.function = pci_dev->function; Now see - there we are again! If virNodeDevCapPCIDev could just have a virPCIDeviceAddress, then this could have been a 4->1 conversion instead of 4->4. Ah well, tomorrow is another day!

> + > +    if (!(pciDev = virPCIDeviceNew(&devAddr))) >           goto cleanup; >       /* We need to be root to read PCI device configs */ > diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c > index f0ba318cc8..ae4ad6677c 100644 > --- a/src/qemu/qemu_domain_address.c > +++ b/src/qemu/qemu_domain_address.c > @@ -857,10 +857,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, >               return 0; >           } > -        if (!(pciDev = virPCIDeviceNew(hostAddr->domain, > -                                       hostAddr->bus, > -                                       hostAddr->slot, > -                                       hostAddr->function))) { > +        if (!(pciDev = virPCIDeviceNew(hostAddr))) { >               /* libvirt should be able to perform all the >                * operations in virPCIDeviceNew() even if it's >                * running unprivileged, so if this fails, the device > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 28781cc34b..7581e3c8cb 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -12000,7 +12000,7 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev, >       if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0) >           goto cleanup; > -    pci = virPCIDeviceNew(devAddr.domain, devAddr.bus, devAddr.slot, devAddr.function); > +    pci = virPCIDeviceNew(&devAddr); >       if (!pci) >           goto cleanup; > @@ -12081,7 +12081,7 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev) >       if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0) >           goto cleanup; > -    pci = virPCIDeviceNew(devAddr.domain, devAddr.bus, devAddr.slot, devAddr.function); > +    pci = virPCIDeviceNew(&devAddr); >       if (!pci) >           goto cleanup; > @@ -12135,7 +12135,7 @@ qemuNodeDeviceReset(virNodeDevicePtr dev) >       if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0) >           goto cleanup; > -    pci = virPCIDeviceNew(devAddr.domain, devAddr.bus, devAddr.slot, devAddr.function); > +    pci = virPCIDeviceNew(&devAddr); >       if (!pci) >           goto cleanup; > diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c > index 1b035cce2f..b564866678 100644 > --- a/src/security/security_apparmor.c > +++ b/src/security/security_apparmor.c > @@ -876,8 +876,7 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr, >       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { >           virPCIDevicePtr pci = > -            virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, > -                            pcisrc->addr.slot, pcisrc->addr.function); > +            virPCIDeviceNew(&pcisrc->addr); >           if (!pci) >               goto done; > diff --git a/src/security/security_dac.c b/src/security/security_dac.c > index 4f4a0a069e..b5e56feaa8 100644 > --- a/src/security/security_dac.c > +++ b/src/security/security_dac.c > @@ -1268,8 +1268,7 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr, >       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { >           virPCIDevicePtr pci = > -            virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, > -                            pcisrc->addr.slot, pcisrc->addr.function); > +            virPCIDeviceNew(&pcisrc->addr); >           if (!pci) >               return -1; > @@ -1437,8 +1436,7 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr, >       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { >           virPCIDevicePtr pci = > -            virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, > -                            pcisrc->addr.slot, pcisrc->addr.function); > +            virPCIDeviceNew(&pcisrc->addr); >           if (!pci) >               return -1; > diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c > index 2fc6ef2616..76edaed027 100644 > --- a/src/security/security_selinux.c > +++ b/src/security/security_selinux.c > @@ -2103,8 +2103,7 @@ virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManagerPtr mgr, >       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { >           virPCIDevicePtr pci = > -            virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, > -                            pcisrc->addr.slot, pcisrc->addr.function); > +            virPCIDeviceNew(&pcisrc->addr); >           if (!pci) >               return -1; > @@ -2343,8 +2342,7 @@ virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManagerPtr mgr, >       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { >           virPCIDevicePtr pci = > -            virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, > -                            pcisrc->addr.slot, pcisrc->addr.function); > +            virPCIDeviceNew(&pcisrc->addr); >           if (!pci) >               return -1; > diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c > index 5a6f4a5f7d..5efdb61e59 100644 > --- a/src/security/virt-aa-helper.c > +++ b/src/security/virt-aa-helper.c > @@ -1104,11 +1104,7 @@ get_files(vahControl * ctl) >               } >               case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { > -                virPCIDevicePtr pci = virPCIDeviceNew( > -                           dev->source.subsys.u.pci.addr.domain, > -                           dev->source.subsys.u.pci.addr.bus, > -                           dev->source.subsys.u.pci.addr.slot, > -                           dev->source.subsys.u.pci.addr.function); > +                virPCIDevicePtr pci = virPCIDeviceNew(&dev->source.subsys.u.pci.addr); See! Like this!! Anyway, this is giving me some thoughts about solving it, but in the meantime this patch is good on its own: Reviewed-by: Laine Stump <laine(a)redhat.com&gt; >                   virDomainHostdevSubsysPCIBackendType backend = dev->source.subsys.u.pci.backend; >                   if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO || > diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c > index a73e5f72f1..acb3ec960c 100644 > --- a/src/util/virnetdev.c > +++ b/src/util/virnetdev.c > @@ -1141,8 +1141,7 @@ virNetDevGetPCIDevice(const char *devName) >       if (!vfPCIAddr) >           return NULL; > -    return virPCIDeviceNew(vfPCIAddr->domain, vfPCIAddr->bus, > -                           vfPCIAddr->slot, vfPCIAddr->function); > +    return virPCIDeviceNew(vfPCIAddr); >   } >   # endif > diff --git a/src/util/virnvme.c b/src/util/virnvme.c > index b8179aa431..66b73cd1d1 100644 > --- a/src/util/virnvme.c > +++ b/src/util/virnvme.c > @@ -290,10 +290,7 @@ virNVMeDeviceCreatePCIDevice(const virNVMeDevice *nvme) >   { >       g_autoptr(virPCIDevice) pci = NULL; > -    if (!(pci = virPCIDeviceNew(nvme->address.domain, > -                                nvme->address.bus, > -                                nvme->address.slot, > -                                nvme->address.function))) > +    if (!(pci = virPCIDeviceNew(&nvme->address))) >           return NULL; >       /* NVMe devices must be bound to vfio */ > diff --git a/src/util/virpci.c b/src/util/virpci.c > index 5f50f92abb..5a91553b5f 100644 > --- a/src/util/virpci.c > +++ b/src/util/virpci.c > @@ -475,24 +475,24 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate, >           return -1; >       while ((ret = virDirRead(dir, &entry, PCI_SYSFS "devices")) > 0) { > -        unsigned int domain, bus, slot, function; >           g_autoptr(virPCIDevice) check = NULL; > +        virPCIDeviceAddress devAddr; >           char *tmp; >           /* expected format: <domain>:<bus>:<slot>.<function> */ >           if (/* domain */ > -            virStrToLong_ui(entry->d_name, &tmp, 16, &domain) < 0 || *tmp != ':' || > +            virStrToLong_ui(entry->d_name, &tmp, 16, &devAddr.domain) < 0 || *tmp != ':' || >               /* bus */ > -            virStrToLong_ui(tmp + 1, &tmp, 16, &bus) < 0 || *tmp != ':' || > +            virStrToLong_ui(tmp + 1, &tmp, 16, &devAddr.bus) < 0 || *tmp != ':' || >               /* slot */ > -            virStrToLong_ui(tmp + 1, &tmp, 16, &slot) < 0 || *tmp != '.' || > +            virStrToLong_ui(tmp + 1, &tmp, 16, &devAddr.slot) < 0 || *tmp != '.' || >               /* function */ > -            virStrToLong_ui(tmp + 1, NULL, 16, &function) < 0) { > +            virStrToLong_ui(tmp + 1, NULL, 16, &devAddr.function) < 0) { >               VIR_WARN("Unusual entry in " PCI_SYSFS "devices: %s", entry->d_name); Isn't there a utility function that does this? >               continue; >           } > -        check = virPCIDeviceNew(domain, bus, slot, function); > +        check = virPCIDeviceNew(&devAddr); >           if (!check) { >               ret = -1; >               break; > @@ -767,10 +767,7 @@ virPCIDeviceIsParent(virPCIDevicePtr dev, virPCIDevicePtr check, void *data) >        */ >       if (dev->address.bus > secondary && dev->address.bus <= subordinate) { >           if (*best == NULL) { > -            *best = virPCIDeviceNew(check->address.domain, > -                                    check->address.bus, > -                                    check->address.slot, > -                                    check->address.function); > +            *best = virPCIDeviceNew(&check->address); >               if (*best == NULL) { >                   ret = -1; >                   goto cleanup; > @@ -790,10 +787,7 @@ virPCIDeviceIsParent(virPCIDevicePtr dev, virPCIDevicePtr check, void *data) >               if (secondary > best_secondary) { >                   virPCIDeviceFree(*best); > -                *best = virPCIDeviceNew(check->address.domain, > -                                        check->address.bus, > -                                        check->address.slot, > -                                        check->address.function); > +                *best = virPCIDeviceNew(&check->address); >                   if (*best == NULL) { >                       ret = -1; >                       goto cleanup; > @@ -1455,10 +1449,7 @@ virPCIDeviceAddressAsString(const virPCIDeviceAddress *addr) >   } >   virPCIDevicePtr > -virPCIDeviceNew(unsigned int domain, > -                unsigned int bus, > -                unsigned int slot, > -                unsigned int function) > +virPCIDeviceNew(const virPCIDeviceAddress *address) >   { >       g_autoptr(virPCIDevice) dev = NULL; >       g_autofree char *vendor = NULL; > @@ -1466,10 +1457,7 @@ virPCIDeviceNew(unsigned int domain, >       dev = g_new0(virPCIDevice, 1); > -    dev->address.domain = domain; > -    dev->address.bus = bus; > -    dev->address.slot = slot; > -    dev->address.function = function; > +    virPCIDeviceAddressCopy(&dev->address, address); >       dev->name = virPCIDeviceAddressAsString(&dev->address); > @@ -1896,8 +1884,7 @@ virPCIDeviceGetIOMMUGroupAddOne(virPCIDeviceAddressPtr newDevAddr, void *opaque) >       virPCIDeviceListPtr groupList = opaque; >       g_autoptr(virPCIDevice) newDev = NULL; > -    if (!(newDev = virPCIDeviceNew(newDevAddr->domain, newDevAddr->bus, > -                                   newDevAddr->slot, newDevAddr->function))) > +    if (!(newDev = virPCIDeviceNew(newDevAddr))) >           return -1; >       if (virPCIDeviceListAdd(groupList, newDev) < 0) > @@ -2028,10 +2015,7 @@ virPCIDeviceAddressGetIOMMUGroupDev(const virPCIDeviceAddress *devAddr) >   { >       g_autoptr(virPCIDevice) pci = NULL; > -    if (!(pci = virPCIDeviceNew(devAddr->domain, > -                                devAddr->bus, > -                                devAddr->slot, > -                                devAddr->function))) > +    if (!(pci = virPCIDeviceNew(devAddr))) >           return NULL; >       return virPCIDeviceGetIOMMUGroupDev(pci); > diff --git a/src/util/virpci.h b/src/util/virpci.h > index 43828b0a8a..d4451848c1 100644 > --- a/src/util/virpci.h > +++ b/src/util/virpci.h > @@ -113,10 +113,7 @@ struct _virPCIEDeviceInfo { >       virPCIELink *link_sta;   /* Actually negotiated capabilities */ >   }; > -virPCIDevicePtr virPCIDeviceNew(unsigned int domain, > -                                unsigned int bus, > -                                unsigned int slot, > -                                unsigned int function); > +virPCIDevicePtr virPCIDeviceNew(const virPCIDeviceAddress *address); >   virPCIDevicePtr virPCIDeviceCopy(virPCIDevicePtr dev); >   void virPCIDeviceFree(virPCIDevicePtr dev); >   const char *virPCIDeviceGetName(virPCIDevicePtr dev); > diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c > index 385db0849a..40c14a5281 100644 > --- a/tests/virhostdevtest.c > +++ b/tests/virhostdevtest.c > @@ -138,7 +138,8 @@ myInit(void) >       } >       for (i = 0; i < nhostdevs; i++) { > -        if (!(dev[i] = virPCIDeviceNew(0, 0, i + 1, 0))) > +        virDomainHostdevSubsys subsys = hostdevs[i]->source.subsys; > +        if (!(dev[i] = virPCIDeviceNew(&subsys.u.pci.addr))) >               goto cleanup; >           virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_VFIO); > diff --git a/tests/virpcitest.c b/tests/virpcitest.c > index 6f064a3f85..6a4bd5518d 100644 > --- a/tests/virpcitest.c > +++ b/tests/virpcitest.c > @@ -60,8 +60,9 @@ testVirPCIDeviceNew(const void *opaque G_GNUC_UNUSED) >       int ret = -1; >       virPCIDevicePtr dev; >       const char *devName; > +    virPCIDeviceAddress devAddr = {.domain = 0, .bus = 0, .slot = 0, .function = 0}; > -    if (!(dev = virPCIDeviceNew(0, 0, 0, 0))) > +    if (!(dev = virPCIDeviceNew(&devAddr))) >           goto cleanup; >       devName = virPCIDeviceGetName(dev); > @@ -103,7 +104,9 @@ testVirPCIDeviceDetach(const void *opaque G_GNUC_UNUSED) >       CHECK_LIST_COUNT(inactiveDevs, 0); >       for (i = 0; i < nDev; i++) { > -        if (!(dev[i] = virPCIDeviceNew(0, 0, i + 1, 0))) > +        virPCIDeviceAddress devAddr = {.domain = 0, .bus = 0, > +                                       .slot = i + 1, .function = 0}; > +        if (!(dev[i] = virPCIDeviceNew(&devAddr))) >               goto cleanup; >           virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_VFIO); > @@ -144,7 +147,9 @@ testVirPCIDeviceReset(const void *opaque G_GNUC_UNUSED) >       CHECK_LIST_COUNT(inactiveDevs, 0); >       for (i = 0; i < nDev; i++) { > -        if (!(dev[i] = virPCIDeviceNew(0, 0, i + 1, 0))) > +        virPCIDeviceAddress devAddr = {.domain = 0, .bus = 0, > +                                       .slot = i + 1, .function = 0}; > +        if (!(dev[i] = virPCIDeviceNew(&devAddr))) >               goto cleanup; >           virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_VFIO); > @@ -176,7 +181,9 @@ testVirPCIDeviceReattach(const void *opaque G_GNUC_UNUSED) >           goto cleanup; >       for (i = 0; i < nDev; i++) { > -        if (!(dev[i] = virPCIDeviceNew(0, 0, i + 1, 0))) > +        virPCIDeviceAddress devAddr = {.domain = 0, .bus = 0, > +                                       .slot = i + 1, .function = 0}; > +        if (!(dev[i] = virPCIDeviceNew(&devAddr))) >               goto cleanup; >           if (virPCIDeviceListAdd(inactiveDevs, dev[i]) < 0) { > @@ -222,8 +229,10 @@ testVirPCIDeviceIsAssignable(const void *opaque) >       const struct testPCIDevData *data = opaque; >       int ret = -1; >       virPCIDevicePtr dev; > +    virPCIDeviceAddress devAddr = {.domain = data->domain, .bus = data->bus, > +                                   .slot = data->slot, .function = data->function}; > -    if (!(dev = virPCIDeviceNew(data->domain, data->bus, data->slot, data->function))) > +    if (!(dev = virPCIDeviceNew(&devAddr))) >           return -1; >       if (virPCIDeviceIsAssignable(dev, true)) > @@ -239,8 +248,10 @@ testVirPCIDeviceDetachSingle(const void *opaque) >       const struct testPCIDevData *data = opaque; >       int ret = -1; >       virPCIDevicePtr dev; > +    virPCIDeviceAddress devAddr = {.domain = data->domain, .bus = data->bus, > +                                   .slot = data->slot, .function = data->function}; > -    dev = virPCIDeviceNew(data->domain, data->bus, data->slot, data->function); > +    dev = virPCIDeviceNew(&devAddr); >       if (!dev) >           goto cleanup; > @@ -261,8 +272,10 @@ testVirPCIDeviceReattachSingle(const void *opaque) >       const struct testPCIDevData *data = opaque; >       int ret = -1; >       virPCIDevicePtr dev; > +    virPCIDeviceAddress devAddr = {.domain = data->domain, .bus = data->bus, > +                                   .slot = data->slot, .function = data->function}; > -    dev = virPCIDeviceNew(data->domain, data->bus, data->slot, data->function); > +    dev = virPCIDeviceNew(&devAddr); >       if (!dev) >           goto cleanup; > @@ -285,8 +298,10 @@ testVirPCIDeviceCheckDriverTest(const void *opaque) >       const struct testPCIDevData *data = opaque; >       int ret = -1; >       virPCIDevicePtr dev; > +    virPCIDeviceAddress devAddr = {.domain = data->domain, .bus = data->bus, > +                                   .slot = data->slot, .function = data->function}; > -    dev = virPCIDeviceNew(data->domain, data->bus, data->slot, data->function); > +    dev = virPCIDeviceNew(&devAddr); >       if (!dev) >           goto cleanup; > @@ -305,8 +320,10 @@ testVirPCIDeviceUnbind(const void *opaque) >       const struct testPCIDevData *data = opaque; >       int ret = -1; >       virPCIDevicePtr dev; > +    virPCIDeviceAddress devAddr = {.domain = data->domain, .bus = data->bus, > +                                   .slot = data->slot, .function = data->function}; > -    dev = virPCIDeviceNew(data->domain, data->bus, data->slot, data->function); > +    dev = virPCIDeviceNew(&devAddr); >       if (!dev) >           goto cleanup;