Message ID | b848e4cf636e5804b67c5592c3557456a48a5f66.1329413065.git.mprivozn@redhat.com |
---|---|
State | New |
Headers | show |
On Thu, Feb 16, 2012 at 06:25:03PM +0100, Michal Privoznik wrote: > This command returns an array of: > > [ifname, ipaddr, ipaddr_family, prefix, hwaddr] > > for each interface in the system that has an IP address. > Currently, only IPv4 and IPv6 are supported. Cool stuff, seems pretty useful. Some comments below: > > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> > --- > qapi-schema-guest.json | 16 +++++ > qga/guest-agent-commands.c | 156 ++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 172 insertions(+), 0 deletions(-) > > diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json > index 5f8a18d..14de133 100644 > --- a/qapi-schema-guest.json > +++ b/qapi-schema-guest.json > @@ -219,3 +219,19 @@ > ## > { 'command': 'guest-fsfreeze-thaw', > 'returns': 'int' } > + > +## > +# @guest-getip: I'd prefer guest-get-ip. But furthermore, we're returning more than just IPs so maybe something like guest-network-info? It also fits the guest-file-* and guest-fsfreeze-* format better, which is nice if we ever add new guest-network-* commands. > +# > +# Get list of guest IP addresses, MAC address *addresses > +# and netmasks > +# > +# Returns: List of GuestIFAddress > +# > +# Since: 1.1 > +## > +{ 'type': 'GuestIFAddress', > + 'data': {'iface': {'name': 'str', 'ipaddr': 'str', 'ipaddrtype': 'int', ipaddrtype seems like a good candidate for an enum type, this lets us map to something less OS-specific and more user-readable, like "ipv4" and "ipv6". See: GuestFsfreezeStatus. > + 'prefix': 'int', 'hwaddr': 'str'} } } It's not upstream yet, but we've moved to documenting all user-defined types in the same manner we currently do with qapi-schema.json, so let's please do that here as well. > +{ 'command': 'guest-getip', > + 'returns': ['GuestIFAddress'] } > diff --git a/qga/guest-agent-commands.c b/qga/guest-agent-commands.c > index a09c8ca..4caffa7 100644 > --- a/qga/guest-agent-commands.c > +++ b/qga/guest-agent-commands.c > @@ -5,6 +5,7 @@ > * > * Authors: > * Michael Roth <mdroth@linux.vnet.ibm.com> > + * Michal Privoznik <mprivozn@redhat.com> > * > * This work is licensed under the terms of the GNU GPL, version 2 or later. > * See the COPYING file in the top-level directory. > @@ -23,6 +24,10 @@ > > #include <sys/types.h> > #include <sys/ioctl.h> > +#include <ifaddrs.h> > +#include <arpa/inet.h> > +#include <sys/socket.h> > +#include <net/if.h> > #include "qga/guest-agent-core.h" > #include "qga-qmp-commands.h" > #include "qerror.h" > @@ -583,3 +588,154 @@ void ga_command_state_init(GAState *s, GACommandState *cs) > #endif > ga_command_state_add(cs, guest_file_init, NULL); > } > + > +/* Count the number of '1' bits in @x */ > +static int32_t > +count_one_bits(uint32_t x) > +{ > + x = ((x & 0xaaaaaaaaU) >> 1) + (x & 0x55555555U); > + x = ((x & 0xccccccccU) >> 2) + (x & 0x33333333U); > + x = (x >> 16) + (x & 0xffff); > + x = ((x & 0xf0f0) >> 4) + (x & 0x0f0f); > + return (x >> 8) + (x & 0x00ff); > +} I think my brain is too small for this. Why not just: for (i = 0, count = 0; i < 32; i++) { if (x & (1 << i)) { count++; } } return count; ? > + > +/* > + * Get the list of interfaces among with their > + * IP/MAC addresses, prefixes and IP versions > + */ > +GuestIFAddressList * qmp_guest_getip(Error **err) > +{ > + GuestIFAddressList *head = NULL, *cur_item = NULL; > + struct ifaddrs *ifap = NULL, *ifa = NULL; > + char err_msg[512]; > + > + slog("guest-getip called"); > + > + if (getifaddrs(&ifap) < 0) { > + snprintf(err_msg, sizeof(err_msg), > + "getifaddrs failed : %s", strerror(errno)); > + error_set(err, QERR_QGA_COMMAND_FAILED, err_msg); > + goto cleanup; > + } > + > + ifa = ifap; > + while (ifa) { > + GuestIFAddressList *info = NULL; > + char addr4[INET_ADDRSTRLEN]; > + char addr6[INET6_ADDRSTRLEN]; > + unsigned char *mac_addr; > + void *tmpAddrPtr = NULL; *tmp_addr_ptr. Avoid camelcase for non-structured types (QEMU coding guidelines). > + int sock, family; > + int mac_supported; > + struct ifreq ifr; > + > + /* Step over interfaces without an address */ > + if (!ifa->ifa_addr) > + continue; > + > + g_debug("Processing %s interface", ifa->ifa_name); > + > + family = ifa->ifa_addr->sa_family; > + > + if (family == AF_INET) { > + /* interface with IPv4 address */ > + tmpAddrPtr = &((struct sockaddr_in *)ifa->ifa_addr)->sin_addr; > + inet_ntop(AF_INET, tmpAddrPtr, addr4, sizeof(addr4)); > + > + info = g_malloc0(sizeof(*info)); > + info->value = g_malloc0(sizeof(*info->value)); > + info->value->iface.name = g_strdup(ifa->ifa_name); > + info->value->iface.ipaddr = g_strdup(addr4); > + > + /* This is safe as '1' and '0' cannot be shuffled in netmask. */ > + tmpAddrPtr = &((struct sockaddr_in *)ifa->ifa_netmask)->sin_addr; > + info->value->iface.prefix = count_one_bits(((uint32_t *) tmpAddrPtr)[0]); > + } else if (family == AF_INET6) { > + /* interface with IPv6 address */ > + tmpAddrPtr = &((struct sockaddr_in6 *)ifa->ifa_addr)->sin6_addr; > + inet_ntop(AF_INET6, tmpAddrPtr, addr6, sizeof(addr6)); > + > + info = g_malloc0(sizeof(*info)); > + info->value = g_malloc0(sizeof(*info->value)); > + info->value->iface.name = g_strdup(ifa->ifa_name); > + info->value->iface.ipaddr = g_strdup(addr6); > + > + /* This is safe as '1' and '0' cannot be shuffled in netmask. */ > + tmpAddrPtr=&((struct sockaddr_in6 *)ifa->ifa_netmask)->sin6_addr; > + info->value->iface.prefix = count_one_bits(((uint32_t *) tmpAddrPtr)[0]) + > + count_one_bits(((uint32_t *) tmpAddrPtr)[1]) + > + count_one_bits(((uint32_t *) tmpAddrPtr)[2]) + > + count_one_bits(((uint32_t *) tmpAddrPtr)[3]); > + } > + > + /* Add new family here */ > + > + mac_supported = ifa->ifa_flags & SIOCGIFHWADDR; > + > + ifa = ifa->ifa_next; > + > + if (!info) > + continue; > + > + if (!cur_item) { > + head = cur_item = info; > + } else { > + cur_item->next = info; > + cur_item = info; > + } > + > + info->value->iface.ipaddrtype = family; > + > + g_debug("Appending to return: iface=%s, ip=%s, type=%llu, prefix=%llu", > + info->value->iface.name, > + info->value->iface.ipaddr, > + (unsigned long long) info->value->iface.ipaddrtype, > + (unsigned long long) info->value->iface.prefix); > + > + /* If we can't get get MAC address on this interface, > + * don't even try but continue to another interface */ > + if (!mac_supported) > + continue; > + > + sock = socket(PF_INET, SOCK_STREAM, 0); > + > + if (sock == -1) { > + snprintf(err_msg, sizeof(err_msg), > + "failed to create socket: %s", strerror(errno)); > + error_set(err, QERR_QGA_COMMAND_FAILED, err_msg); > + goto cleanup; > + } > + > + memset(&ifr, 0, sizeof(ifr)); > + strncpy(ifr.ifr_name, info->value->iface.name, IF_NAMESIZE); > + if (ioctl(sock, SIOCGIFHWADDR, &ifr) == -1) { > + snprintf(err_msg, sizeof(err_msg), > + "failed to get MAC addres of %s: %s", > + info->value->iface.name, > + strerror(errno)); > + error_set(err, QERR_QGA_COMMAND_FAILED, err_msg); > + goto cleanup; If we propagate an error we should return NULL and make sure head is cleaned up (use qapi_free_GuestIFAddressList()) > + } > + > + mac_addr = (unsigned char *) &ifr.ifr_hwaddr.sa_data; > + > + if (asprintf(&info->value->iface.hwaddr, > + "%02x:%02x:%02x:%02x:%02x:%02x", > + (int) mac_addr[0], (int) mac_addr[1], > + (int) mac_addr[2], (int) mac_addr[3], > + (int) mac_addr[4], (int) mac_addr[5]) == -1) { > + snprintf(err_msg, sizeof(err_msg), > + "failed to format MAC: %s", strerror(errno)); > + error_set(err, QERR_QGA_COMMAND_FAILED, err_msg); > + goto cleanup; > + } > + > + close(sock); > + } > + > +cleanup: > + freeifaddrs(ifap); > + > + return head; > +} > -- > 1.7.3.4 > >
On 16.02.2012 21:10, Michael Roth wrote: > On Thu, Feb 16, 2012 at 06:25:03PM +0100, Michal Privoznik wrote: >> This command returns an array of: >> >> [ifname, ipaddr, ipaddr_family, prefix, hwaddr] >> >> for each interface in the system that has an IP address. >> Currently, only IPv4 and IPv6 are supported. > > Cool stuff, seems pretty useful. Some comments below: > >> >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >> --- >> qapi-schema-guest.json | 16 +++++ >> qga/guest-agent-commands.c | 156 ++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 172 insertions(+), 0 deletions(-) >> >> + >> +/* Count the number of '1' bits in @x */ >> +static int32_t >> +count_one_bits(uint32_t x) >> +{ >> + x = ((x & 0xaaaaaaaaU) >> 1) + (x & 0x55555555U); >> + x = ((x & 0xccccccccU) >> 2) + (x & 0x33333333U); >> + x = (x >> 16) + (x & 0xffff); >> + x = ((x & 0xf0f0) >> 4) + (x & 0x0f0f); >> + return (x >> 8) + (x & 0x00ff); >> +} > > I think my brain is too small for this. Why not just: > > for (i = 0, count = 0; i < 32; i++) { > if (x & (1 << i)) { > count++; > } > } > return count; > > ? That algorithm I've used computes what is known as Hamming weight: http://en.wikipedia.org/wiki/Hamming_weight#Efficient_implementation It has advantage of being constant in time. However, I must admit, it is not easy readable so I'd better use something more obvious. Thanks for review, I'll sent v2. Michal
On 02/17/2012 04:58 AM, Michal Privoznik wrote: > On 16.02.2012 21:10, Michael Roth wrote: >> On Thu, Feb 16, 2012 at 06:25:03PM +0100, Michal Privoznik wrote: >>> This command returns an array of: >>> >>> [ifname, ipaddr, ipaddr_family, prefix, hwaddr] >>> >>> for each interface in the system that has an IP address. >>> Currently, only IPv4 and IPv6 are supported. >> >> Cool stuff, seems pretty useful. Some comments below: >> >>> >>> Signed-off-by: Michal Privoznik<mprivozn@redhat.com> >>> --- >>> qapi-schema-guest.json | 16 +++++ >>> qga/guest-agent-commands.c | 156 ++++++++++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 172 insertions(+), 0 deletions(-) >>> > >>> + >>> +/* Count the number of '1' bits in @x */ >>> +static int32_t >>> +count_one_bits(uint32_t x) >>> +{ >>> + x = ((x& 0xaaaaaaaaU)>> 1) + (x& 0x55555555U); >>> + x = ((x& 0xccccccccU)>> 2) + (x& 0x33333333U); >>> + x = (x>> 16) + (x& 0xffff); >>> + x = ((x& 0xf0f0)>> 4) + (x& 0x0f0f); >>> + return (x>> 8) + (x& 0x00ff); >>> +} >> >> I think my brain is too small for this. Why not just: >> >> for (i = 0, count = 0; i< 32; i++) { >> if (x& (1<< i)) { >> count++; >> } >> } >> return count; >> >> ? > > That algorithm I've used computes what is known as Hamming weight: > > http://en.wikipedia.org/wiki/Hamming_weight#Efficient_implementation > > It has advantage of being constant in time. However, I must admit, it is > not easy readable so I'd better use something more obvious. > > Thanks for review, I'll sent v2. __builtin_popcount() will do this as a single instruction on many architectures. Regards, Anthony Liguori > > Michal > >
diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json index 5f8a18d..14de133 100644 --- a/qapi-schema-guest.json +++ b/qapi-schema-guest.json @@ -219,3 +219,19 @@ ## { 'command': 'guest-fsfreeze-thaw', 'returns': 'int' } + +## +# @guest-getip: +# +# Get list of guest IP addresses, MAC address +# and netmasks +# +# Returns: List of GuestIFAddress +# +# Since: 1.1 +## +{ 'type': 'GuestIFAddress', + 'data': {'iface': {'name': 'str', 'ipaddr': 'str', 'ipaddrtype': 'int', + 'prefix': 'int', 'hwaddr': 'str'} } } +{ 'command': 'guest-getip', + 'returns': ['GuestIFAddress'] } diff --git a/qga/guest-agent-commands.c b/qga/guest-agent-commands.c index a09c8ca..4caffa7 100644 --- a/qga/guest-agent-commands.c +++ b/qga/guest-agent-commands.c @@ -5,6 +5,7 @@ * * Authors: * Michael Roth <mdroth@linux.vnet.ibm.com> + * Michal Privoznik <mprivozn@redhat.com> * * This work is licensed under the terms of the GNU GPL, version 2 or later. * See the COPYING file in the top-level directory. @@ -23,6 +24,10 @@ #include <sys/types.h> #include <sys/ioctl.h> +#include <ifaddrs.h> +#include <arpa/inet.h> +#include <sys/socket.h> +#include <net/if.h> #include "qga/guest-agent-core.h" #include "qga-qmp-commands.h" #include "qerror.h" @@ -583,3 +588,154 @@ void ga_command_state_init(GAState *s, GACommandState *cs) #endif ga_command_state_add(cs, guest_file_init, NULL); } + +/* Count the number of '1' bits in @x */ +static int32_t +count_one_bits(uint32_t x) +{ + x = ((x & 0xaaaaaaaaU) >> 1) + (x & 0x55555555U); + x = ((x & 0xccccccccU) >> 2) + (x & 0x33333333U); + x = (x >> 16) + (x & 0xffff); + x = ((x & 0xf0f0) >> 4) + (x & 0x0f0f); + return (x >> 8) + (x & 0x00ff); +} + +/* + * Get the list of interfaces among with their + * IP/MAC addresses, prefixes and IP versions + */ +GuestIFAddressList * qmp_guest_getip(Error **err) +{ + GuestIFAddressList *head = NULL, *cur_item = NULL; + struct ifaddrs *ifap = NULL, *ifa = NULL; + char err_msg[512]; + + slog("guest-getip called"); + + if (getifaddrs(&ifap) < 0) { + snprintf(err_msg, sizeof(err_msg), + "getifaddrs failed : %s", strerror(errno)); + error_set(err, QERR_QGA_COMMAND_FAILED, err_msg); + goto cleanup; + } + + ifa = ifap; + while (ifa) { + GuestIFAddressList *info = NULL; + char addr4[INET_ADDRSTRLEN]; + char addr6[INET6_ADDRSTRLEN]; + unsigned char *mac_addr; + void *tmpAddrPtr = NULL; + int sock, family; + int mac_supported; + struct ifreq ifr; + + /* Step over interfaces without an address */ + if (!ifa->ifa_addr) + continue; + + g_debug("Processing %s interface", ifa->ifa_name); + + family = ifa->ifa_addr->sa_family; + + if (family == AF_INET) { + /* interface with IPv4 address */ + tmpAddrPtr = &((struct sockaddr_in *)ifa->ifa_addr)->sin_addr; + inet_ntop(AF_INET, tmpAddrPtr, addr4, sizeof(addr4)); + + info = g_malloc0(sizeof(*info)); + info->value = g_malloc0(sizeof(*info->value)); + info->value->iface.name = g_strdup(ifa->ifa_name); + info->value->iface.ipaddr = g_strdup(addr4); + + /* This is safe as '1' and '0' cannot be shuffled in netmask. */ + tmpAddrPtr = &((struct sockaddr_in *)ifa->ifa_netmask)->sin_addr; + info->value->iface.prefix = count_one_bits(((uint32_t *) tmpAddrPtr)[0]); + } else if (family == AF_INET6) { + /* interface with IPv6 address */ + tmpAddrPtr = &((struct sockaddr_in6 *)ifa->ifa_addr)->sin6_addr; + inet_ntop(AF_INET6, tmpAddrPtr, addr6, sizeof(addr6)); + + info = g_malloc0(sizeof(*info)); + info->value = g_malloc0(sizeof(*info->value)); + info->value->iface.name = g_strdup(ifa->ifa_name); + info->value->iface.ipaddr = g_strdup(addr6); + + /* This is safe as '1' and '0' cannot be shuffled in netmask. */ + tmpAddrPtr=&((struct sockaddr_in6 *)ifa->ifa_netmask)->sin6_addr; + info->value->iface.prefix = count_one_bits(((uint32_t *) tmpAddrPtr)[0]) + + count_one_bits(((uint32_t *) tmpAddrPtr)[1]) + + count_one_bits(((uint32_t *) tmpAddrPtr)[2]) + + count_one_bits(((uint32_t *) tmpAddrPtr)[3]); + } + + /* Add new family here */ + + mac_supported = ifa->ifa_flags & SIOCGIFHWADDR; + + ifa = ifa->ifa_next; + + if (!info) + continue; + + if (!cur_item) { + head = cur_item = info; + } else { + cur_item->next = info; + cur_item = info; + } + + info->value->iface.ipaddrtype = family; + + g_debug("Appending to return: iface=%s, ip=%s, type=%llu, prefix=%llu", + info->value->iface.name, + info->value->iface.ipaddr, + (unsigned long long) info->value->iface.ipaddrtype, + (unsigned long long) info->value->iface.prefix); + + /* If we can't get get MAC address on this interface, + * don't even try but continue to another interface */ + if (!mac_supported) + continue; + + sock = socket(PF_INET, SOCK_STREAM, 0); + + if (sock == -1) { + snprintf(err_msg, sizeof(err_msg), + "failed to create socket: %s", strerror(errno)); + error_set(err, QERR_QGA_COMMAND_FAILED, err_msg); + goto cleanup; + } + + memset(&ifr, 0, sizeof(ifr)); + strncpy(ifr.ifr_name, info->value->iface.name, IF_NAMESIZE); + if (ioctl(sock, SIOCGIFHWADDR, &ifr) == -1) { + snprintf(err_msg, sizeof(err_msg), + "failed to get MAC addres of %s: %s", + info->value->iface.name, + strerror(errno)); + error_set(err, QERR_QGA_COMMAND_FAILED, err_msg); + goto cleanup; + } + + mac_addr = (unsigned char *) &ifr.ifr_hwaddr.sa_data; + + if (asprintf(&info->value->iface.hwaddr, + "%02x:%02x:%02x:%02x:%02x:%02x", + (int) mac_addr[0], (int) mac_addr[1], + (int) mac_addr[2], (int) mac_addr[3], + (int) mac_addr[4], (int) mac_addr[5]) == -1) { + snprintf(err_msg, sizeof(err_msg), + "failed to format MAC: %s", strerror(errno)); + error_set(err, QERR_QGA_COMMAND_FAILED, err_msg); + goto cleanup; + } + + close(sock); + } + +cleanup: + freeifaddrs(ifap); + + return head; +}
This command returns an array of: [ifname, ipaddr, ipaddr_family, prefix, hwaddr] for each interface in the system that has an IP address. Currently, only IPv4 and IPv6 are supported. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- qapi-schema-guest.json | 16 +++++ qga/guest-agent-commands.c | 156 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 0 deletions(-)