All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, peter.maydell@linaro.org, qemu-devel@nongnu.org
Subject: [PULL 04/16] softmmu: fix device deletion events with -device JSON syntax
Date: Fri, 14 Jan 2022 14:52:14 +0100	[thread overview]
Message-ID: <20220114135226.185407-5-kwolf@redhat.com> (raw)
In-Reply-To: <20220114135226.185407-1-kwolf@redhat.com>

From: Daniel P. Berrangé <berrange@redhat.com>

The -device JSON syntax impl leaks a reference on the created
DeviceState instance. As a result when you hot-unplug the
device, the device_finalize method won't be called and thus
it will fail to emit the required DEVICE_DELETED event.

A 'json-cli' feature was previously added against the
'device_add' QMP command QAPI schema to indicated to mgmt
apps that -device supported JSON syntax. Given the hotplug
bug that feature flag is not usable for its purpose, so
we add a new 'json-cli-hotplug' feature to indicate the
-device supports JSON without breaking hotplug.

Fixes: 5dacda5167560b3af8eadbce5814f60ba44b467e
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/802
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20220105123847.4047954-2-berrange@redhat.com>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi/qdev.json                 |  5 ++++-
 softmmu/vl.c                   |  4 +++-
 tests/qtest/device-plug-test.c | 19 +++++++++++++++++++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/qapi/qdev.json b/qapi/qdev.json
index 69656b14df..26cd10106b 100644
--- a/qapi/qdev.json
+++ b/qapi/qdev.json
@@ -44,6 +44,9 @@
 # @json-cli: If present, the "-device" command line option supports JSON
 #            syntax with a structure identical to the arguments of this
 #            command.
+# @json-cli-hotplug: If present, the "-device" command line option supports JSON
+#                    syntax without the reference counting leak that broke
+#                    hot-unplug
 #
 # Notes:
 #
@@ -74,7 +77,7 @@
 { 'command': 'device_add',
   'data': {'driver': 'str', '*bus': 'str', '*id': 'str'},
   'gen': false, # so we can get the additional arguments
-  'features': ['json-cli'] }
+  'features': ['json-cli', 'json-cli-hotplug'] }
 
 ##
 # @device_del:
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 207a9eb8be..5e1b35ba48 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2684,6 +2684,7 @@ static void qemu_create_cli_devices(void)
     qemu_opts_foreach(qemu_find_opts("device"),
                       device_init_func, NULL, &error_fatal);
     QTAILQ_FOREACH(opt, &device_opts, next) {
+        DeviceState *dev;
         loc_push_restore(&opt->loc);
         /*
          * TODO Eventually we should call qmp_device_add() here to make sure it
@@ -2692,7 +2693,8 @@ static void qemu_create_cli_devices(void)
          * from the start, so call qdev_device_add_from_qdict() directly for
          * now.
          */
-        qdev_device_add_from_qdict(opt->opts, true, &error_fatal);
+        dev = qdev_device_add_from_qdict(opt->opts, true, &error_fatal);
+        object_unref(OBJECT(dev));
         loc_pop(&opt->loc);
     }
     rom_reset_order_override();
diff --git a/tests/qtest/device-plug-test.c b/tests/qtest/device-plug-test.c
index 559d47727a..ad79bd4c14 100644
--- a/tests/qtest/device-plug-test.c
+++ b/tests/qtest/device-plug-test.c
@@ -77,6 +77,23 @@ static void test_pci_unplug_request(void)
     qtest_quit(qtest);
 }
 
+static void test_pci_unplug_json_request(void)
+{
+    QTestState *qtest = qtest_initf(
+        "-device '{\"driver\": \"virtio-mouse-pci\", \"id\": \"dev0\"}'");
+
+    /*
+     * Request device removal. As the guest is not running, the request won't
+     * be processed. However during system reset, the removal will be
+     * handled, removing the device.
+     */
+    device_del(qtest, "dev0");
+    system_reset(qtest);
+    wait_device_deleted_event(qtest, "dev0");
+
+    qtest_quit(qtest);
+}
+
 static void test_ccw_unplug(void)
 {
     QTestState *qtest = qtest_initf("-device virtio-balloon-ccw,id=dev0");
@@ -145,6 +162,8 @@ int main(int argc, char **argv)
      */
     qtest_add_func("/device-plug/pci-unplug-request",
                    test_pci_unplug_request);
+    qtest_add_func("/device-plug/pci-unplug-json-request",
+                   test_pci_unplug_json_request);
 
     if (!strcmp(arch, "s390x")) {
         qtest_add_func("/device-plug/ccw-unplug",
-- 
2.31.1



  parent reply	other threads:[~2022-01-14 13:58 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-14 13:52 [PULL 00/16] Block layer patches Kevin Wolf
2022-01-14 13:52 ` [PULL 01/16] block_int: make bdrv_backing_overridden static Kevin Wolf
2022-01-14 13:52 ` [PULL 02/16] include/sysemu/blockdev.h: remove drive_mark_claimed_by_board and inline drive_def Kevin Wolf
2022-01-14 13:52 ` [PULL 03/16] include/sysemu/blockdev.h: remove drive_get_max_devs Kevin Wolf
2022-01-14 13:52 ` Kevin Wolf [this message]
2022-01-14 13:52 ` [PULL 05/16] docs: Correct 'vhost-user-blk' spelling Kevin Wolf
2022-01-14 13:52 ` [PULL 06/16] qemu-storage-daemon: Add vhost-user-blk help Kevin Wolf
2022-01-14 13:52 ` [PULL 07/16] qapi/block: Restrict vhost-user-blk to CONFIG_VHOST_USER_BLK_SERVER Kevin Wolf
2022-01-14 14:20   ` Philippe Mathieu-Daudé via
2022-01-14 13:52 ` [PULL 08/16] block-backend: prevent dangling BDS pointers across aio_poll() Kevin Wolf
2022-01-14 13:52 ` [PULL 09/16] iotests/stream-error-on-reset: New test Kevin Wolf
2022-01-14 13:52 ` [PULL 10/16] iotests/308: Fix for CAP_DAC_OVERRIDE Kevin Wolf
2022-01-14 13:52 ` [PULL 11/16] vvfat: Fix size of temporary qcow file Kevin Wolf
2022-01-14 13:52 ` [PULL 12/16] vvfat: Fix vvfat_write() for writes before the root directory Kevin Wolf
2022-01-14 13:52 ` [PULL 13/16] iotests: Test qemu-img convert of zeroed data cluster Kevin Wolf
2022-01-14 13:52 ` [PULL 14/16] qemu-img: make is_allocated_sectors() more efficient Kevin Wolf
2022-01-14 13:52 ` [PULL 15/16] block: drop BLK_PERM_GRAPH_MOD Kevin Wolf
2022-01-14 13:52 ` [PULL 16/16] iotests/testrunner.py: refactor test_field_width Kevin Wolf
2022-01-15 12:34 ` [PULL 00/16] Block layer patches Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220114135226.185407-5-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.