3:04 p.m.
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(a)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