[libvirt] [PATCH 11/28] qemu: eliminate memory leaks when converting NetDefs to type='ethernet'

[John Ferlan]

On 06/22/2016 01:37 PM, Laine Stump wrote:
> in qemuConnectDomainXMLToNative. This function was only accounting for
> about 1/10 of all the allocated items in the NetDef prior to memseting
> it to all 0's. On top of that, it was going to great pains to learn
> the name of the bridge device, but then never doing anything useful
> with it (just putting it into data.ethernet.dev, which is *never* used
> when building a qemu commandline). (I think this again all started off
> as code with good intentions, but it was never completed, and instead
> was just Frankensteinically cargo-culted into the odd mish mash we
> have today).
> 
> The resulting code is much simpler, produces exactly the same output,
> and doesn't leak memory.
> ---
>  src/qemu/qemu_driver.c | 56 ++++++--------------------------------------------
>  1 file changed, 6 insertions(+), 50 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 517d0b8..4a8cb7a 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -6987,62 +6987,18 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn,
>          unsigned int bootIndex = net->info.bootIndex;
>          char *model = net->model;
>          virMacAddr mac = net->mac;
> +        char *script = net->script;

Based on 3 spots below where net->script was set to NULL, should this
only be set when "(net->type == VIR_DOMAIN_NET_TYPE_BRIDGE)" ?


>  
> -        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
> -            int actualType = virDomainNetGetActualType(net);
> -            const char *brname;
> -
> -            VIR_FREE(net->data.network.name);
> -            VIR_FREE(net->data.network.portgroup);
> -            if ((actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) &&
> -                (brname = virDomainNetGetActualBridgeName(net))) {
> -
> -                char *brnamecopy;
> -
> -                if (VIR_STRDUP(brnamecopy, brname) < 0)
> -                    goto cleanup;
> -
> -                virDomainActualNetDefFree(net->data.network.actual);
> -
> -                memset(net, 0, sizeof(*net));
> -
> -                net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
> -                net->script = NULL;
^^^

> -                net->data.ethernet.dev = brnamecopy;
> -            } else {
> -                /* actualType is either NETWORK or DIRECT. In either
> -                 * case, the best we can do is NULL everything out.
> -                 */
> -                virDomainActualNetDefFree(net->data.network.actual);
> -                memset(net, 0, sizeof(*net));
> -
> -                net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
> -                net->script = NULL;
^^^

> -                net->data.ethernet.dev = NULL;
> -            }
> -        } else if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
> -            VIR_FREE(net->data.direct.linkdev);
> +        net->model = NULL;
> +        net->script = NULL;
>  
> -            memset(net, 0, sizeof(*net));
> -
> -            net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
> -            net->script = NULL;
^^^

ACK - whichever way you go with that 'script' setting. I figure you know
the best answer to my question


John
> -            net->data.ethernet.dev = NULL;
> -        } else if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
> -            char *script = net->script;
> -            char *brname = net->data.bridge.brname;
> -
> -            memset(net, 0, sizeof(*net));
> -
> -            net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
> -            net->script = script;
> -            net->data.ethernet.dev = brname;
> -        }
> +        virDomainNetDefClear(net);
>  
> -        VIR_FREE(net->virtPortProfile);
> +        net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
>          net->info.bootIndex = bootIndex;
>          net->model = model;
>          net->mac = mac;
> +        net->script = script;
>      }
>  
>      if (!(cmd = qemuProcessCreatePretendCmd(conn, driver, vm, NULL,
>