[libvirt] [PATCH 08/10] conf, qemu: Use virDomainDeviceInfoNew()

[Peter Krempa]

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 at 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170630/ebd898c9/attachment-0001.sig>