From 0a982b1bf3953dc8640c4d6e619fb1132ebbebc3 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 25 Nov 2015 10:37:15 -0700 Subject: [PATCH] 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 cf1d7ec8c0..c2ff97021f 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 41f6dd9fed..0654fe4fe7 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 e92c6abafb..238dc6b08d 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 78362e071d..01c9ee48d8 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 3b99d9d5ca..e6a84d17f0 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");