Re: [Qemu-devel] [RFC PATCH] qga: implement guest-network-get-interfaces command for windows

[Michael Roth]

Quoting zhanghailiang (2014-12-24 04:21:20)
> Signed-off-by: zhanghailiang <address@hidden>
> ---
> 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.

> 
> 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