[Qemu-devel] [PULL 00/10] Fixes and new commands for QEMU Guest Agent

[Qemu-devel] [PULL 00/10] Fixes and new commands for QEMU Guest Agent

[Michael Roth]

The following changes since commit cd2d5541271f1934345d8ca42f5fafff1744eee7:

  Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20150212' into staging (2015-02-13 11:44:50 +0000)

are available in the git repository at:

  git://github.com/mdroth/qemu.git tags/qga-pull-2015-02-16-tag

for you to fetch changes up to bf0abffb8d94ba15344c6e7e4326c8da0b9784b1:

  qemu-ga-win: Fail loudly on bare 'set-time' (2015-02-16 20:17:39 -0600)

----------------------------------------------------------------
tag for qga-pull-2015-02-16

----------------------------------------------------------------

Daniel P. Berrange (1):
      qga: add guest-set-user-password command

Michal Privoznik (1):
      qemu-ga-win: Fail loudly on bare 'set-time'

Olga Krishtal (2):
      utils: drop strtok_r from envlist_parse
      qga: implement file commands for Windows guest

Simon Zolin (1):
      guest agent: guest-file-open: refactoring

zhanghailiang (5):
      qga: introduce three guest memory block commmands with stubs
      qga: implement qmp_guest_get_memory_blocks() for Linux with sysfs
      qga: implement qmp_guest_set_memory_blocks() for Linux with sysfs
      qga: implement qmp_guest_get_memory_block_size() for Linux with sysfs
      qga: add memory block command that unsupported

 include/sysemu/os-win32.h |   1 -
 qga/commands-posix.c      | 468 +++++++++++++++++++++++++++++++++++++++++++++-
 qga/commands-win32.c      | 345 +++++++++++++++++++++++++++++-----
 qga/qapi-schema.json      | 145 +++++++++++++-
 util/envlist.c            |  32 ++--
 5 files changed, 921 insertions(+), 70 deletions(-)

[Qemu-devel] [PATCH 01/10] qga: add guest-set-user-password command

[Michael Roth]

From: "Daniel P. Berrange" <berrange@redhat.com>

Add a new 'guest-set-user-password' command for changing the password
of guest OS user accounts. This command is needed to enable OpenStack
to support its API for changing the admin password of guests running
on KVM/QEMU. It is not practical to provide a command at the QEMU
level explicitly targetting administrator account password change
only, since different guest OS have different names for the admin
account. While UNIX systems use 'root', Windows systems typically
use 'Administrator' and even that can be renamed. Higher level apps
like OpenStack have the ability to figure out the correct admin
account name since they have info that QEMU/libvirt do not.

The command accepts either the clear text password string, encoded
in base64 to make it 8-bit safe in JSON:

$ echo -n "123456" | base64
MTIzNDU2
$ virsh -c qemu:///system  qemu-agent-command f21x86_64 \
   '{ "execute": "guest-set-user-password",
      "arguments": { "crypted": false,
                     "username": "root",
                     "password": "MTIzNDU2" } }'
  {"return":{}}

Or a password that has already been run though a crypt(3) like
algorithm appropriate for the guest, again then base64 encoded:

$ echo -n '$6$n01A2Tau$e...snip...DfMOP7of9AJ1I8q0' | base64
JDYkb...snip...YT2Ey
$ virsh -c qemu:///system  qemu-agent-command f21x86_64 \
   '{ "execute": "guest-set-user-password",
      "arguments": { "crypted": true,
                     "username": "root",
                     "password": "JDYkb...snip...YT2Ey" } }'

NB windows support is desirable, but not implemented in this
patch.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++
 qga/commands-win32.c |   9 +++++
 qga/qapi-schema.json |  27 +++++++++++++
 3 files changed, 146 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index f6f3e3c..57409d0 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1875,6 +1875,108 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
     return processed;
 }
 
+void qmp_guest_set_user_password(const char *username,
+                                 const char *password,
+                                 bool crypted,
+                                 Error **errp)
+{
+    Error *local_err = NULL;
+    char *passwd_path = NULL;
+    pid_t pid;
+    int status;
+    int datafd[2] = { -1, -1 };
+    char *rawpasswddata = NULL;
+    size_t rawpasswdlen;
+    char *chpasswddata = NULL;
+    size_t chpasswdlen;
+
+    rawpasswddata = (char *)g_base64_decode(password, &rawpasswdlen);
+    rawpasswddata = g_renew(char, rawpasswddata, rawpasswdlen + 1);
+    rawpasswddata[rawpasswdlen] = '\0';
+
+    if (strchr(rawpasswddata, '\n')) {
+        error_setg(errp, "forbidden characters in raw password");
+        goto out;
+    }
+
+    if (strchr(username, '\n') ||
+        strchr(username, ':')) {
+        error_setg(errp, "forbidden characters in username");
+        goto out;
+    }
+
+    chpasswddata = g_strdup_printf("%s:%s\n", username, rawpasswddata);
+    chpasswdlen = strlen(chpasswddata);
+
+    passwd_path = g_find_program_in_path("chpasswd");
+
+    if (!passwd_path) {
+        error_setg(errp, "cannot find 'passwd' program in PATH");
+        goto out;
+    }
+
+    if (pipe(datafd) < 0) {
+        error_setg(errp, "cannot create pipe FDs");
+        goto out;
+    }
+
+    pid = fork();
+    if (pid == 0) {
+        close(datafd[1]);
+        /* child */
+        setsid();
+        dup2(datafd[0], 0);
+        reopen_fd_to_null(1);
+        reopen_fd_to_null(2);
+
+        if (crypted) {
+            execle(passwd_path, "chpasswd", "-e", NULL, environ);
+        } else {
+            execle(passwd_path, "chpasswd", NULL, environ);
+        }
+        _exit(EXIT_FAILURE);
+    } else if (pid < 0) {
+        error_setg_errno(errp, errno, "failed to create child process");
+        goto out;
+    }
+    close(datafd[0]);
+    datafd[0] = -1;
+
+    if (qemu_write_full(datafd[1], chpasswddata, chpasswdlen) != chpasswdlen) {
+        error_setg_errno(errp, errno, "cannot write new account password");
+        goto out;
+    }
+    close(datafd[1]);
+    datafd[1] = -1;
+
+    ga_wait_child(pid, &status, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto out;
+    }
+
+    if (!WIFEXITED(status)) {
+        error_setg(errp, "child process has terminated abnormally");
+        goto out;
+    }
+
+    if (WEXITSTATUS(status)) {
+        error_setg(errp, "child process has failed to set user password");
+        goto out;
+    }
+
+out:
+    g_free(chpasswddata);
+    g_free(rawpasswddata);
+    g_free(passwd_path);
+    if (datafd[0] != -1) {
+        close(datafd[0]);
+    }
+    if (datafd[1] != -1) {
+        close(datafd[1]);
+    }
+}
+
 #else /* defined(__linux__) */
 
 void qmp_guest_suspend_disk(Error **errp)
@@ -1910,6 +2012,14 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
     return -1;
 }
 
+void qmp_guest_set_user_password(const char *username,
+                                 const char *password,
+                                 bool crypted,
+                                 Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+}
+
 #endif
 
 #if !defined(CONFIG_FSFREEZE)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 3bcbeae..11876f6 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -446,6 +446,14 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
     return -1;
 }
 
+void qmp_guest_set_user_password(const char *username,
+                                 const char *password,
+                                 bool crypted,
+                                 Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+}
+
 /* add unsupported commands to the blacklist */
 GList *ga_command_blacklist_init(GList *blacklist)
 {
@@ -454,6 +462,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
         "guest-file-write", "guest-file-seek", "guest-file-flush",
         "guest-suspend-hybrid", "guest-network-get-interfaces",
         "guest-get-vcpus", "guest-set-vcpus",
+        "guest-set-user-password",
         "guest-fsfreeze-freeze-list", "guest-get-fsinfo",
         "guest-fstrim", NULL};
     char **p = (char **)list_unsupported;
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 376e79f..5117b65 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -738,3 +738,30 @@
 ##
 { 'command': 'guest-get-fsinfo',
   'returns': ['GuestFilesystemInfo'] }
+
+##
+# @guest-set-user-password
+#
+# @username: the user account whose password to change
+# @password: the new password entry string, base64 encoded
+# @crypted: true if password is already crypt()d, false if raw
+#
+# If the @crypted flag is true, it is the caller's responsibility
+# to ensure the correct crypt() encryption scheme is used. This
+# command does not attempt to interpret or report on the encryption
+# scheme. Refer to the documentation of the guest operating system
+# in question to determine what is supported.
+#
+# Note all guest operating systems will support use of the
+# @crypted flag, as they may require the clear-text password
+#
+# The @password parameter must always be base64 encoded before
+# transmission, even if already crypt()d, to ensure it is 8-bit
+# safe when passed as JSON.
+#
+# Returns: Nothing on success.
+#
+# Since 2.3
+##
+{ 'command': 'guest-set-user-password',
+  'data': { 'username': 'str', 'password': 'str', 'crypted': 'bool' } }
-- 
1.9.1

[Qemu-devel] [PATCH 02/10] utils: drop strtok_r from envlist_parse

[Michael Roth]

From: Olga Krishtal <okrishtal@parallels.com>

The problem is that mingw 4.9.1 fails to compile the code with the
following warning:

/mingw/include/string.h:88:9: note: previous declaration of 'strtok_r'
was here
   char *strtok_r(char * __restrict__ _Str,
                  const char * __restrict__ _Delim,
                  char ** __restrict__ __last);
/include/sysemu/os-win32.h:83:7: warning: redundant redeclaration of
   'strtok_r' [-Wredundant-decls]
   char *strtok_r(char *str, const char *delim, char **saveptr);

The problem is that compiles just fine on previous versions of mingw.
Compiler version check here is not a good idea. Though fortunately
strtok_r is used only once in the code and we could simply rewrite
the code without it.

Signed-off-by: Olga Krishtal <okrishtal@parallels.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Eric Blake <eblake@redhat.com>
CC: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 include/sysemu/os-win32.h |  1 -
 util/envlist.c            | 32 ++++++++++++++++----------------
 2 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index af3fbc4..9cc9e08 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -81,7 +81,6 @@ struct tm *gmtime_r(const time_t *timep, struct tm *result);
 #undef localtime_r
 struct tm *localtime_r(const time_t *timep, struct tm *result);
 
-char *strtok_r(char *str, const char *delim, char **saveptr);
 
 static inline void os_setup_signal_handling(void) {}
 static inline void os_daemonize(void) {}
diff --git a/util/envlist.c b/util/envlist.c
index ebc06cf..099a544 100644
--- a/util/envlist.c
+++ b/util/envlist.c
@@ -94,30 +94,30 @@ envlist_parse(envlist_t *envlist, const char *env,
 {
 	char *tmpenv, *envvar;
 	char *envsave = NULL;
-
-	assert(callback != NULL);
+    int ret = 0;
+    assert(callback != NULL);
 
 	if ((envlist == NULL) || (env == NULL))
 		return (EINVAL);
 
-	/*
-	 * We need to make temporary copy of the env string
-	 * as strtok_r(3) modifies it while it tokenizes.
-	 */
 	if ((tmpenv = strdup(env)) == NULL)
 		return (errno);
-
-	envvar = strtok_r(tmpenv, ",", &envsave);
-	while (envvar != NULL) {
-		if ((*callback)(envlist, envvar) != 0) {
-			free(tmpenv);
-			return (errno);
+    envsave = tmpenv;
+
+    do {
+        envvar = strchr(tmpenv, ',');
+        if (envvar != NULL) {
+            *envvar = '\0';
+        }
+        if ((*callback)(envlist, tmpenv) != 0) {
+            ret = errno;
+            break;
 		}
-		envvar = strtok_r(NULL, ",", &envsave);
-	}
+        tmpenv = envvar + 1;
+    } while (envvar != NULL);
 
-	free(tmpenv);
-	return (0);
+    free(envsave);
+    return ret;
 }
 
 /*
-- 
1.9.1

[Qemu-devel] [PATCH 03/10] guest agent: guest-file-open: refactoring

[Michael Roth]

From: Simon Zolin <szolin@parallels.com>

Moved the code that sets non-blocking flag on fd into a separate function.

Signed-off-by: Simon Zolin <szolin@parallels.com>
Reviewed-by: Roman Kagan <rkagan@parallels.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Michael Roth <mdroth@linux.vnet.ibm.com>
CC: Eric Blake <eblake@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 57409d0..ed527a3 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -376,13 +376,33 @@ safe_open_or_create(const char *path, const char *mode, Error **errp)
     return NULL;
 }
 
+static int guest_file_toggle_flags(int fd, int flags, bool set, Error **err)
+{
+    int ret, old_flags;
+
+    old_flags = fcntl(fd, F_GETFL);
+    if (old_flags == -1) {
+        error_set_errno(err, errno, QERR_QGA_COMMAND_FAILED,
+                        "failed to fetch filehandle flags");
+        return -1;
+    }
+
+    ret = fcntl(fd, F_SETFL, set ? (old_flags | flags) : (old_flags & ~flags));
+    if (ret == -1) {
+        error_set_errno(err, errno, QERR_QGA_COMMAND_FAILED,
+                        "failed to set filehandle flags");
+        return -1;
+    }
+
+    return ret;
+}
+
 int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode,
                             Error **errp)
 {
     FILE *fh;
     Error *local_err = NULL;
-    int fd;
-    int64_t ret = -1, handle;
+    int64_t handle;
 
     if (!has_mode) {
         mode = "r";
@@ -397,12 +417,7 @@ int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode,
     /* set fd non-blocking to avoid common use cases (like reading from a
      * named pipe) from hanging the agent
      */
-    fd = fileno(fh);
-    ret = fcntl(fd, F_GETFL);
-    ret = fcntl(fd, F_SETFL, ret | O_NONBLOCK);
-    if (ret == -1) {
-        error_setg_errno(errp, errno, "failed to make file '%s' non-blocking",
-                         path);
+    if (guest_file_toggle_flags(fileno(fh), O_NONBLOCK, true, errp) < 0) {
         fclose(fh);
         return -1;
     }
-- 
1.9.1

[Qemu-devel] [PATCH 04/10] qga: implement file commands for Windows guest

[Michael Roth]

From: Olga Krishtal <okrishtal@parallels.com>

The following commands are implemented:
- guest_file_open
- guest_file_close
- guest_file_write
- guest_file_read
- guest_file_seek
- guest_file_flush

Motivation is quite simple: Windows guests should be supported with the
same set of features as Linux one. Also this patch is a prerequisite for
Windows guest-exec command support.

Signed-off-by: Olga Krishtal <okrishtal@parallels.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-win32.c | 271 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 250 insertions(+), 21 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 11876f6..9af7950 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -14,10 +14,13 @@
 #include <glib.h>
 #include <wtypes.h>
 #include <powrprof.h>
+#include <stdio.h>
+#include <string.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"
 
 #ifndef SHTDN_REASON_FLAG_PLANNED
 #define SHTDN_REASON_FLAG_PLANNED 0x80000000
@@ -29,6 +32,146 @@
                        (365 * (1970 - 1601) +       \
                         (1970 - 1601) / 4 - 3))
 
+#define INVALID_SET_FILE_POINTER ((DWORD)-1)
+
+typedef struct GuestFileHandle {
+    int64_t id;
+    HANDLE fh;
+    QTAILQ_ENTRY(GuestFileHandle) next;
+} GuestFileHandle;
+
+static struct {
+    QTAILQ_HEAD(, GuestFileHandle) filehandles;
+} guest_file_state;
+
+
+typedef struct OpenFlags {
+    const char *forms;
+    DWORD desired_access;
+    DWORD creation_disposition;
+} OpenFlags;
+static OpenFlags guest_file_open_modes[] = {
+    {"r",   GENERIC_READ,               OPEN_EXISTING},
+    {"rb",  GENERIC_READ,               OPEN_EXISTING},
+    {"w",   GENERIC_WRITE,              CREATE_ALWAYS},
+    {"wb",  GENERIC_WRITE,              CREATE_ALWAYS},
+    {"a",   GENERIC_WRITE,              OPEN_ALWAYS  },
+    {"r+",  GENERIC_WRITE|GENERIC_READ, OPEN_EXISTING},
+    {"rb+", GENERIC_WRITE|GENERIC_READ, OPEN_EXISTING},
+    {"r+b", GENERIC_WRITE|GENERIC_READ, OPEN_EXISTING},
+    {"w+",  GENERIC_WRITE|GENERIC_READ, CREATE_ALWAYS},
+    {"wb+", GENERIC_WRITE|GENERIC_READ, CREATE_ALWAYS},
+    {"w+b", GENERIC_WRITE|GENERIC_READ, CREATE_ALWAYS},
+    {"a+",  GENERIC_WRITE|GENERIC_READ, OPEN_ALWAYS  },
+    {"ab+", GENERIC_WRITE|GENERIC_READ, OPEN_ALWAYS  },
+    {"a+b", GENERIC_WRITE|GENERIC_READ, OPEN_ALWAYS  }
+};
+
+static OpenFlags *find_open_flag(const char *mode_str)
+{
+    int mode;
+    Error **errp = NULL;
+
+    for (mode = 0; mode < ARRAY_SIZE(guest_file_open_modes); ++mode) {
+        OpenFlags *flags = guest_file_open_modes + mode;
+
+        if (strcmp(flags->forms, mode_str) == 0) {
+            return flags;
+        }
+    }
+
+    error_setg(errp, "invalid file open mode '%s'", mode_str);
+    return NULL;
+}
+
+static int64_t guest_file_handle_add(HANDLE fh, Error **errp)
+{
+    GuestFileHandle *gfh;
+    int64_t handle;
+
+    handle = ga_get_fd_handle(ga_state, errp);
+    if (handle < 0) {
+        return -1;
+    }
+    gfh = g_malloc0(sizeof(GuestFileHandle));
+    gfh->id = handle;
+    gfh->fh = fh;
+    QTAILQ_INSERT_TAIL(&guest_file_state.filehandles, gfh, next);
+
+    return handle;
+}
+
+static GuestFileHandle *guest_file_handle_find(int64_t id, Error **errp)
+{
+    GuestFileHandle *gfh;
+    QTAILQ_FOREACH(gfh, &guest_file_state.filehandles, next) {
+        if (gfh->id == id) {
+            return gfh;
+        }
+    }
+    error_setg(errp, "handle '%" PRId64 "' has not been found", id);
+    return NULL;
+}
+
+int64_t qmp_guest_file_open(const char *path, bool has_mode,
+                            const char *mode, Error **errp)
+{
+    int64_t fd;
+    HANDLE fh;
+    HANDLE templ_file = NULL;
+    DWORD share_mode = FILE_SHARE_READ;
+    DWORD flags_and_attr = FILE_ATTRIBUTE_NORMAL;
+    LPSECURITY_ATTRIBUTES sa_attr = NULL;
+    OpenFlags *guest_flags;
+
+    if (!has_mode) {
+        mode = "r";
+    }
+    slog("guest-file-open called, filepath: %s, mode: %s", path, mode);
+    guest_flags = find_open_flag(mode);
+    if (guest_flags == NULL) {
+        error_setg(errp, "invalid file open mode");
+        return -1;
+    }
+
+    fh = CreateFile(path, guest_flags->desired_access, share_mode, sa_attr,
+                    guest_flags->creation_disposition, flags_and_attr,
+                    templ_file);
+    if (fh == INVALID_HANDLE_VALUE) {
+        error_setg_win32(errp, GetLastError(), "failed to open file '%s'",
+                         path);
+        return -1;
+    }
+
+    fd = guest_file_handle_add(fh, errp);
+    if (fd < 0) {
+        CloseHandle(&fh);
+        error_setg(errp, "failed to add handle to qmp handle table");
+        return -1;
+    }
+
+    slog("guest-file-open, handle: % " PRId64, fd);
+    return fd;
+}
+
+void qmp_guest_file_close(int64_t handle, Error **errp)
+{
+    bool ret;
+    GuestFileHandle *gfh = guest_file_handle_find(handle, errp);
+    slog("guest-file-close called, handle: %" PRId64, handle);
+    if (gfh == NULL) {
+        return;
+    }
+    ret = CloseHandle(gfh->fh);
+    if (!ret) {
+        error_setg_win32(errp, GetLastError(), "failed close handle");
+        return;
+    }
+
+    QTAILQ_REMOVE(&guest_file_state.filehandles, gfh, next);
+    g_free(gfh);
+}
+
 static void acquire_privilege(const char *name, Error **errp)
 {
     HANDLE token = NULL;
@@ -113,43 +256,130 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
     }
 }
 
-int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode,
-                            Error **errp)
-{
-    error_set(errp, QERR_UNSUPPORTED);
-    return 0;
-}
-
-void qmp_guest_file_close(int64_t handle, Error **errp)
-{
-    error_set(errp, QERR_UNSUPPORTED);
-}
-
 GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count,
                                    int64_t count, Error **errp)
 {
-    error_set(errp, QERR_UNSUPPORTED);
-    return 0;
+    GuestFileRead *read_data = NULL;
+    guchar *buf;
+    HANDLE fh;
+    bool is_ok;
+    DWORD read_count;
+    GuestFileHandle *gfh = guest_file_handle_find(handle, errp);
+
+    if (!gfh) {
+        return NULL;
+    }
+    if (!has_count) {
+        count = QGA_READ_COUNT_DEFAULT;
+    } else if (count < 0) {
+        error_setg(errp, "value '%" PRId64
+                   "' is invalid for argument count", count);
+        return NULL;
+    }
+
+    fh = gfh->fh;
+    buf = g_malloc0(count+1);
+    is_ok = ReadFile(fh, buf, count, &read_count, NULL);
+    if (!is_ok) {
+        error_setg_win32(errp, GetLastError(), "failed to read file");
+        slog("guest-file-read failed, handle %" PRId64, handle);
+    } else {
+        buf[read_count] = 0;
+        read_data = g_malloc0(sizeof(GuestFileRead));
+        read_data->count = (size_t)read_count;
+        read_data->eof = read_count == 0;
+
+        if (read_count != 0) {
+            read_data->buf_b64 = g_base64_encode(buf, read_count);
+        }
+    }
+    g_free(buf);
+
+    return read_data;
 }
 
 GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64,
                                      bool has_count, int64_t count,
                                      Error **errp)
 {
-    error_set(errp, QERR_UNSUPPORTED);
-    return 0;
+    GuestFileWrite *write_data = NULL;
+    guchar *buf;
+    gsize buf_len;
+    bool is_ok;
+    DWORD write_count;
+    GuestFileHandle *gfh = guest_file_handle_find(handle, errp);
+    HANDLE fh;
+
+    if (!gfh) {
+        return NULL;
+    }
+    fh = gfh->fh;
+    buf = g_base64_decode(buf_b64, &buf_len);
+
+    if (!has_count) {
+        count = buf_len;
+    } else if (count < 0 || count > buf_len) {
+        error_setg(errp, "value '%" PRId64
+                   "' is invalid for argument count", count);
+        goto done;
+    }
+
+    is_ok = WriteFile(fh, buf, count, &write_count, NULL);
+    if (!is_ok) {
+        error_setg_win32(errp, GetLastError(), "failed to write to file");
+        slog("guest-file-write-failed, handle: %" PRId64, handle);
+    } else {
+        write_data = g_malloc0(sizeof(GuestFileWrite));
+        write_data->count = (size_t) write_count;
+    }
+
+done:
+    g_free(buf);
+    return write_data;
 }
 
 GuestFileSeek *qmp_guest_file_seek(int64_t handle, int64_t offset,
                                    int64_t whence, Error **errp)
 {
-    error_set(errp, QERR_UNSUPPORTED);
-    return 0;
+    GuestFileHandle *gfh;
+    GuestFileSeek *seek_data;
+    HANDLE fh;
+    LARGE_INTEGER new_pos, off_pos;
+    off_pos.QuadPart = offset;
+    BOOL res;
+    gfh = guest_file_handle_find(handle, errp);
+    if (!gfh) {
+        return NULL;
+    }
+
+    fh = gfh->fh;
+    res = SetFilePointerEx(fh, off_pos, &new_pos, whence);
+    if (!res) {
+        error_setg_win32(errp, GetLastError(), "failed to seek file");
+        return NULL;
+    }
+    seek_data = g_new0(GuestFileSeek, 1);
+    seek_data->position = new_pos.QuadPart;
+    return seek_data;
 }
 
 void qmp_guest_file_flush(int64_t handle, Error **errp)
 {
-    error_set(errp, QERR_UNSUPPORTED);
+    HANDLE fh;
+    GuestFileHandle *gfh = guest_file_handle_find(handle, errp);
+    if (!gfh) {
+        return;
+    }
+
+    fh = gfh->fh;
+    if (!FlushFileBuffers(fh)) {
+        error_setg_win32(errp, GetLastError(), "failed to flush file");
+    }
+}
+
+static void guest_file_init(void)
+{
+    QTAILQ_INIT(&guest_file_state.filehandles);
 }
 
 GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error **errp)
@@ -458,8 +688,6 @@ void qmp_guest_set_user_password(const char *username,
 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-set-user-password",
@@ -491,4 +719,5 @@ void ga_command_state_init(GAState *s, GACommandState *cs)
     if (!vss_initialized()) {
         ga_command_state_add(cs, NULL, guest_fsfreeze_cleanup);
     }
+    ga_command_state_add(cs, guest_file_init, NULL);
 }
-- 
1.9.1

[Qemu-devel] [PATCH 05/10] qga: introduce three guest memory block commmands with stubs

[Michael Roth]

From: zhanghailiang <zhang.zhanghailiang@huawei.com>

Introduce three new guest commands:
guest-get-memory-blocks, guest-set-memory-blocks, guest-get-memory-block-size.

With these three commands, we can support online/offline guest's memory block
(logical memory hotplug/unplug) as required from host.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c |  38 +++++++++++++++++
 qga/commands-win32.c |  19 +++++++++
 qga/qapi-schema.json | 113 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 170 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index ed527a3..0ce27a4 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1992,6 +1992,25 @@ out:
     }
 }
 
+GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
+GuestMemoryBlockResponseList *
+qmp_guest_set_memory_blocks(GuestMemoryBlockList *mem_blks, Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
+int64_t qmp_guest_get_memory_block_size(Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return -1;
+}
+
 #else /* defined(__linux__) */
 
 void qmp_guest_suspend_disk(Error **errp)
@@ -2035,6 +2054,25 @@ void qmp_guest_set_user_password(const char *username,
     error_set(errp, QERR_UNSUPPORTED);
 }
 
+GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
+GuestMemoryBlockResponseList *
+qmp_guest_set_memory_blocks(GuestMemoryBlockList *mem_blks, Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
+int64_t qmp_guest_get_memory_block_size(Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return -1;
+}
+
 #endif
 
 #if !defined(CONFIG_FSFREEZE)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 9af7950..30e377b 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -684,6 +684,25 @@ void qmp_guest_set_user_password(const char *username,
     error_set(errp, QERR_UNSUPPORTED);
 }
 
+GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
+GuestMemoryBlockResponseList *
+qmp_guest_set_memory_blocks(GuestMemoryBlockList *mem_blks, Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
+int64_t qmp_guest_get_memory_block_size(Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return -1;
+}
+
 /* add unsupported commands to the blacklist */
 GList *ga_command_blacklist_init(GList *blacklist)
 {
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 5117b65..b69cfe1 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -765,3 +765,116 @@
 ##
 { 'command': 'guest-set-user-password',
   'data': { 'username': 'str', 'password': 'str', 'crypted': 'bool' } }
+
+# @GuestMemoryBlock:
+#
+# @phys-index: Arbitrary guest-specific unique identifier of the MEMORY BLOCK.
+#
+# @online: Whether the MEMORY BLOCK is enabled in guest.
+#
+# @can-offline: #optional Whether offlining the MEMORY BLOCK is possible.
+#               This member is always filled in by the guest agent when the
+#               structure is returned, and always ignored on input (hence it
+#               can be omitted then).
+#
+# Since: 2.3
+##
+{ 'type': 'GuestMemoryBlock',
+  'data': {'phys-index': 'uint64',
+           'online': 'bool',
+           '*can-offline': 'bool'} }
+
+##
+# @guest-get-memory-blocks:
+#
+# Retrieve the list of the guest's memory blocks.
+#
+# This is a read-only operation.
+#
+# Returns: The list of all memory blocks the guest knows about.
+# Each memory block is put on the list exactly once, but their order
+# is unspecified.
+#
+# Since: 2.3
+##
+{ 'command': 'guest-get-memory-blocks',
+  'returns': ['GuestMemoryBlock'] }
+
+##
+# @GuestMemoryBlockResponseType
+#
+# An enumeration of memory block operation result.
+#
+# @sucess: the operation of online/offline memory block is successful.
+# @not-found: can't find the corresponding memoryXXX directory in sysfs.
+# @operation-not-supported: for some old kernels, it does not support
+#                           online or offline memory block.
+# @operation-failed: the operation of online/offline memory block fails,
+#                    because of some errors happen.
+#
+# Since: 2.3
+##
+{ 'enum': 'GuestMemoryBlockResponseType',
+  'data': ['success', 'not-found', 'operation-not-supported',
+           'operation-failed'] }
+
+##
+# @GuestMemoryBlockResponse:
+#
+# @phys-index: same with the 'phys-index' member of @GuestMemoryBlock.
+#
+# @response: the result of memory block operation.
+#
+# @error-code: #optional the error number.
+#               When memory block operation fails, we assign the value of
+#               'errno' to this member, it indicates what goes wrong.
+#               When the operation succeeds, it will be omitted.
+#
+# Since: 2.3
+##
+{ 'type': 'GuestMemoryBlockResponse',
+  'data': { 'phys-index': 'uint64',
+            'response': 'GuestMemoryBlockResponseType',
+            '*error-code': 'int' }}
+
+##
+# @guest-set-memory-blocks:
+#
+# Attempt to reconfigure (currently: enable/disable) state of memory blocks
+# inside the guest.
+#
+# The input list is processed node by node in order. In each node @phys-index
+# is used to look up the guest MEMORY BLOCK, for which @online specifies the
+# requested state. The set of distinct @phys-index's is only required to be a
+# subset of the guest-supported identifiers. There's no restriction on list
+# length or on repeating the same @phys-index (with possibly different @online
+# field).
+# Preferably the input list should describe a modified subset of
+# @guest-get-memory-blocks' return value.
+#
+# Returns: The operation results, it is a list of @GuestMemoryBlockResponse,
+#          which is corresponding to the input list.
+#
+#          Note: it will return NULL if the @mem-blks list was empty on input,
+#          or there is an error, and in this case, guest state will not be
+#          changed.
+#
+# Since: 2.3
+##
+{ 'command': 'guest-set-memory-blocks',
+  'data':    {'mem-blks': ['GuestMemoryBlock'] },
+  'returns': ['GuestMemoryBlockResponse'] }
+
+##
+# @guest-get-memory-block-size:
+#
+# Get the the size (in bytes) of a memory block in guest.
+# It is the unit of memory block online/offline operation (also called Logical
+# Memory Hotplug).
+#
+# Returns: memory block size in bytes.
+#
+# Since 2.3
+##
+{ 'command': 'guest-get-memory-block-size',
+  'returns': 'int' }
-- 
1.9.1

[Qemu-devel] [PATCH 06/10] qga: implement qmp_guest_get_memory_blocks() for Linux with sysfs

[Michael Roth]

From: zhanghailiang <zhang.zhanghailiang@huawei.com>

We can get guest's memory block information by using command
"guest-get-memory-blocks", the returned value contains a list of memory block
info, such as phys-index, online state, can-offline info.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>

*replaced guest-triggerable assertion with an error msg

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c | 234 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 233 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 0ce27a4..6cd21b2 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1992,9 +1992,241 @@ out:
     }
 }
 
+static void ga_read_sysfs_file(int dirfd, const char *pathname, char *buf,
+                               int size, Error **errp)
+{
+    int fd;
+    int res;
+
+    errno = 0;
+    fd = openat(dirfd, pathname, O_RDONLY);
+    if (fd == -1) {
+        error_setg_errno(errp, errno, "open sysfs file \"%s\"", pathname);
+        return;
+    }
+
+    res = pread(fd, buf, size, 0);
+    if (res == -1) {
+        error_setg_errno(errp, errno, "pread sysfs file \"%s\"", pathname);
+    } else if (res == 0) {
+        error_setg(errp, "pread sysfs file \"%s\": unexpected EOF", pathname);
+    }
+    close(fd);
+}
+
+static void ga_write_sysfs_file(int dirfd, const char *pathname,
+                                const char *buf, int size, Error **errp)
+{
+    int fd;
+
+    errno = 0;
+    fd = openat(dirfd, pathname, O_WRONLY);
+    if (fd == -1) {
+        error_setg_errno(errp, errno, "open sysfs file \"%s\"", pathname);
+        return;
+    }
+
+    if (pwrite(fd, buf, size, 0) == -1) {
+        error_setg_errno(errp, errno, "pwrite sysfs file \"%s\"", pathname);
+    }
+
+    close(fd);
+}
+
+/* Transfer online/offline status between @mem_blk and the guest system.
+ *
+ * On input either @errp or *@errp must be NULL.
+ *
+ * In system-to-@mem_blk direction, the following @mem_blk fields are accessed:
+ * - R: mem_blk->phys_index
+ * - W: mem_blk->online
+ * - W: mem_blk->can_offline
+ *
+ * In @mem_blk-to-system direction, the following @mem_blk fields are accessed:
+ * - R: mem_blk->phys_index
+ * - R: mem_blk->online
+ *-  R: mem_blk->can_offline
+ * Written members remain unmodified on error.
+ */
+static void transfer_memory_block(GuestMemoryBlock *mem_blk, bool sys2memblk,
+                                  GuestMemoryBlockResponse *result,
+                                  Error **errp)
+{
+    char *dirpath;
+    int dirfd;
+    char *status;
+    Error *local_err = NULL;
+
+    if (!sys2memblk) {
+        DIR *dp;
+
+        if (!result) {
+            error_setg(errp, "Internal error, 'result' should not be NULL");
+            return;
+        }
+        errno = 0;
+        dp = opendir("/sys/devices/system/memory/");
+         /* if there is no 'memory' directory in sysfs,
+         * we think this VM does not support online/offline memory block,
+         * any other solution?
+         */
+        if (!dp && errno == ENOENT) {
+            result->response =
+                GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED;
+            goto out1;
+        }
+        closedir(dp);
+    }
+
+    dirpath = g_strdup_printf("/sys/devices/system/memory/memory%" PRId64 "/",
+                              mem_blk->phys_index);
+    dirfd = open(dirpath, O_RDONLY | O_DIRECTORY);
+    if (dirfd == -1) {
+        if (sys2memblk) {
+            error_setg_errno(errp, errno, "open(\"%s\")", dirpath);
+        } else {
+            if (errno == ENOENT) {
+                result->response = GUEST_MEMORY_BLOCK_RESPONSE_TYPE_NOT_FOUND;
+            } else {
+                result->response =
+                    GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_FAILED;
+            }
+        }
+        g_free(dirpath);
+        goto out1;
+    }
+    g_free(dirpath);
+
+    status = g_malloc0(10);
+    ga_read_sysfs_file(dirfd, "state", status, 10, &local_err);
+    if (local_err) {
+        /* treat with sysfs file that not exist in old kernel */
+        if (errno == ENOENT) {
+            error_free(local_err);
+            if (sys2memblk) {
+                mem_blk->online = true;
+                mem_blk->can_offline = false;
+            } else if (!mem_blk->online) {
+                result->response =
+                    GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED;
+            }
+        } else {
+            if (sys2memblk) {
+                error_propagate(errp, local_err);
+            } else {
+                result->response =
+                    GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_FAILED;
+            }
+        }
+        goto out2;
+    }
+
+    if (sys2memblk) {
+        char removable = '0';
+
+        mem_blk->online = (strncmp(status, "online", 6) == 0);
+
+        ga_read_sysfs_file(dirfd, "removable", &removable, 1, &local_err);
+        if (local_err) {
+            /* if no 'removable' file, it does't support offline mem blk */
+            if (errno == ENOENT) {
+                error_free(local_err);
+                mem_blk->can_offline = false;
+            } else {
+                error_propagate(errp, local_err);
+            }
+        } else {
+            mem_blk->can_offline = (removable != '0');
+        }
+    } else {
+        if (mem_blk->online != (strncmp(status, "online", 6) == 0)) {
+            char *new_state = mem_blk->online ? g_strdup("online") :
+                                                g_strdup("offline");
+
+            ga_write_sysfs_file(dirfd, "state", new_state, strlen(new_state),
+                                &local_err);
+            g_free(new_state);
+            if (local_err) {
+                error_free(local_err);
+                result->response =
+                    GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_FAILED;
+                goto out2;
+            }
+
+            result->response = GUEST_MEMORY_BLOCK_RESPONSE_TYPE_SUCCESS;
+            result->has_error_code = false;
+        } /* otherwise pretend successful re-(on|off)-lining */
+    }
+    g_free(status);
+    close(dirfd);
+    return;
+
+out2:
+    g_free(status);
+    close(dirfd);
+out1:
+    if (!sys2memblk) {
+        result->has_error_code = true;
+        result->error_code = errno;
+    }
+}
+
 GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
 {
-    error_set(errp, QERR_UNSUPPORTED);
+    GuestMemoryBlockList *head, **link;
+    Error *local_err = NULL;
+    struct dirent *de;
+    DIR *dp;
+
+    head = NULL;
+    link = &head;
+
+    dp = opendir("/sys/devices/system/memory/");
+    if (!dp) {
+        error_setg_errno(errp, errno, "Can't open directory"
+                         "\"/sys/devices/system/memory/\"\n");
+        return NULL;
+    }
+
+    /* Note: the phys_index of memory block may be discontinuous,
+     * this is because a memblk is the unit of the Sparse Memory design, which
+     * allows discontinuous memory ranges (ex. NUMA), so here we should
+     * traverse the memory block directory.
+     */
+    while ((de = readdir(dp)) != NULL) {
+        GuestMemoryBlock *mem_blk;
+        GuestMemoryBlockList *entry;
+
+        if ((strncmp(de->d_name, "memory", 6) != 0) ||
+            !(de->d_type & DT_DIR)) {
+            continue;
+        }
+
+        mem_blk = g_malloc0(sizeof *mem_blk);
+        /* The d_name is "memoryXXX",  phys_index is block id, same as XXX */
+        mem_blk->phys_index = strtoul(&de->d_name[6], NULL, 10);
+        mem_blk->has_can_offline = true; /* lolspeak ftw */
+        transfer_memory_block(mem_blk, true, NULL, &local_err);
+
+        entry = g_malloc0(sizeof *entry);
+        entry->value = mem_blk;
+
+        *link = entry;
+        link = &entry->next;
+    }
+
+    closedir(dp);
+    if (local_err == NULL) {
+        /* there's no guest with zero memory blocks */
+        if (head == NULL) {
+            error_setg(errp, "guest reported zero memory blocks!");
+            return;
+        }
+        return head;
+    }
+
+    qapi_free_GuestMemoryBlockList(head);
+    error_propagate(errp, local_err);
     return NULL;
 }
 
-- 
1.9.1

[Qemu-devel] [PATCH 07/10] qga: implement qmp_guest_set_memory_blocks() for Linux with sysfs

[Michael Roth]

From: zhanghailiang <zhang.zhanghailiang@huawei.com>

We can change guest's online/offline state of memory blocks, by using
command 'guest-set-memory-blocks'.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 6cd21b2..1c8080f 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2233,7 +2233,35 @@ GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
 GuestMemoryBlockResponseList *
 qmp_guest_set_memory_blocks(GuestMemoryBlockList *mem_blks, Error **errp)
 {
-    error_set(errp, QERR_UNSUPPORTED);
+    GuestMemoryBlockResponseList *head, **link;
+    Error *local_err = NULL;
+
+    head = NULL;
+    link = &head;
+
+    while (mem_blks != NULL) {
+        GuestMemoryBlockResponse *result;
+        GuestMemoryBlockResponseList *entry;
+        GuestMemoryBlock *current_mem_blk = mem_blks->value;
+
+        result = g_malloc0(sizeof(*result));
+        result->phys_index = current_mem_blk->phys_index;
+        transfer_memory_block(current_mem_blk, false, result, &local_err);
+        if (local_err) { /* should never happen */
+            goto err;
+        }
+        entry = g_malloc0(sizeof *entry);
+        entry->value = result;
+
+        *link = entry;
+        link = &entry->next;
+        mem_blks = mem_blks->next;
+    }
+
+    return head;
+err:
+    qapi_free_GuestMemoryBlockResponseList(head);
+    error_propagate(errp, local_err);
     return NULL;
 }
 
-- 
1.9.1

[Qemu-devel] [PATCH 08/10] qga: implement qmp_guest_get_memory_block_size() for Linux with sysfs

[Michael Roth]

From: zhanghailiang <zhang.zhanghailiang@huawei.com>

The size of a memory block is architecture dependent, it represents the logical
unit upon which memory online/offline operations are to be performed.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 1c8080f..d8d7425 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2267,8 +2267,33 @@ err:
 
 int64_t qmp_guest_get_memory_block_size(Error **errp)
 {
-    error_set(errp, QERR_UNSUPPORTED);
-    return -1;
+    Error *local_err = NULL;
+    char *dirpath;
+    int dirfd;
+    char *buf;
+    int64_t block_size;
+
+    dirpath = g_strdup_printf("/sys/devices/system/memory/");
+    dirfd = open(dirpath, O_RDONLY | O_DIRECTORY);
+    if (dirfd == -1) {
+        error_setg_errno(errp, errno, "open(\"%s\")", dirpath);
+        g_free(dirpath);
+        return -1;
+    }
+    g_free(dirpath);
+
+    buf = g_malloc0(20);
+    ga_read_sysfs_file(dirfd, "block_size_bytes", buf, 20, &local_err);
+    if (local_err) {
+        g_free(buf);
+        error_propagate(errp, local_err);
+        return -1;
+    }
+
+    block_size = strtol(buf, NULL, 16); /* the unit is bytes */
+    g_free(buf);
+
+    return block_size;
 }
 
 #else /* defined(__linux__) */
-- 
1.9.1

[Qemu-devel] [PATCH 09/10] qga: add memory block command that unsupported

[Michael Roth]

From: zhanghailiang <zhang.zhanghailiang@huawei.com>

For memory block command, we only support for linux with sysfs.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c | 4 +++-
 qga/commands-win32.c | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index d8d7425..73e375a 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2414,7 +2414,9 @@ GList *ga_command_blacklist_init(GList *blacklist)
         const char *list[] = {
             "guest-suspend-disk", "guest-suspend-ram",
             "guest-suspend-hybrid", "guest-network-get-interfaces",
-            "guest-get-vcpus", "guest-set-vcpus", NULL};
+            "guest-get-vcpus", "guest-set-vcpus",
+            "guest-get-memory-blocks", "guest-set-memory-blocks",
+            "guest-get-memory-block-size", NULL};
         char **p = (char **)list;
 
         while (*p) {
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 30e377b..3344ec1 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -710,6 +710,8 @@ GList *ga_command_blacklist_init(GList *blacklist)
         "guest-suspend-hybrid", "guest-network-get-interfaces",
         "guest-get-vcpus", "guest-set-vcpus",
         "guest-set-user-password",
+        "guest-get-memory-blocks", "guest-set-memory-blocks",
+        "guest-get-memory-block-size",
         "guest-fsfreeze-freeze-list", "guest-get-fsinfo",
         "guest-fstrim", NULL};
     char **p = (char **)list_unsupported;
-- 
1.9.1

[Qemu-devel] [PATCH 10/10] qemu-ga-win: Fail loudly on bare 'set-time'

[Michael Roth]

From: Michal Privoznik <mprivozn@redhat.com>

The command is not implemented correctly yet. The documentation allows
to not pass any value to set, in which case the time is re-read from
RTC. However, reading CMOS on Windows is not trivial to implement. So
instead of pretending we've set the correct time, fail explicitly.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-win32.c | 44 ++++++++++++++++++++++----------------------
 qga/qapi-schema.json |  5 ++++-
 2 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 3344ec1..5219c75 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -625,31 +625,31 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp)
     FILETIME tf;
     LONGLONG time;
 
-    if (has_time) {
-        /* Okay, user passed a time to set. Validate it. */
-        if (time_ns < 0 || time_ns / 100 > INT64_MAX - W32_FT_OFFSET) {
-            error_setg(errp, "Time %" PRId64 "is invalid", time_ns);
-            return;
-        }
+    if (!has_time) {
+        /* Unfortunately, Windows libraries don't provide an easy way to access
+         * RTC yet:
+         *
+         * https://msdn.microsoft.com/en-us/library/aa908981.aspx
+         */
+        error_setg(errp, "Time argument is required on this platform");
+        return;
+    }
+
+    /* Validate time passed by user. */
+    if (time_ns < 0 || time_ns / 100 > INT64_MAX - W32_FT_OFFSET) {
+        error_setg(errp, "Time %" PRId64 "is invalid", time_ns);
+        return;
+    }
 
-        time = time_ns / 100 + W32_FT_OFFSET;
+    time = time_ns / 100 + W32_FT_OFFSET;
 
-        tf.dwLowDateTime = (DWORD) time;
-        tf.dwHighDateTime = (DWORD) (time >> 32);
+    tf.dwLowDateTime = (DWORD) time;
+    tf.dwHighDateTime = (DWORD) (time >> 32);
 
-        if (!FileTimeToSystemTime(&tf, &ts)) {
-            error_setg(errp, "Failed to convert system time %d",
-                       (int)GetLastError());
-            return;
-        }
-    } else {
-        /* Otherwise read the time from RTC which contains the correct value.
-         * Hopefully. */
-        GetSystemTime(&ts);
-        if (ts.wYear < 1601 || ts.wYear > 30827) {
-            error_setg(errp, "Failed to get time");
-            return;
-        }
+    if (!FileTimeToSystemTime(&tf, &ts)) {
+        error_setg(errp, "Failed to convert system time %d",
+                   (int)GetLastError());
+        return;
     }
 
     acquire_privilege(SE_SYSTEMTIME_NAME, &local_err);
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index b69cfe1..4bfc70d 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -121,7 +121,10 @@
 # given value, then sets the Hardware Clock (RTC) to the
 # current System Time. This will make it easier for a guest
 # to resynchronize without waiting for NTP. If no @time is
-# specified, then the time to set is read from RTC.
+# specified, then the time to set is read from RTC. However,
+# this may not be supported on all platforms (i.e. Windows).
+# If that's the case users are advised to always pass a
+# value.
 #
 # @time: #optional time of nanoseconds, relative to the Epoch
 #        of 1970-01-01 in UTC.
-- 
1.9.1

Re: [Qemu-devel] [PATCH 05/10] qga: introduce three guest memory block commmands with stubs

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1083 bytes --]

On 02/16/2015 08:14 PM, Michael Roth wrote:
> From: zhanghailiang <zhang.zhanghailiang@huawei.com>
> 
> Introduce three new guest commands:
> guest-get-memory-blocks, guest-set-memory-blocks, guest-get-memory-block-size.

Sorry for the late review, but I think guest-get-memory-block-size is
the wrong command to add.


> +##
> +# @guest-get-memory-block-size:
> +#
> +# Get the the size (in bytes) of a memory block in guest.
> +# It is the unit of memory block online/offline operation (also called Logical
> +# Memory Hotplug).
> +#
> +# Returns: memory block size in bytes.
> +#
> +# Since 2.3
> +##
> +{ 'command': 'guest-get-memory-block-size',
> +  'returns': 'int' }

Any QAPI command that returns a bare int instead of a dictionary is
non-extensible, and therefore of suspect design.  I think it would be
better to have:

{ 'command': 'guest-get-memory-block-info',
  'returns': { 'size': 'int' } }

to allow for future extension.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 03/10] guest agent: guest-file-open: refactoring

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]

On 02/16/2015 08:14 PM, Michael Roth wrote:
> From: Simon Zolin <szolin@parallels.com>
> 
> Moved the code that sets non-blocking flag on fd into a separate function.
> 
> Signed-off-by: Simon Zolin <szolin@parallels.com>
> Reviewed-by: Roman Kagan <rkagan@parallels.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Michael Roth <mdroth@linux.vnet.ibm.com>
> CC: Eric Blake <eblake@redhat.com>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  qga/commands-posix.c | 31 +++++++++++++++++++++++--------
>  1 file changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 57409d0..ed527a3 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -376,13 +376,33 @@ safe_open_or_create(const char *path, const char *mode, Error **errp)
>      return NULL;
>  }
>  
> +static int guest_file_toggle_flags(int fd, int flags, bool set, Error **err)
> +{

Why are you reinventing qemu_set_nonblock()?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 03/10] guest agent: guest-file-open: refactoring

[Denis V. Lunev]

On 17/02/15 18:27, Eric Blake wrote:
> On 02/16/2015 08:14 PM, Michael Roth wrote:
>> From: Simon Zolin <szolin@parallels.com>
>>
>> Moved the code that sets non-blocking flag on fd into a separate function.
>>
>> Signed-off-by: Simon Zolin <szolin@parallels.com>
>> Reviewed-by: Roman Kagan <rkagan@parallels.com>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Michael Roth <mdroth@linux.vnet.ibm.com>
>> CC: Eric Blake <eblake@redhat.com>
>> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
>> ---
>>   qga/commands-posix.c | 31 +++++++++++++++++++++++--------
>>   1 file changed, 23 insertions(+), 8 deletions(-)
>>
>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
>> index 57409d0..ed527a3 100644
>> --- a/qga/commands-posix.c
>> +++ b/qga/commands-posix.c
>> @@ -376,13 +376,33 @@ safe_open_or_create(const char *path, const char *mode, Error **errp)
>>       return NULL;
>>   }
>>   
>> +static int guest_file_toggle_flags(int fd, int flags, bool set, Error **err)
>> +{
> Why are you reinventing qemu_set_nonblock()?
>
because we are uneducated :)

Anyway, qemu_set_nonblock() does not handle error
and resides in a strange header aka  "include/qemu/sockets.h"
Technically I can switch to it immediately. Though error
check condition will be lost.

What is better at your opinion?

a) return error from qemu_set_nonblock()/qemu_set_block()
b) drop error check here. The descriptor is just opened
     and we know that it is valid. I could not imagine real
     error other than broken descriptor for this exact fcntl.

Regards,
     Den

Re: [Qemu-devel] [PATCH 03/10] guest agent: guest-file-open: refactoring

[Michael Roth]

Quoting Denis V. Lunev (2015-02-17 10:06:49)
> On 17/02/15 18:27, Eric Blake wrote:
> > On 02/16/2015 08:14 PM, Michael Roth wrote:
> >> From: Simon Zolin <szolin@parallels.com>
> >>
> >> Moved the code that sets non-blocking flag on fd into a separate function.
> >>
> >> Signed-off-by: Simon Zolin <szolin@parallels.com>
> >> Reviewed-by: Roman Kagan <rkagan@parallels.com>
> >> Signed-off-by: Denis V. Lunev <den@openvz.org>
> >> CC: Michael Roth <mdroth@linux.vnet.ibm.com>
> >> CC: Eric Blake <eblake@redhat.com>
> >> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> >> ---
> >>   qga/commands-posix.c | 31 +++++++++++++++++++++++--------
> >>   1 file changed, 23 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> >> index 57409d0..ed527a3 100644
> >> --- a/qga/commands-posix.c
> >> +++ b/qga/commands-posix.c
> >> @@ -376,13 +376,33 @@ safe_open_or_create(const char *path, const char *mode, Error **errp)
> >>       return NULL;
> >>   }
> >>   
> >> +static int guest_file_toggle_flags(int fd, int flags, bool set, Error **err)
> >> +{
> > Why are you reinventing qemu_set_nonblock()?
> >
> because we are uneducated :)
> 
> Anyway, qemu_set_nonblock() does not handle error
> and resides in a strange header aka  "include/qemu/sockets.h"
> Technically I can switch to it immediately. Though error
> check condition will be lost.
> 
> What is better at your opinion?
> 
> a) return error from qemu_set_nonblock()/qemu_set_block()

I think making the existing functions a non-error-checking
wrapper around qemu_set_{block,nonblock}_error or something
would be best.

These are meant to be os-agnostic utility functions though,
but in the case of qemu-ga the win32 variant won't work as
expected, so I'm not sure it's a good idea to rely on them.

If the lack of it's usage in net/tap* compared to other parts
of QEMU that build on w32 is any indication, that seems to
be the pattern followed by other users.

In any case, since I was actually the one who re-invented it,
and this code just moves it to another function, I think we
can address it as a seperate patch and leave the PULL
intact (unless there are other objections).

> b) drop error check here. The descriptor is just opened
>      and we know that it is valid. I could not imagine real
>      error other than broken descriptor for this exact fcntl.
> 
> Regards,
>      Den

Re: [Qemu-devel] [PATCH 03/10] guest agent: guest-file-open: refactoring

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 442 bytes --]

On 02/17/2015 10:56 AM, Michael Roth wrote:
> In any case, since I was actually the one who re-invented it,
> and this code just moves it to another function, I think we
> can address it as a seperate patch and leave the PULL
> intact (unless there are other objections).

Agree that a separate cleanup patch would be okay.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 03/10] guest agent: guest-file-open: refactoring

[Denis V. Lunev]

On 17/02/15 20:56, Michael Roth wrote:
> Quoting Denis V. Lunev (2015-02-17 10:06:49)
>> On 17/02/15 18:27, Eric Blake wrote:
>>> On 02/16/2015 08:14 PM, Michael Roth wrote:
>>>> From: Simon Zolin <szolin@parallels.com>
>>>>
>>>> Moved the code that sets non-blocking flag on fd into a separate function.
>>>>
>>>> Signed-off-by: Simon Zolin <szolin@parallels.com>
>>>> Reviewed-by: Roman Kagan <rkagan@parallels.com>
>>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>>> CC: Michael Roth <mdroth@linux.vnet.ibm.com>
>>>> CC: Eric Blake <eblake@redhat.com>
>>>> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
>>>> ---
>>>>    qga/commands-posix.c | 31 +++++++++++++++++++++++--------
>>>>    1 file changed, 23 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
>>>> index 57409d0..ed527a3 100644
>>>> --- a/qga/commands-posix.c
>>>> +++ b/qga/commands-posix.c
>>>> @@ -376,13 +376,33 @@ safe_open_or_create(const char *path, const char *mode, Error **errp)
>>>>        return NULL;
>>>>    }
>>>>
>>>> +static int guest_file_toggle_flags(int fd, int flags, bool set, Error **err)
>>>> +{
>>> Why are you reinventing qemu_set_nonblock()?
>>>
>> because we are uneducated :)
>>
>> Anyway, qemu_set_nonblock() does not handle error
>> and resides in a strange header aka  "include/qemu/sockets.h"
>> Technically I can switch to it immediately. Though error
>> check condition will be lost.
>>
>> What is better at your opinion?
>>
>> a) return error from qemu_set_nonblock()/qemu_set_block()
>
> I think making the existing functions a non-error-checking
> wrapper around qemu_set_{block,nonblock}_error or something
> would be best.
>
> These are meant to be os-agnostic utility functions though,
> but in the case of qemu-ga the win32 variant won't work as
> expected, so I'm not sure it's a good idea to rely on them.
>
> If the lack of it's usage in net/tap* compared to other parts
> of QEMU that build on w32 is any indication, that seems to
> be the pattern followed by other users.
>
> In any case, since I was actually the one who re-invented it,
> and this code just moves it to another function, I think we
> can address it as a seperate patch and leave the PULL
> intact (unless there are other objections).
>

OK to me.

I'll check Win32 version with Olga to be somewhat
sure with upcoming cleanup.

Re: [Qemu-devel] [PATCH 05/10] qga: introduce three guest memory block commmands with stubs

[Michael Roth]

Quoting Eric Blake (2015-02-17 09:26:12)
> On 02/16/2015 08:14 PM, Michael Roth wrote:
> > From: zhanghailiang <zhang.zhanghailiang@huawei.com>
> > 
> > Introduce three new guest commands:
> > guest-get-memory-blocks, guest-set-memory-blocks, guest-get-memory-block-size.
> 
> Sorry for the late review, but I think guest-get-memory-block-size is
> the wrong command to add.
> 
> 
> > +##
> > +# @guest-get-memory-block-size:
> > +#
> > +# Get the the size (in bytes) of a memory block in guest.
> > +# It is the unit of memory block online/offline operation (also called Logical
> > +# Memory Hotplug).
> > +#
> > +# Returns: memory block size in bytes.
> > +#
> > +# Since 2.3
> > +##
> > +{ 'command': 'guest-get-memory-block-size',
> > +  'returns': 'int' }
> 
> Any QAPI command that returns a bare int instead of a dictionary is
> non-extensible, and therefore of suspect design.  I think it would be
> better to have:
> 
> { 'command': 'guest-get-memory-block-info',
>   'returns': { 'size': 'int' } }
> 
> to allow for future extension.

It seems like a reasonable suggestion to me. I can change it in my
tree if there are no objections. zhanghailiang?

> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org

Re: [Qemu-devel] [PULL 00/10] Fixes and new commands for QEMU Guest Agent

[Michael Roth]

Quoting Michael Roth (2015-02-16 21:14:42)
> The following changes since commit cd2d5541271f1934345d8ca42f5fafff1744eee7:

Please disregard, will send a v2 shortly to address review new comments.

> 
>   Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20150212' into staging (2015-02-13 11:44:50 +0000)
> 
> are available in the git repository at:
> 
>   git://github.com/mdroth/qemu.git tags/qga-pull-2015-02-16-tag
> 
> for you to fetch changes up to bf0abffb8d94ba15344c6e7e4326c8da0b9784b1:
> 
>   qemu-ga-win: Fail loudly on bare 'set-time' (2015-02-16 20:17:39 -0600)
> 
> ----------------------------------------------------------------
> tag for qga-pull-2015-02-16
> 
> ----------------------------------------------------------------
> 
> Daniel P. Berrange (1):
>       qga: add guest-set-user-password command
> 
> Michal Privoznik (1):
>       qemu-ga-win: Fail loudly on bare 'set-time'
> 
> Olga Krishtal (2):
>       utils: drop strtok_r from envlist_parse
>       qga: implement file commands for Windows guest
> 
> Simon Zolin (1):
>       guest agent: guest-file-open: refactoring
> 
> zhanghailiang (5):
>       qga: introduce three guest memory block commmands with stubs
>       qga: implement qmp_guest_get_memory_blocks() for Linux with sysfs
>       qga: implement qmp_guest_set_memory_blocks() for Linux with sysfs
>       qga: implement qmp_guest_get_memory_block_size() for Linux with sysfs
>       qga: add memory block command that unsupported
> 
>  include/sysemu/os-win32.h |   1 -
>  qga/commands-posix.c      | 468 +++++++++++++++++++++++++++++++++++++++++++++-
>  qga/commands-win32.c      | 345 +++++++++++++++++++++++++++++-----
>  qga/qapi-schema.json      | 145 +++++++++++++-
>  util/envlist.c            |  32 ++--
>  5 files changed, 921 insertions(+), 70 deletions(-)

Re: [Qemu-devel] [PATCH 03/10] guest agent: guest-file-open: refactoring

[Denis V. Lunev]

On 17/02/15 20:59, Eric Blake wrote:
> On 02/17/2015 10:56 AM, Michael Roth wrote:
>> In any case, since I was actually the one who re-invented it,
>> and this code just moves it to another function, I think we
>> can address it as a seperate patch and leave the PULL
>> intact (unless there are other objections).
> Agree that a separate cleanup patch would be okay.
>
OK. There is a portable way to detect whether
the descriptor is a pipe or socket on Windows.
Thus generic helper is possible.

We'll provide you with one. ETA is something around
next week.

Re: [Qemu-devel] [PATCH 05/10] qga: introduce three guest memory block commmands with stubs

[zhanghailiang]

On 2015/2/18 2:10, Michael Roth wrote:
> Quoting Eric Blake (2015-02-17 09:26:12)
>> On 02/16/2015 08:14 PM, Michael Roth wrote:
>>> From: zhanghailiang <zhang.zhanghailiang@huawei.com>
>>>
>>> Introduce three new guest commands:
>>> guest-get-memory-blocks, guest-set-memory-blocks, guest-get-memory-block-size.
>>
>> Sorry for the late review, but I think guest-get-memory-block-size is
>> the wrong command to add.
>>
>>
>>> +##
>>> +# @guest-get-memory-block-size:
>>> +#
>>> +# Get the the size (in bytes) of a memory block in guest.
>>> +# It is the unit of memory block online/offline operation (also called Logical
>>> +# Memory Hotplug).
>>> +#
>>> +# Returns: memory block size in bytes.
>>> +#
>>> +# Since 2.3
>>> +##
>>> +{ 'command': 'guest-get-memory-block-size',
>>> +  'returns': 'int' }
>>
>> Any QAPI command that returns a bare int instead of a dictionary is
>> non-extensible, and therefore of suspect design.  I think it would be
>> better to have:
>>
>> { 'command': 'guest-get-memory-block-info',
>>    'returns': { 'size': 'int' } }
>>
>> to allow for future extension.
>
> It seems like a reasonable suggestion to me. I can change it in my
> tree if there are no objections. zhanghailiang?
>

OK, you are free to do that, please. Thanks ;)

>>
>> --
>> Eric Blake   eblake redhat com    +1-919-301-3266
>> Libvirt virtualization library http://libvirt.org
>
>
>

Re: [Qemu-devel] [PATCH 05/10] qga: introduce three guest memory block commmands with stubs

[zhanghailiang]

On 2015/2/17 23:26, Eric Blake wrote:
> On 02/16/2015 08:14 PM, Michael Roth wrote:
>> From: zhanghailiang <zhang.zhanghailiang@huawei.com>
>>
>> Introduce three new guest commands:
>> guest-get-memory-blocks, guest-set-memory-blocks, guest-get-memory-block-size.
>
> Sorry for the late review, but I think guest-get-memory-block-size is
> the wrong command to add.
>
>
>> +##
>> +# @guest-get-memory-block-size:
>> +#
>> +# Get the the size (in bytes) of a memory block in guest.
>> +# It is the unit of memory block online/offline operation (also called Logical
>> +# Memory Hotplug).
>> +#
>> +# Returns: memory block size in bytes.
>> +#
>> +# Since 2.3
>> +##
>> +{ 'command': 'guest-get-memory-block-size',
>> +  'returns': 'int' }
>
> Any QAPI command that returns a bare int instead of a dictionary is
> non-extensible, and therefore of suspect design.  I think it would be
> better to have:
>
> { 'command': 'guest-get-memory-block-info',
>    'returns': { 'size': 'int' } }
>
> to allow for future extension.
>

Good idea, and Michael Roth has changed it like that, thanks for you comments.