[libvirt] [PATCH] libxl: add .domainInterfaceAddresses

[Chun Yan Liu]

>>> On 5/14/2016 at 07:47 AM, in message <5736677D.8030209 at suse.com>, Jim Fehlig
<jfehlig at suse.com> wrote: 
> 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?

Well, it can be extracted and placed in bridge_driver.c as networkGetDHCPInterfaces,
but I don't know if that is acceptable?

Chunyan

>  
> Regards, 
> Jim 
>  
>