From 68aa262ad09c81b8b1284340cc0d26b65c605df5 Mon Sep 17 00:00:00 2001 From: Michael Roth Date: Mon, 23 Nov 2015 15:48:58 -0600 Subject: makefile: fix qemu-ga make install for --disable-tools ab59e3e introduced a fix for `make install` on w32 that involved filtering out qemu-ga from $TOOLS install recipe so that we could append $(EXESUF) to it before attempting to install the binary via install-prog function. install-prog takes a list of binaries to install to a particular directory. If the list is empty it breaks. We guard against this by ensuring $TOOLS is not empty prior to calling. However, ab59e3e introduces extra filtering after this check which can still result on us attempting to call install-prog with an empty list of binaries. In particular, this occurs if we build with the --disable-tools configure option, which results in qemu-ga being the only member of $TOOLS. Fix this by doing a simple s/qemu-ga/qemu-ga$(EXESUF)/ pass through $TOOLS instead of filtering out qemu-ga to handle it seperately. Reported-by: Steve Ellcey Cc: Stefan Weil Signed-off-by: Michael Roth --- Makefile | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Makefile b/Makefile index c7fa427..930ac27 100644 --- a/Makefile +++ b/Makefile @@ -440,10 +440,7 @@ endif install: all $(if $(BUILD_DOCS),install-doc) \ install-datadir install-localstatedir ifneq ($(TOOLS),) - $(call install-prog,$(filter-out qemu-ga,$(TOOLS)),$(DESTDIR)$(bindir)) -ifneq (,$(findstring qemu-ga,$(TOOLS))) - $(call install-prog,qemu-ga$(EXESUF),$(DESTDIR)$(bindir)) -endif + $(call install-prog,$(subst qemu-ga,qemu-ga$(EXESUF),$(TOOLS)),$(DESTDIR)$(bindir)) endif ifneq ($(CONFIG_MODULES),) $(INSTALL_DIR) "$(DESTDIR)$(qemu_moddir)" -- cgit v1.1 From f2b608ab80a336b0136d35d9b49419a917656d44 Mon Sep 17 00:00:00 2001 From: Yuri Pudgorodskiy Date: Thu, 19 Nov 2015 15:20:37 +0300 Subject: qga: gspawn() console helper to Windows guest agent msi build This helper, gspawn-win64-helper-console.exe for 64-bit and gspawn-win32-helper-console.exe for 32-bit environment, is needed for gspawn() mingw implementation, used by guest-exec command. Without these files guest-exec command on Windows will not work with "file not found" diagnostic message. Signed-off-by: Yuri Pudgorodskiy Signed-off-by: Denis V. Lunev CC: Michael Roth Signed-off-by: Michael Roth --- qga/installer/qemu-ga.wxs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs index 6804f02..f25afdd 100644 --- a/qga/installer/qemu-ga.wxs +++ b/qga/installer/qemu-ga.wxs @@ -91,6 +91,16 @@ + + + + + + + + + + @@ -148,6 +158,7 @@ + -- cgit v1.1 From 895b00f62a7e86724dc7352d67c7808d37366130 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 25 Nov 2015 13:59:11 +0100 Subject: qga: flush explicitly when needed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit According to the specification: http://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html "the application shall ensure that output is not directly followed by input without an intervening call to fflush() or to a file positioning function (fseek(), fsetpos(), or rewind()), and input is not directly followed by output without an intervening call to a file positioning function, unless the input operation encounters end-of-file." Without this change, an fwrite() followed by an fread() may lose the previously written content, as shown in the following test. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1210246 Signed-off-by: Marc-André Lureau Reviewed-by: Eric Blake * don't confuse {write,read}() with f{write,read}() in commit msg (Laszlo) Signed-off-by: Michael Roth --- qga/commands-posix.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 0ebd473..cf1d7ec 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -216,9 +216,16 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) } } +typedef enum { + RW_STATE_NEW, + RW_STATE_READING, + RW_STATE_WRITING, +} RwState; + typedef struct GuestFileHandle { uint64_t id; FILE *fh; + RwState state; QTAILQ_ENTRY(GuestFileHandle) next; } GuestFileHandle; @@ -460,6 +467,17 @@ struct GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count, } fh = gfh->fh; + + /* explicitly flush when switching from writing to reading */ + if (gfh->state == RW_STATE_WRITING) { + int ret = fflush(fh); + if (ret == EOF) { + error_setg_errno(errp, errno, "failed to flush file"); + return NULL; + } + gfh->state = RW_STATE_NEW; + } + buf = g_malloc0(count+1); read_count = fread(buf, 1, count, fh); if (ferror(fh)) { @@ -473,6 +491,7 @@ struct GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count, if (read_count) { read_data->buf_b64 = g_base64_encode(buf, read_count); } + gfh->state = RW_STATE_READING; } g_free(buf); clearerr(fh); @@ -496,6 +515,16 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64, } fh = gfh->fh; + + if (gfh->state == RW_STATE_READING) { + int ret = fseek(fh, 0, SEEK_CUR); + if (ret == -1) { + error_setg_errno(errp, errno, "failed to seek file"); + return NULL; + } + gfh->state = RW_STATE_NEW; + } + buf = g_base64_decode(buf_b64, &buf_len); if (!has_count) { @@ -515,6 +544,7 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64, write_data = g_new0(GuestFileWrite, 1); write_data->count = write_count; write_data->eof = feof(fh); + gfh->state = RW_STATE_WRITING; } g_free(buf); clearerr(fh); @@ -538,10 +568,15 @@ struct GuestFileSeek *qmp_guest_file_seek(int64_t handle, int64_t offset, ret = fseek(fh, offset, whence); if (ret == -1) { error_setg_errno(errp, errno, "failed to seek file"); + if (errno == ESPIPE) { + /* file is non-seekable, stdio shouldn't be buffering anyways */ + gfh->state = RW_STATE_NEW; + } } else { seek_data = g_new0(GuestFileSeek, 1); seek_data->position = ftell(fh); seek_data->eof = feof(fh); + gfh->state = RW_STATE_NEW; } clearerr(fh); @@ -562,6 +597,8 @@ void qmp_guest_file_flush(int64_t handle, Error **errp) ret = fflush(fh); if (ret == EOF) { error_setg_errno(errp, errno, "failed to flush file"); + } else { + gfh->state = RW_STATE_NEW; } } -- cgit v1.1 From 4eaab85cb1c1ba9c575d29921df81d63c7aa35df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 25 Nov 2015 13:59:12 +0100 Subject: tests: add file-write-read test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This test exhibits a POSIX behaviour regarding switching between write and read. It's undefined result if the application doesn't ensure a flush between the two operations (with glibc, the flush can be implicit when the buffer size is relatively small). The previous commit fixes this test. Related to: https://bugzilla.redhat.com/show_bug.cgi?id=1210246 Signed-off-by: Marc-André Lureau Reviewed-by: Eric Blake Signed-off-by: Michael Roth --- tests/test-qga.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 93 insertions(+), 2 deletions(-) diff --git a/tests/test-qga.c b/tests/test-qga.c index 6473846..3b99d9d 100644 --- a/tests/test-qga.c +++ b/tests/test-qga.c @@ -352,10 +352,10 @@ static void test_qga_network_get_interfaces(gconstpointer fix) static void test_qga_file_ops(gconstpointer fix) { const TestFixture *fixture = fix; - const guchar helloworld[] = "Hello World!\n"; + const unsigned char helloworld[] = "Hello World!\n"; const char *b64; gchar *cmd, *path, *enc; - guchar *dec; + unsigned char *dec; QDict *ret, *val; int64_t id, eof; gsize count; @@ -496,6 +496,96 @@ static void test_qga_file_ops(gconstpointer fix) g_free(cmd); } +static void test_qga_file_write_read(gconstpointer fix) +{ + const TestFixture *fixture = fix; + const unsigned char helloworld[] = "Hello World!\n"; + const char *b64; + gchar *cmd, *enc; + QDict *ret, *val; + int64_t id, eof; + gsize count; + + /* open */ + ret = qmp_fd(fixture->fd, "{'execute': 'guest-file-open'," + " 'arguments': { 'path': 'foo', 'mode': 'w+' } }"); + g_assert_nonnull(ret); + qmp_assert_no_error(ret); + id = qdict_get_int(ret, "return"); + QDECREF(ret); + + enc = g_base64_encode(helloworld, sizeof(helloworld)); + /* write */ + cmd = g_strdup_printf("{'execute': 'guest-file-write'," + " 'arguments': { 'handle': %" PRId64 "," + " 'buf-b64': '%s' } }", id, enc); + ret = qmp_fd(fixture->fd, cmd); + g_assert_nonnull(ret); + qmp_assert_no_error(ret); + + val = qdict_get_qdict(ret, "return"); + count = qdict_get_int(val, "count"); + eof = qdict_get_bool(val, "eof"); + g_assert_cmpint(count, ==, sizeof(helloworld)); + g_assert_cmpint(eof, ==, 0); + QDECREF(ret); + g_free(cmd); + + /* read (check implicit flush) */ + cmd = g_strdup_printf("{'execute': 'guest-file-read'," + " 'arguments': { 'handle': %" PRId64 "} }", + id); + ret = qmp_fd(fixture->fd, cmd); + val = qdict_get_qdict(ret, "return"); + count = qdict_get_int(val, "count"); + eof = qdict_get_bool(val, "eof"); + b64 = qdict_get_str(val, "buf-b64"); + g_assert_cmpint(count, ==, 0); + g_assert(eof); + g_assert_cmpstr(b64, ==, ""); + QDECREF(ret); + g_free(cmd); + + /* seek to 0 */ + cmd = g_strdup_printf("{'execute': 'guest-file-seek'," + " 'arguments': { 'handle': %" PRId64 ", " + " 'offset': %d, 'whence': %d } }", + id, 0, SEEK_SET); + ret = qmp_fd(fixture->fd, cmd); + qmp_assert_no_error(ret); + val = qdict_get_qdict(ret, "return"); + count = qdict_get_int(val, "position"); + eof = qdict_get_bool(val, "eof"); + g_assert_cmpint(count, ==, 0); + g_assert(!eof); + QDECREF(ret); + g_free(cmd); + + /* read */ + cmd = g_strdup_printf("{'execute': 'guest-file-read'," + " 'arguments': { 'handle': %" PRId64 "} }", + id); + ret = qmp_fd(fixture->fd, cmd); + val = qdict_get_qdict(ret, "return"); + count = qdict_get_int(val, "count"); + eof = qdict_get_bool(val, "eof"); + b64 = qdict_get_str(val, "buf-b64"); + g_assert_cmpint(count, ==, sizeof(helloworld)); + g_assert(eof); + g_assert_cmpstr(b64, ==, enc); + QDECREF(ret); + g_free(cmd); + g_free(enc); + + /* close */ + cmd = g_strdup_printf("{'execute': 'guest-file-close'," + " 'arguments': {'handle': %" PRId64 "} }", + id); + ret = qmp_fd(fixture->fd, cmd); + QDECREF(ret); + g_free(cmd); +} + static void test_qga_get_time(gconstpointer fix) { const TestFixture *fixture = fix; @@ -762,6 +852,7 @@ int main(int argc, char **argv) g_test_add_data_func("/qga/get-memory-blocks", &fix, test_qga_get_memory_blocks); g_test_add_data_func("/qga/file-ops", &fix, test_qga_file_ops); + g_test_add_data_func("/qga/file-write-read", &fix, test_qga_file_write_read); g_test_add_data_func("/qga/get-time", &fix, test_qga_get_time); g_test_add_data_func("/qga/invalid-cmd", &fix, test_qga_invalid_cmd); g_test_add_data_func("/qga/fsfreeze-status", &fix, -- cgit v1.1 From 0a982b1bf3953dc8640c4d6e619fb1132ebbebc3 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 25 Nov 2015 10:37:15 -0700 Subject: qga: Better mapping of SEEK_* in guest-file-seek Exposing OS-specific SEEK_ constants in our qapi was a mistake (if the host has SEEK_CUR as 1, but the guest has it as 2, then the semantics are unclear what should happen); if we had a time machine, we would instead expose only a symbolic enum. It's too late to change the fact that we have an integer in qapi, but we can at least document what mapping we want to enforce for all qga clients (and luckily, it happens to be the mapping that both Linux and Windows use); then fix the code to match that mapping. It also helps us filter out unsupported SEEK_DATA and SEEK_HOLE. In the future, we may wish to move our QGA_SEEK_* constants into qga/qapi-schema.json, along with updating the schema to take an alternate type (either the integer, or the string value of the enum name) - but that's too much risk during hard freeze. Signed-off-by: Eric Blake Signed-off-by: Michael Roth --- qga/commands-posix.c | 19 ++++++++++++++++++- qga/commands-win32.c | 20 +++++++++++++++++++- qga/guest-agent-core.h | 7 +++++++ qga/qapi-schema.json | 4 ++-- tests/test-qga.c | 5 +++-- 5 files changed, 49 insertions(+), 6 deletions(-) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index cf1d7ec..c2ff970 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -553,17 +553,34 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64, } struct GuestFileSeek *qmp_guest_file_seek(int64_t handle, int64_t offset, - int64_t whence, Error **errp) + int64_t whence_code, Error **errp) { GuestFileHandle *gfh = guest_file_handle_find(handle, errp); GuestFileSeek *seek_data = NULL; FILE *fh; int ret; + int whence; if (!gfh) { return NULL; } + /* We stupidly exposed 'whence':'int' in our qapi */ + switch (whence_code) { + case QGA_SEEK_SET: + whence = SEEK_SET; + break; + case QGA_SEEK_CUR: + whence = SEEK_CUR; + break; + case QGA_SEEK_END: + whence = SEEK_END; + break; + default: + error_setg(errp, "invalid whence code %"PRId64, whence_code); + return NULL; + } + fh = gfh->fh; ret = fseek(fh, offset, whence); if (ret == -1) { diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 41f6dd9..0654fe4 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -382,7 +382,7 @@ done: } GuestFileSeek *qmp_guest_file_seek(int64_t handle, int64_t offset, - int64_t whence, Error **errp) + int64_t whence_code, Error **errp) { GuestFileHandle *gfh; GuestFileSeek *seek_data; @@ -390,11 +390,29 @@ GuestFileSeek *qmp_guest_file_seek(int64_t handle, int64_t offset, LARGE_INTEGER new_pos, off_pos; off_pos.QuadPart = offset; BOOL res; + int whence; + gfh = guest_file_handle_find(handle, errp); if (!gfh) { return NULL; } + /* We stupidly exposed 'whence':'int' in our qapi */ + switch (whence_code) { + case QGA_SEEK_SET: + whence = SEEK_SET; + break; + case QGA_SEEK_CUR: + whence = SEEK_CUR; + break; + case QGA_SEEK_END: + whence = SEEK_END; + break; + default: + error_setg(errp, "invalid whence code %"PRId64, whence_code); + return NULL; + } + fh = gfh->fh; res = SetFilePointerEx(fh, off_pos, &new_pos, whence); if (!res) { diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h index e92c6ab..238dc6b 100644 --- a/qga/guest-agent-core.h +++ b/qga/guest-agent-core.h @@ -15,6 +15,13 @@ #define QGA_READ_COUNT_DEFAULT 4096 +/* Mapping of whence codes used by guest-file-seek. */ +enum { + QGA_SEEK_SET = 0, + QGA_SEEK_CUR = 1, + QGA_SEEK_END = 2, +}; + typedef struct GAState GAState; typedef struct GACommandState GACommandState; extern GAState *ga_state; diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index 78362e0..01c9ee4 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -318,13 +318,13 @@ # # Seek to a position in the file, as with fseek(), and return the # current file position afterward. Also encapsulates ftell()'s -# functionality, just Set offset=0, whence=SEEK_CUR. +# functionality, with offset=0 and whence=1. # # @handle: filehandle returned by guest-file-open # # @offset: bytes to skip over in the file stream # -# @whence: SEEK_SET, SEEK_CUR, or SEEK_END, as with fseek() +# @whence: 0 for SEEK_SET, 1 for SEEK_CUR, or 2 for SEEK_END # # Returns: @GuestFileSeek on success. # diff --git a/tests/test-qga.c b/tests/test-qga.c index 3b99d9d..e6a84d1 100644 --- a/tests/test-qga.c +++ b/tests/test-qga.c @@ -13,6 +13,7 @@ #include "libqtest.h" #include "config-host.h" +#include "qga/guest-agent-core.h" typedef struct { char *test_dir; @@ -457,7 +458,7 @@ static void test_qga_file_ops(gconstpointer fix) cmd = g_strdup_printf("{'execute': 'guest-file-seek'," " 'arguments': { 'handle': %" PRId64 ", " " 'offset': %d, 'whence': %d } }", - id, 6, SEEK_SET); + id, 6, QGA_SEEK_SET); ret = qmp_fd(fixture->fd, cmd); qmp_assert_no_error(ret); val = qdict_get_qdict(ret, "return"); @@ -550,7 +551,7 @@ static void test_qga_file_write_read(gconstpointer fix) cmd = g_strdup_printf("{'execute': 'guest-file-seek'," " 'arguments': { 'handle': %" PRId64 ", " " 'offset': %d, 'whence': %d } }", - id, 0, SEEK_SET); + id, 0, QGA_SEEK_SET); ret = qmp_fd(fixture->fd, cmd); qmp_assert_no_error(ret); val = qdict_get_qdict(ret, "return"); -- cgit v1.1 From 44c6e00c3fd85b9c496bd3e74108ace126813a59 Mon Sep 17 00:00:00 2001 From: Yuri Pudgorodskiy Date: Wed, 25 Nov 2015 22:02:26 +0300 Subject: qga: added another non-interactive gspawn() helper file. With previous commit we added gspawn-win64-helper-console.exe, required for gspawn() mingw implementation. Unfortunatly when running as a service without interactive desktop, gspawn() also requires another helper app. Added gspawn-win64-helper.exe and gspawn-win32-helper.exe for corresponding architectures. Signed-off-by: Yuri Pudgorodskiy Signed-off-by: Denis V. Lunev CC: Michael Roth * remove trailing whitespace Signed-off-by: Michael Roth --- qga/installer/qemu-ga.wxs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs index f25afdd..9473875 100644 --- a/qga/installer/qemu-ga.wxs +++ b/qga/installer/qemu-ga.wxs @@ -95,11 +95,17 @@ + + + + + + @@ -159,6 +165,7 @@ + -- cgit v1.1