Chun Yan Liu wrote:
>
>>>> On 5/14/2016 at 07:47 AM, in message <5736677D.8030209@xxxxxxxx>, Jim Fehlig
> <jfehlig@xxxxxxxx> 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@xxxxxxxx>
>>> ---
>>> 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?
Hmm, maybe something like networkGetDHCPLeasesAll() is a better name. Regardless
of the name, I see that other functions in bridge_driver.c take a
virDomainDefPtr, so maybe extracting the code to bridge_driver.c is acceptable.
Laine (cc'd) is generally regarded as the libvirt networking expert :-). Let's
see if he has an opinion.
Regards,
Jim
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list