* [Qemu-devel] [PATCH v2 0/2] qga: implement qmp_guest_network_get_interfaces for win32 @ 2015-04-01 15:39 Kirk Allan 2015-04-01 15:39 ` [Qemu-devel] [PATCH v2 1/2] qga: add additional win32 cflags and libraries Kirk Allan 2015-04-01 15:39 ` [Qemu-devel] [PATCH v2 2/2] qga: win32 implementation of qmp_guest_network_get_interfaces Kirk Allan 0 siblings, 2 replies; 5+ messages in thread From: Kirk Allan @ 2015-04-01 15:39 UTC (permalink / raw) To: qemu-devel; +Cc: sw, mdroth This patch set is to implement qmp_guest_network_get_interfaces for win32. This version splits the previous single patch into two patches: configuration and implementation. The configuration patch utilizes the –extra-cflags rather than introduce a new option for setting _WIN32_WINNT and WINVER. The implementation patch for commands-win32.c is that same as before. It will take advantage of _WIN32_WINNT if set to 0x600 or greater for Windows Vista/2008 guests or newer to use inet_ntop and OnLinkPrefixLength for getting addresses and prefixes. Kirk Allan (2): qga: add additional win32 cflags and libraries qga: win32 implementation of qmp_guest_network_get_interfaces configure | 9 +- qga/commands-win32.c | 319 ++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 324 insertions(+), 4 deletions(-) -- 1.8.5.6 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH v2 1/2] qga: add additional win32 cflags and libraries 2015-04-01 15:39 [Qemu-devel] [PATCH v2 0/2] qga: implement qmp_guest_network_get_interfaces for win32 Kirk Allan @ 2015-04-01 15:39 ` Kirk Allan 2015-04-01 15:39 ` [Qemu-devel] [PATCH v2 2/2] qga: win32 implementation of qmp_guest_network_get_interfaces Kirk Allan 1 sibling, 0 replies; 5+ messages in thread From: Kirk Allan @ 2015-04-01 15:39 UTC (permalink / raw) To: qemu-devel; +Cc: sw, mdroth, Kirk Allan Use –extra-cflags to set cflags to such as _WIN32_WINVER and WINVER to add additional functionality offered in Windows Visat/2008 and newer. Add the -DARCH_$ARCH cflag. Add the iphlpapi library to use APIs such as GetAdaptersInfo and GetAdaptersAddresses. Signed-off-by: Kirk Allan <kallan@suse.com> --- configure | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/configure b/configure index 09c9225..3d78bc4 100755 --- a/configure +++ b/configure @@ -700,7 +700,12 @@ fi if test "$mingw32" = "yes" ; then EXESUF=".exe" DSOSUF=".dll" - QEMU_CFLAGS="-DWIN32_LEAN_AND_MEAN -DWINVER=0x501 $QEMU_CFLAGS" + # --extra-cflags can be used to set flags such as -DWINVER and + # -D_WIN32_WINNT. If -DWINVER has not be set, default to XP (0x501). + if [ "$QEMU_CFLAGS" = "${QEMU_CFLAGS%-DWINVER=*}" ] ; then + QEMU_CFLAGS="-DWINVER=0x501 $QEMU_CFLAGS" + fi + QEMU_CFLAGS="-DWIN32_LEAN_AND_MEAN -DARCH_$ARCH $QEMU_CFLAGS" # enable C99/POSIX format strings (needs mingw32-runtime 3.15 or later) QEMU_CFLAGS="-D__USE_MINGW_ANSI_STDIO=1 $QEMU_CFLAGS" LIBS="-lwinmm -lws2_32 -liphlpapi $LIBS" @@ -718,7 +723,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="" -- 1.8.5.6 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] qga: win32 implementation of qmp_guest_network_get_interfaces 2015-04-01 15:39 [Qemu-devel] [PATCH v2 0/2] qga: implement qmp_guest_network_get_interfaces for win32 Kirk Allan 2015-04-01 15:39 ` [Qemu-devel] [PATCH v2 1/2] qga: add additional win32 cflags and libraries Kirk Allan @ 2015-04-01 15:39 ` Kirk Allan 2015-05-20 18:43 ` Olga Krishtal 1 sibling, 1 reply; 5+ messages in thread From: Kirk Allan @ 2015-04-01 15:39 UTC (permalink / raw) To: qemu-devel; +Cc: sw, mdroth, Kirk Allan By default, IP addresses and prefixes will be derived from information obtained by various calls and structures. IPv4 prefixes can be found by matching the address to those returned by GetApaptersInfo. IPv6 prefixes can not be matched this way due to the unpredictable order of entries. In Windows Visa/2008 guests and newer, it is possible to use inet_ntop() and OnLinkPrefixLength to get IPv4 and IPv6 addresses and prefixes. Setting –extra-cflags in the build configuration to ”- D_WIN32_WINNT-0x600 -DWINVER=0x600” or greater enables this functionality for those guests. Setting –ectra-cflags is not required and if not used, the default approach will be taken. Signed-off-by: Kirk Allan <kallan@suse.com> --- qga/commands-win32.c | 319 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 317 insertions(+), 2 deletions(-) diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 3ef0549..635d3ca 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -16,11 +16,17 @@ #include <powrprof.h> #include <stdio.h> #include <string.h> +#include <winsock2.h> +#include <ws2tcpip.h> +#include <ws2ipdef.h> +#include <iptypes.h> +#include <iphlpapi.h> #include "qga/guest-agent-core.h" #include "qga/vss-win32.h" #include "qga-qmp-commands.h" #include "qapi/qmp/qerror.h" #include "qemu/queue.h" +#include "qemu/host-utils.h" #ifndef SHTDN_REASON_FLAG_PLANNED #define SHTDN_REASON_FLAG_PLANNED 0x80000000 @@ -589,9 +595,318 @@ void qmp_guest_suspend_hybrid(Error **errp) error_set(errp, QERR_UNSUPPORTED); } +#define WORKING_BUFFER_SIZE 15000 +#define MAX_ALLOC_TRIES 2 + +static DWORD guest_get_adapter_addresses(IP_ADAPTER_ADDRESSES **adptr_addrs) +{ + ULONG out_buf_len = 0; + int alloc_tries = 0; + DWORD ret = ERROR_SUCCESS; + + /* Allocate a 15 KB buffer to start with. If not enough, out_buf_len + * will have the amount needed after the call to GetAdaptersAddresses. + */ + out_buf_len = WORKING_BUFFER_SIZE; + + do { + *adptr_addrs = (IP_ADAPTER_ADDRESSES *) g_malloc(out_buf_len); + if (*adptr_addrs == NULL) { + return ERROR_NOT_ENOUGH_MEMORY; + } + + ret = GetAdaptersAddresses(AF_UNSPEC, GAA_FLAG_INCLUDE_PREFIX, + NULL, *adptr_addrs, &out_buf_len); + + if (ret == ERROR_BUFFER_OVERFLOW) { + g_free(*adptr_addrs); + *adptr_addrs = NULL; + alloc_tries++; + } + + } while ((ret == ERROR_BUFFER_OVERFLOW) && (alloc_tries < MAX_ALLOC_TRIES)); + + if (ret != ERROR_SUCCESS) { + if (*adptr_addrs) { + g_free(*adptr_addrs); + *adptr_addrs = NULL; + } + } + return ret; +} + +#if (_WIN32_WINNT < 0x0600) +static DWORD guest_get_adapters_info(IP_ADAPTER_INFO **adptr_info) +{ + ULONG out_buf_len = 0; + int alloc_tries = 0; + DWORD ret = ERROR_SUCCESS; + + out_buf_len = sizeof(IP_ADAPTER_INFO); + do { + *adptr_info = (IP_ADAPTER_INFO *)g_malloc(out_buf_len); + if (*adptr_info == NULL) { + return ERROR_NOT_ENOUGH_MEMORY; + } + + ret = GetAdaptersInfo(*adptr_info, &out_buf_len); + + if (ret == ERROR_BUFFER_OVERFLOW) { + g_free(*adptr_info); + *adptr_info = NULL; + alloc_tries++; + } + + } while ((ret == ERROR_BUFFER_OVERFLOW) && (alloc_tries < MAX_ALLOC_TRIES)); + + if (ret != ERROR_SUCCESS) { + if (*adptr_info) { + g_free(*adptr_info); + *adptr_info = NULL; + } + } + return ret; +} +#endif + +static char *guest_wcstombs_dup(WCHAR *wstr) +{ + char *str; + int i; + + i = wcslen(wstr) + 1; + str = g_malloc(i); + if (str) { + wcstombs(str, wstr, i); + } + return str; +} + +static char *guest_inet_ntop(int af, void *cp, char *buf, socklen_t len) +{ +#if (_WIN32_WINNT >= 0x0600) && defined(ARCH_x86_64) + /* If built for 64 bit Windows Vista/2008 or newer, inet_ntop() is + * available for use. Otherwise, do our best to derive it. + */ + return (char *)inet_ntop(af, cp, buf, len); +#else + u_char *p; + + p = (u_char *)cp; + if (af == AF_INET) { + snprintf(buf, len, "%u.%u.%u.%u", p[0], p[1], p[2], p[3]); + } else if (af == AF_INET6) { + int i, compressed_zeros, added_to_buf; + char t[sizeof "::ffff"]; + + buf[0] = '\0'; + compressed_zeros = 0; + added_to_buf = 0; + for (i = 0; i < 16; i += 2) { + if (p[i] != 0 || p[i + 1] != 0) { + if (compressed_zeros) { + if (len > sizeof "::") { + strcat(buf, "::"); + len -= sizeof "::" - 1; + } + added_to_buf++; + } else { + if (added_to_buf) { + if (len > 1) { + strcat(buf, ":"); + len--; + } + } + added_to_buf++; + } + + /* Take into account leading zeros. */ + if (p[i]) { + len -= snprintf(t, sizeof(t), "%x%02x", p[i], p[i+1]); + } else { + len -= snprintf(t, sizeof(t), "%x", p[i+1]); + } + + /* Ensure there's enough room for the NULL. */ + if (len > 0) { + strcat(buf, t); + } + compressed_zeros = 0; + } else { + compressed_zeros++; + } + } + if (compressed_zeros && !added_to_buf) { + /* The address was all zeros. */ + strcat(buf, "::"); + } + } + return buf; +#endif +} + +static int64_t guest_ip_prefix(IP_ADAPTER_UNICAST_ADDRESS *ip_addr) +{ +#if (_WIN32_WINNT >= 0x0600) + /* If built for Windows Vista/2008 or newer, use the OnLinkPrefixLength + * field to obtain the prefix. Otherwise, do our best to figure it out. + */ + return ip_addr->OnLinkPrefixLength; +#else + int64_t prefix = -1; /* Use for AF_INET6 and unknown/undetermined values. */ + + if (ip_addr->Address.lpSockaddr->sa_family == AF_INET) { + IP_ADAPTER_INFO *adptr_info, *info; + IP_ADDR_STRING *ip; + struct in_addr *p; + + if (guest_get_adapters_info(&adptr_info) != ERROR_SUCCESS) { + return prefix; + } + + /* Match up the passed in ip_addr with one found in adaptr_info. + * The matching one in adptr_info will have the netmask. + */ + p = &((struct sockaddr_in *)ip_addr->Address.lpSockaddr)->sin_addr; + for (info = adptr_info; info && prefix == -1; info = info->Next) { + for (ip = &info->IpAddressList; ip; ip = ip->Next) { + if (p->S_un.S_addr == inet_addr(ip->IpAddress.String)) { + prefix = ctpop32(inet_addr(ip->IpMask.String)); + break; + } + } + } + g_free(adptr_info); + } + return prefix; +#endif +} + GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp) { - error_set(errp, QERR_UNSUPPORTED); + IP_ADAPTER_ADDRESSES *adptr_addrs, *addr; + GuestNetworkInterfaceList *head = NULL, *cur_item = NULL; + GuestIpAddressList *head_addr, *cur_addr; + DWORD ret; + + ret = guest_get_adapter_addresses(&adptr_addrs); + if (ret != ERROR_SUCCESS) { + error_setg(errp, "failed to get adapter addresses %lu", ret); + return NULL; + } + + for (addr = adptr_addrs; addr; addr = addr->Next) { + GuestNetworkInterfaceList *info; + GuestIpAddressList *address_item = NULL; + char addr4[INET_ADDRSTRLEN]; + char addr6[INET6_ADDRSTRLEN]; + unsigned char *mac_addr; + void *p; + IP_ADAPTER_UNICAST_ADDRESS *ip_addr = NULL; + + info = g_malloc0(sizeof(*info)); + if (!info) { + error_setg(errp, "failed to alloc a network interface list"); + goto error; + } + + if (!cur_item) { + head = cur_item = info; + } else { + cur_item->next = info; + cur_item = info; + } + + info->value = g_malloc0(sizeof(*info->value)); + if (!info->value) { + error_setg(errp, "failed to alloc a network interface"); + goto error; + } + info->value->name = guest_wcstombs_dup(addr->FriendlyName); + + if (addr->PhysicalAddressLength) { + mac_addr = addr->PhysicalAddress; + + info->value->hardware_address = + g_strdup_printf("%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]); + + info->value->has_hardware_address = true; + } + + head_addr = NULL; + cur_addr = NULL; + for (ip_addr = addr->FirstUnicastAddress; + ip_addr; + ip_addr = ip_addr->Next) { + address_item = g_malloc0(sizeof(*address_item)); + if (!address_item) { + error_setg(errp, "failed to alloc an Ip address list"); + goto error; + } + + if (!cur_addr) { + head_addr = cur_addr = address_item; + } else { + cur_addr->next = address_item; + cur_addr = address_item; + } + + address_item->value = g_malloc0(sizeof(*address_item->value)); + if (!address_item->value) { + error_setg(errp, "failed to alloc an Ip address"); + goto error; + } + + if (ip_addr->Address.lpSockaddr->sa_family == AF_INET) { + p = &((struct sockaddr_in *) + ip_addr->Address.lpSockaddr)->sin_addr; + + if (!guest_inet_ntop(AF_INET, p, addr4, sizeof(addr4))) { + error_setg(errp, + "failed address presentation form conversion"); + goto error; + } + + address_item->value->ip_address = g_strdup(addr4); + address_item->value->ip_address_type = + GUEST_IP_ADDRESS_TYPE_IPV4; + } else if (ip_addr->Address.lpSockaddr->sa_family == AF_INET6) { + p = &((struct sockaddr_in6 *) + ip_addr->Address.lpSockaddr)->sin6_addr; + + if (!guest_inet_ntop(AF_INET6, p, addr6, sizeof(addr6))) { + error_setg(errp, + "failed address presentation form conversion"); + goto error; + } + + address_item->value->ip_address = g_strdup(addr6); + address_item->value->ip_address_type = + GUEST_IP_ADDRESS_TYPE_IPV6; + } + address_item->value->prefix = guest_ip_prefix(ip_addr); + } + if (head_addr) { + info->value->has_ip_addresses = true; + info->value->ip_addresses = head_addr; + } + } + + if (adptr_addrs) { + g_free(adptr_addrs); + } + return head; + +error: + if (adptr_addrs) { + g_free(adptr_addrs); + } + if (head) { + qapi_free_GuestNetworkInterfaceList(head); + } return NULL; } @@ -707,7 +1022,7 @@ GuestMemoryBlockInfo *qmp_guest_get_memory_block_info(Error **errp) GList *ga_command_blacklist_init(GList *blacklist) { const char *list_unsupported[] = { - "guest-suspend-hybrid", "guest-network-get-interfaces", + "guest-suspend-hybrid", "guest-get-vcpus", "guest-set-vcpus", "guest-set-user-password", "guest-get-memory-blocks", "guest-set-memory-blocks", -- 1.8.5.6 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] qga: win32 implementation of qmp_guest_network_get_interfaces 2015-04-01 15:39 ` [Qemu-devel] [PATCH v2 2/2] qga: win32 implementation of qmp_guest_network_get_interfaces Kirk Allan @ 2015-05-20 18:43 ` Olga Krishtal 2015-05-28 14:33 ` Kirk Allan 0 siblings, 1 reply; 5+ messages in thread From: Olga Krishtal @ 2015-05-20 18:43 UTC (permalink / raw) To: Kirk Allan, qemu-devel; +Cc: sw, mdroth On 01/04/15 18:39, Kirk Allan wrote: > By default, IP addresses and prefixes will be derived from information > obtained by various calls and structures. IPv4 prefixes can be found > by matching the address to those returned by GetApaptersInfo. IPv6 > prefixes can not be matched this way due to the unpredictable order of > entries. GetAdaptersInfo. > In Windows Visa/2008 guests and newer, it is possible to use inet_ntop() > and OnLinkPrefixLength to get IPv4 and IPv6 addresses and prefixes. > Setting –extra-cflags in the build configuration to > ”- D_WIN32_WINNT-0x600 -DWINVER=0x600” or greater enables this functionality > for those guests. Setting –ectra-cflags is not required and if not used, > the default approach will be taken. > > Signed-off-by: Kirk Allan <kallan@suse.com> > --- > qga/commands-win32.c | 319 ++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 317 insertions(+), 2 deletions(-) > > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > index 3ef0549..635d3ca 100644 > --- a/qga/commands-win32.c > +++ b/qga/commands-win32.c > @@ -16,11 +16,17 @@ > #include <powrprof.h> > #include <stdio.h> > #include <string.h> > +#include <winsock2.h> > +#include <ws2tcpip.h> > +#include <ws2ipdef.h> > +#include <iptypes.h> > +#include <iphlpapi.h> > #include "qga/guest-agent-core.h" > #include "qga/vss-win32.h" > #include "qga-qmp-commands.h" > #include "qapi/qmp/qerror.h" > #include "qemu/queue.h" > +#include "qemu/host-utils.h" > > #ifndef SHTDN_REASON_FLAG_PLANNED > #define SHTDN_REASON_FLAG_PLANNED 0x80000000 > @@ -589,9 +595,318 @@ void qmp_guest_suspend_hybrid(Error **errp) > error_set(errp, QERR_UNSUPPORTED); > } > > +#define WORKING_BUFFER_SIZE 15000 > +#define MAX_ALLOC_TRIES 2 > + > +static DWORD guest_get_adapter_addresses(IP_ADAPTER_ADDRESSES **adptr_addrs) > +{ > + ULONG out_buf_len = 0; > + int alloc_tries = 0; > + DWORD ret = ERROR_SUCCESS; > + > + /* Allocate a 15 KB buffer to start with. If not enough, out_buf_len > + * will have the amount needed after the call to GetAdaptersAddresses. > + */ may be we should not allocate memory in the beginning and just make a call with just pointer, and get necessary size. > + out_buf_len = WORKING_BUFFER_SIZE; > + > + do { > + *adptr_addrs = (IP_ADAPTER_ADDRESSES *) g_malloc(out_buf_len); > + if (*adptr_addrs == NULL) { > + return ERROR_NOT_ENOUGH_MEMORY; I think we should use error handling with win32 macro error_setg_win32( errno, GetLastError().. > + } > + > + ret = GetAdaptersAddresses(AF_UNSPEC, GAA_FLAG_INCLUDE_PREFIX, > + NULL, *adptr_addrs, &out_buf_len); > + > + if (ret == ERROR_BUFFER_OVERFLOW) { > + g_free(*adptr_addrs); > + *adptr_addrs = NULL; > + alloc_tries++; > + } > + > + } while ((ret == ERROR_BUFFER_OVERFLOW) && (alloc_tries < MAX_ALLOC_TRIES)); Why we do it twice? > + if (ret != ERROR_SUCCESS) { > + if (*adptr_addrs) { > + g_free(*adptr_addrs); > + *adptr_addrs = NULL; > + } > + } In this case we always have ERROR_SUCCESS. > + return ret; > +} > + The comments for this function is the same. Moreover, they look very similar, can we merge them and have #if (_WIN32_WINNT < 0x0600) inside the function body? > +#if (_WIN32_WINNT < 0x0600) > +static DWORD guest_get_adapters_info(IP_ADAPTER_INFO **adptr_info) > +{ > + ULONG out_buf_len = 0; > + int alloc_tries = 0; > + DWORD ret = ERROR_SUCCESS; > + > + out_buf_len = sizeof(IP_ADAPTER_INFO); > + do { > + *adptr_info = (IP_ADAPTER_INFO *)g_malloc(out_buf_len); > + if (*adptr_info == NULL) { > + return ERROR_NOT_ENOUGH_MEMORY; > + } > + > + ret = GetAdaptersInfo(*adptr_info, &out_buf_len); > + > + if (ret == ERROR_BUFFER_OVERFLOW) { > + g_free(*adptr_info); > + *adptr_info = NULL; > + alloc_tries++; > + } > + > + } while ((ret == ERROR_BUFFER_OVERFLOW) && (alloc_tries < MAX_ALLOC_TRIES)); > + > + if (ret != ERROR_SUCCESS) { > + if (*adptr_info) { > + g_free(*adptr_info); > + *adptr_info = NULL; > + } > + } > + return ret; > +} > +#endif > +static char *guest_wcstombs_dup(WCHAR *wstr) > +{ > + char *str; > + int i; > + > + i = wcslen(wstr) + 1; > + str = g_malloc(i); > + if (str) { This is Windows-specific part of qemu-ga, so we should win32 API use WideCharToMultiByte. I am not sure that wcstombs uses Unicode UTF-16 and afaik it is deprecated. > + wcstombs(str, wstr, i); > + } > + return str; > +} > + > +static char *guest_inet_ntop(int af, void *cp, char *buf, socklen_t len) > +{ > +#if (_WIN32_WINNT >= 0x0600) && defined(ARCH_x86_64) > + /* If built for 64 bit Windows Vista/2008 or newer, inet_ntop() is > + * available for use. Otherwise, do our best to derive it. > + */ > + return (char *)inet_ntop(af, cp, buf, len); > +#else > + u_char *p; > + > + p = (u_char *)cp; > + if (af == AF_INET) { May be we should do some struct IP_addr that will hold this information in appropriate format. > + snprintf(buf, len, "%u.%u.%u.%u", p[0], p[1], p[2], p[3]); > + } else if (af == AF_INET6) { > + int i, compressed_zeros, added_to_buf; > + char t[sizeof "::ffff"]; > + > + buf[0] = '\0'; > + compressed_zeros = 0; > + added_to_buf = 0; > + for (i = 0; i < 16; i += 2) { > + if (p[i] != 0 || p[i + 1] != 0) { > + if (compressed_zeros) { > + if (len > sizeof "::") { > + strcat(buf, "::"); > + len -= sizeof "::" - 1; > + } > + added_to_buf++; > + } else { > + if (added_to_buf) { > + if (len > 1) { > + strcat(buf, ":"); > + len--; > + } > + } > + added_to_buf++; > + } > + > + /* Take into account leading zeros. */ > + if (p[i]) { > + len -= snprintf(t, sizeof(t), "%x%02x", p[i], p[i+1]); > + } else { > + len -= snprintf(t, sizeof(t), "%x", p[i+1]); > + } > + > + /* Ensure there's enough room for the NULL. */ > + if (len > 0) { > + strcat(buf, t); > + } > + compressed_zeros = 0; > + } else { > + compressed_zeros++; > + } > + } > + if (compressed_zeros && !added_to_buf) { > + /* The address was all zeros. */ > + strcat(buf, "::"); > + } > + } > + return buf; > +#endif > +} > + > +static int64_t guest_ip_prefix(IP_ADAPTER_UNICAST_ADDRESS *ip_addr) > +{ > +#if (_WIN32_WINNT >= 0x0600) > + /* If built for Windows Vista/2008 or newer, use the OnLinkPrefixLength > + * field to obtain the prefix. Otherwise, do our best to figure it out. > + */ > + return ip_addr->OnLinkPrefixLength; > +#else > + int64_t prefix = -1; /* Use for AF_INET6 and unknown/undetermined values. */ > + > + if (ip_addr->Address.lpSockaddr->sa_family == AF_INET) { > + IP_ADAPTER_INFO *adptr_info, *info; > + IP_ADDR_STRING *ip; > + struct in_addr *p; > + > + if (guest_get_adapters_info(&adptr_info) != ERROR_SUCCESS) { > + return prefix; > + } > + > + /* Match up the passed in ip_addr with one found in adaptr_info. > + * The matching one in adptr_info will have the netmask. > + */ > + p = &((struct sockaddr_in *)ip_addr->Address.lpSockaddr)->sin_addr; > + for (info = adptr_info; info && prefix == -1; info = info->Next) { > + for (ip = &info->IpAddressList; ip; ip = ip->Next) { > + if (p->S_un.S_addr == inet_addr(ip->IpAddress.String)) { > + prefix = ctpop32(inet_addr(ip->IpMask.String)); > + break; > + } > + } > + } > + g_free(adptr_info); > + } > + return prefix; > +#endif > +} > + > GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp) > { > - error_set(errp, QERR_UNSUPPORTED); > + IP_ADAPTER_ADDRESSES *adptr_addrs, *addr; > + GuestNetworkInterfaceList *head = NULL, *cur_item = NULL; > + GuestIpAddressList *head_addr, *cur_addr; > + DWORD ret; > + > + ret = guest_get_adapter_addresses(&adptr_addrs); > + if (ret != ERROR_SUCCESS) { > + error_setg(errp, "failed to get adapter addresses %lu", ret); > + return NULL; > + } > + > + for (addr = adptr_addrs; addr; addr = addr->Next) { > + GuestNetworkInterfaceList *info; > + GuestIpAddressList *address_item = NULL; > + char addr4[INET_ADDRSTRLEN]; > + char addr6[INET6_ADDRSTRLEN]; > + unsigned char *mac_addr; > + void *p; Variables should not be declared in function body. I think better way is to put them in the beginning. > + IP_ADAPTER_UNICAST_ADDRESS *ip_addr = NULL; > + > + info = g_malloc0(sizeof(*info)); > + if (!info) { > + error_setg(errp, "failed to alloc a network interface list"); > + goto error; > + } > + > + if (!cur_item) { > + head = cur_item = info; > + } else { > + cur_item->next = info; > + cur_item = info; > + } > + > + info->value = g_malloc0(sizeof(*info->value)); > + if (!info->value) { > + error_setg(errp, "failed to alloc a network interface"); > + goto error; > + } > + info->value->name = guest_wcstombs_dup(addr->FriendlyName); > + > + if (addr->PhysicalAddressLength) { > + mac_addr = addr->PhysicalAddress; > + > + info->value->hardware_address = > + g_strdup_printf("%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]); > + > + info->value->has_hardware_address = true; > + } > + > + head_addr = NULL; > + cur_addr = NULL; > + for (ip_addr = addr->FirstUnicastAddress; > + ip_addr; > + ip_addr = ip_addr->Next) { > + address_item = g_malloc0(sizeof(*address_item)); > + if (!address_item) { > + error_setg(errp, "failed to alloc an Ip address list"); > + goto error; > + } > + > + if (!cur_addr) { > + head_addr = cur_addr = address_item; > + } else { > + cur_addr->next = address_item; > + cur_addr = address_item; > + } > + > + address_item->value = g_malloc0(sizeof(*address_item->value)); > + if (!address_item->value) { > + error_setg(errp, "failed to alloc an Ip address"); > + goto error; > + } > + > + if (ip_addr->Address.lpSockaddr->sa_family == AF_INET) { > + p = &((struct sockaddr_in *) > + ip_addr->Address.lpSockaddr)->sin_addr; > + > + if (!guest_inet_ntop(AF_INET, p, addr4, sizeof(addr4))) { > + error_setg(errp, win32 error macro > + "failed address presentation form conversion"); > + goto error; > + } > + > + address_item->value->ip_address = g_strdup(addr4); > + address_item->value->ip_address_type = > + GUEST_IP_ADDRESS_TYPE_IPV4; > + } else if (ip_addr->Address.lpSockaddr->sa_family == AF_INET6) { > + p = &((struct sockaddr_in6 *) > + ip_addr->Address.lpSockaddr)->sin6_addr; > + > + if (!guest_inet_ntop(AF_INET6, p, addr6, sizeof(addr6))) { > + error_setg(errp, > + "failed address presentation form conversion"); win32 error macro > goto error; > + } > + > + address_item->value->ip_address = g_strdup(addr6); > + address_item->value->ip_address_type = > + GUEST_IP_ADDRESS_TYPE_IPV6; > + } > + address_item->value->prefix = guest_ip_prefix(ip_addr); > + } > + if (head_addr) { > + info->value->has_ip_addresses = true; > + info->value->ip_addresses = head_addr; > + } > + } > + > + if (adptr_addrs) { > + g_free(adptr_addrs); > + } > + return head; > + > +error: > + if (adptr_addrs) { > + g_free(adptr_addrs); > + } > + if (head) { > + qapi_free_GuestNetworkInterfaceList(head); > + } > return NULL; > } > > @@ -707,7 +1022,7 @@ GuestMemoryBlockInfo *qmp_guest_get_memory_block_info(Error **errp) > GList *ga_command_blacklist_init(GList *blacklist) > { > const char *list_unsupported[] = { > - "guest-suspend-hybrid", "guest-network-get-interfaces", > + "guest-suspend-hybrid", > "guest-get-vcpus", "guest-set-vcpus", > "guest-set-user-password", > "guest-get-memory-blocks", "guest-set-memory-blocks", I think 2 last functions GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp) and static char *guest_inet_ntop(int af, void *cp, char *buf, socklen_t len)should be redesigned and refactored very attentively. Secondly you should decide what kind of functions to use Win32 API or POSIX API. It is bed idea to mix them. Pls, pay attention to to error handling. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] qga: win32 implementation of qmp_guest_network_get_interfaces 2015-05-20 18:43 ` Olga Krishtal @ 2015-05-28 14:33 ` Kirk Allan 0 siblings, 0 replies; 5+ messages in thread From: Kirk Allan @ 2015-05-28 14:33 UTC (permalink / raw) To: qemu-devel, Olga Krishtal; +Cc: sw, mdroth Thanks for the feedback. I'll make the necessary adjustments and create a new patch. Kirk >>> > On 01/04/15 18:39, Kirk Allan wrote: >> By default, IP addresses and prefixes will be derived from information >> obtained by various calls and structures. IPv4 prefixes can be found >> by matching the address to those returned by GetApaptersInfo. IPv6 >> prefixes can not be matched this way due to the unpredictable order of >> entries. > GetAdaptersInfo. I'll fix the typo. >> In Windows Visa/2008 guests and newer, it is possible to use inet_ntop() >> and OnLinkPrefixLength to get IPv4 and IPv6 addresses and prefixes. >> Setting *extra-cflags in the build configuration to >> *- D_WIN32_WINNT-0x600 -DWINVER=0x600* or greater enables this functionality >> for those guests. Setting *ectra-cflags is not required and if not used, >> the default approach will be taken. >> >> Signed-off-by: Kirk Allan <kallan@suse.com> >> --- >> qga/commands-win32.c | 319 > ++++++++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 317 insertions(+), 2 deletions(-) >> >> diff --git a/qga/commands-win32.c b/qga/commands-win32.c >> index 3ef0549..635d3ca 100644 >> --- a/qga/commands-win32.c >> +++ b/qga/commands-win32.c >> @@ -16,11 +16,17 @@ >> #include <powrprof.h> >> #include <stdio.h> >> #include <string.h> >> +#include <winsock2.h> >> +#include <ws2tcpip.h> >> +#include <ws2ipdef.h> >> +#include <iptypes.h> >> +#include <iphlpapi.h> >> #include "qga/guest-agent-core.h" >> #include "qga/vss-win32.h" >> #include "qga-qmp-commands.h" >> #include "qapi/qmp/qerror.h" >> #include "qemu/queue.h" >> +#include "qemu/host-utils.h" >> >> #ifndef SHTDN_REASON_FLAG_PLANNED >> #define SHTDN_REASON_FLAG_PLANNED 0x80000000 >> @@ -589,9 +595,318 @@ void qmp_guest_suspend_hybrid(Error **errp) >> error_set(errp, QERR_UNSUPPORTED); >> } >> >> +#define WORKING_BUFFER_SIZE 15000 >> +#define MAX_ALLOC_TRIES 2 >> + >> +static DWORD guest_get_adapter_addresses(IP_ADAPTER_ADDRESSES > **adptr_addrs) >> +{ >> + ULONG out_buf_len = 0; >> + int alloc_tries = 0; >> + DWORD ret = ERROR_SUCCESS; >> + >> + /* Allocate a 15 KB buffer to start with. If not enough, out_buf_len >> + * will have the amount needed after the call to GetAdaptersAddresses. >> + */ > may be we should not allocate memory in the beginning and just make a > call with just pointer, > and get necessary size. I'll get the size first and make one allocation. >> + out_buf_len = WORKING_BUFFER_SIZE; >> + >> + do { >> + *adptr_addrs = (IP_ADAPTER_ADDRESSES *) g_malloc(out_buf_len); >> + if (*adptr_addrs == NULL) { >> + return ERROR_NOT_ENOUGH_MEMORY; > I think we should use error handling with win32 macro error_setg_win32( > errno, GetLastError().. I'll use error_setg_win32() when the api sets last error. >> + } >> + >> + ret = GetAdaptersAddresses(AF_UNSPEC, GAA_FLAG_INCLUDE_PREFIX, >> + NULL, *adptr_addrs, &out_buf_len); >> + >> + if (ret == ERROR_BUFFER_OVERFLOW) { >> + g_free(*adptr_addrs); >> + *adptr_addrs = NULL; >> + alloc_tries++; >> + } >> + >> + } while ((ret == ERROR_BUFFER_OVERFLOW) && (alloc_tries < > MAX_ALLOC_TRIES)); > Why we do it twice? >> + if (ret != ERROR_SUCCESS) { >> + if (*adptr_addrs) { >> + g_free(*adptr_addrs); >> + *adptr_addrs = NULL; >> + } >> + } > In this case we always have ERROR_SUCCESS. >> + return ret; >> +} >> + > The comments for this function is the same. Moreover, they look very > similar, > can we merge them and have #if (_WIN32_WINNT < 0x0600) inside the > function body? I'll do the same as above, get the size first and do one allocation. Since we are doing GetAdaptersAddresses and GetAdaptersInfo, I don't see usable way in combining these two. Simplifying th e memory allocation should help though. >> +#if (_WIN32_WINNT < 0x0600) >> +static DWORD guest_get_adapters_info(IP_ADAPTER_INFO **adptr_info) >> +{ >> + ULONG out_buf_len = 0; >> + int alloc_tries = 0; >> + DWORD ret = ERROR_SUCCESS; >> + >> + out_buf_len = sizeof(IP_ADAPTER_INFO); >> + do { >> + *adptr_info = (IP_ADAPTER_INFO *)g_malloc(out_buf_len); >> + if (*adptr_info == NULL) { >> + return ERROR_NOT_ENOUGH_MEMORY; >> + } >> + >> + ret = GetAdaptersInfo(*adptr_info, &out_buf_len); >> + >> + if (ret == ERROR_BUFFER_OVERFLOW) { >> + g_free(*adptr_info); >> + *adptr_info = NULL; >> + alloc_tries++; >> + } >> + >> + } while ((ret == ERROR_BUFFER_OVERFLOW) && (alloc_tries < > MAX_ALLOC_TRIES)); >> + >> + if (ret != ERROR_SUCCESS) { >> + if (*adptr_info) { >> + g_free(*adptr_info); >> + *adptr_info = NULL; >> + } >> + } >> + return ret; >> +} >> +#endif >> +static char *guest_wcstombs_dup(WCHAR *wstr) >> +{ >> + char *str; >> + int i; >> + >> + i = wcslen(wstr) + 1; >> + str = g_malloc(i); >> + if (str) { > This is Windows-specific part of qemu-ga, so we should win32 API use > WideCharToMultiByte. > I am not sure that wcstombs uses Unicode UTF-16 and afaik it is deprecated. Will switch to WideCharToMultiByte. >> + wcstombs(str, wstr, i); >> + } >> + return str; >> +} >> + >> +static char *guest_inet_ntop(int af, void *cp, char *buf, socklen_t len) >> +{ >> +#if (_WIN32_WINNT >= 0x0600) && defined(ARCH_x86_64) >> + /* If built for 64 bit Windows Vista/2008 or newer, inet_ntop() is >> + * available for use. Otherwise, do our best to derive it. >> + */ >> + return (char *)inet_ntop(af, cp, buf, len); >> +#else >> + u_char *p; >> + >> + p = (u_char *)cp; >> + if (af == AF_INET) { > May be we should do some struct IP_addr that will hold this information > in appropriate format. inet_ntop just returns a string, so I'm not sure what else could be done here. >> + snprintf(buf, len, "%u.%u.%u.%u", p[0], p[1], p[2], p[3]); >> + } else if (af == AF_INET6) { >> + int i, compressed_zeros, added_to_buf; >> + char t[sizeof "::ffff"]; >> + >> + buf[0] = '\0'; >> + compressed_zeros = 0; >> + added_to_buf = 0; >> + for (i = 0; i < 16; i += 2) { >> + if (p[i] != 0 || p[i + 1] != 0) { >> + if (compressed_zeros) { >> + if (len > sizeof "::") { >> + strcat(buf, "::"); >> + len -= sizeof "::" - 1; >> + } >> + added_to_buf++; >> + } else { >> + if (added_to_buf) { >> + if (len > 1) { >> + strcat(buf, ":"); >> + len--; >> + } >> + } >> + added_to_buf++; >> + } >> + >> + /* Take into account leading zeros. */ >> + if (p[i]) { >> + len -= snprintf(t, sizeof(t), "%x%02x", p[i], p[i+1]); >> + } else { >> + len -= snprintf(t, sizeof(t), "%x", p[i+1]); >> + } >> + >> + /* Ensure there's enough room for the NULL. */ >> + if (len > 0) { >> + strcat(buf, t); >> + } >> + compressed_zeros = 0; >> + } else { >> + compressed_zeros++; >> + } >> + } >> + if (compressed_zeros && !added_to_buf) { >> + /* The address was all zeros. */ >> + strcat(buf, "::"); >> + } >> + } >> + return buf; >> +#endif >> +} >> + >> +static int64_t guest_ip_prefix(IP_ADAPTER_UNICAST_ADDRESS *ip_addr) >> +{ >> +#if (_WIN32_WINNT >= 0x0600 ) >> + /* If built for Windows Vista/2008 or newer, use the OnLinkPrefixLength >> + * field to obtain the prefix. Otherwise, do our best to figure it > out. >> + */ >> + return ip_addr->OnLinkPrefixLength; >> +#else >> + int64_t prefix = -1; /* Use for AF_INET6 and unknown/undetermined > values. */ >> + >> + if (ip_addr->Address.lpSockaddr->sa_family == AF_INET) { >> + IP_ADAPTER_INFO *adptr_info, *info; >> + IP_ADDR_STRING *ip; >> + struct in_addr *p; >> + >> + if (guest_get_adapters_info(&adptr_info) != ERROR_SUCCESS) { >> + return prefix; >> + } >> + >> + /* Match up the passed in ip_addr with one found in adaptr_info. >> + * The matching one in adptr_info will have the netmask. >> + */ >> + p = &((struct sockaddr_in *)ip_addr->Address.lpSockaddr)->sin_addr; >> + for (info = adptr_info; info && prefix == -1; info = info->Next) { >> + for (ip = &info->IpAddressList; ip; ip = ip->Next) { >> + if (p->S_un.S_addr == inet_addr(ip->IpAddress.String)) { >> + prefix = ctpop32(inet_addr(ip->IpMask.String)); >> + break; >> + } >> + } >> + } >> + g_free(adptr_info); >> + } >> + return prefix; >> +#endif >> +} >> + >> GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp) >> { >> - error_set(errp, QERR_UNSUPPORTED); >> + IP_ADAPTER_ADDRESSES *adptr_addrs, *addr; >> + GuestNetworkInterfaceList *head = NULL, *cur_item = NULL; >> + GuestIpAddressList *head_addr, *cur_addr; >> + DWORD ret; >> + >> + ret = guest_get_adapter_addresses(&adptr_addrs); >> + if (ret != ERROR_SUCCESS) { >> + error_setg(errp, "failed to get adapter addresses %lu", ret); >> + return NULL; >> + } >> + >> + for (addr = adptr_addrs; addr; addr = addr->Next) { >> + GuestNetworkInterfaceList *info; >> + GuestIpAddressList *address_item = NULL; >> + char addr4[INET_ADDRSTRLEN]; >> + char addr6[INET6_ADDRSTRLEN]; >> + unsigned char *mac_addr; >> + void *p; > Variables should not be declared in function body. I think better way is > to put them in the beginning. I'll move the variable to the beginning. >> + IP_ADAPTER_UNICAST_ADDRESS *ip_addr = NULL; >> + >> + info = g_malloc0(sizeof(*info)); >> + if (!info) { >> + error_setg(errp, "failed to alloc a network interface list"); >> + goto error; >> + } >> + >> + if (!cur_item) { >> + head = cur_item = info; >> + } else { >> + cur_item->next = info; >> + cur_item = info; >> + } >> + >> + info->value = g_malloc0(sizeof(*info->value)); >> + if (!info->value) { >> + error_setg(errp, "failed to alloc a network interface"); >> + goto error; >> + } >> + info->value->name = guest_wcstombs_dup(addr->FriendlyName); >> + >> + if (addr->PhysicalAddressLength) { >> + mac_addr = addr->PhysicalAddress; >> + >> + info->value->hardware_address = >> + g_strdup_printf("%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]); >> + >> + info->value->has_hardware_address = true; >> + } >> + >> + head_addr = NULL; >> + cur_addr = NULL; >> + for (ip_addr = addr->FirstUnicastAddress; >> + ip_addr; >> + ip_addr = ip_addr->Next) { >> + address_item = g_malloc0(sizeof(*address_item)); >> + if (!address_item) { >> + error_setg(errp, "failed to alloc an Ip address list"); >> + goto error; >> + } >> + >> + if (!cur_addr) { >> + head_addr = cur_addr = address_item; >> + } else { >> + cur_addr->next = address_item; >> + cur_addr = address_item; >> + } >> + >> + address_item->value = g_malloc0(sizeof(*address_item->value)); >> + if (!address_item->value) { >> + error_setg(errp, "failed to alloc an Ip address"); >> + goto error; >> + } >> + >> + if (ip_addr->Address.lpSockaddr->sa_family == AF_INET) { >> + p = &((struct sockaddr_in *) >> + ip_addr->Address.lpSockaddr)->sin_addr; >> + >> + if (!guest_inet_ntop(AF_INET, p, addr4, sizeof(addr4))) { >> + error_setg(errp, > win32 error macro Since the error can be retrieved with WSAGetLastErorr, I'll switch to error_setg_win32(errp, WSAGetLastError(), ... >> + "failed address presentation form > conversion"); >> + goto error; >> + } >> + >> + address_item->value->ip_address = g_strdup(addr4); >> + address_item->value->ip_address_type = >> + GUEST_IP_ADDRESS_TYPE_IPV4; >> + } else if (ip_addr->Address.lpSockaddr->sa_family == AF_INET6) { >> + p = &((struct sockaddr_in6 *) >> + ip_addr->Address.lpSockaddr)->sin6_addr; >> + >> + if (!guest_inet_ntop(AF_INET6, p, addr6, sizeof(addr6))) { >> + error_setg(errp, >> + "failed address presentation form > conversion"); > win32 error macro Same as above. >> goto error; >> + } >> + >> + address_item->value->ip_address = g_strdup(addr6); >> + address_item->value->ip_address_type = >> + GUEST_IP_ADDRESS_TYPE_IPV6; >> + } >> + address_item->value->prefix = guest_ip_prefix(ip_addr); >> + } >> + if (head_addr) { >> + info->value->has_ip_addresses = true; >> + info->value->ip_addresses = head_addr; >> + } >> + } >> + >> + if (adptr_addrs) { >> + g_free(adptr_addrs); >> + } >> + return head; >> + >> +error: >> + if (adptr_addrs) { >> + g_free(adptr_addrs); >> + } >> + if (head) { >> + qapi_free_GuestNetworkInterfaceList(head); >> + } >> return NULL; >> } >> >> @@ -707,7 +1022,7 @@ GuestMemoryBlockInfo > *qmp_guest_get_memory_block_info(Error **errp) >> GList *ga_command_blacklist_init(GList *blacklist) >> { >> const char *list_unsupported[] = { >> - "guest-suspend-hybrid", "guest-network-get-interfaces", >> + "guest-suspend-hybrid", >> "guest-get-vcpus", "guest-set-vcpus", >> "guest-set-user-password", >> "guest-get-memory-blocks", "guest-set-memory-blocks", > I think 2 last functions GuestNetworkInterfaceList > *qmp_guest_network_get_interfaces(Error **errp) and static char > *guest_inet_ntop(int af, void *cp, char *buf, socklen_t len)should be > redesigned and refactored very attentively. Secondly you should decide > what kind of functions to use Win32 API or POSIX API. > It is bed idea to mix them. Where InetNtop is available where inet_ntop is, I'll switch the win32 api. > Pls, pay attention to to error handling. For apis that can retrieve the error form GetLastError, I'll switch to the error_setg_win32. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-05-28 14:33 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-04-01 15:39 [Qemu-devel] [PATCH v2 0/2] qga: implement qmp_guest_network_get_interfaces for win32 Kirk Allan 2015-04-01 15:39 ` [Qemu-devel] [PATCH v2 1/2] qga: add additional win32 cflags and libraries Kirk Allan 2015-04-01 15:39 ` [Qemu-devel] [PATCH v2 2/2] qga: win32 implementation of qmp_guest_network_get_interfaces Kirk Allan 2015-05-20 18:43 ` Olga Krishtal 2015-05-28 14:33 ` Kirk Allan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).