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@xxxxxxxxx> --- 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list