[Qemu-devel] [PATCH for-2.3 v2 0/4] qtest: Fix remaining test paths to include architecture

[Qemu-devel] [PATCH for-2.3 v2 0/4] qtest: Fix remaining test paths to include architecture

[Andreas Färber]

Hello,

This mini-series fixes the last remaining GTester paths to indicate the
architecture they are executed for (i386 vs. x86_64).

Unlike the previous fw_cfg-test patch, there are no libqtest wrappers adding the
architecture yet; other tests manually constructed a correct path to pass to
g_test_add_data_func(). For convenience, add wrappers for g_test_add_data_func()
and g_test_add().

Based on "[PATCH for-2.3] fw_cfg-test: Fix test path to include architecture".

Regards,
Andreas

v1 -> v2:
* Added a qtest_add() wrapper macro (John, Paolo).
* Made data argument of qtest_add_data_func() const.
* Converted remaining QTest users of g_test_add_data_func() to
  qtest_add_data_func() (John).

Andreas Färber (4):
  qtest: Add qtest_add_data_func() wrapper function
  qtest: Add qtest_add() wrapper macro
  i440fx-test: Fix test paths to include architecture
  tests: Use qtest_add_data_func() consistently

 tests/ahci-test.c       |  9 ++-------
 tests/e1000-test.c      |  4 ++--
 tests/eepro100-test.c   |  5 ++---
 tests/endianness-test.c | 18 +++++++++---------
 tests/i440fx-test.c     | 12 ++++++------
 tests/libqtest.c        |  7 +++++++
 tests/libqtest.h        | 32 ++++++++++++++++++++++++++++++++
 tests/pc-cpu-test.c     | 13 ++++++-------
 tests/qom-test.c        |  4 ++--
 9 files changed, 68 insertions(+), 36 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH for-2.3 v2 1/4] qtest: Add qtest_add_data_func() wrapper function

[Andreas Färber]

It calls g_test_add_data_func() with a path supplemented by the
architecture, like qtest_add_func() does.

Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 tests/libqtest.c |  7 +++++++
 tests/libqtest.h | 12 ++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 9a92aa7..12d65bd 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -652,6 +652,13 @@ void qtest_add_func(const char *str, void (*fn))
     g_free(path);
 }
 
+void qtest_add_data_func(const char *str, const void *data, void (*fn))
+{
+    gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
+    g_test_add_data_func(path, data, fn);
+    g_free(path);
+}
+
 void qtest_memwrite(QTestState *s, uint64_t addr, const void *data, size_t size)
 {
     const uint8_t *ptr = data;
diff --git a/tests/libqtest.h b/tests/libqtest.h
index e7413d5..9281f5c 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -345,6 +345,18 @@ const char *qtest_get_arch(void);
 void qtest_add_func(const char *str, void (*fn));
 
 /**
+ * qtest_add_data_func:
+ * @str: Test case path.
+ * @data: Test case data
+ * @fn: Test case function
+ *
+ * Add a GTester testcase with the given name, data and function.
+ * The path is prefixed with the architecture under test, as
+ * returned by qtest_get_arch().
+ */
+void qtest_add_data_func(const char *str, const void *data, void (*fn));
+
+/**
  * qtest_start:
  * @args: other arguments to pass to QEMU
  *
-- 
2.1.4

[Qemu-devel] [PATCH for-2.3 v2 2/4] qtest: Add qtest_add() wrapper macro

[Andreas Färber]

It extends g_test_add() macro with the architecture path.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 tests/libqtest.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/tests/libqtest.h b/tests/libqtest.h
index 9281f5c..03469b8 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -357,6 +357,26 @@ void qtest_add_func(const char *str, void (*fn));
 void qtest_add_data_func(const char *str, const void *data, void (*fn));
 
 /**
+ * qtest_add:
+ * @testpath: Test case path
+ * @Fixture: Fixture type
+ * @tdata: Test case data
+ * @fsetup: Test case setup function
+ * @ftest: Test case function
+ * @fteardown: Test case teardown function
+ *
+ * Add a GTester testcase with the given name, data and functions.
+ * The path is prefixed with the architecture under test, as
+ * returned by qtest_get_arch().
+ */
+#define qtest_add(testpath, Fixture, tdata, fsetup, ftest, fteardown) \
+    do { \
+        char *path = g_strdup_printf("/%s/%s", qtest_get_arch(), testpath); \
+        g_test_add(path, Fixture, tdata, fsetup, ftest, fteardown); \
+        g_free(path); \
+    } while (0)
+
+/**
  * qtest_start:
  * @args: other arguments to pass to QEMU
  *
-- 
2.1.4

[Qemu-devel] [PATCH for-2.3 v2 3/4] i440fx-test: Fix test paths to include architecture

[Andreas Färber]

Replace g_test_add_func() with new qtest_add_func() and g_test_add()
macro with qtest_add() macro.

Cc: qemu-stable@nongnu.org
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 tests/i440fx-test.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/i440fx-test.c b/tests/i440fx-test.c
index a3f7279..d0bc8de 100644
--- a/tests/i440fx-test.c
+++ b/tests/i440fx-test.c
@@ -383,8 +383,8 @@ static void add_firmware_test(const char *testpath,
                               void (*setup_fixture)(FirmwareTestFixture *f,
                                                     gconstpointer test_data))
 {
-    g_test_add(testpath, FirmwareTestFixture, NULL, setup_fixture,
-               test_i440fx_firmware, NULL);
+    qtest_add(testpath, FirmwareTestFixture, NULL, setup_fixture,
+              test_i440fx_firmware, NULL);
 }
 
 static void request_bios(FirmwareTestFixture *fixture,
@@ -408,10 +408,10 @@ int main(int argc, char **argv)
 
     data.num_cpus = 1;
 
-    g_test_add_data_func("/i440fx/defaults", &data, test_i440fx_defaults);
-    g_test_add_data_func("/i440fx/pam", &data, test_i440fx_pam);
-    add_firmware_test("/i440fx/firmware/bios", request_bios);
-    add_firmware_test("/i440fx/firmware/pflash", request_pflash);
+    qtest_add_data_func("i440fx/defaults", &data, test_i440fx_defaults);
+    qtest_add_data_func("i440fx/pam", &data, test_i440fx_pam);
+    add_firmware_test("i440fx/firmware/bios", request_bios);
+    add_firmware_test("i440fx/firmware/pflash", request_pflash);
 
     ret = g_test_run();
     return ret;
-- 
2.1.4

[Qemu-devel] [PATCH v2 4/4] tests: Use qtest_add_data_func() consistently

[Andreas Färber]

Replace uses of g_test_add_data_func() for QTest test cases.

It is still valid to use it for any non-QTest test cases,
which are not run for multiple target binaries.

Suggested-by: John Snow <jsnow@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 tests/ahci-test.c       |  9 ++-------
 tests/e1000-test.c      |  4 ++--
 tests/eepro100-test.c   |  5 ++---
 tests/endianness-test.c | 18 +++++++++---------
 tests/pc-cpu-test.c     | 13 ++++++-------
 tests/qom-test.c        |  4 ++--
 6 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index ea62e24..4daa96e 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -1045,7 +1045,6 @@ static void test_io_interface(gconstpointer opaque)
 static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
                                 enum BuffLen len)
 {
-    static const char *arch;
     char *name;
     AHCIIOTestOptions *opts = g_malloc(sizeof(AHCIIOTestOptions));
 
@@ -1053,16 +1052,12 @@ static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
     opts->address_type = addr;
     opts->io_type = type;
 
-    if (!arch) {
-        arch = qtest_get_arch();
-    }
-
-    name = g_strdup_printf("/%s/ahci/io/%s/%s/%s", arch,
+    name = g_strdup_printf("ahci/io/%s/%s/%s",
                            io_mode_str[type],
                            addr_mode_str[addr],
                            buff_len_str[len]);
 
-    g_test_add_data_func(name, opts, test_io_interface);
+    qtest_add_data_func(name, opts, test_io_interface);
     g_free(name);
 }
 
diff --git a/tests/e1000-test.c b/tests/e1000-test.c
index 81f164d..7ca6d7e 100644
--- a/tests/e1000-test.c
+++ b/tests/e1000-test.c
@@ -44,8 +44,8 @@ int main(int argc, char **argv)
     for (i = 0; i < ARRAY_SIZE(models); i++) {
         char *path;
 
-        path = g_strdup_printf("/%s/e1000/%s", qtest_get_arch(), models[i]);
-        g_test_add_data_func(path, models[i], test_device);
+        path = g_strdup_printf("e1000/%s", models[i]);
+        qtest_add_data_func(path, models[i], test_device);
     }
 
     return g_test_run();
diff --git a/tests/eepro100-test.c b/tests/eepro100-test.c
index bf82526..8bfaccd 100644
--- a/tests/eepro100-test.c
+++ b/tests/eepro100-test.c
@@ -54,9 +54,8 @@ int main(int argc, char **argv)
     for (i = 0; i < ARRAY_SIZE(models); i++) {
         char *path;
 
-        path = g_strdup_printf("/%s/eepro100/%s",
-                               qtest_get_arch(), models[i]);
-        g_test_add_data_func(path, models[i], test_device);
+        path = g_strdup_printf("eepro100/%s", models[i]);
+        qtest_add_data_func(path, models[i], test_device);
     }
 
     return g_test_run();
diff --git a/tests/endianness-test.c b/tests/endianness-test.c
index 92e17d2..7482ff7 100644
--- a/tests/endianness-test.c
+++ b/tests/endianness-test.c
@@ -298,17 +298,17 @@ int main(int argc, char **argv)
         if (strcmp(test_cases[i].arch, arch) != 0) {
             continue;
         }
-        path = g_strdup_printf("/%s/endianness/%s",
-                               arch, test_cases[i].machine);
-        g_test_add_data_func(path, &test_cases[i], test_endianness);
+        path = g_strdup_printf("endianness/%s",
+                               test_cases[i].machine);
+        qtest_add_data_func(path, &test_cases[i], test_endianness);
 
-        path = g_strdup_printf("/%s/endianness/split/%s",
-                               arch, test_cases[i].machine);
-        g_test_add_data_func(path, &test_cases[i], test_endianness_split);
+        path = g_strdup_printf("endianness/split/%s",
+                               test_cases[i].machine);
+        qtest_add_data_func(path, &test_cases[i], test_endianness_split);
 
-        path = g_strdup_printf("/%s/endianness/combine/%s",
-                               arch, test_cases[i].machine);
-        g_test_add_data_func(path, &test_cases[i], test_endianness_combine);
+        path = g_strdup_printf("endianness/combine/%s",
+                               test_cases[i].machine);
+        qtest_add_data_func(path, &test_cases[i], test_endianness_combine);
     }
 
     ret = g_test_run();
diff --git a/tests/pc-cpu-test.c b/tests/pc-cpu-test.c
index a0122d3..3505c7c 100644
--- a/tests/pc-cpu-test.c
+++ b/tests/pc-cpu-test.c
@@ -75,7 +75,6 @@ static void test_pc_without_cpu_add(gconstpointer data)
 
 static void add_pc_test_cases(void)
 {
-    const char *arch = qtest_get_arch();
     QDict *response, *minfo;
     QList *list;
     const QListEntry *p;
@@ -119,15 +118,15 @@ static void add_pc_test_cases(void)
             (strcmp(mname, "pc-0.12") == 0) ||
             (strcmp(mname, "pc-0.11") == 0) ||
             (strcmp(mname, "pc-0.10") == 0)) {
-            path = g_strdup_printf("/%s/cpu/%s/init/%ux%ux%u&maxcpus=%u",
-                                   arch, mname, data->sockets, data->cores,
+            path = g_strdup_printf("cpu/%s/init/%ux%ux%u&maxcpus=%u",
+                                   mname, data->sockets, data->cores,
                                    data->threads, data->maxcpus);
-            g_test_add_data_func(path, data, test_pc_without_cpu_add);
+            qtest_add_data_func(path, data, test_pc_without_cpu_add);
         } else {
-            path = g_strdup_printf("/%s/cpu/%s/add/%ux%ux%u&maxcpus=%u",
-                                   arch, mname, data->sockets, data->cores,
+            path = g_strdup_printf("cpu/%s/add/%ux%ux%u&maxcpus=%u",
+                                   mname, data->sockets, data->cores,
                                    data->threads, data->maxcpus);
-            g_test_add_data_func(path, data, test_pc_with_cpu_add);
+            qtest_add_data_func(path, data, test_pc_with_cpu_add);
         }
     }
     qtest_end();
diff --git a/tests/qom-test.c b/tests/qom-test.c
index 4246382..fde04e7 100644
--- a/tests/qom-test.c
+++ b/tests/qom-test.c
@@ -128,8 +128,8 @@ static void add_machine_test_cases(void)
         g_assert(qstr);
         mname = qstring_get_str(qstr);
         if (!is_blacklisted(arch, mname)) {
-            path = g_strdup_printf("/%s/qom/%s", arch, mname);
-            g_test_add_data_func(path, mname, test_machine);
+            path = g_strdup_printf("qom/%s", mname);
+            qtest_add_data_func(path, mname, test_machine);
         }
     }
     qtest_end();
-- 
2.1.4

Re: [Qemu-devel] [PATCH for-2.3 v2 2/4] qtest: Add qtest_add() wrapper macro

[John Snow]

On 03/25/2015 02:20 PM, Andreas Färber wrote:
> It extends g_test_add() macro with the architecture path.
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>   tests/libqtest.h | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
>
> diff --git a/tests/libqtest.h b/tests/libqtest.h
> index 9281f5c..03469b8 100644
> --- a/tests/libqtest.h
> +++ b/tests/libqtest.h
> @@ -357,6 +357,26 @@ void qtest_add_func(const char *str, void (*fn));
>   void qtest_add_data_func(const char *str, const void *data, void (*fn));
>
>   /**
> + * qtest_add:
> + * @testpath: Test case path
> + * @Fixture: Fixture type
> + * @tdata: Test case data
> + * @fsetup: Test case setup function
> + * @ftest: Test case function
> + * @fteardown: Test case teardown function
> + *
> + * Add a GTester testcase with the given name, data and functions.
> + * The path is prefixed with the architecture under test, as
> + * returned by qtest_get_arch().
> + */
> +#define qtest_add(testpath, Fixture, tdata, fsetup, ftest, fteardown) \
> +    do { \
> +        char *path = g_strdup_printf("/%s/%s", qtest_get_arch(), testpath); \
> +        g_test_add(path, Fixture, tdata, fsetup, ftest, fteardown); \
> +        g_free(path); \
> +    } while (0)
> +
> +/**
>    * qtest_start:
>    * @args: other arguments to pass to QEMU
>    *
>

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [Qemu-devel] [PATCH for-2.3 v2 3/4] i440fx-test: Fix test paths to include architecture

[John Snow]

On 03/25/2015 02:20 PM, Andreas Färber wrote:
> Replace g_test_add_func() with new qtest_add_func() and g_test_add()
> macro with qtest_add() macro.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>   tests/i440fx-test.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tests/i440fx-test.c b/tests/i440fx-test.c
> index a3f7279..d0bc8de 100644
> --- a/tests/i440fx-test.c
> +++ b/tests/i440fx-test.c
> @@ -383,8 +383,8 @@ static void add_firmware_test(const char *testpath,
>                                 void (*setup_fixture)(FirmwareTestFixture *f,
>                                                       gconstpointer test_data))
>   {
> -    g_test_add(testpath, FirmwareTestFixture, NULL, setup_fixture,
> -               test_i440fx_firmware, NULL);
> +    qtest_add(testpath, FirmwareTestFixture, NULL, setup_fixture,
> +              test_i440fx_firmware, NULL);
>   }
>
>   static void request_bios(FirmwareTestFixture *fixture,
> @@ -408,10 +408,10 @@ int main(int argc, char **argv)
>
>       data.num_cpus = 1;
>
> -    g_test_add_data_func("/i440fx/defaults", &data, test_i440fx_defaults);
> -    g_test_add_data_func("/i440fx/pam", &data, test_i440fx_pam);
> -    add_firmware_test("/i440fx/firmware/bios", request_bios);
> -    add_firmware_test("/i440fx/firmware/pflash", request_pflash);
> +    qtest_add_data_func("i440fx/defaults", &data, test_i440fx_defaults);
> +    qtest_add_data_func("i440fx/pam", &data, test_i440fx_pam);
> +    add_firmware_test("i440fx/firmware/bios", request_bios);
> +    add_firmware_test("i440fx/firmware/pflash", request_pflash);
>
>       ret = g_test_run();
>       return ret;
>

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [Qemu-devel] [PATCH v2 4/4] tests: Use qtest_add_data_func() consistently

[John Snow]

On 03/25/2015 02:20 PM, Andreas Färber wrote:
> Replace uses of g_test_add_data_func() for QTest test cases.
>
> It is still valid to use it for any non-QTest test cases,
> which are not run for multiple target binaries.
>
> Suggested-by: John Snow <jsnow@redhat.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>   tests/ahci-test.c       |  9 ++-------
>   tests/e1000-test.c      |  4 ++--
>   tests/eepro100-test.c   |  5 ++---
>   tests/endianness-test.c | 18 +++++++++---------
>   tests/pc-cpu-test.c     | 13 ++++++-------
>   tests/qom-test.c        |  4 ++--
>   6 files changed, 23 insertions(+), 30 deletions(-)
>
> diff --git a/tests/ahci-test.c b/tests/ahci-test.c
> index ea62e24..4daa96e 100644
> --- a/tests/ahci-test.c
> +++ b/tests/ahci-test.c
> @@ -1045,7 +1045,6 @@ static void test_io_interface(gconstpointer opaque)
>   static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
>                                   enum BuffLen len)
>   {
> -    static const char *arch;
>       char *name;
>       AHCIIOTestOptions *opts = g_malloc(sizeof(AHCIIOTestOptions));
>
> @@ -1053,16 +1052,12 @@ static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
>       opts->address_type = addr;
>       opts->io_type = type;
>
> -    if (!arch) {
> -        arch = qtest_get_arch();
> -    }
> -
> -    name = g_strdup_printf("/%s/ahci/io/%s/%s/%s", arch,
> +    name = g_strdup_printf("ahci/io/%s/%s/%s",
>                              io_mode_str[type],
>                              addr_mode_str[addr],
>                              buff_len_str[len]);
>
> -    g_test_add_data_func(name, opts, test_io_interface);
> +    qtest_add_data_func(name, opts, test_io_interface);
>       g_free(name);
>   }
>
> diff --git a/tests/e1000-test.c b/tests/e1000-test.c
> index 81f164d..7ca6d7e 100644
> --- a/tests/e1000-test.c
> +++ b/tests/e1000-test.c
> @@ -44,8 +44,8 @@ int main(int argc, char **argv)
>       for (i = 0; i < ARRAY_SIZE(models); i++) {
>           char *path;
>
> -        path = g_strdup_printf("/%s/e1000/%s", qtest_get_arch(), models[i]);
> -        g_test_add_data_func(path, models[i], test_device);
> +        path = g_strdup_printf("e1000/%s", models[i]);
> +        qtest_add_data_func(path, models[i], test_device);
>       }
>
>       return g_test_run();
> diff --git a/tests/eepro100-test.c b/tests/eepro100-test.c
> index bf82526..8bfaccd 100644
> --- a/tests/eepro100-test.c
> +++ b/tests/eepro100-test.c
> @@ -54,9 +54,8 @@ int main(int argc, char **argv)
>       for (i = 0; i < ARRAY_SIZE(models); i++) {
>           char *path;
>
> -        path = g_strdup_printf("/%s/eepro100/%s",
> -                               qtest_get_arch(), models[i]);
> -        g_test_add_data_func(path, models[i], test_device);
> +        path = g_strdup_printf("eepro100/%s", models[i]);
> +        qtest_add_data_func(path, models[i], test_device);
>       }
>
>       return g_test_run();
> diff --git a/tests/endianness-test.c b/tests/endianness-test.c
> index 92e17d2..7482ff7 100644
> --- a/tests/endianness-test.c
> +++ b/tests/endianness-test.c
> @@ -298,17 +298,17 @@ int main(int argc, char **argv)
>           if (strcmp(test_cases[i].arch, arch) != 0) {
>               continue;
>           }
> -        path = g_strdup_printf("/%s/endianness/%s",
> -                               arch, test_cases[i].machine);
> -        g_test_add_data_func(path, &test_cases[i], test_endianness);
> +        path = g_strdup_printf("endianness/%s",
> +                               test_cases[i].machine);
> +        qtest_add_data_func(path, &test_cases[i], test_endianness);
>
> -        path = g_strdup_printf("/%s/endianness/split/%s",
> -                               arch, test_cases[i].machine);
> -        g_test_add_data_func(path, &test_cases[i], test_endianness_split);
> +        path = g_strdup_printf("endianness/split/%s",
> +                               test_cases[i].machine);
> +        qtest_add_data_func(path, &test_cases[i], test_endianness_split);
>
> -        path = g_strdup_printf("/%s/endianness/combine/%s",
> -                               arch, test_cases[i].machine);
> -        g_test_add_data_func(path, &test_cases[i], test_endianness_combine);
> +        path = g_strdup_printf("endianness/combine/%s",
> +                               test_cases[i].machine);
> +        qtest_add_data_func(path, &test_cases[i], test_endianness_combine);
>       }
>
>       ret = g_test_run();
> diff --git a/tests/pc-cpu-test.c b/tests/pc-cpu-test.c
> index a0122d3..3505c7c 100644
> --- a/tests/pc-cpu-test.c
> +++ b/tests/pc-cpu-test.c
> @@ -75,7 +75,6 @@ static void test_pc_without_cpu_add(gconstpointer data)
>
>   static void add_pc_test_cases(void)
>   {
> -    const char *arch = qtest_get_arch();
>       QDict *response, *minfo;
>       QList *list;
>       const QListEntry *p;
> @@ -119,15 +118,15 @@ static void add_pc_test_cases(void)
>               (strcmp(mname, "pc-0.12") == 0) ||
>               (strcmp(mname, "pc-0.11") == 0) ||
>               (strcmp(mname, "pc-0.10") == 0)) {
> -            path = g_strdup_printf("/%s/cpu/%s/init/%ux%ux%u&maxcpus=%u",
> -                                   arch, mname, data->sockets, data->cores,
> +            path = g_strdup_printf("cpu/%s/init/%ux%ux%u&maxcpus=%u",
> +                                   mname, data->sockets, data->cores,
>                                      data->threads, data->maxcpus);
> -            g_test_add_data_func(path, data, test_pc_without_cpu_add);
> +            qtest_add_data_func(path, data, test_pc_without_cpu_add);
>           } else {
> -            path = g_strdup_printf("/%s/cpu/%s/add/%ux%ux%u&maxcpus=%u",
> -                                   arch, mname, data->sockets, data->cores,
> +            path = g_strdup_printf("cpu/%s/add/%ux%ux%u&maxcpus=%u",
> +                                   mname, data->sockets, data->cores,
>                                      data->threads, data->maxcpus);
> -            g_test_add_data_func(path, data, test_pc_with_cpu_add);
> +            qtest_add_data_func(path, data, test_pc_with_cpu_add);
>           }
>       }
>       qtest_end();
> diff --git a/tests/qom-test.c b/tests/qom-test.c
> index 4246382..fde04e7 100644
> --- a/tests/qom-test.c
> +++ b/tests/qom-test.c
> @@ -128,8 +128,8 @@ static void add_machine_test_cases(void)
>           g_assert(qstr);
>           mname = qstring_get_str(qstr);
>           if (!is_blacklisted(arch, mname)) {
> -            path = g_strdup_printf("/%s/qom/%s", arch, mname);
> -            g_test_add_data_func(path, mname, test_machine);
> +            path = g_strdup_printf("qom/%s", mname);
> +            qtest_add_data_func(path, mname, test_machine);
>           }
>       }
>       qtest_end();
>

Seems fine to me. The time lost with the nested printfs during test 
initialization is likely not worth crying over in the glorious name of 
consistency.

((Biased.))

Also, what happened to the subject of this mail? Are only patches 1-3 
for-2.3?

All the same:

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [Qemu-devel] [PATCH v2 4/4] tests: Use qtest_add_data_func() consistently

[Andreas Färber]

Am 25.03.2015 um 23:14 schrieb John Snow:
> On 03/25/2015 02:20 PM, Andreas Färber wrote:
>> Replace uses of g_test_add_data_func() for QTest test cases.
>>
>> It is still valid to use it for any non-QTest test cases,
>> which are not run for multiple target binaries.
>>
>> Suggested-by: John Snow <jsnow@redhat.com>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
>>   tests/ahci-test.c       |  9 ++-------
>>   tests/e1000-test.c      |  4 ++--
>>   tests/eepro100-test.c   |  5 ++---
>>   tests/endianness-test.c | 18 +++++++++---------
>>   tests/pc-cpu-test.c     | 13 ++++++-------
>>   tests/qom-test.c        |  4 ++--
>>   6 files changed, 23 insertions(+), 30 deletions(-)
[...]
> Seems fine to me. The time lost with the nested printfs during test
> initialization is likely not worth crying over in the glorious name of
> consistency.
> 
> ((Biased.))
> 
> Also, what happened to the subject of this mail? Are only patches 1-3
> for-2.3?

Yes, I tend to be conservative during the Hard Freeze and 4/4 is not
fixing a bug or improving test coverage. I don't think it would harm,
but I don't push for it. Opinions?

> All the same:
> 
> Reviewed-by: John Snow <jsnow@redhat.com>

Thanks,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH v2 4/4] tests: Use qtest_add_data_func() consistently

[John Snow]

On 03/26/2015 11:41 AM, Andreas Färber wrote:
> Am 25.03.2015 um 23:14 schrieb John Snow:
>> On 03/25/2015 02:20 PM, Andreas Färber wrote:
>>> Replace uses of g_test_add_data_func() for QTest test cases.
>>>
>>> It is still valid to use it for any non-QTest test cases,
>>> which are not run for multiple target binaries.
>>>
>>> Suggested-by: John Snow <jsnow@redhat.com>
>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>> ---
>>>    tests/ahci-test.c       |  9 ++-------
>>>    tests/e1000-test.c      |  4 ++--
>>>    tests/eepro100-test.c   |  5 ++---
>>>    tests/endianness-test.c | 18 +++++++++---------
>>>    tests/pc-cpu-test.c     | 13 ++++++-------
>>>    tests/qom-test.c        |  4 ++--
>>>    6 files changed, 23 insertions(+), 30 deletions(-)
> [...]
>> Seems fine to me. The time lost with the nested printfs during test
>> initialization is likely not worth crying over in the glorious name of
>> consistency.
>>
>> ((Biased.))
>>
>> Also, what happened to the subject of this mail? Are only patches 1-3
>> for-2.3?
>
> Yes, I tend to be conservative during the Hard Freeze and 4/4 is not
> fixing a bug or improving test coverage. I don't think it would harm,
> but I don't push for it. Opinions?
>

Playing it safe is totally fine by me, I was just curious.
My R-b stands.

Thank you,
--John

>> All the same:
>>
>> Reviewed-by: John Snow <jsnow@redhat.com>
>
> Thanks,
> Andreas
>

Re: [Qemu-devel] [PATCH v2 4/4] tests: Use qtest_add_data_func() consistently

[Andreas Färber]

Am 27.03.2015 um 19:46 schrieb John Snow:
> On 03/26/2015 11:41 AM, Andreas Färber wrote:
>> Am 25.03.2015 um 23:14 schrieb John Snow:
>>> On 03/25/2015 02:20 PM, Andreas Färber wrote:
>>>> Replace uses of g_test_add_data_func() for QTest test cases.
>>>>
>>>> It is still valid to use it for any non-QTest test cases,
>>>> which are not run for multiple target binaries.
>>>>
>>>> Suggested-by: John Snow <jsnow@redhat.com>
>>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>>> ---
>>>>    tests/ahci-test.c       |  9 ++-------
>>>>    tests/e1000-test.c      |  4 ++--
>>>>    tests/eepro100-test.c   |  5 ++---
>>>>    tests/endianness-test.c | 18 +++++++++---------
>>>>    tests/pc-cpu-test.c     | 13 ++++++-------
>>>>    tests/qom-test.c        |  4 ++--
>>>>    6 files changed, 23 insertions(+), 30 deletions(-)
>> [...]
>>> Seems fine to me. The time lost with the nested printfs during test
>>> initialization is likely not worth crying over in the glorious name of
>>> consistency.
>>>
>>> ((Biased.))
>>>
>>> Also, what happened to the subject of this mail? Are only patches 1-3
>>> for-2.3?
>>
>> Yes, I tend to be conservative during the Hard Freeze and 4/4 is not
>> fixing a bug or improving test coverage. I don't think it would harm,
>> but I don't push for it. Opinions?
>>
> 
> Playing it safe is totally fine by me, I was just curious.
> My R-b stands.
> 
> Thank you,
> --John
> 
>>> All the same:
>>>
>>> Reviewed-by: John Snow <jsnow@redhat.com>

John, I've rebased this to apply on top of your fourth ahci-test
argument and applied it to qom-next now:
https://github.com/afaerber/qemu-cpu/commits/qom-next

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH v2 4/4] tests: Use qtest_add_data_func() consistently

[John Snow]

On 05/19/2015 08:35 AM, Andreas Färber wrote:
> Am 27.03.2015 um 19:46 schrieb John Snow:
>> On 03/26/2015 11:41 AM, Andreas Färber wrote:
>>> Am 25.03.2015 um 23:14 schrieb John Snow:
>>>> On 03/25/2015 02:20 PM, Andreas Färber wrote:
>>>>> Replace uses of g_test_add_data_func() for QTest test cases.
>>>>>
>>>>> It is still valid to use it for any non-QTest test cases,
>>>>> which are not run for multiple target binaries.
>>>>>
>>>>> Suggested-by: John Snow <jsnow@redhat.com>
>>>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>>>> ---
>>>>>    tests/ahci-test.c       |  9 ++-------
>>>>>    tests/e1000-test.c      |  4 ++--
>>>>>    tests/eepro100-test.c   |  5 ++---
>>>>>    tests/endianness-test.c | 18 +++++++++---------
>>>>>    tests/pc-cpu-test.c     | 13 ++++++-------
>>>>>    tests/qom-test.c        |  4 ++--
>>>>>    6 files changed, 23 insertions(+), 30 deletions(-)
>>> [...]
>>>> Seems fine to me. The time lost with the nested printfs during test
>>>> initialization is likely not worth crying over in the glorious name of
>>>> consistency.
>>>>
>>>> ((Biased.))
>>>>
>>>> Also, what happened to the subject of this mail? Are only patches 1-3
>>>> for-2.3?
>>>
>>> Yes, I tend to be conservative during the Hard Freeze and 4/4 is not
>>> fixing a bug or improving test coverage. I don't think it would harm,
>>> but I don't push for it. Opinions?
>>>
>>
>> Playing it safe is totally fine by me, I was just curious.
>> My R-b stands.
>>
>> Thank you,
>> --John
>>
>>>> All the same:
>>>>
>>>> Reviewed-by: John Snow <jsnow@redhat.com>
> 
> John, I've rebased this to apply on top of your fourth ahci-test
> argument and applied it to qom-next now:
> https://github.com/afaerber/qemu-cpu/commits/qom-next
> 
> Regards,
> Andreas
> 

Oh yes, I forgot about this. Thanks!

I don't have a crazy large queue of ahci tests at the moment, so you
aren't hurting anything. :)

--js