|
On 03/06/15 13:19, Olga Krishtal wrote:
On
29/05/15 19:46, Kirk Allan wrote:
>>>
On 28/05/15 23:54, Denis V. Lunev wrote:
On 28/05/15 21:41, 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
GetAdaptersInfo. 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<address@hidden>
---
qga/commands-win32.c | 292
++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 290 insertions(+), 2 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 3ef0549..3bf9011 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,291 @@ void qmp_guest_suspend_hybrid(Error
**errp)
error_set(errp, QERR_UNSUPPORTED);
}
+static DWORD
guest_get_adapters_addresses(IP_ADAPTER_ADDRESSES
**adptr_addrs)
+{
+ ULONG adptr_addrs_len = 0;
+ DWORD ret = ERROR_SUCCESS;
+
+ /* Call the first time to get the adptr_addrs_len. */
+ *adptr_addrs = NULL;
+ GetAdaptersAddresses(AF_UNSPEC,
GAA_FLAG_INCLUDE_PREFIX,
+ NULL, *adptr_addrs,
&adptr_addrs_len);
+
+ *adptr_addrs = g_malloc(adptr_addrs_len);
+ if (*adptr_addrs == NULL) {
+ ret = ERROR_NOT_ENOUGH_MEMORY;
+ } else {
+ ret = GetAdaptersAddresses(AF_UNSPEC,
GAA_FLAG_INCLUDE_PREFIX,
+ NULL, *adptr_addrs,
&adptr_addrs_len);
+ if (ret != ERROR_SUCCESS) {
+ g_free(*adptr_addrs);
+ *adptr_addrs = NULL;
+ }
+ }
+ return ret;
https://developer.gnome.org/glib/2.28/glib-Memory-Allocation.html#g-malloc
g_malloc never fail
If any call to allocate memory fails, the application is
terminated.
This also means that there is no need to check if the call
succeeded.
I'll cleanup all the g_malloc testing for failures.
this means that IP_ADAPTER_ADDRESSES
could be returned from function
I think that check is still necessary, because we may suffer
some other
problems:
*ERROR_ADDRESS_NOT_ASSOCIATED*
*ERROR_INVALID_PARAMETER*
and even *Other*, according to MSDN.
From my point of view, the prototype of function should be
smth like this:
static IP_ADAPTER_ADDRESSES *guest_get_adapters_addresses()
{
PIP_ADAPTER_ADDRESSES ip_add = NULL;
/* Memory allocation, etc */
......
if (GetAdaptersAddresses(....)) {
error_setg_win32(errp, GetLastError(), "Failed smth");
.....
}
return ip_add;
}
I'll make this change. It does mean that errp will need to be
passed in and then do something like
ret = GetAdaptersAddresses(....);
if (ret != ERROR_SUCCESS) {
error_setg_win32(errp, ret, "Failed smth");
.....
}
because GetAdpatersAddress is not documented to set last error.
Oh, yes. Sorry, but still I think that such prototype is better.
**
+}
+
+#if (_WIN32_WINNT < 0x0600)
are you correct with the condition? according to MSDN
On Windows XP and later: Use the GetAdaptersAddresses
function
instead of GetAdaptersInfo.
Thus you should have above branch of code undefined.
It is better to use #ifdef (smth), #else and #endif to
separate win version.
I'll redo the #ifdefs so that for the < 0x0600 case,
guest_get_adapters_info and guest_ip_prefix will be together to
better show what is happening for the XP and 2003 case.
Den is right, the first func will be
compiled always, and the winxp
variation
will be compiled for winserver 2003, and winxp. Have you
tested if for
2003 server?
I have tested on Windows 2003 with no issue. The only way I
have found to find the correct prefix on XP and 2003 is to use
GetAdaptersInfo and do the matching. The prefixes can be
obtained from GetAdaptersAddresses but the order is not
specified. That's why I am just returning on ipv6 and calling
GetAdaptersInfo to match addresses for ipv4. Rather than do all
this matching using GetAdaptersInfo, I could just return -1 as
in the ipv6 case. What do you recommend?
Actually, I have looked more attentive at GetAdaptersInfo :
Minimum supported client
Windows 2000 Professional [desktop apps only]
Minimum supported server
Windows 2000 Server [desktop apps only]
and GetAdaptersAddresses:
Minimum supported client
Windows XP [desktop apps only]
Minimum supported server
Windows Server 2003 [desktop apps only]
May be we should drop the support of 2000 and do this
functionality starting with XP. As the result we will not need
this #ifdef thing.
I have been looking through the list of function with minimum
supported client Win 2003:
OpenProcessToken, AdjustTokenPrivileges in qmp_guest_shutdown
SetFilePointerEx in
qmp_guest_file_seek
BOOL WINAPI FlushFileBuffers
in qmp_guest_file_flush
BOOL WINAPI GetExitCodeProcess
im qmp_guest_exec
Some IOCLS such as IOCTL_STORAGE_QUERY_PROPERTY
and etc.
As you can see at the moment support of Windows 2000 is very
questionable.
I wonder if we could just drop it?
+static DWORD
guest_get_adapters_info(IP_ADAPTER_INFO **adptr_info)
+{
+ ULONG adptr_info_len = 0;
+ DWORD ret = ERROR_SUCCESS;
+
+ /* Call the first time to get the adptr_info_len. */
+ *adptr_info = NULL;
+ GetAdaptersInfo(*adptr_info, &adptr_info_len);
+
+ *adptr_info = g_malloc(adptr_info_len);
+ if (*adptr_info == NULL) {
+ ret = ERROR_NOT_ENOUGH_MEMORY;
+ } else {
+ ret = GetAdaptersInfo(*adptr_info,
&adptr_info_len);
+ if (ret != ERROR_SUCCESS) {
+ g_free(*adptr_info);
+ *adptr_info = NULL;
+ }
+ }
same note about the allocation
Same prototype as before.
I'll clean up all the memory allocation checks.
+ return ret;
+}
+#endif
+
+static char *guest_wctomb_dup(WCHAR *wstr)
+{
+ char *str;
+ size_t i;
+
+ i = wcslen(wstr) + 1;
+ str = g_malloc(i);
+ if (str) {
+ WideCharToMultiByte(CP_ACP, WC_COMPOSITECHECK,
+ wstr, -1, str, i, NULL,
NULL);
+ }
same allocation
I'll clean up all the memory allocation checks.
+ return str;
+}
+
+static char *guest_inet_ntop(int af, void *cp, char *buf,
size_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.
+ */
nothing about 64bit only is present here. This seems strange
to me
https://msdn.microsoft.com/ru-ru/library/windows/desktop/cc805843(v=vs.85).as
px
You're right, I'll remove the defined(ARCH_x86_64)
If you will use glib utility:
nm_utils_inet4/6_ntop() you won't need
this separation.
This would work, but I'm not seeing the header or lib to compile
this in for Windows. Maybe I'm missing some part of the
environment?
I have searched rpm with nm-utils.h and the result is
disappointing: NetworkManager-libnm-devel.XXX.rpm.
It would be too difficult task to take all this functionality to
Windows.
Pls, look through this and say is it possible to support usage
of nm-utils functions in win32 realization. (I will do it too, but
a bit latter).
However, we still can look at how it was implemented.
+ return (char *)InetNtop(af, cp,
buf, len);
+#else
+ u_char *p;
I'll change to use uint8_t.
+
+ 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) {
What do you recommend here? I'm not finding a form of inet_ntop
for all flavors of Window. The purpose of this code is to do the
address compression. For the XP and 2003 case, I could just
simply produce the string of the full address. Although not
optimal, it would eliminate complexity and the need for the
string manipulation.
+ int i, compressed_zeros,
added_to_buf;
+ char t[sizeof "::ffff"];
sizeof without braces and from string could be tricky but I
am not
quite sure
Tests have shown that this isn't an issue, but I can add the ().
+
+ 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;
with len == 1 you will definitely have problem with wrong
conversion
+ }
+ 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]);
+ }
snprintf can produce non-zero terminated character arrays
and the same problem with length...
+
+ /* 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, "::");
there is not length check here
+ }
+ }
+ return buf;
+#endif
+}
+
I still recommend to think over this function, here useful
link
https://developer.gnome.org/glib/stable/glib-String-Utility-Functions.html
glib provides a lot of function to work with strings, and
there you do
not need think
about memory allocations and etc. Pls, look.
+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. */
+ IP_ADAPTER_INFO *adptr_info, *info;
+ IP_ADDR_STRING *ip;
+ struct in_addr *p;
+
+ if (ip_addr->Address.lpSockaddr->sa_family ==
AF_INET) {
why not to return immediately? You will have 1 less indent
I'll do that.
+ if
(guest_get_adapters_info(&adptr_info) !=
ERROR_SUCCESS) {
+ return prefix;
+ }
adptr_info = guest_get_adapters_info();
if(adptr_info == NULL )
goto out;
If you will use the prototypes, I have written before it will
be easier.
I'll make the changes.
+ /* 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;
why not goto here. Moving out from inner loop with goto is a
good thing.
There is no necessity to use prefix == -1 above in this case
I'll do that.
+ }
+ }
+ }
+ 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;
+ IP_ADAPTER_UNICAST_ADDRESS *ip_addr = NULL;
+ GuestNetworkInterfaceList *head = NULL, *cur_item =
NULL;
+ GuestIpAddressList *head_addr, *cur_addr;
+ GuestNetworkInterfaceList *info;
+ GuestIpAddressList *address_item = NULL;
+ unsigned char *mac_addr;
+ void *p;
+ char addr4[INET_ADDRSTRLEN];
+ char addr6[INET6_ADDRSTRLEN];
+ DWORD ret;
+
May be you should return pointer to structure by
guest_get_adapters_addresses, and
make this function take void? As the result you won't need if
(ret !=
ERROR_SUCCESS)
It will be smth like this:
adptr_addrs = guest_get_adapters_addresses();
if (adptr_addrs == NULL)
goto out;
And all error, that may happen in guest_get_adapters_addresses
will be
held there.
I'll do that.
+ ret =
guest_get_adapters_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)
{
+ info = g_malloc0(sizeof(*info));
+ if (!info) {
no need to check, see above
I'll fix all the memory checks.
+ error_setg(errp,
"failed to alloc a network interface
list");
+ goto error;
+ }
+
+ if (!cur_item) {
it is better to reserve ! condition to logical checks. For
pointers it
is better to check using != NULL
I'll fix these
+ head = cur_item = info;
+ } else {
+ cur_item->next = info;
+ cur_item = info;
+ }
+
+ info->value =
g_malloc0(sizeof(*info->value));
+ if (!info->value) {
same
+ error_setg(errp,
"failed to alloc a network interface");
+ goto error;
+ }
+ info->value->name =
guest_wctomb_dup(addr->FriendlyName);
+
+ if (addr->PhysicalAddressLength) {
same for check
+ 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;
+ }
+
no need check if (!address_item), as Den had explained before.
+ 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_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_win32(errp,
WSAGetLastError(),
+ "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);
if is not needed
I'll clean up all the memory allocation checks.
https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html#g-free
+ }
+ return head;
+
+error:
+ if (adptr_addrs) {
+ g_free(adptr_addrs);
+ }
+ if (head) {
+ qapi_free_GuestNetworkInterfaceList(head);
+ }
return NULL;
}
@@ -707,7 +995,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",
|