From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59480) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1eOU-00006z-79 for qemu-devel@nongnu.org; Wed, 25 Nov 2015 13:02:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a1eOP-0002b0-Om for qemu-devel@nongnu.org; Wed, 25 Nov 2015 13:02:14 -0500 Received: from mail-ob0-x22b.google.com ([2607:f8b0:4003:c01::22b]:36017) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1eOP-0002au-Jc for qemu-devel@nongnu.org; Wed, 25 Nov 2015 13:02:09 -0500 Received: by obdgf3 with SMTP id gf3so45241664obd.3 for ; Wed, 25 Nov 2015 10:02:09 -0800 (PST) Sender: fluxion Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <1448473035-12351-1-git-send-email-eblake@redhat.com> References: <1448473035-12351-1-git-send-email-eblake@redhat.com> Message-ID: <20151125180151.6284.33888@loki> Date: Wed, 25 Nov 2015 12:01:51 -0600 Subject: Re: [Qemu-devel] [PATCH for-2.5 v2] qga: Better mapping of SEEK_* in guest-file-seek List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: mlureau@redhat.com, armbru@redhat.com 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 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 **e= rrp) > { > GuestFileHandle *gfh =3D guest_file_handle_find(handle, errp); > GuestFileSeek *seek_data =3D 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 =3D SEEK_SET; > + break; > + case QGA_SEEK_CUR: > + whence =3D SEEK_CUR; > + break; > + case QGA_SEEK_END: > + whence =3D SEEK_END; > + break; > + default: > + error_setg(errp, "invalid whence code %"PRId64, whence_code); > + return NULL; > + } > + > fh =3D gfh->fh; > ret =3D fseek(fh, offset, whence); > if (ret =3D=3D -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 =3D offset; > BOOL res; > + int whence; > + > gfh =3D 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 =3D SEEK_SET; > + break; > + case QGA_SEEK_CUR: > + whence =3D SEEK_CUR; > + break; > + case QGA_SEEK_END: > + whence =3D SEEK_END; > + break; > + default: > + error_setg(errp, "invalid whence code %"PRId64, whence_code); > + return NULL; > + } > + > fh =3D gfh->fh; > res =3D 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 =3D 0, > + QGA_SEEK_CUR =3D 1, > + QGA_SEEK_END =3D 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=3D0, whence=3DSEEK_CUR. > +# functionality, with offset=3D0 and whence=3D1. > # > # @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 =3D g_strdup_printf("{'execute': 'guest-file-seek'," > " 'arguments': { 'handle': %" PRId64 ", " > " 'offset': %d, 'whence': %d } }", > - id, 6, SEEK_SET); > + id, 6, QGA_SEEK_SET); > ret =3D qmp_fd(fixture->fd, cmd); > qmp_assert_no_error(ret); > val =3D qdict_get_qdict(ret, "return"); > @@ -550,7 +551,7 @@ static void test_qga_file_write_read(gconstpointer fi= x) > cmd =3D g_strdup_printf("{'execute': 'guest-file-seek'," > " 'arguments': { 'handle': %" PRId64 ", " > " 'offset': %d, 'whence': %d } }", > - id, 0, SEEK_SET); > + id, 0, QGA_SEEK_SET); > ret =3D qmp_fd(fixture->fd, cmd); > qmp_assert_no_error(ret); > val =3D qdict_get_qdict(ret, "return"); > -- = > 2.4.3 >=20