From 215a2771a7b6b29037ee8deba484815d816b6fdd Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 11 Feb 2015 11:26:12 +0000 Subject: [PATCH 01/10] qga: add guest-set-user-password command 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 Signed-off-by: Michael Roth --- 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 f6f3e3cd8ee1..57409d0ea672 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 3bcbeae8ff6c..11876f6846a9 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 376e79f58360..5117b65b2636 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' } } From 459db780be10f7adac723a5d3a4ffeac8ae6e768 Mon Sep 17 00:00:00 2001 From: Olga Krishtal Date: Fri, 6 Feb 2015 20:59:53 +0300 Subject: [PATCH 02/10] utils: drop strtok_r from envlist_parse 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 Signed-off-by: Denis V. Lunev CC: Eric Blake CC: Michael Roth Signed-off-by: Michael Roth --- 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 af3fbc47d800..9cc9e0813922 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 ebc06cf0f3f1..099a544a41b0 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; } /* From 85b6f6f53596fd29eee0a6d2475c6a4322eede6b Mon Sep 17 00:00:00 2001 From: Simon Zolin Date: Fri, 6 Feb 2015 20:59:54 +0300 Subject: [PATCH 03/10] guest agent: guest-file-open: refactoring Moved the code that sets non-blocking flag on fd into a separate function. Signed-off-by: Simon Zolin Reviewed-by: Roman Kagan Signed-off-by: Denis V. Lunev CC: Michael Roth CC: Eric Blake Signed-off-by: Michael Roth --- 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 57409d0ea672..ed527a3d3de9 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; } From fa193594fbc27fde7c9062b3bf5c232534887ec7 Mon Sep 17 00:00:00 2001 From: Olga Krishtal Date: Fri, 6 Feb 2015 20:59:55 +0300 Subject: [PATCH 04/10] qga: implement file commands for Windows guest 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 Signed-off-by: Denis V. Lunev CC: Michael Roth Signed-off-by: Michael Roth --- 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 11876f6846a9..9af7950aef5b 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -14,10 +14,13 @@ #include #include #include +#include +#include #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); } From a065aaa9204ecd4a0d18f5eae49aa350a5f76b63 Mon Sep 17 00:00:00 2001 From: zhanghailiang Date: Thu, 22 Jan 2015 10:40:02 +0800 Subject: [PATCH 05/10] qga: introduce three guest memory block commmands with stubs 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 *generalized guest-get-memory-block-size to get-get-memory-block-info for future extensibility Signed-off-by: Michael Roth --- qga/commands-posix.c | 38 +++++++++++++ qga/commands-win32.c | 19 +++++++ qga/qapi-schema.json | 123 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 180 insertions(+) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index ed527a3d3de9..b7b7efe2ec48 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -1992,6 +1992,25 @@ void qmp_guest_set_user_password(const char *username, } } +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; +} + +GuestMemoryBlockInfo *qmp_guest_get_memory_block_info(Error **errp) +{ + error_set(errp, QERR_UNSUPPORTED); + return NULL; +} + #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; +} + +GuestMemoryBlockInfo *qmp_guest_get_memory_block_info(Error **errp) +{ + error_set(errp, QERR_UNSUPPORTED); + return NULL; +} + #endif #if !defined(CONFIG_FSFREEZE) diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 9af7950aef5b..990a8ddf5360 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; +} + +GuestMemoryBlockInfo *qmp_guest_get_memory_block_info(Error **errp) +{ + error_set(errp, QERR_UNSUPPORTED); + return NULL; +} + /* 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 5117b65b2636..33e7d0676dbe 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -765,3 +765,126 @@ ## { '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'] } + +# @GuestMemoryBlockInfo: +# +# @size: the size (in bytes) of the guest memory blocks, +# which are the minimal units of memory block online/offline +# operations (also called Logical Memory Hotplug). +# +# Since: 2.3 +## +{ 'type': 'GuestMemoryBlockInfo', + 'data': {'size': 'uint64'} } + +## +# @guest-get-memory-block-info: +# +# Get information relating to guest memory blocks. +# +# Returns: memory block size in bytes. +# Returns: @GuestMemoryBlockInfo +# +# Since 2.3 +## +{ 'command': 'guest-get-memory-block-info', + 'returns': 'GuestMemoryBlockInfo' } From bd240fca42d5f072fb758a71720d9de9990ac553 Mon Sep 17 00:00:00 2001 From: zhanghailiang Date: Thu, 22 Jan 2015 10:40:03 +0800 Subject: [PATCH 06/10] qga: implement qmp_guest_get_memory_blocks() for Linux with sysfs 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 *replaced guest-triggerable assertion with an error msg Signed-off-by: Michael Roth --- qga/commands-posix.c | 233 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 232 insertions(+), 1 deletion(-) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index b7b7efe2ec48..5d4101dc791d 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -1992,9 +1992,240 @@ void qmp_guest_set_user_password(const char *username, } } +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 head; + } + + qapi_free_GuestMemoryBlockList(head); + error_propagate(errp, local_err); return NULL; } From 32ca7927c7d66371abafb4cabfab9438a5905784 Mon Sep 17 00:00:00 2001 From: zhanghailiang Date: Thu, 22 Jan 2015 10:40:04 +0800 Subject: [PATCH 07/10] qga: implement qmp_guest_set_memory_blocks() for Linux with sysfs We can change guest's online/offline state of memory blocks, by using command 'guest-set-memory-blocks'. Signed-off-by: zhanghailiang Signed-off-by: Michael Roth --- 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 5d4101dc791d..0fd5d959d218 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -2232,7 +2232,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; } From ef82b60be13b18198b84a2157f59c50fd53f5408 Mon Sep 17 00:00:00 2001 From: zhanghailiang Date: Thu, 22 Jan 2015 10:40:05 +0800 Subject: [PATCH 08/10] qga: implement qmp_guest_get_memory_block_info() for Linux with sysfs This conveys general information about guest memory blocks. Currently, just the memory block size. 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 *generalized guest-get-memory-block-size to get-get-memory-block-info for future extensibility Signed-off-by: Michael Roth --- qga/commands-posix.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 0fd5d959d218..6575c491917f 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -2266,8 +2266,35 @@ qmp_guest_set_memory_blocks(GuestMemoryBlockList *mem_blks, Error **errp) GuestMemoryBlockInfo *qmp_guest_get_memory_block_info(Error **errp) { - error_set(errp, QERR_UNSUPPORTED); - return NULL; + Error *local_err = NULL; + char *dirpath; + int dirfd; + char *buf; + GuestMemoryBlockInfo *info; + + 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 NULL; + } + 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 NULL; + } + + info = g_new0(GuestMemoryBlockInfo, 1); + info->size = strtol(buf, NULL, 16); /* the unit is bytes */ + + g_free(buf); + + return info; } #else /* defined(__linux__) */ From 0dd38a03f5e1498aabf7d053a9fab792a5eeec5c Mon Sep 17 00:00:00 2001 From: zhanghailiang Date: Thu, 22 Jan 2015 10:40:06 +0800 Subject: [PATCH 09/10] qga: add memory block command that unsupported For memory block command, we only support for linux with sysfs. Signed-off-by: zhanghailiang Reviewed-by: Michael Roth Signed-off-by: Michael Roth --- 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 6575c491917f..d5bb5cb5dae8 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -2415,7 +2415,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 990a8ddf5360..0238fbbcad74 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; From ee17cbdc3c21f5cb6144a434191ffcd08b7de5fe Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Wed, 21 Jan 2015 12:09:50 +0100 Subject: [PATCH 10/10] qemu-ga-win: Fail loudly on bare 'set-time' 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 Signed-off-by: Michael Roth --- 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 0238fbbcad74..3ef0549c0f5c 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 33e7d0676dbe..95f49e369c1e 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.