[libvirt] [PATCH v2 0/2] a few cleanups in qemu_domain_address.c

Second round of these cleanups after the reviews from Eric. changes from v1: - fixed the commit message of patch 1 - ditched the virPCIAddressSet() approach - use direct assignment instead v1: https://www.redhat.com/archives/libvir-list/2019-September/msg00823.html Daniel Henrique Barboza (2): qemu_domain_address.c: use VIR_AUTOFREE() in strings qemu_domain_address: use virPCIDeviceAddressEqual() in conditionals src/qemu/qemu_domain_address.c | 125 +++++++++++++++------------------ 1 file changed, 55 insertions(+), 70 deletions(-) -- 2.21.0

A few 'cleanup' labels gone after using VIR_AUTOFREE() on the @addrStr variable. Reviewed-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_domain_address.c | 69 +++++++++++++++------------------- 1 file changed, 30 insertions(+), 39 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 216ba6235e..e20481607f 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1524,12 +1524,11 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, * inappropriate address types. */ if (!info->pciConnectFlags) { - char *addrStr = virPCIDeviceAddressAsString(&info->addr.pci); + VIR_AUTOFREE(char *) addrStr = virPCIDeviceAddressAsString(&info->addr.pci); VIR_WARN("qemuDomainDeviceCalculatePCIConnectFlags() thinks that the " "device with PCI address %s should not have a PCI address", addrStr ? addrStr : "(unknown)"); - VIR_FREE(addrStr); info->pciConnectFlags = VIR_PCI_CONNECT_TYPE_PCI_DEVICE; } @@ -1720,11 +1719,10 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virDomainPCIAddressSetPtr addrs) { - int ret = -1; size_t i; virPCIDeviceAddress tmp_addr; bool qemuDeviceVideoUsable = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); - char *addrStr = NULL; + VIR_AUTOFREE(char *) addrStr = NULL; virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI_DEVICE); @@ -1742,7 +1740,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, cont->info.addr.pci.function != 1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Primary IDE controller must have PCI address 0:0:1.1")); - goto cleanup; + return -1; } } else { cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; @@ -1762,7 +1760,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, cont->info.addr.pci.function != 2) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("PIIX3 USB controller at index 0 must have PCI address 0:0:1.2")); - goto cleanup; + return -1; } } else { cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; @@ -1777,7 +1775,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, } if (addrs->nbuses && virDomainPCIAddressReserveAddr(addrs, &cont->info.addr.pci, flags, 0) < 0) - goto cleanup; + return -1; } /* Implicit PIIX3 devices living on slot 1 not handled above */ @@ -1786,11 +1784,11 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, tmp_addr.slot = 1; /* ISA Bridge at 00:01.0 */ if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) - goto cleanup; + return -1; /* Bridge at 00:01.3 */ tmp_addr.function = 3; if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) - goto cleanup; + return -1; } if (def->nvideos > 0 && @@ -1808,26 +1806,26 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, tmp_addr.slot = 2; if (!(addrStr = virPCIDeviceAddressAsString(&tmp_addr))) - goto cleanup; + return -1; if (!virDomainPCIAddressValidate(addrs, &tmp_addr, addrStr, flags, true)) - goto cleanup; + return -1; if (virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { if (qemuDeviceVideoUsable) { if (qemuDomainPCIAddressReserveNextAddr(addrs, &primaryVideo->info) < 0) { - goto cleanup; + return -1; } } else { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("PCI address 0:0:2.0 is in use, " "QEMU needs it for primary video")); - goto cleanup; + return -1; } } else { if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) - goto cleanup; + return -1; primaryVideo->info.addr.pci = tmp_addr; primaryVideo->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; } @@ -1838,7 +1836,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, primaryVideo->info.addr.pci.function != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Primary video card must have PCI address 0:0:2.0")); - goto cleanup; + return -1; } /* If TYPE == PCI, then qemuDomainCollectPCIAddress() function * has already reserved the address, so we must skip */ @@ -1852,13 +1850,10 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, " device will not be possible without manual" " intervention"); } else if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) { - goto cleanup; + return -1; } } - ret = 0; - cleanup: - VIR_FREE(addrStr); - return ret; + return 0; } @@ -1867,11 +1862,10 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virDomainPCIAddressSetPtr addrs) { - int ret = -1; size_t i; virPCIDeviceAddress tmp_addr; bool qemuDeviceVideoUsable = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); - char *addrStr = NULL; + VIR_AUTOFREE(char *) addrStr = NULL; virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPE_PCIE_DEVICE; for (i = 0; i < def->ncontrollers; i++) { @@ -1891,7 +1885,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, cont->info.addr.pci.function != 2) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Primary SATA controller must have PCI address 0:0:1f.2")); - goto cleanup; + return -1; } } else { cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; @@ -1928,7 +1922,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, } if (assign) { if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) - goto cleanup; + return -1; cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; cont->info.addr.pci.domain = 0; @@ -1951,7 +1945,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, tmp_addr.slot = 0x1E; if (!virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) - goto cleanup; + return -1; cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; cont->info.addr.pci.domain = 0; @@ -1975,12 +1969,12 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, tmp_addr.function = 0; tmp_addr.multi = VIR_TRISTATE_SWITCH_ON; if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) - goto cleanup; + return -1; tmp_addr.function = 3; tmp_addr.multi = VIR_TRISTATE_SWITCH_ABSENT; if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) - goto cleanup; + return -1; } if (def->nvideos > 0 && @@ -1997,25 +1991,25 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, tmp_addr.slot = 1; if (!(addrStr = virPCIDeviceAddressAsString(&tmp_addr))) - goto cleanup; + return -1; if (!virDomainPCIAddressValidate(addrs, &tmp_addr, addrStr, flags, true)) - goto cleanup; + return -1; if (virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { if (qemuDeviceVideoUsable) { if (qemuDomainPCIAddressReserveNextAddr(addrs, &primaryVideo->info) < 0) - goto cleanup; + return -1; } else { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("PCI address 0:0:1.0 is in use, " "QEMU needs it for primary video")); - goto cleanup; + return -1; } } else { if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) - goto cleanup; + return -1; primaryVideo->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; primaryVideo->info.addr.pci = tmp_addr; } @@ -2026,7 +2020,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, primaryVideo->info.addr.pci.function != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Primary video card must have PCI address 0:0:1.0")); - goto cleanup; + return -1; } /* If TYPE == PCI, then qemuDomainCollectPCIAddress() function * has already reserved the address, so we must skip */ @@ -2041,7 +2035,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, " intervention"); virResetLastError(); } else if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) { - goto cleanup; + return -1; } } @@ -2062,7 +2056,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, continue; } if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) - goto cleanup; + return -1; sound->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; sound->info.addr.pci = tmp_addr; @@ -2070,10 +2064,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, } } - ret = 0; - cleanup: - VIR_FREE(addrStr); - return ret; + return 0; } -- 2.21.0

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, as long as we provide a virPCIDeviceAddress to compare it to. The later can be done by direct assignment of the now existing virPCIDeviceAddress struct. The defined values of domain, bus, slot and function will be assigned to info->addr.pci, the other values are zeroed (which happens to be their default values too). It's also worth noticing that all these assignments are being conditioned by virDeviceInfoPCIAddressIsPresent() calls, thus it's sensible to discard any non-zero values that might happen to exist in @cont->info.addr, if we settled beforehand that @cont->info.addr is not present or bogus. Suggested-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_domain_address.c | 56 +++++++++++++++------------------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index e20481607f..4f9ae1f37d 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; + 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; + 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,16 @@ 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; + cont->info.addr.pci = primarySATAAddr; } } break; -- 2.21.0

On Fri, Sep 20, 2019 at 10:52:55AM -0300, Daniel Henrique Barboza wrote:
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, as long as we provide a virPCIDeviceAddress to compare it to.
The later can be done by direct assignment of the now existing virPCIDeviceAddress struct. The defined values of domain, bus, slot and function will be assigned to info->addr.pci, the other values are zeroed (which happens to be their default values too). It's also worth noticing that all these assignments are being conditioned by virDeviceInfoPCIAddressIsPresent() calls, thus it's sensible to discard any non-zero values that might happen to exist in @cont->info.addr, if we settled beforehand that @cont->info.addr is not present or bogus.
Suggested-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_domain_address.c | 56 +++++++++++++++------------------- 1 file changed, 25 insertions(+), 31 deletions(-)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index e20481607f..4f9ae1f37d 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; + 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; + 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,16 @@ 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)) {
actually, indent is off here, I'll fix it before pushing... Erik

On Fri, Sep 20, 2019 at 10:52:53AM -0300, Daniel Henrique Barboza wrote:
Second round of these cleanups after the reviews from Eric.
changes from v1: - fixed the commit message of patch 1 - ditched the virPCIAddressSet() approach - use direct assignment instead
v1: https://www.redhat.com/archives/libvir-list/2019-September/msg00823.html
Daniel Henrique Barboza (2): qemu_domain_address.c: use VIR_AUTOFREE() in strings qemu_domain_address: use virPCIDeviceAddressEqual() in conditionals
src/qemu/qemu_domain_address.c | 125 +++++++++++++++------------------ 1 file changed, 55 insertions(+), 70 deletions(-)
Reviewed-by: Erik Skultety <eskultet@redhat.com>
participants (2)
-
Daniel Henrique Barboza
-
Erik Skultety