[libvirt] [PATCH 2/2] virpci, qemu_domain_address: adding virPCIDeviceSetAddress

Daniel Henrique Barboza danielhb413 at gmail.com
Thu Sep 19 20:04:47 UTC 2019


A common operation in qemu_domain_address is comparing a
virPCIDeviceAddress and assigning domain, bus, slot and function
to a specific value. The former can be done with the existing
virPCIDeviceAddressEqual() helper.

A new virpci helper called virPCIDeviceSetAddress() was created to
make it easier to assign the same domain/bus/slot/function of an
already existent virPCIDeviceAddress. Using both in
qemu_domain_address made the code tidier, since the object
was already created to use virPCIDeviceAddressEqual().

Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
---

I wasn't sure if this change was worth contributing it back,
since it feels more like a 'look and feel' change. But
since it made the code inside qemu_domain_address tidier, 
I decided to take a shot.


 src/libvirt_private.syms       |  1 +
 src/qemu/qemu_domain_address.c | 57 ++++++++++++++++------------------
 src/util/virpci.c              | 10 ++++++
 src/util/virpci.h              |  3 ++
 4 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 39812227aa..e878494d3e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2697,6 +2697,7 @@ virPCIDeviceNew;
 virPCIDeviceReattach;
 virPCIDeviceRebind;
 virPCIDeviceReset;
+virPCIDeviceSetAddress;
 virPCIDeviceSetManaged;
 virPCIDeviceSetRemoveSlot;
 virPCIDeviceSetReprobe;
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index e20481607f..8eb1f0656f 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -1729,45 +1729,41 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
     /* Verify that first IDE and USB controllers (if any) is on the PIIX3, fn 1 */
     for (i = 0; i < def->ncontrollers; i++) {
         virDomainControllerDefPtr cont = def->controllers[i];
+        virPCIDeviceAddress primaryIDEAddr = {.domain = 0, .bus = 0,
+                                              .slot = 1, .function = 1};
+        virPCIDeviceAddress piix3USBAddr = {.domain = 0, .bus = 0,
+                                            .slot = 1, .function = 2};
 
         /* First IDE controller lives on the PIIX3 at slot=1, function=1 */
         if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE &&
             cont->idx == 0) {
             if (virDeviceInfoPCIAddressIsPresent(&cont->info)) {
-                if (cont->info.addr.pci.domain != 0 ||
-                    cont->info.addr.pci.bus != 0 ||
-                    cont->info.addr.pci.slot != 1 ||
-                    cont->info.addr.pci.function != 1) {
+                if (!virPCIDeviceAddressEqual(&cont->info.addr.pci,
+                                              &primaryIDEAddr)) {
                     virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                                   _("Primary IDE controller must have PCI address 0:0:1.1"));
+                                   _("Primary IDE controller must have PCI "
+                                     "address 0:0:1.1"));
                     return -1;
                 }
             } else {
                 cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
-                cont->info.addr.pci.domain = 0;
-                cont->info.addr.pci.bus = 0;
-                cont->info.addr.pci.slot = 1;
-                cont->info.addr.pci.function = 1;
+                virPCIDeviceSetAddress(&cont->info.addr.pci, &primaryIDEAddr);
             }
         } else if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
                    cont->idx == 0 &&
                    (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI ||
                     cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT)) {
             if (virDeviceInfoPCIAddressIsPresent(&cont->info)) {
-                if (cont->info.addr.pci.domain != 0 ||
-                    cont->info.addr.pci.bus != 0 ||
-                    cont->info.addr.pci.slot != 1 ||
-                    cont->info.addr.pci.function != 2) {
+                if (!virPCIDeviceAddressEqual(&cont->info.addr.pci,
+                                              &piix3USBAddr)) {
                     virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                                   _("PIIX3 USB controller at index 0 must have PCI address 0:0:1.2"));
+                                   _("PIIX3 USB controller at index 0 must "
+                                     "have PCI address 0:0:1.2"));
                     return -1;
                 }
             } else {
                 cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
-                cont->info.addr.pci.domain = 0;
-                cont->info.addr.pci.bus = 0;
-                cont->info.addr.pci.slot = 1;
-                cont->info.addr.pci.function = 2;
+                virPCIDeviceSetAddress(&cont->info.addr.pci, &piix3USBAddr);
             }
         } else {
             /* this controller is not skipped in qemuDomainCollectPCIAddress */
@@ -1800,6 +1796,8 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
          * at slot 2.
          */
         virDomainVideoDefPtr primaryVideo = def->videos[0];
+        virPCIDeviceAddress primaryCardAddr = {.domain = 0, .bus = 0,
+                                               .slot = 2, .function = 0};
 
         if (virDeviceInfoPCIAddressIsWanted(&primaryVideo->info)) {
             memset(&tmp_addr, 0, sizeof(tmp_addr));
@@ -1830,10 +1828,8 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
                 primaryVideo->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
             }
         } else if (!qemuDeviceVideoUsable) {
-            if (primaryVideo->info.addr.pci.domain != 0 ||
-                primaryVideo->info.addr.pci.bus != 0 ||
-                primaryVideo->info.addr.pci.slot != 2 ||
-                primaryVideo->info.addr.pci.function != 0) {
+            if (!virPCIDeviceAddressEqual(&primaryVideo->info.addr.pci,
+                                          &primaryCardAddr)) {
                 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                                _("Primary video card must have PCI address 0:0:2.0"));
                 return -1;
@@ -1870,6 +1866,8 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
 
     for (i = 0; i < def->ncontrollers; i++) {
         virDomainControllerDefPtr cont = def->controllers[i];
+        virPCIDeviceAddress primarySATAAddr = {.domain = 0, .bus = 0,
+                                               .slot = 0x1F, .function = 2};
 
         switch (cont->type) {
         case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
@@ -1879,20 +1877,17 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
              */
             if (cont->idx == 0) {
                 if (virDeviceInfoPCIAddressIsPresent(&cont->info)) {
-                    if (cont->info.addr.pci.domain != 0 ||
-                        cont->info.addr.pci.bus != 0 ||
-                        cont->info.addr.pci.slot != 0x1F ||
-                        cont->info.addr.pci.function != 2) {
+                    if (!virPCIDeviceAddressEqual(&cont->info.addr.pci,
+                                                 &primarySATAAddr)) {
                         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                                       _("Primary SATA controller must have PCI address 0:0:1f.2"));
+                                       _("Primary SATA controller must have "
+                                         "PCI address 0:0:1f.2"));
                         return -1;
                     }
                 } else {
                     cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
-                    cont->info.addr.pci.domain = 0;
-                    cont->info.addr.pci.bus = 0;
-                    cont->info.addr.pci.slot = 0x1F;
-                    cont->info.addr.pci.function = 2;
+                    virPCIDeviceSetAddress(&cont->info.addr.pci,
+                                           &primarySATAAddr);
                 }
             }
             break;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index ee78151e74..12ec05ab9e 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1483,6 +1483,16 @@ virPCIDeviceGetName(virPCIDevicePtr dev)
     return dev->name;
 }
 
+void
+virPCIDeviceSetAddress(virPCIDeviceAddress *addr1,
+                       const virPCIDeviceAddress *addr2)
+{
+    addr1->domain = addr2->domain;
+    addr1->bus = addr2->bus;
+    addr1->slot = addr2->slot;
+    addr1->function = addr2->function;
+}
+
 /**
  * virPCIDeviceGetConfigPath:
  *
diff --git a/src/util/virpci.h b/src/util/virpci.h
index dc20f91710..b192e1cf19 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -234,6 +234,9 @@ bool virPCIDeviceAddressIsEmpty(const virPCIDeviceAddress *addr);
 bool virPCIDeviceAddressEqual(const virPCIDeviceAddress *addr1,
                               const virPCIDeviceAddress *addr2);
 
+void virPCIDeviceSetAddress(virPCIDeviceAddress *addr1,
+                            const virPCIDeviceAddress *addr2);
+
 char *virPCIDeviceAddressAsString(const virPCIDeviceAddress *addr)
       ATTRIBUTE_NONNULL(1);
 
-- 
2.21.0




More information about the libvir-list mailing list