[libvirt] [PATCH] libxl: add .domainInterfaceAddresses

[Jim Fehlig]

On 05/13/2016 12:21 AM, Chunyan Liu wrote:
> Add .domainInterfaceAddresses so that user can have a way to
> get domain interface address by 'virsh domifaddr'. Currently
> it only supports '--source lease'.
>
> Signed-off: Chunyan Liu <cyliu at suse.com>
> ---
>  src/libxl/libxl_driver.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 140 insertions(+)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 062d6f8..f2bd6fa 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -5425,6 +5425,145 @@ static int libxlNodeGetSecurityModel(virConnectPtr conn,
>      return 0;
>  }
>  
> +static int
> +libxlGetDHCPInterfaces(virDomainPtr dom,
> +                       virDomainObjPtr vm,
> +                       virDomainInterfacePtr **ifaces)
> +{
> +    int rv = -1;
> +    int n_leases = 0;
> +    size_t i, j;
> +    size_t ifaces_count = 0;
> +    virNetworkPtr network = NULL;
> +    char macaddr[VIR_MAC_STRING_BUFLEN];
> +    virDomainInterfacePtr iface = NULL;
> +    virNetworkDHCPLeasePtr *leases = NULL;
> +    virDomainInterfacePtr *ifaces_ret = NULL;
> +
> +    if (!dom->conn->networkDriver ||
> +        !dom->conn->networkDriver->networkGetDHCPLeases) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Network driver does not support DHCP lease query"));
> +        return -1;
> +    }
> +
> +    for (i = 0; i < vm->def->nnets; i++) {
> +        if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK)
> +            continue;
> +
> +        virMacAddrFormat(&(vm->def->nets[i]->mac), macaddr);
> +        virObjectUnref(network);
> +        network = virNetworkLookupByName(dom->conn,
> +                                         vm->def->nets[i]->data.network.name);
> +
> +        if ((n_leases = virNetworkGetDHCPLeases(network, macaddr,
> +                                                &leases, 0)) < 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;
> +        }
> +
> +        for (j = 0; j < n_leases; j++)
> +            virNetworkDHCPLeaseFree(leases[j]);
> +
> +        VIR_FREE(leases);
> +    }
> +
> +    *ifaces = ifaces_ret;
> +    ifaces_ret = NULL;
> +    rv = ifaces_count;
> +
> + cleanup:
> +    virObjectUnref(network);
> +    if (leases) {
> +        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;
> +}

It's unfortunate this is a copy-paste from the qemu driver. The code is not
trivial and any bug fixes in one copy could be missed in the other. A lot of the
function is domain related, so probably can't be abstracted further to the
network code. Have you considered approaches that allow the drivers to share
this code?

Regards,
Jim