[Qemu-devel] [PULL 0/4] qemu-ga patch queue

[Qemu-devel] [PULL 0/4] qemu-ga patch queue

[Michael Roth]

The following changes since commit ae49fbbcd8e4e9d8bf7131add34773f579e1aff7:

  Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20171025' into staging (2017-10-25 16:38:57 +0100)

are available in the git repository at:

  git://github.com/mdroth/qemu.git tags/qga-pull-2017-10-26-tag

for you to fetch changes up to 8cedc80555e5a395a4fda7130b0115015e775c48:

  qga-win: fix error-handling in getNameByStringSID() (2017-10-26 20:01:32 -0500)

----------------------------------------------------------------
qemu-ga patch queue for 2.11

* support for network interface stats
* w32: improvements for guest-set-time
* w32: fix a hang with guest-fsfreeze-freeze when timeout occurs
  during heavy I/O
* w32: fix faulty error-handling in VSS/fsfreeze COM registration

----------------------------------------------------------------
Bishara AbuHattoum (1):
      qga-win: Updating guest_set_time action

Chen Hanxiao (1):
      qga-win: don't hang if vss hold writes timeout

Michael Roth (1):
      qga-win: fix error-handling in getNameByStringSID()

ZhiPeng Lu (1):
      qga: add network stats to guest-network-get-interfaces

 configure                   |  2 +-
 qga/commands-posix.c        | 74 +++++++++++++++++++++++++++++++++++++-
 qga/commands-win32.c        | 86 ++++++++++++++++++++++++++++++++++++++++++++-
 qga/qapi-schema.json        | 38 +++++++++++++++++++-
 qga/vss-win32/install.cpp   | 13 ++++---
 qga/vss-win32/requester.cpp | 12 +++++++
 6 files changed, 217 insertions(+), 8 deletions(-)

[Qemu-devel] [PULL 1/4] qga-win: don't hang if vss hold writes timeout

[Michael Roth]

From: Chen Hanxiao <chenhanxiao@gmail.com>

When VM is in a heavy IO, if the command "guest-fsfreeze-freeze"
is executed, VSS may timeout when trying to hold writes.

Inside guest, Event ID 12298(VSS_ERROR_HOLD_WRITES_TIMEOUT)
is logged in the Event Viewer.

At that time, if we call AbortBackup, qga may hang forever.

This patch will solve this issue.

Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Cc: Tomoki Sekiyama <tomoki.sekiyama@gmail.com>

Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/vss-win32/requester.cpp | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
index 301762d8b1..3d9c9716c0 100644
--- a/qga/vss-win32/requester.cpp
+++ b/qga/vss-win32/requester.cpp
@@ -419,6 +419,16 @@ void requester_freeze(int *num_vols, ErrorSet *errset)
             break;
         }
     }
+
+    if (wait_status == WAIT_TIMEOUT) {
+        err_set(errset, E_FAIL,
+                "timeout when try to receive Frozen event from VSS provider");
+        /* If we are here, VSS had timeout.
+         * Don't call AbortBackup, just return directly.
+         */
+        goto out1;
+    }
+
     if (wait_status != WAIT_OBJECT_0) {
         err_set(errset, E_FAIL,
                 "couldn't receive Frozen event from VSS provider");
@@ -432,6 +442,8 @@ out:
     if (vss_ctx.pVssbc) {
         vss_ctx.pVssbc->AbortBackup();
     }
+
+out1:
     requester_cleanup();
     CoUninitialize();
 }
-- 
2.11.0

[Qemu-devel] [PULL 2/4] qga-win: Updating guest_set_time action

[Michael Roth]

From: Bishara AbuHattoum <bishara@daynix.com>

  At the moment, Windows libraries don't provide a way to access
  RTC, so, a workaround is to use the Windows w32tm command to
  resync the time.
  Related bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1183874

Signed-off-by: Bishara AbuHattoum <bishara@daynix.com>
Reviewed-by: Sameeh Jubran <sameeh@daynix.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 configure            |  2 +-
 qga/commands-win32.c | 36 +++++++++++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 03547cea6a..325b08056c 100755
--- a/configure
+++ b/configure
@@ -827,7 +827,7 @@ if test "$mingw32" = "yes" ; then
   sysconfdir="\${prefix}"
   local_statedir=
   confsuffix=""
-  libs_qga="-lws2_32 -lwinmm -lpowrprof -lwtsapi32 -liphlpapi -lnetapi32 $libs_qga"
+  libs_qga="-lws2_32 -lwinmm -lpowrprof -lwtsapi32 -lwininet -liphlpapi -lnetapi32 $libs_qga"
 fi
 
 werror=""
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 619dbd2bc2..fbd7eb7bbb 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -29,6 +29,7 @@
 #endif
 #include <lm.h>
 #include <wtsapi32.h>
+#include <wininet.h>
 
 #include "qga/guest-agent-core.h"
 #include "qga/vss-win32.h"
@@ -1277,8 +1278,41 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp)
          * RTC yet:
          *
          * https://msdn.microsoft.com/en-us/library/aa908981.aspx
+         *
+         * Instead, a workaround is to use the Windows win32tm command to
+         * resync the time using the Windows Time service.
          */
-        error_setg(errp, "Time argument is required on this platform");
+        LPVOID msg_buffer;
+        DWORD ret_flags;
+
+        HRESULT hr = system("w32tm /resync /nowait");
+
+        if (GetLastError() != 0) {
+            strerror_s((LPTSTR) & msg_buffer, 0, errno);
+            error_setg(errp, "system(...) failed: %s", (LPCTSTR)msg_buffer);
+        } else if (hr != 0) {
+            if (hr == HRESULT_FROM_WIN32(ERROR_SERVICE_NOT_ACTIVE)) {
+                error_setg(errp, "Windows Time service not running on the "
+                                 "guest");
+            } else {
+                if (!FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER |
+                                   FORMAT_MESSAGE_FROM_SYSTEM |
+                                   FORMAT_MESSAGE_IGNORE_INSERTS, NULL,
+                                   (DWORD)hr, MAKELANGID(LANG_NEUTRAL,
+                                   SUBLANG_DEFAULT), (LPTSTR) & msg_buffer, 0,
+                                   NULL)) {
+                    error_setg(errp, "w32tm failed with error (0x%lx), couldn'"
+                                     "t retrieve error message", hr);
+                } else {
+                    error_setg(errp, "w32tm failed with error (0x%lx): %s", hr,
+                               (LPCTSTR)msg_buffer);
+                    LocalFree(msg_buffer);
+                }
+            }
+        } else if (!InternetGetConnectedState(&ret_flags, 0)) {
+            error_setg(errp, "No internet connection on guest, sync not "
+                             "accurate");
+        }
         return;
     }
 
-- 
2.11.0

[Qemu-devel] [PULL 3/4] qga: add network stats to guest-network-get-interfaces

[Michael Roth]

From: ZhiPeng Lu <lu.zhipeng@zte.com.cn>

we can get the network interface statistics inside a virtual machine by
guest-network-get-interfaces command. it is very useful for us tomonitor
and analyze network traffic.

Signed-off-by: ZhiPeng Lu <lu.zhipeng@zte.com.cn>
* don't rely on sizeof(wchar[]) for wchar[] indexing
* avoid camelCase variable names
* fix up getline() usage
* condensed commit subject line
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 qga/commands-win32.c | 50 +++++++++++++++++++++++++++++++++++
 qga/qapi-schema.json | 38 ++++++++++++++++++++++++++-
 3 files changed, 160 insertions(+), 2 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index ab0c63d931..e809e382eb 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1643,6 +1643,67 @@ guest_find_interface(GuestNetworkInterfaceList *head,
     return head;
 }
 
+static int guest_get_network_stats(const char *name,
+                       GuestNetworkInterfaceStat *stats)
+{
+    int name_len;
+    char const *devinfo = "/proc/net/dev";
+    FILE *fp;
+    char *line = NULL, *colon;
+    size_t n = 0;
+    fp = fopen(devinfo, "r");
+    if (!fp) {
+        return -1;
+    }
+    name_len = strlen(name);
+    while (getline(&line, &n, fp) != -1) {
+        long long dummy;
+        long long rx_bytes;
+        long long rx_packets;
+        long long rx_errs;
+        long long rx_dropped;
+        long long tx_bytes;
+        long long tx_packets;
+        long long tx_errs;
+        long long tx_dropped;
+        char *trim_line;
+        trim_line = g_strchug(line);
+        if (trim_line[0] == '\0') {
+            continue;
+        }
+        colon = strchr(trim_line, ':');
+        if (!colon) {
+            continue;
+        }
+        if (colon - name_len  == trim_line &&
+           strncmp(trim_line, name, name_len) == 0) {
+            if (sscanf(colon + 1,
+                "%lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld",
+                  &rx_bytes, &rx_packets, &rx_errs, &rx_dropped,
+                  &dummy, &dummy, &dummy, &dummy,
+                  &tx_bytes, &tx_packets, &tx_errs, &tx_dropped,
+                  &dummy, &dummy, &dummy, &dummy) != 16) {
+                continue;
+            }
+            stats->rx_bytes = rx_bytes;
+            stats->rx_packets = rx_packets;
+            stats->rx_errs = rx_errs;
+            stats->rx_dropped = rx_dropped;
+            stats->tx_bytes = tx_bytes;
+            stats->tx_packets = tx_packets;
+            stats->tx_errs = tx_errs;
+            stats->tx_dropped = tx_dropped;
+            fclose(fp);
+            g_free(line);
+            return 0;
+        }
+    }
+    fclose(fp);
+    g_free(line);
+    g_debug("/proc/net/dev: Interface '%s' not found", name);
+    return -1;
+}
+
 /*
  * Build information about guest interfaces
  */
@@ -1659,6 +1720,7 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
     for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
         GuestNetworkInterfaceList *info;
         GuestIpAddressList **address_list = NULL, *address_item = NULL;
+        GuestNetworkInterfaceStat  *interface_stat = NULL;
         char addr4[INET_ADDRSTRLEN];
         char addr6[INET6_ADDRSTRLEN];
         int sock;
@@ -1778,7 +1840,17 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
 
         info->value->has_ip_addresses = true;
 
-
+        if (!info->value->has_statistics) {
+            interface_stat = g_malloc0(sizeof(*interface_stat));
+            if (guest_get_network_stats(info->value->name,
+                interface_stat) == -1) {
+                info->value->has_statistics = false;
+                g_free(interface_stat);
+            } else {
+                info->value->statistics = interface_stat;
+                info->value->has_statistics = true;
+            }
+        }
     }
 
     freeifaddrs(ifap);
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index fbd7eb7bbb..0322188a73 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1153,6 +1153,44 @@ out:
 }
 #endif
 
+#define INTERFACE_PATH_BUF_SZ 512
+
+static DWORD get_interface_index(const char *guid)
+{
+    ULONG index;
+    DWORD status;
+    wchar_t wbuf[INTERFACE_PATH_BUF_SZ];
+    snwprintf(wbuf, INTERFACE_PATH_BUF_SZ, L"\\device\\tcpip_%s", guid);
+    wbuf[INTERFACE_PATH_BUF_SZ - 1] = 0;
+    status = GetAdapterIndex (wbuf, &index);
+    if (status != NO_ERROR) {
+        return (DWORD)~0;
+    } else {
+        return index;
+    }
+}
+static int guest_get_network_stats(const char *name,
+                       GuestNetworkInterfaceStat *stats)
+{
+    DWORD if_index = 0;
+    MIB_IFROW a_mid_ifrow;
+    memset(&a_mid_ifrow, 0, sizeof(a_mid_ifrow));
+    if_index = get_interface_index(name);
+    a_mid_ifrow.dwIndex = if_index;
+    if (NO_ERROR == GetIfEntry(&a_mid_ifrow)) {
+        stats->rx_bytes = a_mid_ifrow.dwInOctets;
+        stats->rx_packets = a_mid_ifrow.dwInUcastPkts;
+        stats->rx_errs = a_mid_ifrow.dwInErrors;
+        stats->rx_dropped = a_mid_ifrow.dwInDiscards;
+        stats->tx_bytes = a_mid_ifrow.dwOutOctets;
+        stats->tx_packets = a_mid_ifrow.dwOutUcastPkts;
+        stats->tx_errs = a_mid_ifrow.dwOutErrors;
+        stats->tx_dropped = a_mid_ifrow.dwOutDiscards;
+        return 0;
+    }
+    return -1;
+}
+
 GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
 {
     IP_ADAPTER_ADDRESSES *adptr_addrs, *addr;
@@ -1160,6 +1198,7 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
     GuestNetworkInterfaceList *head = NULL, *cur_item = NULL;
     GuestIpAddressList *head_addr, *cur_addr;
     GuestNetworkInterfaceList *info;
+    GuestNetworkInterfaceStat *interface_stat = NULL;
     GuestIpAddressList *address_item = NULL;
     unsigned char *mac_addr;
     char *addr_str;
@@ -1239,6 +1278,17 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
             info->value->has_ip_addresses = true;
             info->value->ip_addresses = head_addr;
         }
+        if (!info->value->has_statistics) {
+            interface_stat = g_malloc0(sizeof(*interface_stat));
+            if (guest_get_network_stats(addr->AdapterName,
+                interface_stat) == -1) {
+                info->value->has_statistics = false;
+                g_free(interface_stat);
+            } else {
+                info->value->statistics = interface_stat;
+                info->value->has_statistics = true;
+            }
+        }
     }
     WSACleanup();
 out:
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 90a0c8602b..17884c7c70 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -643,6 +643,38 @@
            'prefix': 'int'} }
 
 ##
+# @GuestNetworkInterfaceStat:
+#
+# @rx-bytes: total bytes received
+#
+# @rx-packets: total packets received
+#
+# @rx-errs: bad packets received
+#
+# @rx-dropped: receiver dropped packets
+#
+# @tx-bytes: total bytes transmitted
+#
+# @tx-packets: total packets transmitted
+#
+# @tx-errs: packet transmit problems
+#
+# @tx-dropped: dropped packets transmitted
+#
+# Since: 2.11
+##
+{ 'struct': 'GuestNetworkInterfaceStat',
+  'data': {'rx-bytes': 'uint64',
+            'rx-packets': 'uint64',
+            'rx-errs': 'uint64',
+            'rx-dropped': 'uint64',
+            'tx-bytes': 'uint64',
+            'tx-packets': 'uint64',
+            'tx-errs': 'uint64',
+            'tx-dropped': 'uint64'
+           } }
+
+##
 # @GuestNetworkInterface:
 #
 # @name: The name of interface for which info are being delivered
@@ -651,12 +683,16 @@
 #
 # @ip-addresses: List of addresses assigned to @name
 #
+# @statistics: various statistic counters related to @name
+# (since 2.11)
+#
 # Since: 1.1
 ##
 { 'struct': 'GuestNetworkInterface',
   'data': {'name': 'str',
            '*hardware-address': 'str',
-           '*ip-addresses': ['GuestIpAddress'] } }
+           '*ip-addresses': ['GuestIpAddress'],
+           '*statistics': 'GuestNetworkInterfaceStat' } }
 
 ##
 # @guest-network-get-interfaces:
-- 
2.11.0

[Qemu-devel] [PULL 4/4] qga-win: fix error-handling in getNameByStringSID()

[Michael Roth]

In one case we misconstrue a BOOL return as an HRESULT, and in the
other case we don't check the BOOL return from LookupAccountSidW()
before extracting the HRESULT from GetLastError(). Both can lead to
getNameByStringSID() misreporting an error.

Reported-by: Chen Hanxiao <chenhanxiao@gmail.com>
Suggested-by: Tomáš Golembiovský <tgolembi@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/vss-win32/install.cpp | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
index ba7c94eb25..6713e58670 100644
--- a/qga/vss-win32/install.cpp
+++ b/qga/vss-win32/install.cpp
@@ -148,10 +148,15 @@ static HRESULT getNameByStringSID(
     DWORD domainNameLen = BUFFER_SIZE;
     wchar_t domainName[BUFFER_SIZE];
 
-    chk(ConvertStringSidToSidW(sid, &psid));
-    LookupAccountSidW(NULL, psid, buffer, bufferLen,
-                domainName, &domainNameLen, &groupType);
-    hr = HRESULT_FROM_WIN32(GetLastError());
+    if (!ConvertStringSidToSidW(sid, &psid)) {
+        hr = HRESULT_FROM_WIN32(GetLastError());
+        goto out;
+    }
+    if (!LookupAccountSidW(NULL, psid, buffer, bufferLen,
+                           domainName, &domainNameLen, &groupType)) {
+        hr = HRESULT_FROM_WIN32(GetLastError());
+        /* Fall through and free psid */
+    }
 
     LocalFree(psid);
 
-- 
2.11.0

Re: [Qemu-devel] [PULL 0/4] qemu-ga patch queue

[Peter Maydell]

On 27 October 2017 at 02:18, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> The following changes since commit ae49fbbcd8e4e9d8bf7131add34773f579e1aff7:
>
>   Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20171025' into staging (2017-10-25 16:38:57 +0100)
>
> are available in the git repository at:
>
>   git://github.com/mdroth/qemu.git tags/qga-pull-2017-10-26-tag
>
> for you to fetch changes up to 8cedc80555e5a395a4fda7130b0115015e775c48:
>
>   qga-win: fix error-handling in getNameByStringSID() (2017-10-26 20:01:32 -0500)
>
> ----------------------------------------------------------------
> qemu-ga patch queue for 2.11
>
> * support for network interface stats
> * w32: improvements for guest-set-time
> * w32: fix a hang with guest-fsfreeze-freeze when timeout occurs
>   during heavy I/O
> * w32: fix faulty error-handling in VSS/fsfreeze COM registration
>

Applied, thanks.

-- PMM