Skip to contentSkip to conversation

another round of pci fixes

PATCH Superseded Hu Tao 5 participants 14 messages Jun 20, 2014 – Jun 29, 2014

lore
Share & tools
lore

Hu Tao

author
#0 lore raw

cover another round of pci fixes

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(-)
Signature · 1 line
1.9.3
author
#1 lore raw

PATCH 1/3 fixup! libqemustub: add more stubs for qemu-char

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

Cc Nikolay Nikolaev <[email protected]>
Signed-off-by Hu Tao <[email protected]>
---
 stubs/Makefile.objs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
stubs/Makefile.objs +1 -1
index 2312076..03210ad 100644
@@ -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
Signature · 1 line
1.9.3
#2 lore raw

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

Quoted 23 lines · 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 <[email protected]>
Signed-off-by Hu Tao <[email protected]>
---
 stubs/Makefile.objs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
stubs/Makefile.objs +1 -2
index 2312076..03210ad 100644
@@ -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
--
Quoted 3 lines

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

author
#3 lore raw

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

Quoted 32 lines · Nikolay Nikolaev

On Fri, Jun 20, 2014 at 8:55 AM, Hu Tao <[email protected]> wrote:

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

    Cc: Nikolay Nikolaev <[email protected]>
    Signed-off-by: Hu Tao <[email protected]>
    ---
     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
Quoted 4 lines

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

author
#4 lore raw

PATCH v2 1/1 fixup! libqemustub: add more stubs for qemu-char

In reply to 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 <[email protected]>
---
 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()

stubs/Makefile.objs +1 -1
index 2312076..03210ad 100644
@@ -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
stubs/qemu-chr-open-spice.c +2 -0
index 40a29a9..f1c4849 100644
@@ -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
Signature · 1 line
1.9.3
#5 lore raw

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

In reply to Hu Tao
Quoted 2 lines · Hu Tao

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

author
#6 lore raw

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

Quoted 4 lines · Peter Maydell
Quoted 2 lines · Hu Tao

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

Quoted 8 lines

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

author
#7 lore raw

PATCH 2/3 fixup! qapi/string-output-visitor: fix bugs

In reply to Hu Tao

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

Signed-off-by Hu Tao <[email protected]>
---
 qapi/string-output-visitor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
qapi/string-output-visitor.c +1 -1
index e9aca3b..1ab8574 100644
@@ -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 {
Signature · 1 line
1.9.3
Reviewed-by on v1 2/3
#8 lore raw

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

Quoted 6 lines · Hu Tao

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

Signed-off-by Hu Tao <[email protected]>
---
 qapi/string-output-visitor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by Eric Blake <[email protected]>
Quoted 15 lines
qapi/string-output-visitor.c +1 -1
index e9aca3b..1ab8574 100644
@@ -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 {
Signature · 2 lines
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org
author
#9 lore raw

PATCH 3/3 tests: add human format test for string output visitor

In reply to Hu Tao
Signed-off-by Hu Tao <[email protected]>
---
 tests/test-string-output-visitor.c | 109 ++++++++++++++++++++++++++++++-------
 1 file changed, 90 insertions(+), 19 deletions(-)
tests/test-string-output-visitor.c +90 -19
index 28e7359..e89e43c 100644
@@ -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();
Signature · 1 line
1.9.3
#10 lore raw

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

tests/test-string-output-visitor.c static void test_visitor_out_enum(TestOutputVisitorData *data, · line 208

Quoted 12 lines · Hu Tao
Signed-off-by Hu Tao <[email protected]>
---
 tests/test-string-output-visitor.c | 109 ++++++++++++++++++++++++++++++-------
 1 file changed, 90 insertions(+), 19 deletions(-)
tests/test-string-output-visitor.c +5 -0
+        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.

tests/test-string-output-visitor.c static void test_visitor_out_enum_errors(TestOutputVisitorData *data, · line 237

Quoted 6 lines
tests/test-string-output-visitor.c +2 -3
-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.

Signature · 2 lines
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org
#11 lore raw

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

Quoted 19 lines · Eric Blake

tests/test-string-output-visitor.c static void test_visitor_out_enum(TestOutputVisitorData *data, · line 208

Quoted 12 lines · Hu Tao
Signed-off-by Hu Tao <[email protected]>
---
 tests/test-string-output-visitor.c | 109 ++++++++++++++++++++++++++++++-------
 1 file changed, 90 insertions(+), 19 deletions(-)
tests/test-string-output-visitor.c +5 -0
+        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.

Quoted 1 line
Quoted 7 lines

tests/test-string-output-visitor.c static void test_visitor_out_enum_errors(TestOutputVisitorData *data, · line 237

Quoted 5 lines
tests/test-string-output-visitor.c +2 -3
-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.

Quoted 4 lines
Signature · 2 lines
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org
#12 lore raw
In reply to Hu Tao
Quoted 6 lines · 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!

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.

Quoted 13 lines

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(-)
Signature · 1 line
1.9.3
#13 lore raw
In reply to Hu Tao
Quoted 6 lines · 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!

All applied, thanks!

Quoted 13 lines

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(-)
Signature · 1 line
1.9.3