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@xxxxxxxxxx>
> ---
> 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.
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
