* Fix and tests for -readconfig

* Fixes for changeable block size
 -----BEGIN PGP SIGNATURE-----
 
 iQFIBAABCAAyFiEE8TM4V0tmI4mGbHaCv/vSX3jHroMFAmLxB6wUHHBib256aW5p
 QHJlZGhhdC5jb20ACgkQv/vSX3jHroMbgQgAmj38xh/KurUvjAwlki9OF+jIXem/
 r7eHtc6wG/dGN7CyL+EK08c2GfPIphhii3JGfLT3P/xHvHRVgYow2AELNvYSG85M
 SBQGfDsHLoKvY6Wni3AolECvtGycXkd30RtrzqdTT8iZyIhTTsikEG2hSgE+Z6Yy
 9XGPHN9puTkkD5HYnDV1+T4+yc28F8UTVob3fv9b7LMH7mSYz7UN5Tw4zB7DjOL1
 JNTuYqW9JN1X1vIYKxYw9Y5Jb3qNjnl8y6if8bPuvrLRw94sd0ax23yEQHwtgcUj
 cYGcvZ2/X0SQg4AHTsqGJCuffKNDfHhmmA7w0Xb2DEvBvoveYOaDp3dKZQ==
 =L9FI
 -----END PGP SIGNATURE-----

Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging

* Fix and tests for -readconfig
* Fixes for changeable block size

# -----BEGIN PGP SIGNATURE-----
#
# iQFIBAABCAAyFiEE8TM4V0tmI4mGbHaCv/vSX3jHroMFAmLxB6wUHHBib256aW5p
# QHJlZGhhdC5jb20ACgkQv/vSX3jHroMbgQgAmj38xh/KurUvjAwlki9OF+jIXem/
# r7eHtc6wG/dGN7CyL+EK08c2GfPIphhii3JGfLT3P/xHvHRVgYow2AELNvYSG85M
# SBQGfDsHLoKvY6Wni3AolECvtGycXkd30RtrzqdTT8iZyIhTTsikEG2hSgE+Z6Yy
# 9XGPHN9puTkkD5HYnDV1+T4+yc28F8UTVob3fv9b7LMH7mSYz7UN5Tw4zB7DjOL1
# JNTuYqW9JN1X1vIYKxYw9Y5Jb3qNjnl8y6if8bPuvrLRw94sd0ax23yEQHwtgcUj
# cYGcvZ2/X0SQg4AHTsqGJCuffKNDfHhmmA7w0Xb2DEvBvoveYOaDp3dKZQ==
# =L9FI
# -----END PGP SIGNATURE-----
# gpg: Signature made Mon 08 Aug 2022 05:55:08 AM PDT
# gpg:                using RSA key F13338574B662389866C7682BFFBD25F78C7AE83
# gpg:                issuer "pbonzini@redhat.com"
# gpg: Good signature from "Paolo Bonzini <bonzini@gnu.org>" [undefined]
# gpg:                 aka "Paolo Bonzini <pbonzini@redhat.com>" [undefined]
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg:          There is no indication that the signature belongs to the owner.
# Primary key fingerprint: 46F5 9FBD 57D6 12E7 BFD4  E2F7 7E15 100C CD36 69B1
#      Subkey fingerprint: F133 3857 4B66 2389 866C  7682 BFFB D25F 78C7 AE83

* tag 'for-upstream' of https://gitlab.com/bonzini/qemu:
  tests/qtest: add scenario for -readconfig handling
  vl: remove dead code in parse_memory_options()
  vl: fix [memory] section with -readconfig
  scsi-disk: ensure block size is non-zero and changes limited to bits 8-15
  scsi-disk: fix overflow when block size is not a multiple of BDRV_SECTOR_SIZE

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

hw/scsi/scsi-disk.c

@@ -1591,7 +1591,7 @@ static void scsi_disk_emulate_mode_select(SCSIDiskReq *r, uint8_t *inbuf)
     int cmd = r->req.cmd.buf[0];
     int len = r->req.cmd.xfer;
     int hdr_len = (cmd == MODE_SELECT ? 4 : 8);
-    int bd_len;
+    int bd_len, bs;
     int pass;
 
     if ((r->req.cmd.buf[1] & 0x11) != 0x10) {
@@ -1617,9 +1617,19 @@ static void scsi_disk_emulate_mode_select(SCSIDiskReq *r, uint8_t *inbuf)
     }
 
     /* Allow changing the block size */
-    if (bd_len && p[6] != (s->qdev.blocksize >> 8)) {
+    if (bd_len) {
-        s->qdev.blocksize = p[6] << 8;
+        bs = p[5] << 16 | p[6] << 8 | p[7];
-        trace_scsi_disk_mode_select_set_blocksize(s->qdev.blocksize);
+
+        /*
+         * Since the existing code only checks/updates bits 8-15 of the block
+         * size, restrict ourselves to the same requirement for now to ensure
+         * that a block size set by a block descriptor and then read back by
+         * a subsequent SCSI command will be the same
+         */
+        if (bs && !(bs & ~0xff00) && bs != s->qdev.blocksize) {
+            s->qdev.blocksize = bs;
+            trace_scsi_disk_mode_select_set_blocksize(s->qdev.blocksize);
+        }
     }
 
     len -= bd_len;
@@ -1849,7 +1859,7 @@ static void scsi_disk_emulate_write_same(SCSIDiskReq *r, uint8_t *inbuf)
     uint32_t nb_sectors = scsi_data_cdb_xfer(r->req.cmd.buf);
     WriteSameCBData *data;
     uint8_t *buf;
-    int i;
+    int i, l;
 
     /* Fail if PBDATA=1 or LBDATA=1 or ANCHOR=1.  */
     if (nb_sectors == 0 || (req->cmd.buf[1] & 0x16)) {
@@ -1891,8 +1901,9 @@ static void scsi_disk_emulate_write_same(SCSIDiskReq *r, uint8_t *inbuf)
                                               data->iov.iov_len);
     qemu_iovec_init_external(&data->qiov, &data->iov, 1);
 
-    for (i = 0; i < data->iov.iov_len; i += s->qdev.blocksize) {
+    for (i = 0; i < data->iov.iov_len; i += l) {
-        memcpy(&buf[i], inbuf, s->qdev.blocksize);
+        l = MIN(s->qdev.blocksize, data->iov.iov_len - i);
+        memcpy(&buf[i], inbuf, l);
     }
 
     scsi_req_ref(&r->req);

softmmu/vl.c

@@ -1947,27 +1947,21 @@ static void qemu_resolve_machine_memdev(void)
     }
 }
 
-static void parse_memory_options(const char *arg)
+static void parse_memory_options(void)
 {
-    QemuOpts *opts;
+    QemuOpts *opts = qemu_find_opts_singleton("memory");
     QDict *dict, *prop;
     const char *mem_str;
+    Location loc;
 
-    opts = qemu_opts_parse_noisily(qemu_find_opts("memory"), arg, true);
+    loc_push_none(&loc);
-    if (!opts) {
+    qemu_opts_loc_restore(opts);
-        exit(EXIT_FAILURE);
-    }
 
     prop = qdict_new();
 
     if (qemu_opt_get_size(opts, "size", 0) != 0) {
-        mem_str = qemu_opt_get(opts, "size");
-        if (!*mem_str) {
-            error_report("missing 'size' option value");
-            exit(EXIT_FAILURE);
-        }
-
         /* Fix up legacy suffix-less format */
+        mem_str = qemu_opt_get(opts, "size");
         if (g_ascii_isdigit(mem_str[strlen(mem_str) - 1])) {
             g_autofree char *mib_str = g_strdup_printf("%sM", mem_str);
             qdict_put_str(prop, "size", mib_str);
@@ -1987,6 +1981,7 @@ static void parse_memory_options(const char *arg)
     qdict_put(dict, "memory", prop);
     keyval_merge(machine_opts_dict, dict, &error_fatal);
     qobject_unref(dict);
+    loc_pop(&loc);
 }
 
 static void qemu_create_machine(QDict *qdict)
@@ -2053,8 +2048,7 @@ static bool is_qemuopts_group(const char *group)
     if (g_str_equal(group, "object") ||
         g_str_equal(group, "machine") ||
         g_str_equal(group, "smp-opts") ||
-        g_str_equal(group, "boot-opts") ||
+        g_str_equal(group, "boot-opts")) {
-        g_str_equal(group, "memory")) {
         return false;
     }
     return true;
@@ -2078,8 +2072,6 @@ static void qemu_record_config_group(const char *group, QDict *dict,
         machine_merge_property("smp", dict, &error_fatal);
     } else if (g_str_equal(group, "boot-opts")) {
         machine_merge_property("boot", dict, &error_fatal);
-    } else if (g_str_equal(group, "memory")) {
-        machine_merge_property("memory", dict, &error_fatal);
     } else {
         abort();
     }
@@ -2882,7 +2874,10 @@ void qemu_init(int argc, char **argv, char **envp)
                 exit(0);
                 break;
             case QEMU_OPTION_m:
-                parse_memory_options(optarg);
+                opts = qemu_opts_parse_noisily(qemu_find_opts("memory"), optarg, true);
+                if (opts == NULL) {
+                    exit(1);
+                }
                 break;
 #ifdef CONFIG_TPM
             case QEMU_OPTION_tpmdev:
@@ -3515,6 +3510,9 @@ void qemu_init(int argc, char **argv, char **envp)
 
     configure_rtc(qemu_find_opts_singleton("rtc"));
 
+    /* Transfer QemuOpts options into machine options */
+    parse_memory_options();
+
     qemu_create_machine(machine_opts_dict);
 
     suspend_mux_open();

tests/qtest/meson.build

@@ -26,6 +26,7 @@ qtests_generic = [
   'qom-test',
   'test-hmp',
   'qos-test',
+  'readconfig-test',
 ]
 if config_host.has_key('CONFIG_MODULES')
   qtests_generic += [ 'modules-test' ]

tests/qtest/readconfig-test.c

@@ -0,0 +1,195 @@
+/*
+ * Validate -readconfig
+ *
+ * Copyright (c) 2022 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "qapi/error.h"
+#include "qapi/qapi-visit-machine.h"
+#include "qapi/qapi-visit-qom.h"
+#include "qapi/qapi-visit-ui.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qlist.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qmp/qstring.h"
+#include "qemu/units.h"
+
+static QTestState *qtest_init_with_config(const char *cfgdata)
+{
+    GError *error = NULL;
+    g_autofree char *args = NULL;
+    int cfgfd = -1;
+    g_autofree char *cfgpath = NULL;
+    QTestState *qts;
+    ssize_t ret;
+
+    cfgfd = g_file_open_tmp("readconfig-test-XXXXXX", &cfgpath, &error);
+    g_assert_no_error(error);
+    g_assert_cmpint(cfgfd, >=, 0);
+
+    ret = qemu_write_full(cfgfd, cfgdata, strlen(cfgdata));
+    if (ret < 0) {
+        unlink(cfgpath);
+    }
+    g_assert_cmpint(ret, ==, strlen(cfgdata));
+
+    close(cfgfd);
+
+    args = g_strdup_printf("-nodefaults -machine none -readconfig %s", cfgpath);
+
+    qts = qtest_init(args);
+
+    unlink(cfgpath);
+
+    return qts;
+}
+
+static void test_x86_memdev_resp(QObject *res)
+{
+    Visitor *v;
+    g_autoptr(MemdevList) memdevs = NULL;
+    Memdev *memdev;
+
+    g_assert(res);
+    v = qobject_input_visitor_new(res);
+    visit_type_MemdevList(v, NULL, &memdevs, &error_abort);
+
+    g_assert(memdevs);
+    g_assert(memdevs->value);
+    g_assert(!memdevs->next);
+
+    memdev = memdevs->value;
+    g_assert_cmpstr(memdev->id, ==, "ram");
+    g_assert_cmpint(memdev->size, ==, 200 * MiB);
+
+    visit_free(v);
+}
+
+static void test_x86_memdev(void)
+{
+    QDict *resp;
+    QTestState *qts;
+    const char *cfgdata =
+        "[memory]\n"
+        "size = \"200\"";
+
+    qts = qtest_init_with_config(cfgdata);
+   /* Test valid command */
+    resp = qtest_qmp(qts, "{ 'execute': 'query-memdev' }");
+    test_x86_memdev_resp(qdict_get(resp, "return"));
+    qobject_unref(resp);
+
+    qtest_quit(qts);
+}
+
+
+#ifdef CONFIG_SPICE
+static void test_spice_resp(QObject *res)
+{
+    Visitor *v;
+    g_autoptr(SpiceInfo) spice = NULL;
+
+    g_assert(res);
+    v = qobject_input_visitor_new(res);
+    visit_type_SpiceInfo(v, "spcie", &spice, &error_abort);
+
+    g_assert(spice);
+    g_assert(spice->enabled);
+
+    visit_free(v);
+}
+
+static void test_spice(void)
+{
+    QDict *resp;
+    QTestState *qts;
+    const char *cfgdata =
+        "[spice]\n"
+        "disable-ticketing = \"on\"\n"
+        "unix = \"on\"\n";
+
+    qts = qtest_init_with_config(cfgdata);
+   /* Test valid command */
+    resp = qtest_qmp(qts, "{ 'execute': 'query-spice' }");
+    test_spice_resp(qdict_get(resp, "return"));
+    qobject_unref(resp);
+
+    qtest_quit(qts);
+}
+#endif
+
+static void test_object_rng_resp(QObject *res)
+{
+    Visitor *v;
+    g_autoptr(ObjectPropertyInfoList) objs = NULL;
+    ObjectPropertyInfoList *tmp;
+    ObjectPropertyInfo *obj;
+    bool seen_rng = false;
+
+    g_assert(res);
+    v = qobject_input_visitor_new(res);
+    visit_type_ObjectPropertyInfoList(v, NULL, &objs, &error_abort);
+
+    g_assert(objs);
+    tmp = objs;
+    while (tmp) {
+        g_assert(tmp->value);
+
+        obj = tmp->value;
+        if (g_str_equal(obj->name, "rng0") &&
+            g_str_equal(obj->type, "child<rng-builtin>")) {
+            seen_rng = true;
+        }
+
+        tmp = tmp->next;
+    }
+
+    g_assert(seen_rng);
+
+    visit_free(v);
+}
+
+static void test_object_rng(void)
+{
+    QDict *resp;
+    QTestState *qts;
+    const char *cfgdata =
+        "[object]\n"
+        "qom-type = \"rng-builtin\"\n"
+        "id = \"rng0\"\n";
+
+    qts = qtest_init_with_config(cfgdata);
+   /* Test valid command */
+    resp = qtest_qmp(qts,
+                     "{ 'execute': 'qom-list',"
+                     "  'arguments': {'path': '/objects' }}");
+    test_object_rng_resp(qdict_get(resp, "return"));
+    qobject_unref(resp);
+
+    qtest_quit(qts);
+}
+
+int main(int argc, char *argv[])
+{
+    const char *arch;
+    g_test_init(&argc, &argv, NULL);
+
+    arch = qtest_get_arch();
+
+    if (g_str_equal(arch, "i386") ||
+        g_str_equal(arch, "x86_64")) {
+        qtest_add_func("readconfig/x86/memdev", test_x86_memdev);
+    }
+#ifdef CONFIG_SPICE
+    qtest_add_func("readconfig/spice", test_spice);
+#endif
+
+    qtest_add_func("readconfig/object-rng", test_object_rng);
+
+    return g_test_run();
+}