[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.5 v2] qga: Better mapping of SEEK_* in gue
From: |
Michael Roth |
Subject: |
Re: [Qemu-devel] [PATCH for-2.5 v2] qga: Better mapping of SEEK_* in guest-file-seek |
Date: |
Wed, 25 Nov 2015 12:01:51 -0600 |
User-agent: |
alot/0.3.6 |
Quoting Eric Blake (2015-11-25 11:37:15)
> 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 <address@hidden>
Thanks, applied to qga tree:
https://github.com/mdroth/qemu/commits/qga
>
> ---
> v2: rebase to mdroth/qga, add QGA_SEEK_* constants instead of magic
> numbers
> ---
> 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");
> --
> 2.4.3
>