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,
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list