On Thu, Jun 29, 2017 at 20:04:01 +0200, Andrea Bolognani wrote:
Instances allocated on the stack or using VIR_ALLOC() directly skip the initialization process. Use the proper allocation function instead to ensure all virDomainDeviceInfo instances are properly initialized.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_addr.c | 6 ++++-- src/conf/domain_conf.c | 4 ++-- src/libvirt_private.syms | 2 ++ src/qemu/qemu_domain_address.c | 29 +++++++++++++++++------------ 4 files changed, 25 insertions(+), 16 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 178aefc..48e9e33 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -91,6 +91,8 @@ virCPUModeTypeToString; # conf/device_conf.h virDomainDeviceInfoAddressIsEqual; virDomainDeviceInfoCopy; +virDomainDeviceInfoFree; +virDomainDeviceInfoNew;
Looks like this belongs more like it belongs to the patch moving the functions.
virInterfaceLinkFormat; virInterfaceLinkParseXML; virPCIDeviceAddressEqual; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index b5b863f..93ab304 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1983,6 +1983,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, int ret = -1; virDomainPCIAddressSetPtr addrs = NULL; qemuDomainObjPrivatePtr priv = NULL; + virDomainDeviceInfoPtr info = virDomainDeviceInfoNew(); int max_idx = -1; int nbuses = 0; size_t i; @@ -2031,16 +2032,15 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, */
if (qemuDomainHasPCIRoot(def)) { - /* This is a dummy info used to reserve a slot for a + bool buses_reserved = true; + + /* The dummy info is used to reserve a slot for a * legacy PCI device that doesn't exist, but may in the * future, e.g. if another device is hotplugged into the * domain. */ - virDomainDeviceInfo info = { - .pciConnectFlags = (VIR_PCI_CONNECT_HOTPLUGGABLE | - VIR_PCI_CONNECT_TYPE_PCI_DEVICE) - }; - bool buses_reserved = true; + info->pciConnectFlags = (VIR_PCI_CONNECT_HOTPLUGGABLE |
@info may be NULL here.
+ VIR_PCI_CONNECT_TYPE_PCI_DEVICE);
for (i = 0; i < addrs->nbuses; i++) { if (!qemuDomainPCIBusFullyReserved(&addrs->buses[i])) { @@ -2049,7 +2049,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, } } if (!buses_reserved && - qemuDomainPCIAddressReserveNextAddr(addrs, &info) < 0) + qemuDomainPCIAddressReserveNextAddr(addrs, info) < 0) goto cleanup; }
@@ -2073,15 +2073,19 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, if (max_idx <= 0 && addrs->nbuses > max_idx + 1 && qemuDomainHasPCIeRoot(def)) { - virDomainDeviceInfo info = { - .pciConnectFlags = (VIR_PCI_CONNECT_HOTPLUGGABLE | - VIR_PCI_CONNECT_TYPE_PCIE_DEVICE) - }; + + /* The dummy info is used to reserve a slot for a + * PCI Express device that doesn't exist, but may in the + * future, e.g. if another device is hotplugged into the + * domain. + */ + info->pciConnectFlags = (VIR_PCI_CONNECT_HOTPLUGGABLE |
Here too.
+ VIR_PCI_CONNECT_TYPE_PCIE_DEVICE);
/* if there isn't an empty pcie-root-port, this will * cause one to be added */ - if (qemuDomainPCIAddressReserveNextAddr(addrs, &info) < 0) + if (qemuDomainPCIAddressReserveNextAddr(addrs, info) < 0) goto cleanup; }
ACK if you make sure that @info is valid, which kind of wasn't necessary with stack alloc'd variables.