Re: [Qemu-devel] [PATCH v3 2/2] tests: add file-write-read test

2015-11-25 Thread Eric Blake 
On 11/25/2015 05:59 AM, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> 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 
> ---
>  tests/test-qga.c | 95 
> ++--
>  1 file changed, 93 insertions(+), 2 deletions(-)

Reviewed-by: Eric Blake 

> +/* seek to 0 */
> +cmd = g_strdup_printf("{'execute': 'guest-file-seek',"
> +  " 'arguments': { 'handle': %" PRId64 ", "
> +  " 'offset': %d, 'whence': %d } }",
> +  id, 0, SEEK_SET);

We still have a conflict between this series and my proposal to codify 0
rather than relying on platform-specific SEEK_SET; Markus had the
suggestion of using QGA_SET (or QGA_SEEK_SET).  Are we trying to get
both your series and my v2 patch into 2.5?  Knowing that will help me
decide whether my v2 should be rebased on top of your patches.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 2/2] tests: add file-write-read test

2015-11-25 Thread Eric Blake 
On 11/25/2015 09:46 AM, Michael Roth wrote:
> Quoting Eric Blake (2015-11-25 10:41:35)
>> On 11/25/2015 09:21 AM, Michael Roth wrote:
>>
> +/* seek to 0 */
> +cmd = g_strdup_printf("{'execute': 'guest-file-seek',"
> +  " 'arguments': { 'handle': %" PRId64 ", "
> +  " 'offset': %d, 'whence': %d } }",
> +  id, 0, SEEK_SET);

 We still have a conflict between this series and my proposal to codify 0
 rather than relying on platform-specific SEEK_SET; Markus had the
 suggestion of using QGA_SET (or QGA_SEEK_SET).  Are we trying to get
 both your series and my v2 patch into 2.5?  Knowing that will help me
 decide whether my v2 should be rebased on top of your patches.
>>>
>>> I was planning on pulling in your patch on top of this for the next
>>> 2.5 pull, so rebasing on top of this series is probably best.
>>
>> Okay, will do, and will do quickly so I'm not holding up your pull request.
> 
> Thanks! FYI I now have this series applied here if you'd like to base
> on that:
> 
>   https://github.com/mdroth/qemu/commits/qga

On that branch, commit 7a38932 has two typos:
s/is commit msg (Lazlo)/in commit msg (Laszlo)/

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 2/2] tests: add file-write-read test

2015-11-25 Thread Eric Blake 
On 11/25/2015 09:21 AM, Michael Roth wrote:

>>> +/* seek to 0 */
>>> +cmd = g_strdup_printf("{'execute': 'guest-file-seek',"
>>> +  " 'arguments': { 'handle': %" PRId64 ", "
>>> +  " 'offset': %d, 'whence': %d } }",
>>> +  id, 0, SEEK_SET);
>>
>> We still have a conflict between this series and my proposal to codify 0
>> rather than relying on platform-specific SEEK_SET; Markus had the
>> suggestion of using QGA_SET (or QGA_SEEK_SET).  Are we trying to get
>> both your series and my v2 patch into 2.5?  Knowing that will help me
>> decide whether my v2 should be rebased on top of your patches.
> 
> I was planning on pulling in your patch on top of this for the next
> 2.5 pull, so rebasing on top of this series is probably best.

Okay, will do, and will do quickly so I'm not holding up your pull request.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 2/2] tests: add file-write-read test

2015-11-25 Thread Eric Blake 
On 11/25/2015 10:14 AM, Michael Roth wrote:
>>> Thanks! FYI I now have this series applied here if you'd like to base
>>> on that:
>>>
>>>   https://github.com/mdroth/qemu/commits/qga
>>
>> On that branch, commit 7a38932 has two typos:
>> s/is commit msg (Lazlo)/in commit msg (Laszlo)/
> 
> Thanks, fixed now.

Except that in e8c9ea9, the message still references write()/read()
instead of the intended fwrite()/fread().  We'll get it eventually :)

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 2/2] tests: add file-write-read test

2015-11-25 Thread Michael Roth 
Quoting Eric Blake (2015-11-25 10:02:55)
> On 11/25/2015 05:59 AM, marcandre.lur...@redhat.com wrote:
> > From: Marc-André Lureau 
> > 
> > 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 
> > ---
> >  tests/test-qga.c | 95 
> > ++--
> >  1 file changed, 93 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Eric Blake 
> 
> > +/* seek to 0 */
> > +cmd = g_strdup_printf("{'execute': 'guest-file-seek',"
> > +  " 'arguments': { 'handle': %" PRId64 ", "
> > +  " 'offset': %d, 'whence': %d } }",
> > +  id, 0, SEEK_SET);
> 
> We still have a conflict between this series and my proposal to codify 0
> rather than relying on platform-specific SEEK_SET; Markus had the
> suggestion of using QGA_SET (or QGA_SEEK_SET).  Are we trying to get
> both your series and my v2 patch into 2.5?  Knowing that will help me
> decide whether my v2 should be rebased on top of your patches.

I was planning on pulling in your patch on top of this for the next
2.5 pull, so rebasing on top of this series is probably best.

> 
> -- 
> Eric Blake   eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>

Re: [Qemu-devel] [PATCH v3 2/2] tests: add file-write-read test

2015-11-25 Thread Michael Roth 
Quoting Eric Blake (2015-11-25 11:10:46)
> On 11/25/2015 09:46 AM, Michael Roth wrote:
> > Quoting Eric Blake (2015-11-25 10:41:35)
> >> On 11/25/2015 09:21 AM, Michael Roth wrote:
> >>
> > +/* seek to 0 */
> > +cmd = g_strdup_printf("{'execute': 'guest-file-seek',"
> > +  " 'arguments': { 'handle': %" PRId64 ", "
> > +  " 'offset': %d, 'whence': %d } }",
> > +  id, 0, SEEK_SET);
> 
>  We still have a conflict between this series and my proposal to codify 0
>  rather than relying on platform-specific SEEK_SET; Markus had the
>  suggestion of using QGA_SET (or QGA_SEEK_SET).  Are we trying to get
>  both your series and my v2 patch into 2.5?  Knowing that will help me
>  decide whether my v2 should be rebased on top of your patches.
> >>>
> >>> I was planning on pulling in your patch on top of this for the next
> >>> 2.5 pull, so rebasing on top of this series is probably best.
> >>
> >> Okay, will do, and will do quickly so I'm not holding up your pull request.
> > 
> > Thanks! FYI I now have this series applied here if you'd like to base
> > on that:
> > 
> >   https://github.com/mdroth/qemu/commits/qga
> 
> On that branch, commit 7a38932 has two typos:
> s/is commit msg (Lazlo)/in commit msg (Laszlo)/

Thanks, fixed now.

> 
> -- 
> Eric Blake   eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>

Re: [Qemu-devel] [PATCH v3 2/2] tests: add file-write-read test

2015-11-25 Thread Michael Roth 
Quoting Eric Blake (2015-11-25 11:18:24)
> On 11/25/2015 10:14 AM, Michael Roth wrote:
> >>> Thanks! FYI I now have this series applied here if you'd like to base
> >>> on that:
> >>>
> >>>   https://github.com/mdroth/qemu/commits/qga
> >>
> >> On that branch, commit 7a38932 has two typos:
> >> s/is commit msg (Lazlo)/in commit msg (Laszlo)/
> > 
> > Thanks, fixed now.
> 
> Except that in e8c9ea9, the message still references write()/read()
> instead of the intended fwrite()/fread().  We'll get it eventually :)

Argh! Sorry, fixed now.

> 
> -- 
> Eric Blake   eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>

Re: [Qemu-devel] [PATCH v3 2/2] tests: add file-write-read test

2015-11-25 Thread Michael Roth 
Quoting Eric Blake (2015-11-25 10:41:35)
> On 11/25/2015 09:21 AM, Michael Roth wrote:
> 
> >>> +/* seek to 0 */
> >>> +cmd = g_strdup_printf("{'execute': 'guest-file-seek',"
> >>> +  " 'arguments': { 'handle': %" PRId64 ", "
> >>> +  " 'offset': %d, 'whence': %d } }",
> >>> +  id, 0, SEEK_SET);
> >>
> >> We still have a conflict between this series and my proposal to codify 0
> >> rather than relying on platform-specific SEEK_SET; Markus had the
> >> suggestion of using QGA_SET (or QGA_SEEK_SET).  Are we trying to get
> >> both your series and my v2 patch into 2.5?  Knowing that will help me
> >> decide whether my v2 should be rebased on top of your patches.
> > 
> > I was planning on pulling in your patch on top of this for the next
> > 2.5 pull, so rebasing on top of this series is probably best.
> 
> Okay, will do, and will do quickly so I'm not holding up your pull request.

Thanks! FYI I now have this series applied here if you'd like to base
on that:

  https://github.com/mdroth/qemu/commits/qga

> 
> -- 
> Eric Blake   eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>

[Qemu-devel] [PATCH v3 2/2] tests: add file-write-read test

2015-11-25 Thread marcandre . lureau 
From: Marc-André Lureau 

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 
---
 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", ,
  test_qga_get_memory_blocks);
 g_test_add_data_func("/qga/file-ops", , test_qga_file_ops);
+g_test_add_data_func("/qga/file-write-read", , 
test_qga_file_write_read);
 g_test_add_data_func("/qga/get-time", , test_qga_get_time);
 g_test_add_data_func("/qga/invalid-cmd", , test_qga_invalid_cmd);
 g_test_add_data_func("/qga/fsfreeze-status", ,
-- 
2.5.0