| Message ID | 1427902795-12024-3-git-send-email-kallan@suse.com |
|---|---|
| State | New |
| Headers | show |
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.
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.
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",
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(-)