[libvirt] [PATCHv2 1/4] util: introduce 2 new helpers for get interface IPv6 address

Luyao Huang lhuang at redhat.com
Mon Mar 9 15:37:53 UTC 2015


On 03/09/2015 07:50 AM, John Ferlan wrote:
> On 02/28/2015 04:08 AM, Luyao Huang wrote:
>> Introduce virNetDevGetIfaddrsAddress to help to get interface IPv6
>> and IPv4 address.
>>
>> Introduce virNetDevGetIPAddress to use virNetDevGetIfaddrsAddress
>> and virNetDevGetIPv4Address to get IP address.
>>
>> Signed-off-by: Luyao Huang <lhuang at redhat.com>
>> ---
>> v2: add a new function virNetDevGetIPAddress to call virNetDevGetIfaddrsAddress
>> , and make virNetDevGetIfaddrsAddress can get ipv4 address for a interface.
>> Also add a error output in virNetDevGetIfaddrsAddress when get multiple ip
>> address for a host interface.
>>
>>   src/libvirt_private.syms |  1 +
>>   src/util/virnetdev.c     | 98 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   src/util/virnetdev.h     |  4 ++
>>   3 files changed, 103 insertions(+)
>>
...
>
>> +        if (ifa->ifa_addr->sa_family == (want_ipv6 ? AF_INET6 : AF_INET)) {
>> +            if (++nIPaddr > 1)
>> +                break;
> [1]... hmm not sure about this...
>
>> +            if (want_ipv6) {
>> +                addr->len = sizeof(addr->data.inet6);
>> +                memcpy(&addr->data.inet6, ifa->ifa_addr, addr->len);
>> +            } else {
>> +                addr->len = sizeof(addr->data.inet4);
>> +                memcpy(&addr->data.inet4, ifa->ifa_addr, addr->len);
>> +            }
>> +            addr->data.stor.ss_family = ifa->ifa_addr->sa_family;
>> +        }
>> +    }
>> +
>> +    if (nIPaddr == 1)
>> +        ret = 0;
>> +    else if (nIPaddr > 1)
>> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                       _("Interface '%s' has multiple %s address"),
> s/address/addresses
>
>> +                       ifname, want_ipv6 ? "IPv6" : "IPv4");
> [1]
> But is this really desired... It seems from the previous review, if
> someone wants a specific address they use "<listen type='address'...".
>
> Otherwise, they use this function.  Since you'll be returning either an
> IPv4 or IPv6 address here you'd be causing an error here if the network
> had more than one IPv4 address defined; whereas, the previous code just
> returned the "first" from the ioctl(SIOCGIFADDR...).
>
> I think once you have an address you just return the first one found and
> document it that way avoiding this path completely. You could also note
> that if you want a specific address use the type='address'
>
>
I had an idea but my eyes almost close ;)  so i write here without test 
them.

I think it will be better if we make this function to get mutli address 
and return the number of address we get, like this:

+static int
+virNetDevGetIfaddrsAddress(const char *ifname,
+                           bool want_ipv6,
+                           virSocketAddrPtr *addrs)
+{
+    struct ifaddrs *ifap, *ifa;
+    int nIPaddr = 0;
+
+    if (getifaddrs(&ifap) < 0) {
+        virReportSystemError(errno,
+                             _("Could not get interface list for '%s'"),
+                             ifname);
+        return -1;
+    }
+
+    for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
+        virSocketAddrPtr tmpaddr;
+
+        if (!ifa->ifa_addr ||
+            STRNEQ_NULLABLE(ifa->ifa_name, ifname)) {
+            continue;
+        }
+
+        if (ifa->ifa_addr->sa_family == (want_ipv6 ? AF_INET6 : AF_INET)) {
+            memset(tmpaddr, 0, sizeof(*tmpaddr));
+
+        if (!ifa->ifa_addr ||
+            STRNEQ_NULLABLE(ifa->ifa_name, ifname)) {
+            continue;
+        }
+
+        if (ifa->ifa_addr->sa_family == (want_ipv6 ? AF_INET6 : AF_INET)) {
+            memset(tmpaddr, 0, sizeof(*tmpaddr));
+
+            if (want_ipv6) {
+                tmpaddr->len = sizeof(tmpaddr->data.inet6);
+                memcpy(&tmpaddr->data.inet6, ifa->ifa_addr, tmpaddr->len);
+            } else {
+                tmpaddr->len = sizeof(tmpaddr->data.inet4);
+                memcpy(&tmpaddr->data.inet4, ifa->ifa_addr, tmpaddr->len);
+            }
+            tmpaddr->data.stor.ss_family = ifa->ifa_addr->sa_family;
+            addrs[nIPaddr++] = tmpaddr;
+        }
+    }
+    if (nIPaddr == 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Interface '%s' not found"),
+                       ifname);
+        return -1;
+    }
+    freeifaddrs(ifap);
+    return nIPaddr;
+}

>> +int virNetDevGetIPAddress(const char *ifname,
>> +                          bool want_ipv6,
>> +                          virSocketAddrPtr addr)
and then rename this function to virNetDevGetOneIPAddress()

and rework the code like this:

+int virNetDevGetOneIPAddress(const char *ifname,
+                                                 bool want_ipv6,
+                                                 virSocketAddrPtr addr)
+{
+    int ret;
+    virSocketAddrPtr *addrs = NULL;
+
+    ret = virNetDevGetIfaddrsAddress(ifname, want_ipv6, addrs);
+    if (ret > 0) {
+        addr = addrs[0];
+    } else if (ret == -2 && !want_ipv6) {
+        return virNetDevGetIPv4Address(ifname, addr);
+    }
+
+    return ret;
+}

...
>
> These changes require modifying src/network/bridge_driver.c (temporarily
> until we add the IPv6 aware code later):
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 2a61991..7d6e173 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -4608,7 +4608,7 @@ networkGetNetworkAddress(const char *netname, char
> **netaddr)
>       }
>
>       if (dev_name) {
> -        if (virNetDevGetIPv4Address(dev_name, &addr) < 0)
> +        if (virNetDevGetIPAddress(dev_name, false, &addr) < 0)
>               goto cleanup;
>           addrptr = &addr;
>       }
>

At last, add the multiple address check to here and this place will like 
this:

+        int num = virNetDevGetOneIPAddress(dev_name, want_ipv6, &addr);
+        if (num > 1) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("interface '%s' has multiple %s addresses"),
+                           dev_name, want_ipv6 ? "IPv6" : "IPv4");
+            goto cleanup;
+        } else if (num < 0) {
              goto cleanup;
+        }


because i am not test them so there will be some mistake, I will test 
them tomorrow, hope it will work :)
>
>
> John

Luyao




More information about the libvir-list mailing list