[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH] qga: implement guest-network-get-interfaces
From: |
Michael Roth |
Subject: |
Re: [Qemu-devel] [RFC PATCH] qga: implement guest-network-get-interfaces command for windows |
Date: |
Thu, 08 Jan 2015 17:29:43 -0600 |
User-agent: |
alot/0.3.4 |
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
- Re: [Qemu-devel] [RFC PATCH] qga: implement guest-network-get-interfaces command for windows,
Michael Roth <=