[libvirt] [PATCHv7 3/4] domifaddr: Implement the API for qemu

[Wang Rui]

On 2014/12/17 8:16, Nehal J Wani wrote:
> By querying the qemu guest agent with the QMP command
> "guest-network-get-interfaces" and converting the received JSON
> output to structured objects.
> 
> Although "ifconfig" is deprecated, IP aliases created by "ifconfig"
> are supported by this API. The legacy syntax of an IP alias is:
> "<ifname>:<alias-name>". Since we want all aliases to be clubbed
> under parent interface, simply stripping ":<alias-name>" suffices.
> Note that IP aliases formed by "ip" aren't visible to "ifconfig",
> and aliases created by "ip" do not have any specific name. But
> we are lucky, as qemu guest agent detects aliases created by both.
> 
[...]

> +static int
> +qemuGetDHCPInterfaces(virDomainPtr dom,
> +                      virDomainObjPtr vm,
> +                      virDomainInterfacePtr **ifaces)
> +{
> +    int rv = -1;
> +    int n_leases = 0;
> +    size_t i, j;
> +    size_t ifaces_count = 0;
> +    virNetworkPtr network;
> +    char macaddr[VIR_MAC_STRING_BUFLEN];
> +    virDomainInterfacePtr iface = NULL;
> +    virNetworkDHCPLeasePtr *leases = NULL;
> +    virDomainInterfacePtr *ifaces_ret = NULL;
> +
> +    for (i = 0; i < vm->def->nnets; i++) {
> +        virMacAddrFormat(&(vm->def->nets[i]->mac), macaddr);
> +        network = virNetworkLookupByName(dom->conn,
> +                                         vm->def->nets[i]->data.network.name);
> +
> +        if (dom->conn->networkDriver &&
> +            dom->conn->networkDriver->networkGetDHCPLeases) {
Is it better to move this check out of the loop?

> +            n_leases = virNetworkGetDHCPLeases(network, macaddr,
> +                                         &leases, 0);
Memory is allocated for 'leases' in the loop 'nnets' times. But it is freed only
once at the end. Should it be freed in the loop, not at the end of the function?

> +            if (n_leases < 0)
> +                goto error;
> +        }
> +
> +        if (n_leases) {
> +            if (VIR_EXPAND_N(ifaces_ret, ifaces_count, 1) < 0)
> +                goto error;
> +
> +            if (VIR_ALLOC(ifaces_ret[ifaces_count - 1]) < 0)
> +                goto error;
> +
> +            iface = ifaces_ret[ifaces_count - 1];
> +            /* Assuming each lease corresponds to a separate IP */
> +            iface->naddrs = n_leases;
> +
> +            if (VIR_ALLOC_N(iface->addrs, iface->naddrs) < 0)
> +                goto error;
> +
> +            if (VIR_STRDUP(iface->name, vm->def->nets[i]->ifname) < 0)
> +                goto cleanup;
> +
> +            if (VIR_STRDUP(iface->hwaddr, macaddr) < 0)
> +                goto cleanup;
> +        }
> +
> +        for (j = 0; j < n_leases; j++) {
> +            virNetworkDHCPLeasePtr lease = leases[j];
> +            virDomainIPAddressPtr ip_addr = &iface->addrs[j];
> +
> +            if (VIR_STRDUP(ip_addr->addr, lease->ipaddr) < 0)
> +                goto cleanup;
> +
> +            ip_addr->type = lease->type;
> +            ip_addr->prefix = lease->prefix;
> +        }
> +    }
> +
> +    *ifaces = ifaces_ret;
> +    ifaces_ret = NULL;
> +    rv = ifaces_count;
> +
> + cleanup:
> +    for (i = 0; i < n_leases; i++)
> +        virNetworkDHCPLeaseFree(leases[i]);
> +
> +    VIR_FREE(leases);
> +    return rv;
> +
> + error:
> +    if (ifaces_ret) {
> +        for (i = 0; i < ifaces_count; i++)
> +            virDomainInterfaceFree(ifaces_ret[i]);
> +    }
> +    VIR_FREE(ifaces_ret);
> +
> +    goto cleanup;
> +}