Quoting Michael Roth (2015-01-08 17:29:43)
> Quoting zhanghailiang (2014-12-24 04:21:20)
> > Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com>
> > ---
> > Hi,
> >
> > This patch implements guest-network-get-interfaces command for
> > Windows.
> >
> > This patch is RFC because the value of network 'prefix' length may be wrong
> > When there is an adapter with multiple IP which have different netmask.
> >
> > The main reason is I get this value by hunting for matching prefix in
> > linked list,
> > But unfortunately the order of linked IP_ADAPTER_UNICAST_ADDRESS structures
> > pointed
> > to by the FirstUnicastAddress member does not have any relationship with the
> > order of linked IP_ADAPTER_PREFIX structures pointed to by the FirstPrefix
> > member. So actually, we cannot match exactly prefix with unicast struct. :(
>
> Kenth Andersson's original patch attempted to match the SOCKET_ADDRESS
> structure
> in PIP_ADAPTER_PREFIX->Address to that of PIP_ADAPTER_UNICAST_ADDRESS->Address
> to determine if it corresponded. Is this not workable for the pre-Vista case?
>
> Ideally there's something better we can do than a memcmp of the contents
> though...
>
> Cc'ing Kenth as maybe he has some insight or an update of his original
> patch he'd like to pursue.
Actually Cc'ing this time.
>
> >
> > Yes, MSDN suggests we get prefix length value by reference to
> > OnLinkPrefixLength which
> > is a member of struct IP_ADAPTER_UNICAST_ADDRESS, but this structure member
> > is only
> > available on Windows Vista and later, and it seems that the cross compiling
> > environment is like Windows XP. Who know this?
> >
> > Any comments and suggestion are welcomed.
> >
> > You can test this by command:
> > '{"execute":"guest-network-get-interfaces"}'
> > The return value is like:
> > {"return":[{"name":"{FE2D1285-75FF-48E7-BDEF-50D19DA7D6B4}","ip-addresses":[{"ip-address-type":"ipv6","ip-address":"fe80::e4ca:8658:61e3:8b83","prefix":64},{"ip-address-type":"ipv4","ip-address":"9.61.170.170","prefix":16}],"hardware-address":"52:54:00:7b:4b:19"},{"name":"{846EE342-7039-11DE-9D20-806E6F6E6963}","ip-addresses":[{"ip-address-type":"ipv6","ip-address":"::1","prefix":128},{"ip-address-type":"ipv4","ip-address":"127.0.0.1","prefix":8}],"hardware-address":"52:54:00:7b:4b:19"}]}
> >
> > ---
> > configure | 2 +-
> > qga/Makefile.objs | 2 +-
> > qga/commands-win32.c | 201
> > ++++++++++++++++++++++++++++++++++++++++++++++++-
> > qga/guest-agent-core.h | 11 +++
> > qga/inet_ntop-win32.c | 184 ++++++++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 394 insertions(+), 6 deletions(-)
> > create mode 100644 qga/inet_ntop-win32.c
> >
> > diff --git a/configure b/configure
> > index cae588c..7cafbdd 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/Makefile.objs b/qga/Makefile.objs
> > index 1c5986c..47ef4aa 100644
> > --- a/qga/Makefile.objs
> > +++ b/qga/Makefile.objs
> > @@ -1,6 +1,6 @@
> > qga-obj-y = commands.o guest-agent-command-state.o main.o
> > qga-obj-$(CONFIG_POSIX) += commands-posix.o channel-posix.o
> > -qga-obj-$(CONFIG_WIN32) += commands-win32.o channel-win32.o service-win32.o
> > +qga-obj-$(CONFIG_WIN32) += commands-win32.o channel-win32.o
> > service-win32.o inet_ntop-win32.o
> > qga-obj-$(CONFIG_WIN32) += vss-win32.o
> > qga-obj-y += qapi-generated/qga-qapi-types.o
> > qapi-generated/qga-qapi-visit.o
> > qga-obj-y += qapi-generated/qga-qmp-marshal.o
> > diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> > index 3bcbeae..af4eb31 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 <ws2tcpip.h>
> > +#include <iphlpapi.h>
> > #include "qga/guest-agent-core.h"
> > #include "qga/vss-win32.h"
> > #include "qga-qmp-commands.h"
> > @@ -359,9 +362,200 @@ void qmp_guest_suspend_hybrid(Error **errp)
> > error_set(errp, QERR_UNSUPPORTED);
> > }
> >
> > -GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
> > +#define WORKING_BUFFER_SIZE 15000
> > +#define MAX_TRIES 3
> > +#define IN_LINKLOCAL(a) ((((uint32_t) (a)) & 0xaffff0000) == 0xa9fe0000)
> > +
> > +/*
> > + * For Vista and later version, we can get the prefix length value from
> > + * OnLinkPrefixLength which is a member of IP_ADAPTER_UNICAST_ADDRESS
> > structure.
> > + * Otherwise we must hunt for matching prefix in linked list.
> > + *
> > + * Note:The order of linked IP_ADAPTER_UNICAST_ADDRESS structures pointed
> > to by
> > + * the FirstUnicastAddress member does not have any relationship with the
> > + * order of linked IP_ADAPTER_PREFIX structures pointed to by the
> > FirstPrefix
> > + * member, So the result may be incorrect for an adapter with multiple IP
> > !!!
> > + *
> > + * More info can be found at:
> > +
> > *http://msdn.microsoft.com/en-us/library/windows/desktop/aa366066(v=vs.85).aspx
> > + */
> > +static int64_t get_adapter_unicast_prefixlength(PIP_ADAPTER_ADDRESSES
> > pAdapter,
> > + PIP_ADAPTER_UNICAST_ADDRESS
> > pUnicast)
> > {
> > - error_set(errp, QERR_UNSUPPORTED);
> > + IP_ADAPTER_PREFIX *prefix;
> > +/*
> > +* Actually, here the cross compiling envirtonment for windows qemu-ga,
> > +* the IP_ADAPTER_UNICAST_ADDRESS structure is defined as
> > +* IP_ADAPTER_UNICAST_ADDRESS_XP.
>
> What environment is this? For FC18 mingw crossbuilds it seems to be
> IP_ADAPTER_UNICAST_ADDRESS, or at least there's a typedef for it since
> I was able to compile/test Kenth's patch.
>
> > +*/
> > +#if 0
> > + if (IsWindowsVistaOrGreater()) {
> > + return pUnicast->OnLinkPrefixLength;
> > + }
> > +#endif
> > + for (prefix = pAdapter->FirstPrefix; prefix; prefix = prefix->Next) {
> > + LPSOCKADDR lpSockaddr = prefix->Address.lpSockaddr;
> > +
> > + if (lpSockaddr->sa_family !=
> > pUnicast->Address.lpSockaddr->sa_family) {
> > + continue;
> > + }
> > + if (lpSockaddr->sa_family == AF_INET) {
> > + char addr4[INET_ADDRSTRLEN];
> > + struct sockaddr_in* sa_in = (struct sockaddr_in *)lpSockaddr;
> > + inet_ntop(AF_INET, &(sa_in->sin_addr), addr4, sizeof(addr4));
> > + g_debug("fuck:%s\n", addr4);
> > + }
> > + /* special cases */
> > + /* RFC2863: IPv4 interface not up */
> > + if (lpSockaddr->sa_family == AF_INET &&
> > + pAdapter->OperStatus != IfOperStatusUp) {
> > + /* RFC3927: link-local IPv4 always has 16-bit CIDR */
> > + if (IN_LINKLOCAL(ntohl(((struct sockaddr_in *)
> > + (pUnicast->Address.lpSockaddr))->sin_addr.s_addr))) {
> > + g_debug("Assuming 16-bit prefix length for"
> > + "link-local IPv4 adapter %s",
> > pAdapter->AdapterName);
> > + return 16;
> > + } else {
> > + g_debug("Prefix length unavailable for IPv4 adapter %s.",
> > + pAdapter->AdapterName);
> > + }
> > + break;
> > + }
> > + /* default IPv6 route */
> > + if (lpSockaddr->sa_family == AF_INET6 && 0 == prefix->PrefixLength
> > &&
> > + IN6_IS_ADDR_UNSPECIFIED(&((struct sockaddr_in6 *)
> > + (lpSockaddr))->sin6_addr)) {
> > + continue;
> > + }
> > +
> > + return prefix->PrefixLength;
> > + }
> > + return -1;
> > +}
> > +
> > +GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error** errp)
> > +{
> > + PIP_ADAPTER_ADDRESSES pAddresses = NULL, pCurrAddresses = NULL;
>
> Small nit, but since there's so many uses of "addresses" going on below,
> and since we call them "adapters" in the argument to
> get_adapter_unicast_prefixlength() anyway, can we do the same here for
> readability?
>
> > + ULONG outBufLen = 0;
> > + DWORD dwRetVal = 0;
> > + ULONG Iterations = 0;
>
> I know it goes against the grain with win32 code, but please stick to QEMU
> coding style where possible.
>
> > + ULONG flags = (GAA_FLAG_INCLUDE_PREFIX | GAA_FLAG_SKIP_ANYCAST |
> > + GAA_FLAG_SKIP_DNS_SERVER | GAA_FLAG_SKIP_FRIENDLY_NAME |
> > + GAA_FLAG_SKIP_MULTICAST);
> > + GuestNetworkInterfaceList *head = NULL, *cur_item = NULL;
> > +
> > + /* Allocate a 15 KB buffer to start with */
> > + outBufLen = WORKING_BUFFER_SIZE;
> > + do {
> > + pAddresses = (IP_ADAPTER_ADDRESSES *) malloc(outBufLen);
> > + if (pAddresses == NULL) {
> > + error_setg(errp, "Memory allocation failed for"
> > + "IP_ADAPTER_ADDRESSES struct");
> > + return NULL;
> > + }
> > +
> > + dwRetVal = GetAdaptersAddresses(AF_UNSPEC, flags, NULL,
> > + pAddresses, &outBufLen);
> > +
> > + if (dwRetVal == ERROR_BUFFER_OVERFLOW) {
> > + free(pAddresses);
> > + pAddresses = NULL;
> > + } else {
> > + break;
> > + }
> > + Iterations++;
> > +
> > + } while ((dwRetVal == ERROR_BUFFER_OVERFLOW) && (Iterations <
> > MAX_TRIES));
> > +
> > + if (dwRetVal != NO_ERROR) {
> > + error_setg(errp, "Call to GetAdaptersAddresses failed with error:
> > %d",
> > + (int)dwRetVal);
> > + goto error;
> > + }
> > +
> > + for (pCurrAddresses = pAddresses; pCurrAddresses;
> > + pCurrAddresses = pCurrAddresses->Next) {
> > + GuestNetworkInterfaceList *info;
> > + PIP_ADAPTER_UNICAST_ADDRESS pUnicast = NULL;
> > + unsigned char *mac_addr;
> > +
> > + if (pCurrAddresses->IfType == IF_TYPE_TUNNEL) { /* skip tunnel
> > type? */
> > + continue;
> > + }
>
> I don't think we skip these on POSIX at least, so we probably shouldn't here.
>
> > +
> > + info = g_malloc0(sizeof(*info));
> > + info->value = g_malloc0(sizeof(*info->value));
> > + info->value->name = g_strdup(pCurrAddresses->AdapterName);
> > +
> > + if (!cur_item) {
> > + head = cur_item = info;
> > + } else {
> > + cur_item->next = info;
> > + cur_item = info;
> > + }
> > +
> > + mac_addr = pAddresses->PhysicalAddress;
> > + info->value->hardware_address =
> > + g_strdup_printf("%02x:%02x:%02x:%02x:%02x:%02x",
>
> ^ indent here
>
> > + (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;
> > +
> > + for (pUnicast = pCurrAddresses->FirstUnicastAddress; pUnicast;
> > + pUnicast = pUnicast->Next) {
> > + GuestIpAddressList **address_list = NULL, *address_item = NULL;
> > + char addr4[INET_ADDRSTRLEN];
> > + char addr6[INET6_ADDRSTRLEN];
> > +
> > + address_item = g_malloc0(sizeof(*address_item));
> > + address_item->value = g_malloc0(sizeof(*address_item->value));
> > + address_item->value->prefix = get_adapter_unicast_prefixlength(
> > + pCurrAddresses,
> > pUnicast);
> > + switch (pUnicast->Address.lpSockaddr->sa_family) {
> > + case AF_INET: {
> > + struct sockaddr_in *sa_in = (struct sockaddr_in *)
> > + pUnicast->Address.lpSockaddr;
> > + inet_ntop(AF_INET, &(sa_in->sin_addr), addr4,
> > sizeof(addr4));
>
> Kenth's original patch avoided the need for these 3rd party inet_ntop*
> functions and used WSAAddressToString() instead, which I think it
> is preferable...
>
> ...but, I see it's also listed as Vista+, which is likely why it wasn't used
> here. Darn.
>
> AFAIK the code is ISC-licensed and GPL-compatible but since it's the
> first example of such code in the QEMU tree I'd probably want to get
> an ACK from other maintainers before committing.
>
> > + address_item->value->ip_address_type
> > + = GUEST_IP_ADDRESS_TYPE_IPV4;
> > + address_item->value->ip_address = g_strdup(addr4);
> > + break;
> > + }
> > + case AF_INET6: {
> > + struct sockaddr_in6 *sa_in6 = (struct sockaddr_in6 *)
> > +
> > pUnicast->Address.lpSockaddr;
> > + inet_ntop(AF_INET6, &(sa_in6->sin6_addr), addr6,
> > sizeof(addr6));
> > + address_item->value->ip_address_type
> > + = GUEST_IP_ADDRESS_TYPE_IPV6;
> > + address_item->value->ip_address = g_strdup(addr6);
> > + break;
> > + }
> > + default:
> > + break;
> > + }
> > +
> > + address_list = &info->value->ip_addresses;
> > +
> > + while (*address_list && (*address_list)->next) {
> > + address_list = &(*address_list)->next;
> > + }
> > +
> > + if (!*address_list) {
> > + *address_list = address_item;
> > + } else {
> > + (*address_list)->next = address_item;
> > + }
> > +
> > + info->value->has_ip_addresses = true;
> > + }
> > + }
> > + free(pAddresses);
> > + return head;
> > +error:
> > + free(pAddresses);
> > + qapi_free_GuestNetworkInterfaceList(head);
> > return NULL;
> > }
> >
> > @@ -452,8 +646,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;
> > diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
> > index e92c6ab..e82afc5 100644
> > --- a/qga/guest-agent-core.h
> > +++ b/qga/guest-agent-core.h
> > @@ -12,6 +12,9 @@
> > */
> > #include "qapi/qmp/dispatch.h"
> > #include "qemu-common.h"
> > +#ifdef _WIN32
> > +#include <ws2tcpip.h>
> > +#endif
> >
> > #define QGA_READ_COUNT_DEFAULT 4096
> >
> > @@ -41,3 +44,11 @@ int64_t ga_get_fd_handle(GAState *s, Error **errp);
> > #ifndef _WIN32
> > void reopen_fd_to_null(int fd);
> > #endif
> > +
> > +#ifdef _WIN32
> > +/* Convert a Internet address in binary network format for interface
> > + type AF in buffer starting at CP to presentation form and place
> > + result in buffer of length LEN astarting at BUF. */
> > +extern const char *inet_ntop(int __af, const void *__cp,
> > + char *__buf, socklen_t __len);
> > +#endif
> > diff --git a/qga/inet_ntop-win32.c b/qga/inet_ntop-win32.c
> > new file mode 100644
> > index 0000000..2de961d
> > --- /dev/null
> > +++ b/qga/inet_ntop-win32.c
> > @@ -0,0 +1,184 @@
> > +/*
> > + * Copyright (c) 1996-1999 by Internet Software Consortium.
> > + *
> > + * Permission to use, copy, modify, and distribute this software for any
> > + * purpose with or without fee is hereby granted, provided that the above
> > + * copyright notice and this permission notice appear in all copies.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SOFTWARE CONSORTIUM
> > DISCLAIMS
> > + * ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
> > WARRANTIES
> > + * OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL INTERNET SOFTWARE
> > + * CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL
> > + * DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR
> > + * PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS
> > + * ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
> > THIS
> > + * SOFTWARE.
> > + */
> > +#include <errno.h>
> > +#include <ws2tcpip.h>
> > +#include "qga/guest-agent-core.h"
> > +
> > +
> > +#define SPRINTF(x) ((socklen_t)sprintf x)
> > +#define NS_INT16SZ 2
> > +#define NS_IN6ADDRSZ 16
> > +/*
> > + * WARNING: Don't even consider trying to compile this on a system where
> > + * sizeof(int) < 4. sizeof(int) > 4 is fine; all the world's not a VAX.
> > + */
> > +
> > +static const char *inet_ntop4(const u_char *src, char *dst, socklen_t
> > size);
> > +static const char *inet_ntop6(const u_char *src, char *dst, socklen_t
> > size);
> > +
> > +/* char *
> > + * inet_ntop(af, src, dst, size)
> > + * convert a network format address to presentation format.
> > + * return:
> > + * pointer to presentation format address (`dst'), or NULL (see errno).
> > + * author:
> > + * Paul Vixie, 1996.
> > + */
> > +const char *
> > +inet_ntop(int af, const void *src, char *dst, socklen_t size)
> > +{
> > + switch (af) {
> > + case AF_INET:
> > + return inet_ntop4(src, dst, size);
> > + case AF_INET6:
> > + return inet_ntop6(src, dst, size);
> > + default:
> > + errno = EAFNOSUPPORT;
> > + return NULL;
> > + }
> > + /* NOTREACHED */
> > +}
> > +
> > +/* const char *
> > + * inet_ntop4(src, dst, size)
> > + * format an IPv4 address
> > + * return:
> > + * `dst' (as a const)
> > + * notes:
> > + * (1) uses no statics
> > + * (2) takes a u_char* not an in_addr as input
> > + * author:
> > + * Paul Vixie, 1996.
> > + */
> > +static const char *
> > +inet_ntop4(const u_char *src, char *dst, socklen_t size)
> > +{
> > + static const char fmt[] = "%u.%u.%u.%u";
> > + char tmp[sizeof "255.255.255.255"];
> > +
> > + if (SPRINTF((tmp, fmt, src[0], src[1], src[2], src[3])) >= size) {
> > + errno = ENOSPC;
> > + return NULL;
> > + }
> > + return strcpy(dst, tmp);
> > +}
> > +
> > +/* const char *
> > + * inet_ntop6(src, dst, size)
> > + * convert IPv6 binary address into presentation (printable) format
> > + * author:
> > + * Paul Vixie, 1996.
> > + */
> > +static const char *
> > +inet_ntop6(const u_char *src, char *dst, socklen_t size)
> > +{
> > + /*
> > + * Note that int32_t and int16_t need only be "at least" large enough
> > + * to contain a value of the specified size. On some systems, like
> > + * Crays, there is no such thing as an integer variable with 16 bits.
> > + * Keep this in mind if you think this function should have been coded
> > + * to use pointer overlays. All the world's not a VAX.
> > + */
> > + char tmp[sizeof "ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255"], *tp;
> > + struct { int base, len; } best, cur;
> > + u_int words[NS_IN6ADDRSZ / NS_INT16SZ];
> > + int i;
> > +
> > + /*
> > + * Preprocess:
> > + * Copy the input (bytewise) array into a wordwise array.
> > + * Find the longest run of 0x00's in src[] for :: shorthanding.
> > + */
> > + memset(words, '\0', sizeof words);
> > + for (i = 0; i < NS_IN6ADDRSZ; i += 2) {
> > + words[i / 2] = (src[i] << 8) | src[i + 1];
> > + }
> > + best.base = -1;
> > + cur.base = -1;
> > + best.len = 0;
> > + cur.len = 0;
> > + for (i = 0; i < (NS_IN6ADDRSZ / NS_INT16SZ); i++) {
> > + if (words[i] == 0) {
> > + if (cur.base == -1) {
> > + cur.base = i;
> > + cur.len = 1;
> > + } else {
> > + cur.len++;
> > + }
> > + } else {
> > + if (cur.base != -1) {
> > + if (best.base == -1 || cur.len > best.len) {
> > + best = cur;
> > + }
> > + cur.base = -1;
> > + }
> > + }
> > + }
> > + if (cur.base != -1) {
> > + if (best.base == -1 || cur.len > best.len) {
> > + best = cur;
> > + }
> > + }
> > + if (best.base != -1 && best.len < 2) {
> > + best.base = -1;
> > + }
> > +
> > + /*
> > + * Format the result.
> > + */
> > + tp = tmp;
> > + for (i = 0; i < (NS_IN6ADDRSZ / NS_INT16SZ); i++) {
> > + /* Are we inside the best run of 0x00's? */
> > + if (best.base != -1 && i >= best.base &&
> > + i < (best.base + best.len)) {
> > + if (i == best.base) {
> > + *tp++ = ':';
> > + }
> > + continue;
> > + }
> > + /* Are we following an initial run of 0x00s or any real hex? */
> > + if (i != 0) {
> > + *tp++ = ':';
> > + }
> > + /* Is this address an encapsulated IPv4? */
> > + if (i == 6 && best.base == 0 &&
> > + (best.len == 6 || (best.len == 5 && words[5] == 0xffff))) {
> > + if (!inet_ntop4(src + 12, tp, sizeof tmp - (tp - tmp))) {
> > + return NULL;
> > + }
> > + tp += strlen(tp);
> > + break;
> > + }
> > + tp += SPRINTF((tp, "%x", words[i]));
> > + }
> > + /* Was it a trailing run of 0x00's? */
> > + if (best.base != -1 && (best.base + best.len) ==
> > + (NS_IN6ADDRSZ / NS_INT16SZ)) {
> > + *tp++ = ':';
> > + }
> > + *tp++ = '\0';
> > +
> > + /*
> > + * Check for overflow, copy, and we're done.
> > + */
> > + if ((socklen_t)(tp - tmp) > size) {
> > + errno = ENOSPC;
> > + return NULL;
> > + }
> > + return strcpy(dst, tmp);
> > +}
> > +
> > --
> > 1.7.12.4
