All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 00/13] Misc bugfix patches for 2021-06-04
@ 2021-06-04 15:17 Paolo Bonzini
  2021-06-04 15:17 ` [PULL 01/13] meson: allow optional dependencies for block modules Paolo Bonzini
                   ` (13 more replies)
  0 siblings, 14 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-06-04 15:17 UTC (permalink / raw)
  To: qemu-devel

The following changes since commit 5a95f5ce3cd5842cc8f61a91ecd4fb4e8d10104f:

  Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-fpu-20210603' into staging (2021-06-04 10:04:11 +0100)

are available in the Git repository at:

  https://gitlab.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 49e987695a1873a769a823604f9065aa88e00c55:

  vl: plug -object back into -readconfig (2021-06-04 13:50:04 +0200)

----------------------------------------------------------------
* OpenBSD cleanup (Brad)
* fixes for the i386 accel/cpu refactoring (Claudio)
* unmap test for emulated SCSI (Kit)
* fix for iscsi module (myself)
* fix for -readconfig of objects (myself)
* fixes for x86 16-bit task switching (myself)
* fix for x86 MOV from/to CR8 (Richard)

----------------------------------------------------------------
Brad Smith (1):
      oslib-posix: Remove OpenBSD workaround for fcntl("/dev/null", F_SETFL, O_NONBLOCK) failure

Claudio Fontana (2):
      i386: reorder call to cpu_exec_realizefn
      i386: run accel_cpu_instance_init as post_init

Kit Westneat (1):
      tests/qtest/virtio-scsi-test: add unmap large LBA with 4k blocks test

Paolo Bonzini (8):
      meson: allow optional dependencies for block modules
      iscsi: link libm into the module
      target/i386: tcg: fix segment register offsets for 16-bit TSS
      target/i386: tcg: fix loading of registers from 16-bit TSS
      target/i386: tcg: fix switching from 16-bit to 32-bit tasks or vice versa
      qemu-config: parse configuration files to a QDict
      vl: plumb keyval-based options into -readconfig
      vl: plug -object back into -readconfig

Richard Henderson (1):
      target/i386: Fix decode of cr8

 block/meson.build              |  18 +++----
 include/block/qdict.h          |   2 -
 include/qapi/qmp/qdict.h       |   3 ++
 include/qemu/config-file.h     |   7 ++-
 meson.build                    |   4 +-
 softmmu/vl.c                   | 105 +++++++++++++++++++++++++++++------------
 target/i386/cpu.c              |  89 ++++++++++++++++++++++------------
 target/i386/kvm/kvm-cpu.c      |  12 ++++-
 target/i386/tcg/seg_helper.c   |  31 ++++++------
 target/i386/tcg/translate.c    |   1 +
 tests/qtest/virtio-scsi-test.c |  51 ++++++++++++++++++++
 util/oslib-posix.c             |  11 -----
 util/qemu-config.c             |  98 ++++++++++++++++++++++++++------------
 13 files changed, 298 insertions(+), 134 deletions(-)
-- 
2.31.1



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PULL 01/13] meson: allow optional dependencies for block modules
  2021-06-04 15:17 [PULL 00/13] Misc bugfix patches for 2021-06-04 Paolo Bonzini
@ 2021-06-04 15:17 ` Paolo Bonzini
  2021-06-04 15:17 ` [PULL 02/13] iscsi: link libm into the module Paolo Bonzini
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-06-04 15:17 UTC (permalink / raw)
  To: qemu-devel

Right now all dependencies for block modules are passed to
module_ss.add(when: ...), so they are mandatory.  In the next patch we
will need to add a libm dependency to a module, but libm does not exist
on all systems.  So, modify the creation of module_ss and modsrc so that
dependencies can also be passed to module_ss.add(if_true: ...).

While touching the array, remove the useless dependency of the curl
module on glib.  glib is always linked in QEMU and in fact all other
block modules also need it, but they don't have to specify it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/meson.build | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/block/meson.build b/block/meson.build
index e687c54dbc..9e3388f633 100644
--- a/block/meson.build
+++ b/block/meson.build
@@ -71,19 +71,19 @@ block_modules = {}
 
 modsrc = []
 foreach m : [
-  [curl, 'curl', [curl, glib], 'curl.c'],
-  [glusterfs, 'gluster', glusterfs, 'gluster.c'],
-  [libiscsi, 'iscsi', libiscsi, 'iscsi.c'],
-  [libnfs, 'nfs', libnfs, 'nfs.c'],
-  [libssh, 'ssh', libssh, 'ssh.c'],
-  [rbd, 'rbd', rbd, 'rbd.c'],
+  [curl, 'curl', files('curl.c')],
+  [glusterfs, 'gluster', files('gluster.c')],
+  [libiscsi, 'iscsi', files('iscsi.c')],
+  [libnfs, 'nfs', files('nfs.c')],
+  [libssh, 'ssh', files('ssh.c')],
+  [rbd, 'rbd', files('rbd.c')],
 ]
   if m[0].found()
+    module_ss = ss.source_set()
+    module_ss.add(when: m[0], if_true: m[2])
     if enable_modules
-      modsrc += files(m[3])
+      modsrc += module_ss.all_sources()
     endif
-    module_ss = ss.source_set()
-    module_ss.add(when: m[2], if_true: files(m[3]))
     block_modules += {m[1] : module_ss}
   endif
 endforeach
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 02/13] iscsi: link libm into the module
  2021-06-04 15:17 [PULL 00/13] Misc bugfix patches for 2021-06-04 Paolo Bonzini
  2021-06-04 15:17 ` [PULL 01/13] meson: allow optional dependencies for block modules Paolo Bonzini
@ 2021-06-04 15:17 ` Paolo Bonzini
  2021-06-04 15:17 ` [PULL 03/13] oslib-posix: Remove OpenBSD workaround for fcntl("/dev/null", F_SETFL, O_NONBLOCK) failure Paolo Bonzini
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-06-04 15:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Yi Sun

Depending on the configuration of QEMU, some binaries might not need libm
at all.  In that case libiscsi, which uses exp(), will fail to load.
Link it in the module explicitly.

Reported-by: Yi Sun <yisun@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/meson.build | 2 +-
 meson.build       | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/meson.build b/block/meson.build
index 9e3388f633..01861e1545 100644
--- a/block/meson.build
+++ b/block/meson.build
@@ -73,7 +73,7 @@ modsrc = []
 foreach m : [
   [curl, 'curl', files('curl.c')],
   [glusterfs, 'gluster', files('gluster.c')],
-  [libiscsi, 'iscsi', files('iscsi.c')],
+  [libiscsi, 'iscsi', [files('iscsi.c'), libm]],
   [libnfs, 'nfs', files('nfs.c')],
   [libssh, 'ssh', files('ssh.c')],
   [rbd, 'rbd', files('rbd.c')],
diff --git a/meson.build b/meson.build
index a45f1a844f..913cf2a41a 100644
--- a/meson.build
+++ b/meson.build
@@ -163,7 +163,7 @@ if targetos != 'linux' and get_option('multiprocess').enabled()
 endif
 multiprocess_allowed = targetos == 'linux' and not get_option('multiprocess').disabled()
 
-m = cc.find_library('m', required: false)
+libm = cc.find_library('m', required: false)
 util = cc.find_library('util', required: false)
 winmm = []
 socket = []
@@ -1899,7 +1899,7 @@ util_ss.add_all(trace_ss)
 util_ss = util_ss.apply(config_all, strict: false)
 libqemuutil = static_library('qemuutil',
                              sources: util_ss.sources() + stub_ss.sources() + genh,
-                             dependencies: [util_ss.dependencies(), m, glib, socket, malloc, pixman])
+                             dependencies: [util_ss.dependencies(), libm, glib, socket, malloc, pixman])
 qemuutil = declare_dependency(link_with: libqemuutil,
                               sources: genh + version_res)
 
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 03/13] oslib-posix: Remove OpenBSD workaround for fcntl("/dev/null", F_SETFL, O_NONBLOCK) failure
  2021-06-04 15:17 [PULL 00/13] Misc bugfix patches for 2021-06-04 Paolo Bonzini
  2021-06-04 15:17 ` [PULL 01/13] meson: allow optional dependencies for block modules Paolo Bonzini
  2021-06-04 15:17 ` [PULL 02/13] iscsi: link libm into the module Paolo Bonzini
@ 2021-06-04 15:17 ` Paolo Bonzini
  2021-06-04 15:17 ` [PULL 04/13] target/i386: tcg: fix segment register offsets for 16-bit TSS Paolo Bonzini
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-06-04 15:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Brad Smith

From: Brad Smith <brad@comstyle.com>

OpenBSD prior to 6.3 required a workaround to utilize fcntl(F_SETFL) on memory
devices.

Since modern verions of OpenBSD that are only officialy supported and buildable
on do not have this issue I am garbage collecting this workaround.

Signed-off-by: Brad Smith <brad@comstyle.com>

Message-Id: <YGYECGXQhdamEJgC@humpty.home.comstyle.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 util/oslib-posix.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 36820fec16..7b4bec1402 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -273,17 +273,6 @@ int qemu_try_set_nonblock(int fd)
         return -errno;
     }
     if (fcntl(fd, F_SETFL, f | O_NONBLOCK) == -1) {
-#ifdef __OpenBSD__
-        /*
-         * Previous to OpenBSD 6.3, fcntl(F_SETFL) is not permitted on
-         * memory devices and sets errno to ENODEV.
-         * It's OK if we fail to set O_NONBLOCK on devices like /dev/null,
-         * because they will never block anyway.
-         */
-        if (errno == ENODEV) {
-            return 0;
-        }
-#endif
         return -errno;
     }
     return 0;
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 04/13] target/i386: tcg: fix segment register offsets for 16-bit TSS
  2021-06-04 15:17 [PULL 00/13] Misc bugfix patches for 2021-06-04 Paolo Bonzini
                   ` (2 preceding siblings ...)
  2021-06-04 15:17 ` [PULL 03/13] oslib-posix: Remove OpenBSD workaround for fcntl("/dev/null", F_SETFL, O_NONBLOCK) failure Paolo Bonzini
@ 2021-06-04 15:17 ` Paolo Bonzini
  2021-06-04 15:17 ` [PULL 05/13] target/i386: tcg: fix loading of registers from " Paolo Bonzini
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-06-04 15:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell

The TSS offsets in the manuals have only 2-byte slots for the
segment registers.  QEMU incorrectly uses 4-byte slots, so
that SS overlaps the LDT selector.

Resolves: #382
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/tcg/seg_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index 2f6cdc8239..547b959689 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -281,7 +281,7 @@ static void switch_tss_ra(CPUX86State *env, int tss_selector,
                                              retaddr) | 0xffff0000;
         }
         for (i = 0; i < 4; i++) {
-            new_segs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x22 + i * 4),
+            new_segs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x22 + i * 2),
                                              retaddr);
         }
         new_ldt = cpu_lduw_kernel_ra(env, tss_base + 0x2a, retaddr);
@@ -349,7 +349,7 @@ static void switch_tss_ra(CPUX86State *env, int tss_selector,
         cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 6 * 2), env->regs[R_ESI], retaddr);
         cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 7 * 2), env->regs[R_EDI], retaddr);
         for (i = 0; i < 4; i++) {
-            cpu_stw_kernel_ra(env, env->tr.base + (0x22 + i * 4),
+            cpu_stw_kernel_ra(env, env->tr.base + (0x22 + i * 2),
                               env->segs[i].selector, retaddr);
         }
     }
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 05/13] target/i386: tcg: fix loading of registers from 16-bit TSS
  2021-06-04 15:17 [PULL 00/13] Misc bugfix patches for 2021-06-04 Paolo Bonzini
                   ` (3 preceding siblings ...)
  2021-06-04 15:17 ` [PULL 04/13] target/i386: tcg: fix segment register offsets for 16-bit TSS Paolo Bonzini
@ 2021-06-04 15:17 ` Paolo Bonzini
  2021-06-04 15:17 ` [PULL 06/13] target/i386: tcg: fix switching from 16-bit to 32-bit tasks or vice versa Paolo Bonzini
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-06-04 15:17 UTC (permalink / raw)
  To: qemu-devel

According to the manual, the high 16-bit of the registers are preserved
when switching to a 16-bit task.  Implement this in switch_tss_ra.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/tcg/seg_helper.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index 547b959689..2112c5fc51 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -277,8 +277,7 @@ static void switch_tss_ra(CPUX86State *env, int tss_selector,
         new_eip = cpu_lduw_kernel_ra(env, tss_base + 0x0e, retaddr);
         new_eflags = cpu_lduw_kernel_ra(env, tss_base + 0x10, retaddr);
         for (i = 0; i < 8; i++) {
-            new_regs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x12 + i * 2),
-                                             retaddr) | 0xffff0000;
+            new_regs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x12 + i * 2), retaddr);
         }
         for (i = 0; i < 4; i++) {
             new_segs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x22 + i * 2),
@@ -391,19 +390,17 @@ static void switch_tss_ra(CPUX86State *env, int tss_selector,
     env->eip = new_eip;
     eflags_mask = TF_MASK | AC_MASK | ID_MASK |
         IF_MASK | IOPL_MASK | VM_MASK | RF_MASK | NT_MASK;
-    if (!(type & 8)) {
-        eflags_mask &= 0xffff;
+    if (type & 8) {
+        cpu_load_eflags(env, new_eflags, eflags_mask);
+        for (i = 0; i < 8; i++) {
+            env->regs[i] = new_regs[i];
+        }
+    } else {
+        cpu_load_eflags(env, new_eflags, eflags_mask & 0xffff);
+        for (i = 0; i < 8; i++) {
+            env->regs[i] = (env->regs[i] & 0xffff0000) | new_regs[i];
+        }
     }
-    cpu_load_eflags(env, new_eflags, eflags_mask);
-    /* XXX: what to do in 16 bit case? */
-    env->regs[R_EAX] = new_regs[0];
-    env->regs[R_ECX] = new_regs[1];
-    env->regs[R_EDX] = new_regs[2];
-    env->regs[R_EBX] = new_regs[3];
-    env->regs[R_ESP] = new_regs[4];
-    env->regs[R_EBP] = new_regs[5];
-    env->regs[R_ESI] = new_regs[6];
-    env->regs[R_EDI] = new_regs[7];
     if (new_eflags & VM_MASK) {
         for (i = 0; i < 6; i++) {
             load_seg_vm(env, i, new_segs[i]);
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 06/13] target/i386: tcg: fix switching from 16-bit to 32-bit tasks or vice versa
  2021-06-04 15:17 [PULL 00/13] Misc bugfix patches for 2021-06-04 Paolo Bonzini
                   ` (4 preceding siblings ...)
  2021-06-04 15:17 ` [PULL 05/13] target/i386: tcg: fix loading of registers from " Paolo Bonzini
@ 2021-06-04 15:17 ` Paolo Bonzini
  2021-06-04 15:17 ` [PULL 07/13] target/i386: Fix decode of cr8 Paolo Bonzini
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-06-04 15:17 UTC (permalink / raw)
  To: qemu-devel

The format of the task state segment is governed by bit 3 in the
descriptor type field.  On a task switch, the format for saving
is given by the current value of TR's type field, while the
format for loading is given by the new descriptor.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/tcg/seg_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index 2112c5fc51..3ed20ca31d 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -319,7 +319,7 @@ static void switch_tss_ra(CPUX86State *env, int tss_selector,
     }
 
     /* save the current state in the old TSS */
-    if (type & 8) {
+    if (old_type & 8) {
         /* 32 bit */
         cpu_stl_kernel_ra(env, env->tr.base + 0x20, next_eip, retaddr);
         cpu_stl_kernel_ra(env, env->tr.base + 0x24, old_eflags, retaddr);
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 07/13] target/i386: Fix decode of cr8
  2021-06-04 15:17 [PULL 00/13] Misc bugfix patches for 2021-06-04 Paolo Bonzini
                   ` (5 preceding siblings ...)
  2021-06-04 15:17 ` [PULL 06/13] target/i386: tcg: fix switching from 16-bit to 32-bit tasks or vice versa Paolo Bonzini
@ 2021-06-04 15:17 ` Paolo Bonzini
  2021-06-04 15:17 ` [PULL 08/13] tests/qtest/virtio-scsi-test: add unmap large LBA with 4k blocks test Paolo Bonzini
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-06-04 15:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Richard Henderson

From: Richard Henderson <richard.henderson@linaro.org>

A recent cleanup did not recognize that there are two ways
to encode cr8: one via the LOCK and the other via REX.

Fixes: 7eff2e7c
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/380
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20210602035511.96834-1-richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/tcg/translate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 834186bcae..a7f5c0c8f2 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -8091,6 +8091,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
         case 2:
         case 3:
         case 4:
+        case 8:
             break;
         default:
             goto unknown_op;
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 08/13] tests/qtest/virtio-scsi-test: add unmap large LBA with 4k blocks test
  2021-06-04 15:17 [PULL 00/13] Misc bugfix patches for 2021-06-04 Paolo Bonzini
                   ` (6 preceding siblings ...)
  2021-06-04 15:17 ` [PULL 07/13] target/i386: Fix decode of cr8 Paolo Bonzini
@ 2021-06-04 15:17 ` Paolo Bonzini
  2021-06-04 15:17 ` [PULL 09/13] i386: reorder call to cpu_exec_realizefn Paolo Bonzini
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-06-04 15:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kit Westneat

From: Kit Westneat <kit.westneat@gmail.com>

Add test for issue #345

Signed-off-by: Kit Westneat <kit.westneat@gmail.com>
Message-Id: <20210603142022.676395-1-kit.westneat@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/virtio-scsi-test.c | 51 ++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/tests/qtest/virtio-scsi-test.c b/tests/qtest/virtio-scsi-test.c
index 1b7ecc1c8f..8ceb12aacd 100644
--- a/tests/qtest/virtio-scsi-test.c
+++ b/tests/qtest/virtio-scsi-test.c
@@ -200,6 +200,42 @@ static void test_unaligned_write_same(void *obj, void *data,
     qvirtio_scsi_pci_free(vs);
 }
 
+/* Test UNMAP with a large LBA, issue #345 */
+static void test_unmap_large_lba(void *obj, void *data,
+                                      QGuestAllocator *t_alloc)
+{
+    QVirtioSCSI *scsi = obj;
+    QVirtioSCSIQueues *vs;
+    const uint8_t unmap[VIRTIO_SCSI_CDB_SIZE] = {
+        0x42, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18, 0x00
+    };
+
+    /*
+     * Default null-co device size is 2**30
+     * LBA 0x7fff is ~ 1/8 into device, with 4k blocks
+     * if check_lba_range incorrectly using 512 bytes, will trigger sense error
+     */
+    uint8_t unmap_params[0x18] = {
+        0x00, 0x16, /* unmap data length */
+        0x00, 0x10, /* unmap block descriptor data length */
+        0x00, 0x00, 0x00, 0x00, /* reserved */
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x7f, 0xff, /* LBA */
+        0x00, 0x00, 0x03, 0xff, /* sector count */
+        0x00, 0x00, 0x00, 0x00, /* reserved */
+    };
+    struct virtio_scsi_cmd_resp resp;
+
+    alloc = t_alloc;
+    vs = qvirtio_scsi_init(scsi->vdev);
+
+    virtio_scsi_do_command(vs, unmap, NULL, 0, unmap_params,
+                           sizeof(unmap_params), &resp);
+    g_assert_cmphex(resp.response, ==, 0);
+    g_assert_cmphex(resp.status, !=, CHECK_CONDITION);
+
+    qvirtio_scsi_pci_free(vs);
+}
+
 static void test_write_to_cdrom(void *obj, void *data,
                                 QGuestAllocator *t_alloc)
 {
@@ -293,6 +329,17 @@ static void *virtio_scsi_setup(GString *cmd_line, void *arg)
     return arg;
 }
 
+static void *virtio_scsi_setup_4k(GString *cmd_line, void *arg)
+{
+    g_string_append(cmd_line,
+                    " -drive file=blkdebug::null-co://,"
+                    "file.image.read-zeroes=on,"
+                    "if=none,id=dr1,format=raw "
+                    "-device scsi-hd,drive=dr1,lun=0,scsi-id=1"
+                    ",logical_block_size=4k,physical_block_size=4k");
+    return arg;
+}
+
 static void *virtio_scsi_setup_cd(GString *cmd_line, void *arg)
 {
     g_string_append(cmd_line,
@@ -323,6 +370,10 @@ static void register_virtio_scsi_test(void)
     qos_add_test("unaligned-write-same", "virtio-scsi",
                  test_unaligned_write_same, &opts);
 
+    opts.before = virtio_scsi_setup_4k;
+    qos_add_test("large-lba-unmap", "virtio-scsi",
+                 test_unmap_large_lba, &opts);
+
     opts.before = virtio_scsi_setup_cd;
     qos_add_test("write-to-cdrom", "virtio-scsi", test_write_to_cdrom, &opts);
 
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 09/13] i386: reorder call to cpu_exec_realizefn
  2021-06-04 15:17 [PULL 00/13] Misc bugfix patches for 2021-06-04 Paolo Bonzini
                   ` (7 preceding siblings ...)
  2021-06-04 15:17 ` [PULL 08/13] tests/qtest/virtio-scsi-test: add unmap large LBA with 4k blocks test Paolo Bonzini
@ 2021-06-04 15:17 ` Paolo Bonzini
  2021-06-04 15:17 ` [PULL 10/13] i386: run accel_cpu_instance_init as post_init Paolo Bonzini
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-06-04 15:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vitaly Kuznetsov, Claudio Fontana, Eduardo Habkost

From: Claudio Fontana <cfontana@suse.de>

i386 realizefn code is sensitive to ordering, and recent commits
aimed at refactoring it, splitting accelerator-specific code,
broke assumptions which need to be fixed.

We need to:

* process hyper-v enlightements first, as they assume features
  not to be expanded

* only then, expand features

* after expanding features, attempt to check them and modify them in the
  accel-specific realizefn code called by cpu_exec_realizefn().

* after the framework has been called via cpu_exec_realizefn,
  the code can check for what has or hasn't been set by accel-specific
  code, or extend its results, ie:

  - check and evenually set code_urev default
  - modify cpu->mwait after potentially being set from host CPUID.
  - finally check for phys_bits assuming all user and accel-specific
    adjustments have already been taken into account.

Fixes: f5cc5a5c ("i386: split cpu accelerators from cpu.c"...)
Fixes: 30565f10 ("cpu: call AccelCPUClass::cpu_realizefn in"...)
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Claudio Fontana <cfontana@suse.de>
Message-Id: <20210603123001.17843-2-cfontana@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/cpu.c         | 79 +++++++++++++++++++++++++--------------
 target/i386/kvm/kvm-cpu.c | 12 +++++-
 2 files changed, 61 insertions(+), 30 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index e0ba36cc23..9c47daa409 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6089,39 +6089,17 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
     Error *local_err = NULL;
     static bool ht_warned;
 
-    /* Process Hyper-V enlightenments */
-    x86_cpu_hyperv_realize(cpu);
-
-    cpu_exec_realizefn(cs, &local_err);
-    if (local_err != NULL) {
-        error_propagate(errp, local_err);
-        return;
-    }
-
-    if (xcc->host_cpuid_required && !accel_uses_host_cpuid()) {
-        g_autofree char *name = x86_cpu_class_get_model_name(xcc);
-        error_setg(&local_err, "CPU model '%s' requires KVM or HVF", name);
-        goto out;
-    }
-
-    if (cpu->ucode_rev == 0) {
-        /* The default is the same as KVM's.  */
-        if (IS_AMD_CPU(env)) {
-            cpu->ucode_rev = 0x01000065;
-        } else {
-            cpu->ucode_rev = 0x100000000ULL;
-        }
-    }
-
-    /* mwait extended info: needed for Core compatibility */
-    /* We always wake on interrupt even if host does not have the capability */
-    cpu->mwait.ecx |= CPUID_MWAIT_EMX | CPUID_MWAIT_IBE;
-
     if (cpu->apic_id == UNASSIGNED_APIC_ID) {
         error_setg(errp, "apic-id property was not initialized properly");
         return;
     }
 
+    /*
+     * Process Hyper-V enlightenments.
+     * Note: this currently has to happen before the expansion of CPU features.
+     */
+    x86_cpu_hyperv_realize(cpu);
+
     x86_cpu_expand_features(cpu, &local_err);
     if (local_err) {
         goto out;
@@ -6146,11 +6124,56 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
            & CPUID_EXT2_AMD_ALIASES);
     }
 
+    /*
+     * note: the call to the framework needs to happen after feature expansion,
+     * but before the checks/modifications to ucode_rev, mwait, phys_bits.
+     * These may be set by the accel-specific code,
+     * and the results are subsequently checked / assumed in this function.
+     */
+    cpu_exec_realizefn(cs, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    if (xcc->host_cpuid_required && !accel_uses_host_cpuid()) {
+        g_autofree char *name = x86_cpu_class_get_model_name(xcc);
+        error_setg(&local_err, "CPU model '%s' requires KVM or HVF", name);
+        goto out;
+    }
+
+    if (cpu->ucode_rev == 0) {
+        /*
+         * The default is the same as KVM's. Note that this check
+         * needs to happen after the evenual setting of ucode_rev in
+         * accel-specific code in cpu_exec_realizefn.
+         */
+        if (IS_AMD_CPU(env)) {
+            cpu->ucode_rev = 0x01000065;
+        } else {
+            cpu->ucode_rev = 0x100000000ULL;
+        }
+    }
+
+    /*
+     * mwait extended info: needed for Core compatibility
+     * We always wake on interrupt even if host does not have the capability.
+     *
+     * requires the accel-specific code in cpu_exec_realizefn to
+     * have already acquired the CPUID data into cpu->mwait.
+     */
+    cpu->mwait.ecx |= CPUID_MWAIT_EMX | CPUID_MWAIT_IBE;
+
     /* For 64bit systems think about the number of physical bits to present.
      * ideally this should be the same as the host; anything other than matching
      * the host can cause incorrect guest behaviour.
      * QEMU used to pick the magic value of 40 bits that corresponds to
      * consumer AMD devices but nothing else.
+     *
+     * Note that this code assumes features expansion has already been done
+     * (as it checks for CPUID_EXT2_LM), and also assumes that potential
+     * phys_bits adjustments to match the host have been already done in
+     * accel-specific code in cpu_exec_realizefn.
      */
     if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) {
         if (cpu->phys_bits &&
diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
index 5235bce8dc..00369c2000 100644
--- a/target/i386/kvm/kvm-cpu.c
+++ b/target/i386/kvm/kvm-cpu.c
@@ -26,10 +26,18 @@ static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
     /*
      * The realize order is important, since x86_cpu_realize() checks if
      * nothing else has been set by the user (or by accelerators) in
-     * cpu->ucode_rev and cpu->phys_bits.
+     * cpu->ucode_rev and cpu->phys_bits, and updates the CPUID results in
+     * mwait.ecx.
+     * This accel realization code also assumes cpu features are already expanded.
      *
      * realize order:
-     * kvm_cpu -> host_cpu -> x86_cpu
+     *
+     * x86_cpu_realize():
+     *  -> x86_cpu_expand_features()
+     *  -> cpu_exec_realizefn():
+     *            -> accel_cpu_realizefn()
+     *               kvm_cpu_realizefn() -> host_cpu_realizefn()
+     *  -> check/update ucode_rev, phys_bits, mwait
      */
     if (cpu->max_features) {
         if (enable_cpu_pm && kvm_has_waitpkg()) {
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 10/13] i386: run accel_cpu_instance_init as post_init
  2021-06-04 15:17 [PULL 00/13] Misc bugfix patches for 2021-06-04 Paolo Bonzini
                   ` (8 preceding siblings ...)
  2021-06-04 15:17 ` [PULL 09/13] i386: reorder call to cpu_exec_realizefn Paolo Bonzini
@ 2021-06-04 15:17 ` Paolo Bonzini
  2021-06-04 15:17 ` [PULL 11/13] qemu-config: parse configuration files to a QDict Paolo Bonzini
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-06-04 15:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Claudio Fontana, Eduardo Habkost

From: Claudio Fontana <cfontana@suse.de>

This fixes host and max cpu initialization, by running the accel cpu
initialization only after all instance init functions are called for all
X86 cpu subclasses.

The bug this is fixing is related to the "max" and "host" i386 cpu
subclasses, which set cpu->max_features, which is then used at cpu
realization time.

In order to properly split the accel-specific max features code that
needs to be executed at cpu instance initialization time,

we cannot call the accel cpu initialization at the end of the x86 base
class initialization, or we will have no way to specialize
"max features" cpu behavior, overriding the "max" cpu class defaults,
and checking for the "max features" flag itself.

This patch moves the accel-specific cpu instance initialization to after
all x86 cpu instance code has been executed, including subclasses,

so that proper initialization of cpu "host" and "max" can be restored.

Fixes: f5cc5a5c ("i386: split cpu accelerators from cpu.c,"...)
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Claudio Fontana <cfontana@suse.de>
Message-Id: <20210603123001.17843-3-cfontana@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/cpu.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9c47daa409..a9fe1662d3 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6401,6 +6401,11 @@ static void x86_cpu_register_feature_bit_props(X86CPUClass *xcc,
     x86_cpu_register_bit_prop(xcc, name, w, bitnr);
 }
 
+static void x86_cpu_post_initfn(Object *obj)
+{
+    accel_cpu_instance_init(CPU(obj));
+}
+
 static void x86_cpu_initfn(Object *obj)
 {
     X86CPU *cpu = X86_CPU(obj);
@@ -6452,9 +6457,6 @@ static void x86_cpu_initfn(Object *obj)
     if (xcc->model) {
         x86_cpu_load_model(cpu, xcc->model);
     }
-
-    /* if required, do accelerator-specific cpu initializations */
-    accel_cpu_instance_init(CPU(obj));
 }
 
 static int64_t x86_cpu_get_arch_id(CPUState *cs)
@@ -6799,6 +6801,8 @@ static const TypeInfo x86_cpu_type_info = {
     .parent = TYPE_CPU,
     .instance_size = sizeof(X86CPU),
     .instance_init = x86_cpu_initfn,
+    .instance_post_init = x86_cpu_post_initfn,
+
     .abstract = true,
     .class_size = sizeof(X86CPUClass),
     .class_init = x86_cpu_common_class_init,
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 11/13] qemu-config: parse configuration files to a QDict
  2021-06-04 15:17 [PULL 00/13] Misc bugfix patches for 2021-06-04 Paolo Bonzini
                   ` (9 preceding siblings ...)
  2021-06-04 15:17 ` [PULL 10/13] i386: run accel_cpu_instance_init as post_init Paolo Bonzini
@ 2021-06-04 15:17 ` Paolo Bonzini
  2021-06-04 15:17 ` [PULL 12/13] vl: plumb keyval-based options into -readconfig Paolo Bonzini
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-06-04 15:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Markus Armbruster, qemu-stable

Change the parser to put the values into a QDict and pass them
to a callback.  qemu_config_parse's QemuOpts creation is
itself turned into a callback function.

This is useful for -readconfig to support keyval-based options;
getting a QDict from the parser removes a roundtrip from
QDict to QemuOpts and then back to QDict.

Unfortunately there is a disadvantage in that semantic errors will
point to the last line of the group, because the entries of the QDict
do not have a location attached.

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20210524105752.3318299-2-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/config-file.h |  7 ++-
 softmmu/vl.c               |  4 +-
 util/qemu-config.c         | 98 ++++++++++++++++++++++++++------------
 3 files changed, 76 insertions(+), 33 deletions(-)

diff --git a/include/qemu/config-file.h b/include/qemu/config-file.h
index 0500b3668d..f605423321 100644
--- a/include/qemu/config-file.h
+++ b/include/qemu/config-file.h
@@ -1,6 +1,8 @@
 #ifndef QEMU_CONFIG_FILE_H
 #define QEMU_CONFIG_FILE_H
 
+typedef void QEMUConfigCB(const char *group, QDict *qdict, void *opaque, Error **errp);
+
 void qemu_load_module_for_opts(const char *group);
 QemuOptsList *qemu_find_opts(const char *group);
 QemuOptsList *qemu_find_opts_err(const char *group, Error **errp);
@@ -14,7 +16,10 @@ void qemu_config_write(FILE *fp);
 int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname,
                       Error **errp);
 
-int qemu_read_config_file(const char *filename, Error **errp);
+/* A default callback for qemu_read_config_file().  */
+void qemu_config_do_parse(const char *group, QDict *qdict, void *opaque, Error **errp);
+
+int qemu_read_config_file(const char *filename, QEMUConfigCB *f, Error **errp);
 
 /* Parse QDict options as a replacement for a config file (allowing multiple
    enumerated (0..(n-1)) configuration "sections") */
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 6054f6f0b9..47dfdd704f 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2133,7 +2133,7 @@ static void qemu_read_default_config_file(Error **errp)
     int ret;
     g_autofree char *file = get_relocated_path(CONFIG_QEMU_CONFDIR "/qemu.conf");
 
-    ret = qemu_read_config_file(file, errp);
+    ret = qemu_read_config_file(file, qemu_config_do_parse, errp);
     if (ret < 0) {
         if (ret == -ENOENT) {
             error_free(*errp);
@@ -3399,7 +3399,7 @@ void qemu_init(int argc, char **argv, char **envp)
                 qemu_plugin_opt_parse(optarg, &plugin_list);
                 break;
             case QEMU_OPTION_readconfig:
-                qemu_read_config_file(optarg, &error_fatal);
+                qemu_read_config_file(optarg, qemu_config_do_parse, &error_fatal);
                 break;
             case QEMU_OPTION_spice:
                 olist = qemu_find_opts_err("spice", NULL);
diff --git a/util/qemu-config.c b/util/qemu-config.c
index 34974c4b47..374f3bc460 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -2,6 +2,7 @@
 #include "block/qdict.h" /* for qdict_extract_subqdict() */
 #include "qapi/error.h"
 #include "qapi/qapi-commands-misc.h"
+#include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qlist.h"
 #include "qemu/error-report.h"
@@ -351,19 +352,19 @@ void qemu_config_write(FILE *fp)
 }
 
 /* Returns number of config groups on success, -errno on error */
-int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, Error **errp)
+static int qemu_config_foreach(FILE *fp, QEMUConfigCB *cb, void *opaque,
+                               const char *fname, Error **errp)
 {
-    char line[1024], group[64], id[64], arg[64], value[1024];
+    char line[1024], prev_group[64], group[64], arg[64], value[1024];
     Location loc;
-    QemuOptsList *list = NULL;
     Error *local_err = NULL;
-    QemuOpts *opts = NULL;
+    QDict *qdict = NULL;
     int res = -EINVAL, lno = 0;
     int count = 0;
 
     loc_push_none(&loc);
     while (fgets(line, sizeof(line), fp) != NULL) {
-        loc_set_file(fname, ++lno);
+        ++lno;
         if (line[0] == '\n') {
             /* skip empty lines */
             continue;
@@ -372,39 +373,39 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, Error *
             /* comment */
             continue;
         }
-        if (sscanf(line, "[%63s \"%63[^\"]\"]", group, id) == 2) {
-            /* group with id */
-            list = find_list(lists, group, &local_err);
-            if (local_err) {
-                error_propagate(errp, local_err);
-                goto out;
+        if (line[0] == '[') {
+            QDict *prev = qdict;
+            if (sscanf(line, "[%63s \"%63[^\"]\"]", group, value) == 2) {
+                qdict = qdict_new();
+                qdict_put_str(qdict, "id", value);
+                count++;
+            } else if (sscanf(line, "[%63[^]]]", group) == 1) {
+                qdict = qdict_new();
+                count++;
             }
-            opts = qemu_opts_create(list, id, 1, NULL);
-            count++;
-            continue;
-        }
-        if (sscanf(line, "[%63[^]]]", group) == 1) {
-            /* group without id */
-            list = find_list(lists, group, &local_err);
-            if (local_err) {
-                error_propagate(errp, local_err);
-                goto out;
+            if (qdict != prev) {
+                if (prev) {
+                    cb(prev_group, prev, opaque, &local_err);
+                    qobject_unref(prev);
+                    if (local_err) {
+                        error_propagate(errp, local_err);
+                        goto out;
+                    }
+                }
+                strcpy(prev_group, group);
+                continue;
             }
-            opts = qemu_opts_create(list, NULL, 0, &error_abort);
-            count++;
-            continue;
         }
+        loc_set_file(fname, lno);
         value[0] = '\0';
         if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2 ||
             sscanf(line, " %63s = \"\"", arg) == 1) {
             /* arg = value */
-            if (opts == NULL) {
+            if (qdict == NULL) {
                 error_setg(errp, "no group defined");
                 goto out;
             }
-            if (!qemu_opt_set(opts, arg, value, errp)) {
-                goto out;
-            }
+            qdict_put_str(qdict, arg, value);
             continue;
         }
         error_setg(errp, "parse error");
@@ -417,11 +418,48 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, Error *
     }
     res = count;
 out:
+    if (qdict) {
+        cb(group, qdict, opaque, errp);
+        qobject_unref(qdict);
+    }
     loc_pop(&loc);
     return res;
 }
 
-int qemu_read_config_file(const char *filename, Error **errp)
+void qemu_config_do_parse(const char *group, QDict *qdict, void *opaque, Error **errp)
+{
+    QemuOptsList **lists = opaque;
+    const char *id = qdict_get_try_str(qdict, "id");
+    QemuOptsList *list;
+    QemuOpts *opts;
+    const QDictEntry *unrecognized;
+
+    list = find_list(lists, group, errp);
+    if (!list) {
+        return;
+    }
+
+    opts = qemu_opts_create(list, id, 1, errp);
+    if (!opts) {
+        return;
+    }
+    if (!qemu_opts_absorb_qdict(opts, qdict, errp)) {
+        qemu_opts_del(opts);
+        return;
+    }
+    unrecognized = qdict_first(qdict);
+    if (unrecognized) {
+        error_setg(errp, QERR_INVALID_PARAMETER, unrecognized->key);
+        qemu_opts_del(opts);
+    }
+}
+
+int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, Error **errp)
+{
+    return qemu_config_foreach(fp, qemu_config_do_parse, lists, fname, errp);
+}
+
+int qemu_read_config_file(const char *filename, QEMUConfigCB *cb, Error **errp)
 {
     FILE *f = fopen(filename, "r");
     int ret;
@@ -431,7 +469,7 @@ int qemu_read_config_file(const char *filename, Error **errp)
         return -errno;
     }
 
-    ret = qemu_config_parse(f, vm_config_groups, filename, errp);
+    ret = qemu_config_foreach(f, cb, vm_config_groups, filename, errp);
     fclose(f);
     return ret;
 }
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 12/13] vl: plumb keyval-based options into -readconfig
  2021-06-04 15:17 [PULL 00/13] Misc bugfix patches for 2021-06-04 Paolo Bonzini
                   ` (10 preceding siblings ...)
  2021-06-04 15:17 ` [PULL 11/13] qemu-config: parse configuration files to a QDict Paolo Bonzini
@ 2021-06-04 15:17 ` Paolo Bonzini
  2021-06-29 12:52   ` Peter Maydell
  2021-06-04 15:17 ` [PULL 13/13] vl: plug -object back " Paolo Bonzini
  2021-06-05 10:25 ` [PULL 00/13] Misc bugfix patches for 2021-06-04 Peter Maydell
  13 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2021-06-04 15:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Markus Armbruster, qemu-stable

Let -readconfig support parsing command line options into QDict or
QemuOpts.  This will be used to add back support for objects in
-readconfig.

Cc: Markus Armbruster <armbru@redhat.com>
Cc: qemu-stable@nongnu.org
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20210524105752.3318299-3-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/block/qdict.h    |  2 -
 include/qapi/qmp/qdict.h |  3 ++
 softmmu/vl.c             | 83 ++++++++++++++++++++++++++++------------
 3 files changed, 62 insertions(+), 26 deletions(-)

diff --git a/include/block/qdict.h b/include/block/qdict.h
index d8cb502d7d..ced2acfb92 100644
--- a/include/block/qdict.h
+++ b/include/block/qdict.h
@@ -20,8 +20,6 @@ void qdict_join(QDict *dest, QDict *src, bool overwrite);
 void qdict_extract_subqdict(QDict *src, QDict **dst, const char *start);
 void qdict_array_split(QDict *src, QList **dst);
 int qdict_array_entries(QDict *src, const char *subqdict);
-QObject *qdict_crumple(const QDict *src, Error **errp);
-void qdict_flatten(QDict *qdict);
 
 typedef struct QDictRenames {
     const char *from;
diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 9934539c1b..d5b5430e21 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -64,4 +64,7 @@ const char *qdict_get_try_str(const QDict *qdict, const char *key);
 
 QDict *qdict_clone_shallow(const QDict *src);
 
+QObject *qdict_crumple(const QDict *src, Error **errp);
+void qdict_flatten(QDict *qdict);
+
 #endif /* QDICT_H */
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 47dfdd704f..5e8240b9d8 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -121,6 +121,7 @@
 #include "qapi/qapi-commands-misc.h"
 #include "qapi/qapi-visit-qom.h"
 #include "qapi/qapi-commands-ui.h"
+#include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qerror.h"
 #include "sysemu/iothread.h"
 #include "qemu/guest-random.h"
@@ -2127,13 +2128,53 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp)
     return 0;
 }
 
+/*
+ * Return whether configuration group @group is stored in QemuOpts, or
+ * recorded as one or more QDicts by qemu_record_config_group.
+ */
+static bool is_qemuopts_group(const char *group)
+{
+    return true;
+}
+
+static void qemu_record_config_group(const char *group, QDict *dict,
+                                     bool from_json, Error **errp)
+{
+    abort();
+}
+
+/*
+ * Parse non-QemuOpts config file groups, pass the rest to
+ * qemu_config_do_parse.
+ */
+static void qemu_parse_config_group(const char *group, QDict *qdict,
+                                    void *opaque, Error **errp)
+{
+    QObject *crumpled;
+    if (is_qemuopts_group(group)) {
+        qemu_config_do_parse(group, qdict, opaque, errp);
+        return;
+    }
+
+    crumpled = qdict_crumple(qdict, errp);
+    if (!crumpled) {
+        return;
+    }
+    if (qobject_type(crumpled) != QTYPE_QDICT) {
+        assert(qobject_type(crumpled) == QTYPE_QLIST);
+        error_setg(errp, "Lists cannot be at top level of a configuration section");
+        return;
+    }
+    qemu_record_config_group(group, qobject_to(QDict, crumpled), false, errp);
+}
+
 static void qemu_read_default_config_file(Error **errp)
 {
     ERRP_GUARD();
     int ret;
     g_autofree char *file = get_relocated_path(CONFIG_QEMU_CONFDIR "/qemu.conf");
 
-    ret = qemu_read_config_file(file, qemu_config_do_parse, errp);
+    ret = qemu_read_config_file(file, qemu_parse_config_group, errp);
     if (ret < 0) {
         if (ret == -ENOENT) {
             error_free(*errp);
@@ -2142,9 +2183,8 @@ static void qemu_read_default_config_file(Error **errp)
     }
 }
 
-static int qemu_set_option(const char *str)
+static void qemu_set_option(const char *str, Error **errp)
 {
-    Error *local_err = NULL;
     char group[64], id[64], arg[64];
     QemuOptsList *list;
     QemuOpts *opts;
@@ -2152,27 +2192,23 @@ static int qemu_set_option(const char *str)
 
     rc = sscanf(str, "%63[^.].%63[^.].%63[^=]%n", group, id, arg, &offset);
     if (rc < 3 || str[offset] != '=') {
-        error_report("can't parse: \"%s\"", str);
-        return -1;
-    }
-
-    list = qemu_find_opts(group);
-    if (list == NULL) {
-        return -1;
-    }
-
-    opts = qemu_opts_find(list, id);
-    if (!opts) {
-        error_report("there is no %s \"%s\" defined",
-                     list->name, id);
-        return -1;
+        error_setg(errp, "can't parse: \"%s\"", str);
+        return;
     }
 
-    if (!qemu_opt_set(opts, arg, str + offset + 1, &local_err)) {
-        error_report_err(local_err);
-        return -1;
+    if (!is_qemuopts_group(group)) {
+        error_setg(errp, "-set is not supported with %s", group);
+    } else {
+        list = qemu_find_opts_err(group, errp);
+        if (list) {
+            opts = qemu_opts_find(list, id);
+            if (!opts) {
+                error_setg(errp, "there is no %s \"%s\" defined", group, id);
+                return;
+            }
+            qemu_opt_set(opts, arg, str + offset + 1, errp);
+        }
     }
-    return 0;
 }
 
 static void user_register_global_props(void)
@@ -2766,8 +2802,7 @@ void qemu_init(int argc, char **argv, char **envp)
                 }
                 break;
             case QEMU_OPTION_set:
-                if (qemu_set_option(optarg) != 0)
-                    exit(1);
+                qemu_set_option(optarg, &error_fatal);
                 break;
             case QEMU_OPTION_global:
                 if (qemu_global_option(optarg) != 0)
@@ -3399,7 +3434,7 @@ void qemu_init(int argc, char **argv, char **envp)
                 qemu_plugin_opt_parse(optarg, &plugin_list);
                 break;
             case QEMU_OPTION_readconfig:
-                qemu_read_config_file(optarg, qemu_config_do_parse, &error_fatal);
+                qemu_read_config_file(optarg, qemu_parse_config_group, &error_fatal);
                 break;
             case QEMU_OPTION_spice:
                 olist = qemu_find_opts_err("spice", NULL);
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 13/13] vl: plug -object back into -readconfig
  2021-06-04 15:17 [PULL 00/13] Misc bugfix patches for 2021-06-04 Paolo Bonzini
                   ` (11 preceding siblings ...)
  2021-06-04 15:17 ` [PULL 12/13] vl: plumb keyval-based options into -readconfig Paolo Bonzini
@ 2021-06-04 15:17 ` Paolo Bonzini
  2021-06-05 10:25 ` [PULL 00/13] Misc bugfix patches for 2021-06-04 Peter Maydell
  13 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-06-04 15:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Markus Armbruster, qemu-stable

Commit bc2f4fcb1d ("qom: move user_creatable_add_opts logic to vl.c
and QAPIfy it", 2021-03-19) switched the creation of objects from
qemu_opts_foreach to a bespoke QTAILQ in preparation for supporting JSON
syntax in -object.

Unfortunately in doing so it lost support for [object] stanzas in
configuration files and also for "-set object.ID.KEY=VAL".  The latter
is hard to re-establish and probably best solved by deprecating -set.
This patch uses the infrastructure introduced by the previous two
patches in order to parse QOM objects correctly from configuration
files.

Cc: Markus Armbruster <armbru@redhat.com>
Cc: qemu-stable@nongnu.org
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20210524105752.3318299-4-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 softmmu/vl.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/softmmu/vl.c b/softmmu/vl.c
index 5e8240b9d8..326c1e9080 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1722,9 +1722,15 @@ static void object_option_foreach_add(bool (*type_opt_predicate)(const char *))
     }
 }
 
+static void object_option_add_visitor(Visitor *v)
+{
+    ObjectOption *opt = g_new0(ObjectOption, 1);
+    visit_type_ObjectOptions(v, NULL, &opt->opts, &error_fatal);
+    QTAILQ_INSERT_TAIL(&object_opts, opt, next);
+}
+
 static void object_option_parse(const char *optarg)
 {
-    ObjectOption *opt;
     QemuOpts *opts;
     const char *type;
     Visitor *v;
@@ -1752,11 +1758,8 @@ static void object_option_parse(const char *optarg)
         v = opts_visitor_new(opts);
     }
 
-    opt = g_new0(ObjectOption, 1);
-    visit_type_ObjectOptions(v, NULL, &opt->opts, &error_fatal);
+    object_option_add_visitor(v);
     visit_free(v);
-
-    QTAILQ_INSERT_TAIL(&object_opts, opt, next);
 }
 
 /*
@@ -2134,13 +2137,22 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp)
  */
 static bool is_qemuopts_group(const char *group)
 {
+    if (g_str_equal(group, "object")) {
+        return false;
+    }
     return true;
 }
 
 static void qemu_record_config_group(const char *group, QDict *dict,
                                      bool from_json, Error **errp)
 {
-    abort();
+    if (g_str_equal(group, "object")) {
+        Visitor *v = qobject_input_visitor_new_keyval(QOBJECT(dict));
+        object_option_add_visitor(v);
+        visit_free(v);
+    } else {
+        abort();
+    }
 }
 
 /*
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PULL 00/13] Misc bugfix patches for 2021-06-04
  2021-06-04 15:17 [PULL 00/13] Misc bugfix patches for 2021-06-04 Paolo Bonzini
                   ` (12 preceding siblings ...)
  2021-06-04 15:17 ` [PULL 13/13] vl: plug -object back " Paolo Bonzini
@ 2021-06-05 10:25 ` Peter Maydell
  13 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2021-06-05 10:25 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers

On Fri, 4 Jun 2021 at 16:19, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The following changes since commit 5a95f5ce3cd5842cc8f61a91ecd4fb4e8d10104f:
>
>   Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-fpu-20210603' into staging (2021-06-04 10:04:11 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 49e987695a1873a769a823604f9065aa88e00c55:
>
>   vl: plug -object back into -readconfig (2021-06-04 13:50:04 +0200)
>
> ----------------------------------------------------------------
> * OpenBSD cleanup (Brad)
> * fixes for the i386 accel/cpu refactoring (Claudio)
> * unmap test for emulated SCSI (Kit)
> * fix for iscsi module (myself)
> * fix for -readconfig of objects (myself)
> * fixes for x86 16-bit task switching (myself)
> * fix for x86 MOV from/to CR8 (Richard)
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.1
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PULL 12/13] vl: plumb keyval-based options into -readconfig
  2021-06-04 15:17 ` [PULL 12/13] vl: plumb keyval-based options into -readconfig Paolo Bonzini
@ 2021-06-29 12:52   ` Peter Maydell
  0 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2021-06-29 12:52 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Kevin Wolf, qemu-stable, QEMU Developers, Markus Armbruster

On Fri, 4 Jun 2021 at 16:23, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Let -readconfig support parsing command line options into QDict or
> QemuOpts.  This will be used to add back support for objects in
> -readconfig.
>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: qemu-stable@nongnu.org
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Message-Id: <20210524105752.3318299-3-pbonzini@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Hi; Coverity suspects a resource leak in this code (CID 1457455):


> +/*
> + * Parse non-QemuOpts config file groups, pass the rest to
> + * qemu_config_do_parse.
> + */
> +static void qemu_parse_config_group(const char *group, QDict *qdict,
> +                                    void *opaque, Error **errp)
> +{
> +    QObject *crumpled;
> +    if (is_qemuopts_group(group)) {
> +        qemu_config_do_parse(group, qdict, opaque, errp);
> +        return;
> +    }
> +
> +    crumpled = qdict_crumple(qdict, errp);

It thinks qdict_crumple() allocates memory...

> +    if (!crumpled) {
> +        return;
> +    }
> +    if (qobject_type(crumpled) != QTYPE_QDICT) {
> +        assert(qobject_type(crumpled) == QTYPE_QLIST);
> +        error_setg(errp, "Lists cannot be at top level of a configuration section");

...but here we return without freeing/derefing it or keeping track
of the pointer anywhere...

> +        return;
> +    }
> +    qemu_record_config_group(group, qobject_to(QDict, crumpled), false, errp);

...and here it thinks that qemu_record_config_group does not free or
keep a pointer to 'crumpled', though in this case I suspect it is wrong.

More general question: where should I look to find documentation on
what functions take 'ownership' of a reference-counted object?
I often find when trying to analyse Coverity reports like these that
I am just as confused as it is about whether a function really has
taken ownership of something or whether the caller kept ownership
and needed to deref it...

thanks
-- PMM


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-06-29 12:53 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-04 15:17 [PULL 00/13] Misc bugfix patches for 2021-06-04 Paolo Bonzini
2021-06-04 15:17 ` [PULL 01/13] meson: allow optional dependencies for block modules Paolo Bonzini
2021-06-04 15:17 ` [PULL 02/13] iscsi: link libm into the module Paolo Bonzini
2021-06-04 15:17 ` [PULL 03/13] oslib-posix: Remove OpenBSD workaround for fcntl("/dev/null", F_SETFL, O_NONBLOCK) failure Paolo Bonzini
2021-06-04 15:17 ` [PULL 04/13] target/i386: tcg: fix segment register offsets for 16-bit TSS Paolo Bonzini
2021-06-04 15:17 ` [PULL 05/13] target/i386: tcg: fix loading of registers from " Paolo Bonzini
2021-06-04 15:17 ` [PULL 06/13] target/i386: tcg: fix switching from 16-bit to 32-bit tasks or vice versa Paolo Bonzini
2021-06-04 15:17 ` [PULL 07/13] target/i386: Fix decode of cr8 Paolo Bonzini
2021-06-04 15:17 ` [PULL 08/13] tests/qtest/virtio-scsi-test: add unmap large LBA with 4k blocks test Paolo Bonzini
2021-06-04 15:17 ` [PULL 09/13] i386: reorder call to cpu_exec_realizefn Paolo Bonzini
2021-06-04 15:17 ` [PULL 10/13] i386: run accel_cpu_instance_init as post_init Paolo Bonzini
2021-06-04 15:17 ` [PULL 11/13] qemu-config: parse configuration files to a QDict Paolo Bonzini
2021-06-04 15:17 ` [PULL 12/13] vl: plumb keyval-based options into -readconfig Paolo Bonzini
2021-06-29 12:52   ` Peter Maydell
2021-06-04 15:17 ` [PULL 13/13] vl: plug -object back " Paolo Bonzini
2021-06-05 10:25 ` [PULL 00/13] Misc bugfix patches for 2021-06-04 Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.