Merge remote-tracking branch 'remotes/armbru/tags/pull-tests-2018-08-…

…16' into staging

Testing patches for 2018-08-16

# gpg: Signature made Thu 16 Aug 2018 09:34:43 BST
# gpg:                using RSA key 3870B400EB918653
# gpg: Good signature from "Markus Armbruster <armbru@redhat.com>"
# gpg:                 aka "Markus Armbruster <armbru@pond.sub.org>"
# Primary key fingerprint: 354B C8B3 D7EB 2A6B 6867  4E5F 3870 B400 EB91 8653

* remotes/armbru/tags/pull-tests-2018-08-16: (25 commits)
  libqtest: Improve error reporting for bad read from QEMU
  tests/libqtest: Improve kill_qemu()
  libqtest: Rename qtest_FOOv() to qtest_vFOO() for consistency
  libqtest: Replace qtest_startf() by qtest_initf()
  libqtest: Enable compile-time format string checking
  migration-test: Clean up string interpolation into QMP, part 3
  migration-test: Clean up string interpolation into QMP, part 2
  migration-test: Clean up string interpolation into QMP, part 1
  migration-test: Make wait_command() cope with '%'
  tests: New helper qtest_qmp_receive_success()
  migration-test: Make wait_command() return the "return" member
  tests: Clean up string interpolation around qtest_qmp_device_add()
  cpu-plug-test: Don't pass integers as strings to device_add
  tests: Clean up string interpolation into QMP input (simple cases)
  tests: Pass literal format strings directly to qmp_FOO()
  qobject: qobject_from_jsonv() is dangerous, hide it away
  test-qobject-input-visitor: Avoid format string ambiguity
  libqtest: Simplify qmp_fd_vsend() a bit
  qobject: New qobject_from_vjsonf_nofail(), qdict_from_vjsonf_nofail()
  qobject: Replace qobject_from_jsonf() by qobject_from_jsonf_nofail()
  ...

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

include/qapi/qmp/qjson.h

@@ -15,11 +15,15 @@
 #define QJSON_H
 
 QObject *qobject_from_json(const char *string, Error **errp);
-QObject *qobject_from_jsonf(const char *string, ...) GCC_FMT_ATTR(1, 2);
-QObject *qobject_from_jsonv(const char *string, va_list *ap, Error **errp)
-    GCC_FMT_ATTR(1, 0);
 
-QDict *qdict_from_jsonf_nofail(const char *string, ...) GCC_FMT_ATTR(1, 2);
+QObject *qobject_from_vjsonf_nofail(const char *string, va_list ap)
+    GCC_FMT_ATTR(1, 0);
+QObject *qobject_from_jsonf_nofail(const char *string, ...)
+    GCC_FMT_ATTR(1, 2);
+QDict *qdict_from_vjsonf_nofail(const char *string, va_list ap)
+    GCC_FMT_ATTR(1, 0);
+QDict *qdict_from_jsonf_nofail(const char *string, ...)
+    GCC_FMT_ATTR(1, 2);
 
 QString *qobject_to_json(const QObject *obj);
 QString *qobject_to_json_pretty(const QObject *obj);

qobject/qjson.c

@@ -39,7 +39,18 @@ static void parse_json(JSONMessageParser *parser, GQueue *tokens)
     s->result = json_parser_parse_err(tokens, s->ap, &s->err);
 }
 
-QObject *qobject_from_jsonv(const char *string, va_list *ap, Error **errp)
+/*
+ * Parse @string as JSON value.
+ * If @ap is non-null, interpolate %-escapes.
+ * Takes ownership of %p arguments.
+ * On success, return the JSON value.
+ * On failure, store an error through @errp and return NULL.
+ * Ownership of %p arguments becomes indeterminate then.  To avoid
+ * leaks, callers passing %p must terminate on error, e.g. by passing
+ * &error_abort.
+ */
+static QObject *qobject_from_jsonv(const char *string, va_list *ap,
+                                   Error **errp)
 {
     JSONParsingState state = {};
 
@@ -59,34 +70,70 @@ QObject *qobject_from_json(const char *string, Error **errp)
     return qobject_from_jsonv(string, NULL, errp);
 }
 
-QObject *qobject_from_jsonf(const char *string, ...)
+/*
+ * Parse @string as JSON value with %-escapes interpolated.
+ * Abort on error.  Do not use with untrusted @string.
+ * Return the resulting QObject.  It is never null.
+ */
+QObject *qobject_from_vjsonf_nofail(const char *string, va_list ap)
+{
+    va_list ap_copy;
+    QObject *obj;
+
+    /* va_copy() is needed when va_list is an array type */
+    va_copy(ap_copy, ap);
+    obj = qobject_from_jsonv(string, &ap_copy, &error_abort);
+    va_end(ap_copy);
+
+    assert(obj);
+    return obj;
+}
+
+/*
+ * Parse @string as JSON value with %-escapes interpolated.
+ * Abort on error.  Do not use with untrusted @string.
+ * Return the resulting QObject.  It is never null.
+ */
+QObject *qobject_from_jsonf_nofail(const char *string, ...)
 {
     QObject *obj;
     va_list ap;
 
     va_start(ap, string);
-    obj = qobject_from_jsonv(string, &ap, &error_abort);
+    obj = qobject_from_vjsonf_nofail(string, ap);
     va_end(ap);
 
     return obj;
 }
 
+/*
+ * Parse @string as JSON object with %-escapes interpolated.
+ * Abort on error.  Do not use with untrusted @string.
+ * Return the resulting QDict.  It is never null.
+ */
+QDict *qdict_from_vjsonf_nofail(const char *string, va_list ap)
+{
+    QDict *qdict;
+
+    qdict = qobject_to(QDict, qobject_from_vjsonf_nofail(string, ap));
+    assert(qdict);
+    return qdict;
+}
+
 /*
  * Parse @string as JSON object with %-escapes interpolated.
  * Abort on error.  Do not use with untrusted @string.
  * Return the resulting QDict.  It is never null.
  */
 QDict *qdict_from_jsonf_nofail(const char *string, ...)
 {
-    QDict *obj;
+    QDict *qdict;
     va_list ap;
 
     va_start(ap, string);
-    obj = qobject_to(QDict, qobject_from_jsonv(string, &ap, &error_abort));
+    qdict = qdict_from_vjsonf_nofail(string, ap);
     va_end(ap);
-
-    assert(obj);
-    return obj;
+    return qdict;
 }
 
 typedef struct ToJsonIterState

tests/ahci-test.c

@@ -37,6 +37,9 @@
 #include "hw/pci/pci_ids.h"
 #include "hw/pci/pci_regs.h"
 
+/* TODO actually test the results and get rid of this */
+#define qmp_discard_response(...) qobject_unref(qmp(__VA_ARGS__))
+
 /* Test images sizes in MB */
 #define TEST_IMAGE_SIZE_MB_LARGE (200 * 1024)
 #define TEST_IMAGE_SIZE_MB_SMALL 64
@@ -1352,7 +1355,6 @@ static void test_flush_migrate(void)
     AHCIQState *src, *dst;
     AHCICommand *cmd;
     uint8_t px;
-    const char *s;
     char *uri = g_strdup_printf("unix:%s", mig_socket);
 
     prepare_blkdebug_script(debug_path, "flush_to_disk");
@@ -1388,8 +1390,7 @@ static void test_flush_migrate(void)
     ahci_migrate(src, dst, uri);
 
     /* Complete the command */
-    s = "{'execute':'cont' }";
-    qmp_async(s);
+    qmp_send("{'execute':'cont' }");
     qmp_eventwait("RESUME");
     ahci_command_wait(dst, cmd);
     ahci_command_verify(dst, cmd);
@@ -1592,8 +1593,8 @@ static void test_atapi_tray(void)
     atapi_wait_tray(false);
 
     /* Remove media */
-    qmp_async("{'execute': 'blockdev-open-tray', "
-               "'arguments': {'id': 'cd0'}}");
+    qmp_send("{'execute': 'blockdev-open-tray',"
+             " 'arguments': {'id': 'cd0'}}");
     atapi_wait_tray(true);
     rsp = qmp_receive();
     qobject_unref(rsp);
@@ -1619,8 +1620,8 @@ static void test_atapi_tray(void)
                                          "'node-name': 'node0' }}");
 
     /* Again, the event shows up first */
-    qmp_async("{'execute': 'blockdev-close-tray', "
-               "'arguments': {'id': 'cd0'}}");
+    qmp_send("{'execute': 'blockdev-close-tray',"
+             " 'arguments': {'id': 'cd0'}}");
     atapi_wait_tray(false);
     rsp = qmp_receive();
     qobject_unref(rsp);

tests/boot-order-test.c

@@ -13,9 +13,12 @@
 #include "qemu/osdep.h"
 #include "libqos/fw_cfg.h"
 #include "libqtest.h"
-
+#include "qapi/qmp/qdict.h"
 #include "hw/nvram/fw_cfg_keys.h"
 
+/* TODO actually test the results and get rid of this */
+#define qmp_discard_response(...) qobject_unref(qmp(__VA_ARGS__))
+
 typedef struct {
     const char *args;
     uint64_t expected_boot;
@@ -30,10 +33,10 @@ static void test_a_boot_order(const char *machine,
 {
     uint64_t actual;
 
-    global_qtest = qtest_startf("-nodefaults%s%s %s",
-                                machine ? " -M " : "",
-                                machine ?: "",
-                                test_args);
+    global_qtest = qtest_initf("-nodefaults%s%s %s",
+                               machine ? " -M " : "",
+                               machine ?: "",
+                               test_args);
     actual = read_boot_order();
     g_assert_cmphex(actual, ==, expected_boot);
     qmp_discard_response("{ 'execute': 'system_reset' }");

tests/boot-serial-test.c

@@ -172,11 +172,11 @@ static void test_machine(const void *data)
      * Make sure that this test uses tcg if available: It is used as a
      * fast-enough smoketest for that.
      */
-    global_qtest = qtest_startf("%s %s -M %s,accel=tcg:kvm "
-                                "-chardev file,id=serial0,path=%s "
-                                "-no-shutdown -serial chardev:serial0 %s",
-                                codeparam, code ? codetmp : "",
-                                test->machine, serialtmp, test->extra);
+    global_qtest = qtest_initf("%s %s -M %s,accel=tcg:kvm "
+                               "-chardev file,id=serial0,path=%s "
+                               "-no-shutdown -serial chardev:serial0 %s",
+                               codeparam, code ? codetmp : "",
+                               test->machine, serialtmp, test->extra);
     if (code) {
         unlink(codetmp);
     }

tests/cdrom-test.c

@@ -99,7 +99,7 @@ static void test_cdrom_param(gconstpointer data)
     QTestState *qts;
     char *resp;
 
-    qts = qtest_startf("-M %s -cdrom %s", (const char *)data, isoimage);
+    qts = qtest_initf("-M %s -cdrom %s", (const char *)data, isoimage);
     resp = qtest_hmp(qts, "info block");
     g_assert(strstr(resp, isoimage) != 0);
     g_free(resp);
@@ -120,8 +120,8 @@ static void test_cdboot(gconstpointer data)
 {
     QTestState *qts;
 
-    qts = qtest_startf("-accel kvm:tcg -no-shutdown %s%s", (const char *)data,
-                       isoimage);
+    qts = qtest_initf("-accel kvm:tcg -no-shutdown %s%s", (const char *)data,
+                      isoimage);
     boot_sector_test(qts);
     qtest_quit(qts);
 }

tests/check-qjson.c

@@ -865,7 +865,8 @@ static void vararg_string(void)
         QString *str;
 
         str = qobject_to(QString,
-                         qobject_from_jsonf("%s", test_cases[i].decoded));
+                         qobject_from_jsonf_nofail("%s",
+                                                   test_cases[i].decoded));
         g_assert(str);
         g_assert(strcmp(qstring_get_str(str), test_cases[i].decoded) == 0);
 
@@ -998,17 +999,17 @@ static void vararg_number(void)
     double valuef = 2.323423423;
     int64_t val;
 
-    qnum = qobject_to(QNum, qobject_from_jsonf("%d", value));
+    qnum = qobject_to(QNum, qobject_from_jsonf_nofail("%d", value));
     g_assert(qnum_get_try_int(qnum, &val));
     g_assert_cmpint(val, ==, value);
     qobject_unref(qnum);
 
-    qnum = qobject_to(QNum, qobject_from_jsonf("%lld", value_ll));
+    qnum = qobject_to(QNum, qobject_from_jsonf_nofail("%lld", value_ll));
     g_assert(qnum_get_try_int(qnum, &val));
     g_assert_cmpint(val, ==, value_ll);
     qobject_unref(qnum);
 
-    qnum = qobject_to(QNum, qobject_from_jsonf("%f", valuef));
+    qnum = qobject_to(QNum, qobject_from_jsonf_nofail("%f", valuef));
     g_assert(qnum_get_double(qnum) == valuef);
     qobject_unref(qnum);
 }
@@ -1042,13 +1043,13 @@ static void keyword_literal(void)
 
     qobject_unref(qbool);
 
-    qbool = qobject_to(QBool, qobject_from_jsonf("%i", false));
+    qbool = qobject_to(QBool, qobject_from_jsonf_nofail("%i", false));
     g_assert(qbool);
     g_assert(qbool_get_bool(qbool) == false);
     qobject_unref(qbool);
 
     /* Test that non-zero values other than 1 get collapsed to true */
-    qbool = qobject_to(QBool, qobject_from_jsonf("%i", 2));
+    qbool = qobject_to(QBool, qobject_from_jsonf_nofail("%i", 2));
     g_assert(qbool);
     g_assert(qbool_get_bool(qbool) == true);
     qobject_unref(qbool);
@@ -1298,7 +1299,7 @@ static void simple_varargs(void)
     embedded_obj = qobject_from_json("[32, 42]", &error_abort);
     g_assert(embedded_obj != NULL);
 
-    obj = qobject_from_jsonf("[%d, 2, %p]", 1, embedded_obj);
+    obj = qobject_from_jsonf_nofail("[%d, 2, %p]", 1, embedded_obj);
     g_assert(qlit_equal_qobject(&decoded, obj));
 
     qobject_unref(obj);

tests/cpu-plug-test.c

@@ -88,8 +88,9 @@ static void test_plug_with_device_add_x86(gconstpointer data)
         for (c = 0; c < td->cores; c++) {
             for (t = 0; t < td->threads; t++) {
                 char *id = g_strdup_printf("id-%i-%i-%i", s, c, t);
-                qtest_qmp_device_add(td->device_model, id, "'socket-id':'%i', "
-                                     "'core-id':'%i', 'thread-id':'%i'",
+                qtest_qmp_device_add(td->device_model, id,
+                                     "{'socket-id':%u, 'core-id':%u,"
+                                     " 'thread-id':%u}",
                                      s, c, t);
                 g_free(id);
             }
@@ -114,7 +115,7 @@ static void test_plug_with_device_add_coreid(gconstpointer data)
 
     for (c = td->cores; c < td->maxcpus / td->sockets / td->threads; c++) {
         char *id = g_strdup_printf("id-%i", c);
-        qtest_qmp_device_add(td->device_model, id, "'core-id':'%i'", c);
+        qtest_qmp_device_add(td->device_model, id, "{'core-id':%u}", c);
         g_free(id);
     }
 

tests/drive_del-test.c

@@ -15,6 +15,9 @@
 #include "libqos/virtio.h"
 #include "qapi/qmp/qdict.h"
 
+/* TODO actually test the results and get rid of this */
+#define qmp_discard_response(...) qobject_unref(qmp(__VA_ARGS__))
+
 static void drive_add(void)
 {
     char *resp = hmp("drive_add 0 if=none,id=drive0");

tests/e1000e-test.c

@@ -456,12 +456,10 @@ static void test_e1000e_multiple_transfers(gconstpointer data)
 
 static void test_e1000e_hotplug(gconstpointer data)
 {
-    static const uint8_t slot = 0x06;
-
     qtest_start("-device e1000e");
 
-    qpci_plug_device_test("e1000e", "e1000e_net", slot, NULL);
-    qpci_unplug_acpi_device_test("e1000e_net", slot);
+    qtest_qmp_device_add("e1000e", "e1000e_net", "{'addr': '0x06'}");
+    qpci_unplug_acpi_device_test("e1000e_net", 0x06);
 
     qtest_end();
 }

tests/endianness-test.c

@@ -115,10 +115,10 @@ static void test_endianness(gconstpointer data)
 {
     const TestCase *test = data;
 
-    global_qtest = qtest_startf("-M %s%s%s -device pc-testdev",
-                                test->machine,
-                                test->superio ? " -device " : "",
-                                test->superio ?: "");
+    global_qtest = qtest_initf("-M %s%s%s -device pc-testdev",
+                               test->machine,
+                               test->superio ? " -device " : "",
+                               test->superio ?: "");
     isa_outl(test, 0xe0, 0x87654321);
     g_assert_cmphex(isa_inl(test, 0xe0), ==, 0x87654321);
     g_assert_cmphex(isa_inw(test, 0xe2), ==, 0x8765);
@@ -187,10 +187,10 @@ static void test_endianness_split(gconstpointer data)
 {
     const TestCase *test = data;
 
-    global_qtest = qtest_startf("-M %s%s%s -device pc-testdev",
-                                test->machine,
-                                test->superio ? " -device " : "",
-                                test->superio ?: "");
+    global_qtest = qtest_initf("-M %s%s%s -device pc-testdev",
+                               test->machine,
+                               test->superio ? " -device " : "",
+                               test->superio ?: "");
     isa_outl(test, 0xe8, 0x87654321);
     g_assert_cmphex(isa_inl(test, 0xe0), ==, 0x87654321);
     g_assert_cmphex(isa_inw(test, 0xe2), ==, 0x8765);
@@ -231,10 +231,10 @@ static void test_endianness_combine(gconstpointer data)
 {
     const TestCase *test = data;
 
-    global_qtest = qtest_startf("-M %s%s%s -device pc-testdev",
-                                test->machine,
-                                test->superio ? " -device " : "",
-                                test->superio ?: "");
+    global_qtest = qtest_initf("-M %s%s%s -device pc-testdev",
+                               test->machine,
+                               test->superio ? " -device " : "",
+                               test->superio ?: "");
     isa_outl(test, 0xe0, 0x87654321);
     g_assert_cmphex(isa_inl(test, 0xe8), ==, 0x87654321);
     g_assert_cmphex(isa_inw(test, 0xea), ==, 0x8765);

tests/fdc-test.c

@@ -26,8 +26,12 @@
 
 
 #include "libqtest.h"
+#include "qapi/qmp/qdict.h"
 #include "qemu-common.h"
 
+/* TODO actually test the results and get rid of this */
+#define qmp_discard_response(...) qobject_unref(qmp(__VA_ARGS__))
+
 #define TEST_IMAGE_SIZE 1440 * 1024
 
 #define FLOPPY_BASE 0x3f0