[Qemu-devel] [PATCH 0/3] another round of pci fixes

[Qemu-devel] [PATCH 0/3] another round of pci fixes

[Hu Tao]

Hi Michael,

This series is on top of your pci tree.

This series includes two fixups and one patch for adding test of human
format of string output visitor, please review. thanks!


Hu Tao (3):
  fixup! libqemustub: add more stubs for qemu-char
  fixup! qapi/string-output-visitor: fix bugs
  tests: add human format test for string output visitor

 qapi/string-output-visitor.c       |   2 +-
 stubs/Makefile.objs                |   2 +-
 tests/test-string-output-visitor.c | 109 ++++++++++++++++++++++++++++++-------
 3 files changed, 92 insertions(+), 21 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 1/3] fixup! libqemustub: add more stubs for qemu-char

[Hu Tao]

build stub/qemu-chr-open-spice.o only with CONFIG_SPICE, this
suppresses the warning when building without CONFIG_SPICE.

Cc: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 stubs/Makefile.objs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 2312076..03210ad 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -25,7 +25,7 @@ stub-obj-y += mon-set-error.o
 stub-obj-y += monitor-init.o
 stub-obj-y += notify-event.o
 stub-obj-y += pci-drive-hot-add.o
-stub-obj-y += qemu-chr-open-spice.o
+stub-obj-$(CONFIG_SPICE) += qemu-chr-open-spice.o
 stub-obj-y += qtest.o
 stub-obj-y += reset.o
 stub-obj-y += runstate-check.o
-- 
1.9.3

[Qemu-devel] [PATCH 2/3] fixup! qapi/string-output-visitor: fix bugs

[Hu Tao]

"0x1-0x10" looks better than "0x1-10"

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 qapi/string-output-visitor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index e9aca3b..1ab8574 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -98,7 +98,7 @@ static void format_string(StringOutputVisitor *sov, Range *r, bool next,
 {
     if (r->end - r->begin > 1) {
         if (human) {
-            g_string_append_printf(sov->string, "0x%" PRIx64 "-%" PRIx64,
+            g_string_append_printf(sov->string, "0x%" PRIx64 "-0x%" PRIx64,
                                    r->begin, r->end - 1);
 
         } else {
-- 
1.9.3

[Qemu-devel] [PATCH 3/3] tests: add human format test for string output visitor

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 tests/test-string-output-visitor.c | 109 ++++++++++++++++++++++++++++++-------
 1 file changed, 90 insertions(+), 19 deletions(-)

diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
index 28e7359..e89e43c 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -21,12 +21,25 @@
 typedef struct TestOutputVisitorData {
     StringOutputVisitor *sov;
     Visitor *ov;
+    bool human;
 } TestOutputVisitorData;
 
 static void visitor_output_setup(TestOutputVisitorData *data,
                                  const void *unused)
 {
-    data->sov = string_output_visitor_new(false);
+    data->human = false;
+    data->sov = string_output_visitor_new(data->human);
+    g_assert(data->sov != NULL);
+
+    data->ov = string_output_get_visitor(data->sov);
+    g_assert(data->ov != NULL);
+}
+
+static void visitor_output_setup_human(TestOutputVisitorData *data,
+                                       const void *unused)
+{
+    data->human = true;
+    data->sov = string_output_visitor_new(data->human);
     g_assert(data->sov != NULL);
 
     data->ov = string_output_get_visitor(data->sov);
@@ -53,7 +66,11 @@ static void test_visitor_out_int(TestOutputVisitorData *data,
 
     str = string_output_get_string(data->sov);
     g_assert(str != NULL);
-    g_assert_cmpstr(str, ==, "42");
+    if (data->human) {
+        g_assert_cmpstr(str, ==, "42 (0x2a)");
+    } else {
+        g_assert_cmpstr(str, ==, "42");
+    }
     g_free(str);
 }
 
@@ -78,8 +95,15 @@ static void test_visitor_out_intList(TestOutputVisitorData *data,
 
     str = string_output_get_string(data->sov);
     g_assert(str != NULL);
-    g_assert_cmpstr(str, ==,
-        "0-1,3-6,9-16,21-22,9223372036854775806-9223372036854775807");
+    if (data->human) {
+        g_assert_cmpstr(str, ==,
+            "0-1,3-6,9-16,21-22,9223372036854775806-9223372036854775807 "
+            "(0x0-0x1,0x3-0x6,0x9-0x10,0x15-0x16,"
+            "0x7ffffffffffffffe-0x7fffffffffffffff)");
+    } else {
+        g_assert_cmpstr(str, ==,
+            "0-1,3-6,9-16,21-22,9223372036854775806-9223372036854775807");
+    }
     g_free(str);
     while (list) {
         intList *tmp2;
@@ -125,6 +149,7 @@ static void test_visitor_out_string(TestOutputVisitorData *data,
                                     const void *unused)
 {
     char *string = (char *) "Q E M U";
+    const char *string_human = "\"Q E M U\"";
     Error *err = NULL;
     char *str;
 
@@ -133,7 +158,11 @@ static void test_visitor_out_string(TestOutputVisitorData *data,
 
     str = string_output_get_string(data->sov);
     g_assert(str != NULL);
-    g_assert_cmpstr(str, ==, string);
+    if (data->human) {
+        g_assert_cmpstr(str, ==, string_human);
+    } else {
+        g_assert_cmpstr(str, ==, string);
+    }
     g_free(str);
 }
 
@@ -150,7 +179,11 @@ static void test_visitor_out_no_string(TestOutputVisitorData *data,
 
     str = string_output_get_string(data->sov);
     g_assert(str != NULL);
-    g_assert_cmpstr(str, ==, "");
+    if (data->human) {
+        g_assert_cmpstr(str, ==, "<null>");
+    } else {
+        g_assert_cmpstr(str, ==, "");
+    }
     g_free(str);
 }
 
@@ -162,12 +195,26 @@ static void test_visitor_out_enum(TestOutputVisitorData *data,
     EnumOne i;
 
     for (i = 0; i < ENUM_ONE_MAX; i++) {
+        char *str_human;
+        int len;
+
         visit_type_EnumOne(data->ov, &i, "unused", &err);
         g_assert(!err);
 
+        len = strlen(EnumOne_lookup[i]) + 2;
+        str_human = g_malloc0(len);
+        str_human[0] = '"';
+        strncpy(str_human + 1, EnumOne_lookup[i], strlen(EnumOne_lookup[i]));
+        str_human[len - 1] = '"';
+
         str = string_output_get_string(data->sov);
         g_assert(str != NULL);
-        g_assert_cmpstr(str, ==, EnumOne_lookup[i]);
+        if (data->human) {
+            g_assert_cmpstr(str, ==, str_human);
+        } else {
+            g_assert_cmpstr(str, ==, EnumOne_lookup[i]);
+        }
+        g_free(str_human);
 	g_free(str);
     }
 }
@@ -186,11 +233,15 @@ static void test_visitor_out_enum_errors(TestOutputVisitorData *data,
     }
 }
 
-static void output_visitor_test_add(const char *testpath,
-                                    TestOutputVisitorData *data,
-                                    void (*test_func)(TestOutputVisitorData *data, const void *user_data))
+static void
+output_visitor_test_add(const char *testpath,
+                        TestOutputVisitorData *data,
+                        void (*test_func)(TestOutputVisitorData *data,
+                                          const void *user_data),
+                        bool human)
 {
-    g_test_add(testpath, TestOutputVisitorData, data, visitor_output_setup,
+    g_test_add(testpath, TestOutputVisitorData, data,
+               human ? visitor_output_setup_human : visitor_output_setup,
                test_func, visitor_output_teardown);
 }
 
@@ -201,21 +252,41 @@ int main(int argc, char **argv)
     g_test_init(&argc, &argv, NULL);
 
     output_visitor_test_add("/string-visitor/output/int",
-                            &out_visitor_data, test_visitor_out_int);
+                            &out_visitor_data, test_visitor_out_int, false);
+    output_visitor_test_add("/string-visitor/output/int",
+                            &out_visitor_data, test_visitor_out_int, true);
+    output_visitor_test_add("/string-visitor/output/bool",
+                            &out_visitor_data, test_visitor_out_bool, false);
     output_visitor_test_add("/string-visitor/output/bool",
-                            &out_visitor_data, test_visitor_out_bool);
+                            &out_visitor_data, test_visitor_out_bool, true);
+    output_visitor_test_add("/string-visitor/output/number",
+                            &out_visitor_data, test_visitor_out_number, false);
     output_visitor_test_add("/string-visitor/output/number",
-                            &out_visitor_data, test_visitor_out_number);
+                            &out_visitor_data, test_visitor_out_number, true);
     output_visitor_test_add("/string-visitor/output/string",
-                            &out_visitor_data, test_visitor_out_string);
+                            &out_visitor_data, test_visitor_out_string, false);
+    output_visitor_test_add("/string-visitor/output/string",
+                            &out_visitor_data, test_visitor_out_string, true);
+    output_visitor_test_add("/string-visitor/output/no-string",
+                            &out_visitor_data, test_visitor_out_no_string,
+                            false);
     output_visitor_test_add("/string-visitor/output/no-string",
-                            &out_visitor_data, test_visitor_out_no_string);
+                            &out_visitor_data, test_visitor_out_no_string,
+                            true);
+    output_visitor_test_add("/string-visitor/output/enum",
+                            &out_visitor_data, test_visitor_out_enum, false);
     output_visitor_test_add("/string-visitor/output/enum",
-                            &out_visitor_data, test_visitor_out_enum);
+                            &out_visitor_data, test_visitor_out_enum, true);
     output_visitor_test_add("/string-visitor/output/enum-errors",
-                            &out_visitor_data, test_visitor_out_enum_errors);
+                            &out_visitor_data, test_visitor_out_enum_errors,
+                            false);
+    output_visitor_test_add("/string-visitor/output/enum-errors",
+                            &out_visitor_data, test_visitor_out_enum_errors,
+                            true);
+    output_visitor_test_add("/string-visitor/output/intList",
+                            &out_visitor_data, test_visitor_out_intList, false);
     output_visitor_test_add("/string-visitor/output/intList",
-                            &out_visitor_data, test_visitor_out_intList);
+                            &out_visitor_data, test_visitor_out_intList, true);
 
     g_test_run();
 
-- 
1.9.3

Re: [Qemu-devel] [PATCH 1/3] fixup! libqemustub: add more stubs for qemu-char

[Nikolay Nikolaev]

[-- Attachment #1: Type: text/plain, Size: 1054 bytes --]

On Fri, Jun 20, 2014 at 8:55 AM, Hu Tao <hutao@cn.fujitsu.com> wrote:

> build stub/qemu-chr-open-spice.o only with CONFIG_SPICE, this
> suppresses the warning when building without CONFIG_SPICE.
>
> Cc: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  stubs/Makefile.objs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> index 2312076..03210ad 100644
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -25,7 +25,7 @@ stub-obj-y += mon-set-error.o
>  stub-obj-y += monitor-init.o
>  stub-obj-y += notify-event.o
>  stub-obj-y += pci-drive-hot-add.o
> -stub-obj-y += qemu-chr-open-spice.o
> +stub-obj-$(CONFIG_SPICE) += qemu-chr-open-spice.o
>  stub-obj-y += qtest.o
>  stub-obj-y += reset.o
>  stub-obj-y += runstate-check.o
> --
> 1.9.3
>
>
I've already send one, based on Andrew Jones proposal:
http://patchwork.ozlabs.org/patch/361926/

But probably this one is better. I'm OK with it.

regards,
Nikolay Nikolaev

[-- Attachment #2: Type: text/html, Size: 1840 bytes --]

Re: [Qemu-devel] [PATCH 1/3] fixup! libqemustub: add more stubs for qemu-char

[Hu Tao]

On Fri, Jun 20, 2014 at 09:03:59AM +0300, Nikolay Nikolaev wrote:
> 
> 
> 
> On Fri, Jun 20, 2014 at 8:55 AM, Hu Tao <hutao@cn.fujitsu.com> wrote:
> 
>     build stub/qemu-chr-open-spice.o only with CONFIG_SPICE, this
>     suppresses the warning when building without CONFIG_SPICE.
> 
>     Cc: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
>     Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
>     ---
>      stubs/Makefile.objs | 2 +-
>      1 file changed, 1 insertion(+), 1 deletion(-)
> 
>     diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
>     index 2312076..03210ad 100644
>     --- a/stubs/Makefile.objs
>     +++ b/stubs/Makefile.objs
>     @@ -25,7 +25,7 @@ stub-obj-y += mon-set-error.o
>      stub-obj-y += monitor-init.o
>      stub-obj-y += notify-event.o
>      stub-obj-y += pci-drive-hot-add.o
>     -stub-obj-y += qemu-chr-open-spice.o
>     +stub-obj-$(CONFIG_SPICE) += qemu-chr-open-spice.o
>      stub-obj-y += qtest.o
>      stub-obj-y += reset.o
>      stub-obj-y += runstate-check.o
>     --
>     1.9.3
> 
> 
> 
> I've already send one, based on Andrew Jones proposal:
> http://patchwork.ozlabs.org/patch/361926/
> 
> But probably this one is better. I'm OK with it.

Didn't see that one. But we need the version check in the stub also,
because in include/ui/qemu-spice.h function qemu_chr_open_spice_port()
is defined if version < 0x000c02, or we can remove the one in
qemu-spice.h.

Regards,
Hu

[Qemu-devel] [PATCH v2] fixup! libqemustub: add more stubs for qemu-char

[Hu Tao]

build stub/qemu-chr-open-spice.o only with CONFIG_SPICE, this
suppresses the warning when building without CONFIG_SPICE.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 stubs/Makefile.objs         | 2 +-
 stubs/qemu-chr-open-spice.c | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

v2:
  - add version check for qemu_chr_open_spice_port()

diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 2312076..03210ad 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -25,7 +25,7 @@ stub-obj-y += mon-set-error.o
 stub-obj-y += monitor-init.o
 stub-obj-y += notify-event.o
 stub-obj-y += pci-drive-hot-add.o
-stub-obj-y += qemu-chr-open-spice.o
+stub-obj-$(CONFIG_SPICE) += qemu-chr-open-spice.o
 stub-obj-y += qtest.o
 stub-obj-y += reset.o
 stub-obj-y += runstate-check.o
diff --git a/stubs/qemu-chr-open-spice.c b/stubs/qemu-chr-open-spice.c
index 40a29a9..f1c4849 100644
--- a/stubs/qemu-chr-open-spice.c
+++ b/stubs/qemu-chr-open-spice.c
@@ -6,7 +6,9 @@ CharDriverState *qemu_chr_open_spice_vmc(const char *type)
     return NULL;
 }
 
+#if SPICE_SERVER_VERSION >= 0x000c02
 CharDriverState *qemu_chr_open_spice_port(const char *name)
 {
     return NULL;
 }
+#endif
-- 
1.9.3

Re: [Qemu-devel] [PATCH 1/3] fixup! libqemustub: add more stubs for qemu-char

[Peter Maydell]

On 20 June 2014 06:55, Hu Tao <hutao@cn.fujitsu.com> wrote:
> build stub/qemu-chr-open-spice.o only with CONFIG_SPICE, this
> suppresses the warning when building without CONFIG_SPICE.

I notice there's been a bunch of these "!fixup" patches on
list recently. Do you think you could add something in the
subject that says which patch queue/maintainer tree they
are fixup patches for, so that I know whether I can ignore them
or not?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/3] fixup! libqemustub: add more stubs for qemu-char

[Hu Tao]

On Fri, Jun 20, 2014 at 09:26:32AM +0100, Peter Maydell wrote:
> On 20 June 2014 06:55, Hu Tao <hutao@cn.fujitsu.com> wrote:
> > build stub/qemu-chr-open-spice.o only with CONFIG_SPICE, this
> > suppresses the warning when building without CONFIG_SPICE.
> 
> I notice there's been a bunch of these "!fixup" patches on
> list recently. Do you think you could add something in the
> subject that says which patch queue/maintainer tree they
> are fixup patches for, so that I know whether I can ignore them
> or not?
> 
> thanks
> -- PMM

Sorry if this brings any nosices. the fixups are for MST's pci tree, see
coverletter.

Regards,
Hu Tao

Re: [Qemu-devel] [PATCH 2/3] fixup! qapi/string-output-visitor: fix bugs

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 990 bytes --]

On 06/19/2014 11:55 PM, Hu Tao wrote:
> "0x1-0x10" looks better than "0x1-10"
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  qapi/string-output-visitor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
> index e9aca3b..1ab8574 100644
> --- a/qapi/string-output-visitor.c
> +++ b/qapi/string-output-visitor.c
> @@ -98,7 +98,7 @@ static void format_string(StringOutputVisitor *sov, Range *r, bool next,
>  {
>      if (r->end - r->begin > 1) {
>          if (human) {
> -            g_string_append_printf(sov->string, "0x%" PRIx64 "-%" PRIx64,
> +            g_string_append_printf(sov->string, "0x%" PRIx64 "-0x%" PRIx64,
>                                     r->begin, r->end - 1);
>  
>          } else {
> 

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 3/3] tests: add human format test for string output visitor

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1100 bytes --]

On 06/19/2014 11:55 PM, Hu Tao wrote:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  tests/test-string-output-visitor.c | 109 ++++++++++++++++++++++++++++++-------
>  1 file changed, 90 insertions(+), 19 deletions(-)
> 

>  
> +        len = strlen(EnumOne_lookup[i]) + 2;
> +        str_human = g_malloc0(len);
> +        str_human[0] = '"';
> +        strncpy(str_human + 1, EnumOne_lookup[i], strlen(EnumOne_lookup[i]));
> +        str_human[len - 1] = '"';

Eww.  Just use g_strdup_printf("\"%s\"", EnumOne_lookup[i]), instead of
futzing around with manual length calculations.


>  
> -static void output_visitor_test_add(const char *testpath,
> -                                    TestOutputVisitorData *data,
> -                                    void (*test_func)(TestOutputVisitorData *data, const void *user_data))
> +static void
> +output_visitor_test_add(const char *testpath,

Why the line split? You moved away from the usual qemu style.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 0/3] another round of pci fixes

[Michael S. Tsirkin]

On Fri, Jun 20, 2014 at 01:55:40PM +0800, Hu Tao wrote:
> Hi Michael,
> 
> This series is on top of your pci tree.
> 
> This series includes two fixups and one patch for adding test of human
> format of string output visitor, please review. thanks!

Please note we can't generally send fixups to patches
for which a pull request was sent.
We had a case recently where pull was rejected and
rebase requested, so that made fixups possible again.



> 
> Hu Tao (3):
>   fixup! libqemustub: add more stubs for qemu-char
>   fixup! qapi/string-output-visitor: fix bugs
>   tests: add human format test for string output visitor
> 
>  qapi/string-output-visitor.c       |   2 +-
>  stubs/Makefile.objs                |   2 +-
>  tests/test-string-output-visitor.c | 109 ++++++++++++++++++++++++++++++-------
>  3 files changed, 92 insertions(+), 21 deletions(-)
> 
> -- 
> 1.9.3

Re: [Qemu-devel] [PATCH 3/3] tests: add human format test for string output visitor

[Michael S. Tsirkin]

On Fri, Jun 20, 2014 at 09:40:31AM -0600, Eric Blake wrote:
> On 06/19/2014 11:55 PM, Hu Tao wrote:
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  tests/test-string-output-visitor.c | 109 ++++++++++++++++++++++++++++++-------
> >  1 file changed, 90 insertions(+), 19 deletions(-)
> > 
> 
> >  
> > +        len = strlen(EnumOne_lookup[i]) + 2;
> > +        str_human = g_malloc0(len);
> > +        str_human[0] = '"';
> > +        strncpy(str_human + 1, EnumOne_lookup[i], strlen(EnumOne_lookup[i]));
> > +        str_human[len - 1] = '"';
> 
> Eww.  Just use g_strdup_printf("\"%s\"", EnumOne_lookup[i]), instead of
> futzing around with manual length calculations.
> 
> 
> >  
> > -static void output_visitor_test_add(const char *testpath,
> > -                                    TestOutputVisitorData *data,
> > -                                    void (*test_func)(TestOutputVisitorData *data, const void *user_data))
> > +static void
> > +output_visitor_test_add(const char *testpath,
> 
> Why the line split? You moved away from the usual qemu style.

We have different styles really, coding style does not say.
And that line's was way too long, so it did need a split.

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

Re: [Qemu-devel] [PATCH 0/3] another round of pci fixes

[Michael S. Tsirkin]

On Fri, Jun 20, 2014 at 01:55:40PM +0800, Hu Tao wrote:
> Hi Michael,
> 
> This series is on top of your pci tree.
> 
> This series includes two fixups and one patch for adding test of human
> format of string output visitor, please review. thanks!

All applied, thanks!

> 
> Hu Tao (3):
>   fixup! libqemustub: add more stubs for qemu-char
>   fixup! qapi/string-output-visitor: fix bugs
>   tests: add human format test for string output visitor
> 
>  qapi/string-output-visitor.c       |   2 +-
>  stubs/Makefile.objs                |   2 +-
>  tests/test-string-output-visitor.c | 109 ++++++++++++++++++++++++++++++-------
>  3 files changed, 92 insertions(+), 21 deletions(-)
> 
> -- 
> 1.9.3