]> xenbits.xensource.com Git - libvirt.git/commitdiff
virpci.c: simplify virPCIDeviceNew() signature
authorDaniel Henrique Barboza <danielhb413@gmail.com>
Mon, 4 Jan 2021 12:54:28 +0000 (09:54 -0300)
committerDaniel Henrique Barboza <danielhb413@gmail.com>
Fri, 29 Jan 2021 20:52:10 +0000 (17:52 -0300)
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.

Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
15 files changed:
src/hypervisor/virhostdev.c
src/libxl/libxl_driver.c
src/node_device/node_device_udev.c
src/qemu/qemu_domain_address.c
src/qemu/qemu_driver.c
src/security/security_apparmor.c
src/security/security_dac.c
src/security/security_selinux.c
src/security/virt-aa-helper.c
src/util/virnetdev.c
src/util/virnvme.c
src/util/virpci.c
src/util/virpci.h
tests/virhostdevtest.c
tests/virpcitest.c

index be32a26164e88a05c790698fd44abaadaaaba707..bd35397f2cdfcd99e88d6742ba5c2dc576926b19 100644 (file)
@@ -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;
index 360d553a22da77bc2ec36785893cc6ce0e134807..3eaf106006432105576ce2b224d267471890145e 100644 (file)
@@ -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;
 
index 55a273168118fffd8d307d7900bc83ebf14eb193..fceb135aa5bb4e6201a9d78bff6085e5ebf42050 100644 (file)
@@ -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;
+
+    if (!(pciDev = virPCIDeviceNew(&devAddr)))
         goto cleanup;
 
     /* We need to be root to read PCI device configs */
index d40edadb4dd93c4620a8a26340a2b3d667515727..1171dc5b14766aaff96a1c5f5de43e2216ddb3e2 100644 (file)
@@ -863,10 +863,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
index 315f15c8c566a27bbcb0d73a4349ead9478960ba..ed840a5c8dd20c967bdee0219cdd2d2f7b8027d8 100644 (file)
@@ -12009,7 +12009,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;
 
@@ -12090,7 +12090,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;
 
@@ -12144,7 +12144,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;
 
index 1f309c0c9fe027a3dc4a99515e06534f074c8976..e6ecb513b1b4dab7854e78e86726adddb0c639f4 100644 (file)
@@ -892,8 +892,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;
index 71d58758c41676f0288e5524b9fef60b846458f4..386a294a2ebdb506f6453367fed97b86bd85ccd7 100644 (file)
@@ -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;
index 3563dbfb86a57ab41528ddcdb5b4b7b943d6a65a..a251478e1a49af040c0444c0029c1f8888b30608 100644 (file)
@@ -2105,8 +2105,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;
@@ -2345,8 +2344,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;
index d33bacde5d9c46d9c75207e0bd8cd28024873aa7..c6153053202d6c87cf91527ecef27add18aa42fc 100644 (file)
@@ -1105,11 +1105,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);
 
                 virDomainHostdevSubsysPCIBackendType backend = dev->source.subsys.u.pci.backend;
                 if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO ||
index 2485718b48fc769546441df8b0f0b2619745b981..1ef7cea20a8b49d203056906c29206e03ec16c6a 100644 (file)
@@ -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
 
index b8179aa4311766377730eb3f9f801fb48e8b25b2..66b73cd1d19c7a0779f09ee1fcee7f0c0c44439c 100644 (file)
@@ -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 */
index 0729388fc804befebdcfb8f739e0a601bf69a516..ce6b132688e7b1463c7908aa0e49c2b7d6a60bc4 100644 (file)
@@ -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);
             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);
index 1a4bf1d751341ee4143adfc8c398881b16a533a9..3f609212c702589289754b44ccc706b440e96838 100644 (file)
@@ -118,10 +118,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);
index 385db0849a728d264c4cdd69692be98c33d4bbc6..40c14a5281f76cbe9ea752abce83558bece0424e 100644 (file)
@@ -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);
index 6f064a3f858fe49aa202c83b55bb8dc46914fedc..6a4bd5518da63dea61fd43c0175f3c52c9d08315 100644 (file)
@@ -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;