Message ID | 098E614A-9C31-4FCB-9F79-5FFC0AA97867@eastmark.net |
---|---|
State | New |
Headers | show |
Quoting Kenth Andersson (2014-09-29 13:22:54) > Implementation of guest-network-get-interfaces for windows > > > Signed-off-by: Kenth Andersson <kenth@eastmark.net> Thanks! I've been testing the functionality and it seems work nicely. Some review comments below though: > --- > configure | 2 +- > qga/commands-win32.c | 247 ++++++++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 244 insertions(+), 5 deletions(-) > > diff --git a/configure b/configure > index 681abfc..7c6c60c 100755 > --- a/configure > +++ b/configure > @@ -717,7 +717,7 @@ EOF > sysconfdir="\${prefix}" > local_statedir= > confsuffix="" > - libs_qga="-lws2_32 -lwinmm -lpowrprof $libs_qga" > + libs_qga="-lws2_32 -lwinmm -lpowrprof -liphlpapi $libs_qga" > fi > > werror="" > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > index 3bcbeae..fb6fdba 100644 > --- a/qga/commands-win32.c > +++ b/qga/commands-win32.c > @@ -14,6 +14,9 @@ > #include <glib.h> > #include <wtypes.h> > #include <powrprof.h> > +#include <winsock2.h> > +#include <iphlpapi.h> > +#include <ws2tcpip.h> > #include "qga/guest-agent-core.h" > #include "qga/vss-win32.h" > #include "qga-qmp-commands.h" > @@ -29,6 +32,9 @@ > (365 * (1970 - 1601) + \ > (1970 - 1601) / 4 - 3)) > > +/* Defines the max length of an IPv6 address in human readable format + pad */ > +#define MAX_IPv6_LEN 50 > + > static void acquire_privilege(const char *name, Error **errp) > { > HANDLE token = NULL; > @@ -359,10 +365,244 @@ void qmp_guest_suspend_hybrid(Error **errp) > error_set(errp, QERR_UNSUPPORTED); > } > > +static int get_prefix_length(PIP_ADAPTER_PREFIX prefix, > + SOCKET_ADDRESS *sockaddr, int socklen) > +{ > + PIP_ADAPTER_PREFIX p; > + struct sockaddr *addr = sockaddr->lpSockaddr; > + > + for (p = prefix; p; p = p->Next) { > + /* Compare the ip-adderss address family with the prefix > + * address family */ > + if (addr->sa_family == p->Address.lpSockaddr->sa_family) { > + int num_bytes = p->PrefixLength / 8; > + unsigned char *src = 0; > + unsigned char *dst = 0; > + int remaining_bits; > + > + if (addr->sa_family == AF_INET) { > + struct sockaddr_in *sin = (struct sockaddr_in *)addr; > + src = (unsigned char *)&(sin->sin_addr.s_addr); > + } else if (addr->sa_family == AF_INET6) { > + struct sockaddr_in6 *sin = (struct sockaddr_in6 *)addr; > + src = (unsigned char *)sin->sin6_addr.s6_addr; > + } > + > + if (p->Address.lpSockaddr->sa_family == AF_INET) { > + struct sockaddr_in *sin = > + (struct sockaddr_in *)p->Address.lpSockaddr; > + dst = (unsigned char *)&(sin->sin_addr.s_addr); > + } else if (p->Address.lpSockaddr->sa_family == AF_INET6) { > + struct sockaddr_in6 *sin = > + (struct sockaddr_in6 *)p->Address.lpSockaddr; > + dst = (unsigned char *)sin->sin6_addr.s6_addr; > + } > + > + /* If non of the addresses are in correct format we will continue > + * to next one */ > + if (src == 0 || dst == 0) { > + continue; > + } > + > + /* Check if prefix network is the same network as we are testing > + * start with whole bytes */ > + > + if (memcmp(src, dst, num_bytes) != 0) { > + continue; > + } > + > + /* Check the remaning bits */ > + remaining_bits = p->PrefixLength % 8; > + > + if (remaining_bits != 0) { > + unsigned char mask = 0xff << (8 - remaining_bits); > + int i = num_bytes; > + > + if ((src[i] & mask) != (dst[i] & mask)) { > + continue; > + } > + } > + > + return p->PrefixLength; > + } > + } > + return 0; > +} > + > + > + > GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp) > { > - error_set(errp, QERR_UNSUPPORTED); > - return NULL; > + GuestNetworkInterfaceList *head = NULL, *curr_item = NULL; > + DWORD ret_val; > + > + PIP_ADAPTER_ADDRESSES adpt_addrs; > + PIP_ADAPTER_ADDRESSES curr_adpt_addrs; > + PIP_ADAPTER_UNICAST_ADDRESS adpt_uc_addr; > + > + /* GetAdaptersAddresses requires ULONG */ > + ULONG bufLen = sizeof(IP_ADAPTER_ADDRESSES); buf_len > + > + /* Startup WinSock32 */ > + WORD ws_version_requested = MAKEWORD(2, 2); > + WSADATA ws_data; > + WSAStartup(ws_version_requested, &ws_data); Since this can fail, we should return an error and bail (after calling WSACleanup). Something like: error_setg(errp, "failed to initialize Windows Socket API v2.2"); > + > + /* Allocate adapter address list with one entry, used to > + * fetch the read length needed by GetAdapterAddresses */ > + adpt_addrs = g_malloc(sizeof(IP_ADAPTER_ADDRESSES)); > + > + /* Get adapter adresses and if it doesn't fit in adpt_addrs addresses > + * we will realloc */ > + if (GetAdaptersAddresses(AF_UNSPEC, > + GAA_FLAG_INCLUDE_PREFIX, > + NULL, > + adpt_addrs, &bufLen) == > + ERROR_BUFFER_OVERFLOW) { > + > + g_free(adpt_addrs); > + adpt_addrs = g_malloc(bufLen); > + } > + > + /* Get adapter address list */ > + ret_val = GetAdaptersAddresses(AF_UNSPEC, GAA_FLAG_INCLUDE_PREFIX, > + NULL, > + adpt_addrs, &bufLen); Doesn't this mean we end up calling GetAdaptersAddresses unconditionally 2 times? Why not only make the second attempt if you get the overflow and need to resize adpt_addrs? Alternatively, maybe you can call GetAdaptersAddresses once with a NULL buffer just to probe the size. This would still be 2 calls, but you can avoid needing to guess at an initial size for adpt_addrs. > + if (ret_val == NO_ERROR) { I would just jump to error-handling if ret_val != NO_ERROR so you don't need to indent the main block of code. > + > + /* Iterate all adapter addresses */ > + curr_adpt_addrs = adpt_addrs; > + > + while (curr_adpt_addrs) { > + /* Check if this adapter is up and running */ > + > + if (curr_adpt_addrs->OperStatus == IfOperStatusUp && > + (curr_adpt_addrs->IfType == IF_TYPE_ETHERNET_CSMACD || > + curr_adpt_addrs->IfType == IF_TYPE_IEEE80211 || > + curr_adpt_addrs->IfType == IF_TYPE_SOFTWARE_LOOPBACK)) { Probably unlikely, but shouldn't IF_TYPE_PPP be included? And do we really need to filter these at all? In the POSIX implementation we include all of these, as well as IF_TYPE_TUNNEL (via tap/tun interfaces). > + > + int len = 0; > + char *temp_description = 0; > + > + GuestNetworkInterfaceList *interface_list = > + g_malloc0(sizeof(*interface_list)); > + interface_list->value = > + g_malloc0(sizeof(*interface_list->value)); Small nit, but it gets a little to keep track of the pointers here. Maybe we should just use sizeof(GuestNetworkInterfaceList) and sizeof(GuestNetworkInterface) explicitly? > + > + len = wcslen(curr_adpt_addrs->Description) + 1; > + temp_description = g_malloc(len); > + > + wcstombs(temp_description, > + curr_adpt_addrs->Description, > + len); > + > + interface_list->value->name = g_strdup(temp_description); > + > + g_free(temp_description); > + > + if (curr_item == NULL) { > + head = curr_item = interface_list; > + } else { > + curr_item->next = interface_list; > + curr_item = interface_list; > + } > + > + /* Format MAC address */ > + interface_list->value->hardware_address = > + g_strdup_printf("%02x:%02x:%02x:%02x:%02x:%02x", > + (unsigned int) curr_adpt_addrs->PhysicalAddress[0], > + (unsigned int) curr_adpt_addrs->PhysicalAddress[1], > + (unsigned int) curr_adpt_addrs->PhysicalAddress[2], > + (unsigned int) curr_adpt_addrs->PhysicalAddress[3], > + (unsigned int) curr_adpt_addrs->PhysicalAddress[4], > + (unsigned int) curr_adpt_addrs->PhysicalAddress[5]); > + > + > + interface_list->value->has_hardware_address = true; > + > + /* Iterate all unicast addresses of this adapter */ > + GuestIpAddressList **address_list = > + &interface_list->value->ip_addresses; > + > + GuestIpAddressList *address_item = NULL; > + > + adpt_uc_addr = curr_adpt_addrs->FirstUnicastAddress; > + > + while (adpt_uc_addr) { > + SOCKET_ADDRESS *saddr = &adpt_uc_addr->Address; > + int socklen; > + char buffer[MAX_IPv6_LEN]; > + DWORD len = MAX_IPv6_LEN; > + > + /* Allocate an address item */ > + address_item = g_malloc0(sizeof(*address_item)); > + > + address_item->value = > + g_malloc0(sizeof(*address_item->value)); > + > + /* Is this IPv4 or IPv6 */ > + if (saddr->lpSockaddr->sa_family == AF_INET) { > + address_item->value->ip_address_type = > + GUEST_IP_ADDRESS_TYPE_IPV4; > + socklen = sizeof(struct sockaddr_in); > + } else if (saddr->lpSockaddr->sa_family == AF_INET6) { > + address_item->value->ip_address_type = > + GUEST_IP_ADDRESS_TYPE_IPV6; > + socklen = sizeof(struct sockaddr_in6); > + } else { > + socklen = 0; What's supposed to happen in this situation? Maybe we should just set skip the address field in this case? And if we do, we should be carefully about setting has_ip_addresses unconditionally below. > + } > + > + /* Temporary buffer to hold human readable address */ > + WSAAddressToString(saddr->lpSockaddr, > + socklen, NULL, buffer, &len); > + buffer[len] = 0; > + > + address_item->value->ip_address = g_strdup(buffer); > + > + /* Get the prefix for this address */ > + address_item->value->prefix = > + get_prefix_length(curr_adpt_addrs->FirstPrefix, > + saddr, socklen); > + > + /* Add this address to the end of the address list */ > + while (*address_list && (*address_list)->next) { > + address_list = &(*address_list)->next; > + } > + > + if (!*address_list) { > + *address_list = address_item; > + } else { > + (*address_list)->next = address_item; > + } Maybe something like this would be cleaner: /* Iterate all unicast addresses of this adapter */ GuestIpAddressList *current_address_item = NULL; adpt_uc_addr = curr_adpt_addrs->FirstUnicastAddress; while (adpt_uc_addr) { ... if (!current_address_item) { interface_list->value->ip_addresses = address_item; current_address_item = address_item; } else { current_address_item->next = address_item; current_address_item = address_item; } } In fact, I just noticed that's already how you're handling GuestNetworkInterfaceList above. > + > + /* Jump to next address */ > + adpt_uc_addr = adpt_uc_addr->Next; > + } > + interface_list->value->has_ip_addresses = true; > + > + } > + > + /* Jump to next adapter */ > + curr_adpt_addrs = curr_adpt_addrs->Next; > + } > + } > + > + /* Free the adapter list */ > + if (adpt_addrs != NULL) { > + g_free(adpt_addrs); > + } > + > + /* Cleanup WinSock32 */ > + WSACleanup(); > + > + if (!head) { > + error_set(errp, > + ERROR_CLASS_GENERIC_ERROR, > + "Could not get network interface information"); you can use error_setg() for generic errors > + } > + > + return head; > } > > int64_t qmp_guest_get_time(Error **errp) > @@ -452,8 +692,7 @@ GList *ga_command_blacklist_init(GList *blacklist) > const char *list_unsupported[] = { > "guest-file-open", "guest-file-close", "guest-file-read", > "guest-file-write", "guest-file-seek", "guest-file-flush", > - "guest-suspend-hybrid", "guest-network-get-interfaces", > - "guest-get-vcpus", "guest-set-vcpus", > + "guest-suspend-hybrid", "guest-get-vcpus", "guest-set-vcpus", > "guest-fsfreeze-freeze-list", "guest-get-fsinfo", > "guest-fstrim", NULL}; > char **p = (char **)list_unsupported; > -- > 1.9.3 (Apple Git-50)
I will take care of the below items as soon as I get some free time. /Kenth On 01 Oct 2014, at 19:45, Michael Roth <mdroth@linux.vnet.ibm.com> wrote: > Quoting Kenth Andersson (2014-09-29 13:22:54) >> Implementation of guest-network-get-interfaces for windows >> >> >> Signed-off-by: Kenth Andersson <kenth@eastmark.net> > > Thanks! I've been testing the functionality and it seems work nicely. Some > review comments below though: > >> --- >> configure | 2 +- >> qga/commands-win32.c | 247 ++++++++++++++++++++++++++++++++++++++++++++++++++- >> 2 files changed, 244 insertions(+), 5 deletions(-) >> >> diff --git a/configure b/configure >> index 681abfc..7c6c60c 100755 >> --- a/configure >> +++ b/configure >> @@ -717,7 +717,7 @@ EOF >> sysconfdir="\${prefix}" >> local_statedir= >> confsuffix="" >> - libs_qga="-lws2_32 -lwinmm -lpowrprof $libs_qga" >> + libs_qga="-lws2_32 -lwinmm -lpowrprof -liphlpapi $libs_qga" >> fi >> >> werror="" >> diff --git a/qga/commands-win32.c b/qga/commands-win32.c >> index 3bcbeae..fb6fdba 100644 >> --- a/qga/commands-win32.c >> +++ b/qga/commands-win32.c >> @@ -14,6 +14,9 @@ >> #include <glib.h> >> #include <wtypes.h> >> #include <powrprof.h> >> +#include <winsock2.h> >> +#include <iphlpapi.h> >> +#include <ws2tcpip.h> >> #include "qga/guest-agent-core.h" >> #include "qga/vss-win32.h" >> #include "qga-qmp-commands.h" >> @@ -29,6 +32,9 @@ >> (365 * (1970 - 1601) + \ >> (1970 - 1601) / 4 - 3)) >> >> +/* Defines the max length of an IPv6 address in human readable format + pad */ >> +#define MAX_IPv6_LEN 50 >> + >> static void acquire_privilege(const char *name, Error **errp) >> { >> HANDLE token = NULL; >> @@ -359,10 +365,244 @@ void qmp_guest_suspend_hybrid(Error **errp) >> error_set(errp, QERR_UNSUPPORTED); >> } >> >> +static int get_prefix_length(PIP_ADAPTER_PREFIX prefix, >> + SOCKET_ADDRESS *sockaddr, int socklen) >> +{ >> + PIP_ADAPTER_PREFIX p; >> + struct sockaddr *addr = sockaddr->lpSockaddr; >> + >> + for (p = prefix; p; p = p->Next) { >> + /* Compare the ip-adderss address family with the prefix >> + * address family */ >> + if (addr->sa_family == p->Address.lpSockaddr->sa_family) { >> + int num_bytes = p->PrefixLength / 8; >> + unsigned char *src = 0; >> + unsigned char *dst = 0; >> + int remaining_bits; >> + >> + if (addr->sa_family == AF_INET) { >> + struct sockaddr_in *sin = (struct sockaddr_in *)addr; >> + src = (unsigned char *)&(sin->sin_addr.s_addr); >> + } else if (addr->sa_family == AF_INET6) { >> + struct sockaddr_in6 *sin = (struct sockaddr_in6 *)addr; >> + src = (unsigned char *)sin->sin6_addr.s6_addr; >> + } >> + >> + if (p->Address.lpSockaddr->sa_family == AF_INET) { >> + struct sockaddr_in *sin = >> + (struct sockaddr_in *)p->Address.lpSockaddr; >> + dst = (unsigned char *)&(sin->sin_addr.s_addr); >> + } else if (p->Address.lpSockaddr->sa_family == AF_INET6) { >> + struct sockaddr_in6 *sin = >> + (struct sockaddr_in6 *)p->Address.lpSockaddr; >> + dst = (unsigned char *)sin->sin6_addr.s6_addr; >> + } >> + >> + /* If non of the addresses are in correct format we will continue >> + * to next one */ >> + if (src == 0 || dst == 0) { >> + continue; >> + } >> + >> + /* Check if prefix network is the same network as we are testing >> + * start with whole bytes */ >> + >> + if (memcmp(src, dst, num_bytes) != 0) { >> + continue; >> + } >> + >> + /* Check the remaning bits */ >> + remaining_bits = p->PrefixLength % 8; >> + >> + if (remaining_bits != 0) { >> + unsigned char mask = 0xff << (8 - remaining_bits); >> + int i = num_bytes; >> + >> + if ((src[i] & mask) != (dst[i] & mask)) { >> + continue; >> + } >> + } >> + >> + return p->PrefixLength; >> + } >> + } >> + return 0; >> +} >> + >> + >> + >> GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp) >> { >> - error_set(errp, QERR_UNSUPPORTED); >> - return NULL; >> + GuestNetworkInterfaceList *head = NULL, *curr_item = NULL; >> + DWORD ret_val; >> + >> + PIP_ADAPTER_ADDRESSES adpt_addrs; >> + PIP_ADAPTER_ADDRESSES curr_adpt_addrs; >> + PIP_ADAPTER_UNICAST_ADDRESS adpt_uc_addr; >> + >> + /* GetAdaptersAddresses requires ULONG */ >> + ULONG bufLen = sizeof(IP_ADAPTER_ADDRESSES); > > buf_len > >> + >> + /* Startup WinSock32 */ >> + WORD ws_version_requested = MAKEWORD(2, 2); >> + WSADATA ws_data; >> + WSAStartup(ws_version_requested, &ws_data); > > Since this can fail, we should return an error and bail (after calling WSACleanup). > Something like: > > error_setg(errp, "failed to initialize Windows Socket API v2.2"); > >> + >> + /* Allocate adapter address list with one entry, used to >> + * fetch the read length needed by GetAdapterAddresses */ >> + adpt_addrs = g_malloc(sizeof(IP_ADAPTER_ADDRESSES)); >> + >> + /* Get adapter adresses and if it doesn't fit in adpt_addrs > > addresses > >> + * we will realloc */ >> + if (GetAdaptersAddresses(AF_UNSPEC, >> + GAA_FLAG_INCLUDE_PREFIX, >> + NULL, >> + adpt_addrs, &bufLen) == >> + ERROR_BUFFER_OVERFLOW) { >> + >> + g_free(adpt_addrs); >> + adpt_addrs = g_malloc(bufLen); >> + } >> + >> + /* Get adapter address list */ >> + ret_val = GetAdaptersAddresses(AF_UNSPEC, GAA_FLAG_INCLUDE_PREFIX, >> + NULL, >> + adpt_addrs, &bufLen); > > Doesn't this mean we end up calling GetAdaptersAddresses unconditionally 2 > times? Why not only make the second attempt if you get the overflow and > need to resize adpt_addrs? > > Alternatively, maybe you can call GetAdaptersAddresses once with a NULL buffer > just to probe the size. This would still be 2 calls, but you can avoid > needing to guess at an initial size for adpt_addrs. > >> + if (ret_val == NO_ERROR) { > > I would just jump to error-handling if ret_val != NO_ERROR so you don't > need to indent the main block of code. > >> + >> + /* Iterate all adapter addresses */ >> + curr_adpt_addrs = adpt_addrs; >> + >> + while (curr_adpt_addrs) { >> + /* Check if this adapter is up and running */ >> + >> + if (curr_adpt_addrs->OperStatus == IfOperStatusUp && >> + (curr_adpt_addrs->IfType == IF_TYPE_ETHERNET_CSMACD || >> + curr_adpt_addrs->IfType == IF_TYPE_IEEE80211 || >> + curr_adpt_addrs->IfType == IF_TYPE_SOFTWARE_LOOPBACK)) { > > Probably unlikely, but shouldn't IF_TYPE_PPP be included? And do we really > need to filter these at all? In the POSIX implementation we include all of > these, as well as IF_TYPE_TUNNEL (via tap/tun interfaces). > >> + >> + int len = 0; >> + char *temp_description = 0; >> + >> + GuestNetworkInterfaceList *interface_list = >> + g_malloc0(sizeof(*interface_list)); >> + interface_list->value = >> + g_malloc0(sizeof(*interface_list->value)); > > Small nit, but it gets a little to keep track of the pointers here. Maybe we > should just use sizeof(GuestNetworkInterfaceList) and > sizeof(GuestNetworkInterface) explicitly? > >> + >> + len = wcslen(curr_adpt_addrs->Description) + 1; >> + temp_description = g_malloc(len); >> + >> + wcstombs(temp_description, >> + curr_adpt_addrs->Description, >> + len); >> + >> + interface_list->value->name = g_strdup(temp_description); >> + >> + g_free(temp_description); >> + >> + if (curr_item == NULL) { >> + head = curr_item = interface_list; >> + } else { >> + curr_item->next = interface_list; >> + curr_item = interface_list; >> + } >> + >> + /* Format MAC address */ >> + interface_list->value->hardware_address = >> + g_strdup_printf("%02x:%02x:%02x:%02x:%02x:%02x", >> + (unsigned int) curr_adpt_addrs->PhysicalAddress[0], >> + (unsigned int) curr_adpt_addrs->PhysicalAddress[1], >> + (unsigned int) curr_adpt_addrs->PhysicalAddress[2], >> + (unsigned int) curr_adpt_addrs->PhysicalAddress[3], >> + (unsigned int) curr_adpt_addrs->PhysicalAddress[4], >> + (unsigned int) curr_adpt_addrs->PhysicalAddress[5]); >> + >> + >> + interface_list->value->has_hardware_address = true; >> + >> + /* Iterate all unicast addresses of this adapter */ >> + GuestIpAddressList **address_list = >> + &interface_list->value->ip_addresses; >> + >> + GuestIpAddressList *address_item = NULL; >> + >> + adpt_uc_addr = curr_adpt_addrs->FirstUnicastAddress; >> + >> + while (adpt_uc_addr) { >> + SOCKET_ADDRESS *saddr = &adpt_uc_addr->Address; >> + int socklen; >> + char buffer[MAX_IPv6_LEN]; >> + DWORD len = MAX_IPv6_LEN; >> + >> + /* Allocate an address item */ >> + address_item = g_malloc0(sizeof(*address_item)); >> + >> + address_item->value = >> + g_malloc0(sizeof(*address_item->value)); >> + >> + /* Is this IPv4 or IPv6 */ >> + if (saddr->lpSockaddr->sa_family == AF_INET) { >> + address_item->value->ip_address_type = >> + GUEST_IP_ADDRESS_TYPE_IPV4; >> + socklen = sizeof(struct sockaddr_in); >> + } else if (saddr->lpSockaddr->sa_family == AF_INET6) { >> + address_item->value->ip_address_type = >> + GUEST_IP_ADDRESS_TYPE_IPV6; >> + socklen = sizeof(struct sockaddr_in6); >> + } else { >> + socklen = 0; > > What's supposed to happen in this situation? Maybe we should just set > skip the address field in this case? And if we do, we should be carefully > about setting has_ip_addresses unconditionally below. > >> + } >> + >> + /* Temporary buffer to hold human readable address */ >> + WSAAddressToString(saddr->lpSockaddr, >> + socklen, NULL, buffer, &len); >> + buffer[len] = 0; >> + >> + address_item->value->ip_address = g_strdup(buffer); >> + >> + /* Get the prefix for this address */ >> + address_item->value->prefix = >> + get_prefix_length(curr_adpt_addrs->FirstPrefix, >> + saddr, socklen); > > >> + >> + /* Add this address to the end of the address list */ >> + while (*address_list && (*address_list)->next) { >> + address_list = &(*address_list)->next; >> + } >> + >> + if (!*address_list) { >> + *address_list = address_item; >> + } else { >> + (*address_list)->next = address_item; >> + } > > Maybe something like this would be cleaner: > > /* Iterate all unicast addresses of this adapter */ > GuestIpAddressList *current_address_item = NULL; > > adpt_uc_addr = curr_adpt_addrs->FirstUnicastAddress; > > while (adpt_uc_addr) { > ... > > if (!current_address_item) { > interface_list->value->ip_addresses = address_item; > current_address_item = address_item; > } else { > current_address_item->next = address_item; > current_address_item = address_item; > } > } > > In fact, I just noticed that's already how you're handling > GuestNetworkInterfaceList above. > >> + >> + /* Jump to next address */ >> + adpt_uc_addr = adpt_uc_addr->Next; >> + } >> + interface_list->value->has_ip_addresses = true; >> + >> + } >> + >> + /* Jump to next adapter */ >> + curr_adpt_addrs = curr_adpt_addrs->Next; >> + } >> + } >> + >> + /* Free the adapter list */ >> + if (adpt_addrs != NULL) { >> + g_free(adpt_addrs); >> + } >> + >> + /* Cleanup WinSock32 */ >> + WSACleanup(); >> + >> + if (!head) { >> + error_set(errp, >> + ERROR_CLASS_GENERIC_ERROR, >> + "Could not get network interface information"); > > you can use error_setg() for generic errors > >> + } >> + >> + return head; >> } >> >> int64_t qmp_guest_get_time(Error **errp) >> @@ -452,8 +692,7 @@ GList *ga_command_blacklist_init(GList *blacklist) >> const char *list_unsupported[] = { >> "guest-file-open", "guest-file-close", "guest-file-read", >> "guest-file-write", "guest-file-seek", "guest-file-flush", >> - "guest-suspend-hybrid", "guest-network-get-interfaces", >> - "guest-get-vcpus", "guest-set-vcpus", >> + "guest-suspend-hybrid", "guest-get-vcpus", "guest-set-vcpus", >> "guest-fsfreeze-freeze-list", "guest-get-fsinfo", >> "guest-fstrim", NULL}; >> char **p = (char **)list_unsupported; >> -- >> 1.9.3 (Apple Git-50) > >
diff --git a/configure b/configure index 681abfc..7c6c60c 100755 --- a/configure +++ b/configure @@ -717,7 +717,7 @@ EOF sysconfdir="\${prefix}" local_statedir= confsuffix="" - libs_qga="-lws2_32 -lwinmm -lpowrprof $libs_qga" + libs_qga="-lws2_32 -lwinmm -lpowrprof -liphlpapi $libs_qga" fi werror="" diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 3bcbeae..fb6fdba 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -14,6 +14,9 @@ #include <glib.h> #include <wtypes.h> #include <powrprof.h> +#include <winsock2.h> +#include <iphlpapi.h> +#include <ws2tcpip.h> #include "qga/guest-agent-core.h" #include "qga/vss-win32.h" #include "qga-qmp-commands.h" @@ -29,6 +32,9 @@ (365 * (1970 - 1601) + \ (1970 - 1601) / 4 - 3)) +/* Defines the max length of an IPv6 address in human readable format + pad */ +#define MAX_IPv6_LEN 50 + static void acquire_privilege(const char *name, Error **errp) { HANDLE token = NULL; @@ -359,10 +365,244 @@ void qmp_guest_suspend_hybrid(Error **errp) error_set(errp, QERR_UNSUPPORTED); } +static int get_prefix_length(PIP_ADAPTER_PREFIX prefix, + SOCKET_ADDRESS *sockaddr, int socklen) +{ + PIP_ADAPTER_PREFIX p; + struct sockaddr *addr = sockaddr->lpSockaddr; + + for (p = prefix; p; p = p->Next) { + /* Compare the ip-adderss address family with the prefix + * address family */ + if (addr->sa_family == p->Address.lpSockaddr->sa_family) { + int num_bytes = p->PrefixLength / 8; + unsigned char *src = 0; + unsigned char *dst = 0; + int remaining_bits; + + if (addr->sa_family == AF_INET) { + struct sockaddr_in *sin = (struct sockaddr_in *)addr; + src = (unsigned char *)&(sin->sin_addr.s_addr); + } else if (addr->sa_family == AF_INET6) { + struct sockaddr_in6 *sin = (struct sockaddr_in6 *)addr; + src = (unsigned char *)sin->sin6_addr.s6_addr; + } + + if (p->Address.lpSockaddr->sa_family == AF_INET) { + struct sockaddr_in *sin = + (struct sockaddr_in *)p->Address.lpSockaddr; + dst = (unsigned char *)&(sin->sin_addr.s_addr); + } else if (p->Address.lpSockaddr->sa_family == AF_INET6) { + struct sockaddr_in6 *sin = + (struct sockaddr_in6 *)p->Address.lpSockaddr; + dst = (unsigned char *)sin->sin6_addr.s6_addr; + } + + /* If non of the addresses are in correct format we will continue + * to next one */ + if (src == 0 || dst == 0) { + continue; + } + + /* Check if prefix network is the same network as we are testing + * start with whole bytes */ + + if (memcmp(src, dst, num_bytes) != 0) { + continue; + } + + /* Check the remaning bits */ + remaining_bits = p->PrefixLength % 8; + + if (remaining_bits != 0) { + unsigned char mask = 0xff << (8 - remaining_bits); + int i = num_bytes; + + if ((src[i] & mask) != (dst[i] & mask)) { + continue; + } + } + + return p->PrefixLength; + } + } + return 0; +} + + + GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp) { - error_set(errp, QERR_UNSUPPORTED); - return NULL; + GuestNetworkInterfaceList *head = NULL, *curr_item = NULL; + DWORD ret_val; + + PIP_ADAPTER_ADDRESSES adpt_addrs; + PIP_ADAPTER_ADDRESSES curr_adpt_addrs; + PIP_ADAPTER_UNICAST_ADDRESS adpt_uc_addr; + + /* GetAdaptersAddresses requires ULONG */ + ULONG bufLen = sizeof(IP_ADAPTER_ADDRESSES); + + /* Startup WinSock32 */ + WORD ws_version_requested = MAKEWORD(2, 2); + WSADATA ws_data; + WSAStartup(ws_version_requested, &ws_data); + + /* Allocate adapter address list with one entry, used to + * fetch the read length needed by GetAdapterAddresses */ + adpt_addrs = g_malloc(sizeof(IP_ADAPTER_ADDRESSES)); + + /* Get adapter adresses and if it doesn't fit in adpt_addrs + * we will realloc */ + if (GetAdaptersAddresses(AF_UNSPEC, + GAA_FLAG_INCLUDE_PREFIX, + NULL, + adpt_addrs, &bufLen) == + ERROR_BUFFER_OVERFLOW) { + + g_free(adpt_addrs); + adpt_addrs = g_malloc(bufLen); + } + + /* Get adapter address list */ + ret_val = GetAdaptersAddresses(AF_UNSPEC, GAA_FLAG_INCLUDE_PREFIX, + NULL, + adpt_addrs, &bufLen); + if (ret_val == NO_ERROR) { + + /* Iterate all adapter addresses */ + curr_adpt_addrs = adpt_addrs; + + while (curr_adpt_addrs) { + /* Check if this adapter is up and running */ + + if (curr_adpt_addrs->OperStatus == IfOperStatusUp && + (curr_adpt_addrs->IfType == IF_TYPE_ETHERNET_CSMACD || + curr_adpt_addrs->IfType == IF_TYPE_IEEE80211 || + curr_adpt_addrs->IfType == IF_TYPE_SOFTWARE_LOOPBACK)) { + + int len = 0; + char *temp_description = 0; + + GuestNetworkInterfaceList *interface_list = + g_malloc0(sizeof(*interface_list)); + interface_list->value = + g_malloc0(sizeof(*interface_list->value)); + + len = wcslen(curr_adpt_addrs->Description) + 1; + temp_description = g_malloc(len); + + wcstombs(temp_description, + curr_adpt_addrs->Description, + len); + + interface_list->value->name = g_strdup(temp_description); + + g_free(temp_description); + + if (curr_item == NULL) { + head = curr_item = interface_list; + } else { + curr_item->next = interface_list; + curr_item = interface_list; + } + + /* Format MAC address */ + interface_list->value->hardware_address = + g_strdup_printf("%02x:%02x:%02x:%02x:%02x:%02x", + (unsigned int) curr_adpt_addrs->PhysicalAddress[0], + (unsigned int) curr_adpt_addrs->PhysicalAddress[1], + (unsigned int) curr_adpt_addrs->PhysicalAddress[2], + (unsigned int) curr_adpt_addrs->PhysicalAddress[3], + (unsigned int) curr_adpt_addrs->PhysicalAddress[4], + (unsigned int) curr_adpt_addrs->PhysicalAddress[5]); + + + interface_list->value->has_hardware_address = true; + + /* Iterate all unicast addresses of this adapter */ + GuestIpAddressList **address_list = + &interface_list->value->ip_addresses; + + GuestIpAddressList *address_item = NULL; + + adpt_uc_addr = curr_adpt_addrs->FirstUnicastAddress; + + while (adpt_uc_addr) { + SOCKET_ADDRESS *saddr = &adpt_uc_addr->Address; + int socklen; + char buffer[MAX_IPv6_LEN]; + DWORD len = MAX_IPv6_LEN; + + /* Allocate an address item */ + address_item = g_malloc0(sizeof(*address_item)); + + address_item->value = + g_malloc0(sizeof(*address_item->value)); + + /* Is this IPv4 or IPv6 */ + if (saddr->lpSockaddr->sa_family == AF_INET) { + address_item->value->ip_address_type = + GUEST_IP_ADDRESS_TYPE_IPV4; + socklen = sizeof(struct sockaddr_in); + } else if (saddr->lpSockaddr->sa_family == AF_INET6) { + address_item->value->ip_address_type = + GUEST_IP_ADDRESS_TYPE_IPV6; + socklen = sizeof(struct sockaddr_in6); + } else { + socklen = 0; + } + + /* Temporary buffer to hold human readable address */ + WSAAddressToString(saddr->lpSockaddr, + socklen, NULL, buffer, &len); + buffer[len] = 0; + + address_item->value->ip_address = g_strdup(buffer); + + /* Get the prefix for this address */ + address_item->value->prefix = + get_prefix_length(curr_adpt_addrs->FirstPrefix, + saddr, socklen); + + /* Add this address to the end of the address list */ + while (*address_list && (*address_list)->next) { + address_list = &(*address_list)->next; + } + + if (!*address_list) { + *address_list = address_item; + } else { + (*address_list)->next = address_item; + } + + /* Jump to next address */ + adpt_uc_addr = adpt_uc_addr->Next; + } + interface_list->value->has_ip_addresses = true; + + } + + /* Jump to next adapter */ + curr_adpt_addrs = curr_adpt_addrs->Next; + } + } + + /* Free the adapter list */ + if (adpt_addrs != NULL) { + g_free(adpt_addrs); + } + + /* Cleanup WinSock32 */ + WSACleanup(); + + if (!head) { + error_set(errp, + ERROR_CLASS_GENERIC_ERROR, + "Could not get network interface information"); + } + + return head; } int64_t qmp_guest_get_time(Error **errp) @@ -452,8 +692,7 @@ GList *ga_command_blacklist_init(GList *blacklist) const char *list_unsupported[] = { "guest-file-open", "guest-file-close", "guest-file-read", "guest-file-write", "guest-file-seek", "guest-file-flush", - "guest-suspend-hybrid", "guest-network-get-interfaces", - "guest-get-vcpus", "guest-set-vcpus", + "guest-suspend-hybrid", "guest-get-vcpus", "guest-set-vcpus", "guest-fsfreeze-freeze-list", "guest-get-fsinfo", "guest-fstrim", NULL}; char **p = (char **)list_unsupported;
Implementation of guest-network-get-interfaces for windows Signed-off-by: Kenth Andersson <kenth@eastmark.net> --- configure | 2 +- qga/commands-win32.c | 247 ++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 244 insertions(+), 5 deletions(-)