Re: [libvirt] [PATCHv7 3/4] domifaddr: Implement the API for qemu
On Wed, Dec 17, 2014 at 11:56 AM, Wang Rui <moon.wangrui(a)huawei.com> wrote:
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?
Yup, seems like a redundant check for each loop. Will fix that.
> + 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?
Whoops! Thats a huge leak! Will fix it in the next revision.
> + 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;
> +}
--
Nehal J Wani