From 5ada09caffb91004cae904ec43cf37b59de3eda3 Mon Sep 17 00:00:00 2001 From: Miroslav Rezanina Date: Mon, 13 Jan 2025 06:49:31 -0500 Subject: [PATCH] * Mon Jan 13 2025 Miroslav Rezanina - 9.1.0-9 - kvm-qdev-Fix-set_pci_devfn-to-visit-option-only-once.patch [RHEL-39948] - kvm-tests-avocado-hotplug_blk-Fix-addr-in-device_add-com.patch [RHEL-39948] - kvm-qdev-monitor-avoid-QemuOpts-in-QMP-device_add.patch [RHEL-39948] - kvm-vl-use-qmp_device_add-in-qemu_create_cli_devices.patch [RHEL-39948] - Resolves: RHEL-39948 (qom-get iothread-vq-mapping is empty on new hotplug disk [rhel-9.5]) --- ..._pci_devfn-to-visit-option-only-once.patch | 128 +++++++++++++++++ ...tor-avoid-QemuOpts-in-QMP-device_add.patch | 131 ++++++++++++++++++ ...tplug_blk-Fix-addr-in-device_add-com.patch | 51 +++++++ ...evice_add-in-qemu_create_cli_devices.patch | 65 +++++++++ qemu-kvm.spec | 18 ++- 5 files changed, 392 insertions(+), 1 deletion(-) create mode 100644 kvm-qdev-Fix-set_pci_devfn-to-visit-option-only-once.patch create mode 100644 kvm-qdev-monitor-avoid-QemuOpts-in-QMP-device_add.patch create mode 100644 kvm-tests-avocado-hotplug_blk-Fix-addr-in-device_add-com.patch create mode 100644 kvm-vl-use-qmp_device_add-in-qemu_create_cli_devices.patch diff --git a/kvm-qdev-Fix-set_pci_devfn-to-visit-option-only-once.patch b/kvm-qdev-Fix-set_pci_devfn-to-visit-option-only-once.patch new file mode 100644 index 0000000..1339593 --- /dev/null +++ b/kvm-qdev-Fix-set_pci_devfn-to-visit-option-only-once.patch @@ -0,0 +1,128 @@ +From dbd3404564697435456f654f297a6ad4a6a7a351 Mon Sep 17 00:00:00 2001 +From: Kevin Wolf +Date: Tue, 19 Nov 2024 13:03:53 +0100 +Subject: [PATCH 1/4] qdev: Fix set_pci_devfn() to visit option only once + +RH-Author: Stefan Hajnoczi +RH-MergeRequest: 316: qdev-monitor: avoid QemuOpts in QMP device_add +RH-Jira: RHEL-39948 +RH-Acked-by: Kevin Wolf +RH-Acked-by: Hanna Czenczek +RH-Commit: [1/4] 03b399f0fb3aff073206bb5280da41ce4f4ea251 (stefanha/centos-stream-qemu-kvm) + +pci_devfn properties accept either a string or an integer as input. To +implement this, set_pci_devfn() first tries to visit the option as a +string, and if that fails, it visits it as an integer instead. While the +QemuOpts visitor happens to accept this, it is invalid according to the +visitor interface. QObject input visitors run into an assertion failure +when this is done. + +QObject input visitors are used with the JSON syntax version of -device +on the command line: + +$ ./qemu-system-x86_64 -enable-kvm -M q35 -device pcie-pci-bridge,id=pci.1,bus=pcie.0 -blockdev null-co,node-name=disk -device '{ "driver": "virtio-blk-pci", "drive": "disk", "id": "virtio-disk0", "bus": "pci.1", "addr": 1 }' +qemu-system-x86_64: ../qapi/qobject-input-visitor.c:143: QObject *qobject_input_try_get_object(QObjectInputVisitor *, const char *, _Bool): Assertion `removed' failed. + +The proper way to accept both strings and integers is using the +alternate mechanism, which tells us the type of the input before it's +visited. With this information, we can directly visit it as the right +type. + +This fixes set_pci_devfn() by using the alternate mechanism. + +Cc: qemu-stable@nongnu.org +Reported-by: Peter Maydell +Signed-off-by: Kevin Wolf +Message-ID: <20241119120353.57812-1-kwolf@redhat.com> +Acked-by: Paolo Bonzini +Reviewed-by: Markus Armbruster +Signed-off-by: Kevin Wolf +(cherry picked from commit 5102f9df4a9a7adfbd902f9515c3f8f53dba288e) +Signed-off-by: Stefan Hajnoczi +--- + hw/core/qdev-properties-system.c | 54 +++++++++++++++++++++----------- + 1 file changed, 36 insertions(+), 18 deletions(-) + +diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c +index 5cd527cdba..b182dc293a 100644 +--- a/hw/core/qdev-properties-system.c ++++ b/hw/core/qdev-properties-system.c +@@ -820,39 +820,57 @@ static void set_pci_devfn(Object *obj, Visitor *v, const char *name, + void *opaque, Error **errp) + { + Property *prop = opaque; ++ g_autofree GenericAlternate *alt; + int32_t value, *ptr = object_field_prop_ptr(obj, prop); + unsigned int slot, fn, n; +- char *str; ++ g_autofree char *str = NULL; ++ ++ if (!visit_start_alternate(v, name, &alt, sizeof(*alt), errp)) { ++ return; ++ } ++ ++ switch (alt->type) { ++ case QTYPE_QSTRING: ++ if (!visit_type_str(v, name, &str, errp)) { ++ goto out; ++ } + +- if (!visit_type_str(v, name, &str, NULL)) { ++ if (sscanf(str, "%x.%x%n", &slot, &fn, &n) != 2) { ++ fn = 0; ++ if (sscanf(str, "%x%n", &slot, &n) != 1) { ++ goto invalid; ++ } ++ } ++ if (str[n] != '\0' || fn > 7 || slot > 31) { ++ goto invalid; ++ } ++ *ptr = slot << 3 | fn; ++ break; ++ ++ case QTYPE_QNUM: + if (!visit_type_int32(v, name, &value, errp)) { +- return; ++ goto out; + } + if (value < -1 || value > 255) { + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, + name ? name : "null", "a value between -1 and 255"); +- return; ++ goto out; + } + *ptr = value; +- return; +- } ++ break; + +- if (sscanf(str, "%x.%x%n", &slot, &fn, &n) != 2) { +- fn = 0; +- if (sscanf(str, "%x%n", &slot, &n) != 1) { +- goto invalid; +- } +- } +- if (str[n] != '\0' || fn > 7 || slot > 31) { +- goto invalid; ++ default: ++ error_setg(errp, "Invalid parameter type for '%s', expected int or str", ++ name ? name : "null"); ++ goto out; + } +- *ptr = slot << 3 | fn; +- g_free(str); +- return; ++ ++ goto out; + + invalid: + error_set_from_qdev_prop_error(errp, EINVAL, obj, name, str); +- g_free(str); ++out: ++ visit_end_alternate(v, (void **) &alt); + } + + static int print_pci_devfn(Object *obj, Property *prop, char *dest, +-- +2.39.3 + diff --git a/kvm-qdev-monitor-avoid-QemuOpts-in-QMP-device_add.patch b/kvm-qdev-monitor-avoid-QemuOpts-in-QMP-device_add.patch new file mode 100644 index 0000000..40d54f5 --- /dev/null +++ b/kvm-qdev-monitor-avoid-QemuOpts-in-QMP-device_add.patch @@ -0,0 +1,131 @@ +From 16596225b9a4c686f6c0b0f5400681f3eed599ca Mon Sep 17 00:00:00 2001 +From: Stefan Hajnoczi +Date: Tue, 27 Aug 2024 15:27:50 -0400 +Subject: [PATCH 3/4] qdev-monitor: avoid QemuOpts in QMP device_add +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +RH-Author: Stefan Hajnoczi +RH-MergeRequest: 316: qdev-monitor: avoid QemuOpts in QMP device_add +RH-Jira: RHEL-39948 +RH-Acked-by: Kevin Wolf +RH-Acked-by: Hanna Czenczek +RH-Commit: [3/4] 1f6efb07255700e00243d067b5468b6840270ed0 (stefanha/centos-stream-qemu-kvm) + +The QMP device_add monitor command converts the QDict arguments to +QemuOpts and then back again to QDict. This process only supports scalar +types. Device properties like virtio-blk-pci's iothread-vq-mapping (an +array of objects) are silently dropped by qemu_opts_from_qdict() during +the QemuOpts conversion even though QAPI is capable of validating them. +As a result, hotplugging virtio-blk-pci devices with the +iothread-vq-mapping property does not work as expected (the property is +ignored). + +Get rid of the QemuOpts conversion in qmp_device_add() and call +qdev_device_add_from_qdict() with from_json=true. Using the QMP +command's QDict arguments directly allows non-scalar properties. + +The HMP is also adjusted since qmp_device_add()'s now expects properly +typed JSON arguments and cannot be used from HMP anymore. Move the code +that was previously in qmp_device_add() (with QemuOpts conversion and +from_json=false) into hmp_device_add() so that its behavior is +unchanged. + +This patch changes the behavior of QMP device_add but not HMP +device_add. QMP clients that sent incorrectly typed device_add QMP +commands no longer work. This is a breaking change but clients should be +using the correct types already. See the netdev_add QAPIfication in +commit db2a380c8457 for similar reasoning and object-add in commit +9151e59a8b6e. Unlike those commits, we continue to rely on 'gen': false +for the time being. + +Markus helped me figure this out and even provided a draft patch. The +code ended up very close to what he suggested. + +Suggested-by: Markus Armbruster +Cc: Daniel P. Berrangé +Signed-off-by: Stefan Hajnoczi +Message-ID: <20240827192751.948633-2-stefanha@redhat.com> +Reviewed-by: Daniel P. Berrangé +Reviewed-by: Kevin Wolf +Signed-off-by: Kevin Wolf +(cherry picked from commit be93fd53723cbdca675bd9ed112dae5cabbe1e91) +Signed-off-by: Stefan Hajnoczi +--- + system/qdev-monitor.c | 42 ++++++++++++++++++++++++++++-------------- + 1 file changed, 28 insertions(+), 14 deletions(-) + +diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c +index 6af6ef7d66..8b27cc42b0 100644 +--- a/system/qdev-monitor.c ++++ b/system/qdev-monitor.c +@@ -849,18 +849,9 @@ void hmp_info_qdm(Monitor *mon, const QDict *qdict) + + void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp) + { +- QemuOpts *opts; + DeviceState *dev; + +- opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict, errp); +- if (!opts) { +- return; +- } +- if (!monitor_cur_is_qmp() && qdev_device_help(opts)) { +- qemu_opts_del(opts); +- return; +- } +- dev = qdev_device_add(opts, errp); ++ dev = qdev_device_add_from_qdict(qdict, true, errp); + if (!dev) { + /* + * Drain all pending RCU callbacks. This is done because +@@ -872,9 +863,6 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp) + * to the user + */ + drain_call_rcu(); +- +- qemu_opts_del(opts); +- return; + } + object_unref(OBJECT(dev)); + } +@@ -967,8 +955,34 @@ void qmp_device_del(const char *id, Error **errp) + void hmp_device_add(Monitor *mon, const QDict *qdict) + { + Error *err = NULL; ++ QemuOpts *opts; ++ DeviceState *dev; + +- qmp_device_add((QDict *)qdict, NULL, &err); ++ opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict, &err); ++ if (!opts) { ++ goto out; ++ } ++ if (qdev_device_help(opts)) { ++ qemu_opts_del(opts); ++ return; ++ } ++ dev = qdev_device_add(opts, &err); ++ if (!dev) { ++ /* ++ * Drain all pending RCU callbacks. This is done because ++ * some bus related operations can delay a device removal ++ * (in this case this can happen if device is added and then ++ * removed due to a configuration error) ++ * to a RCU callback, but user might expect that this interface ++ * will finish its job completely once qmp command returns result ++ * to the user ++ */ ++ drain_call_rcu(); ++ ++ qemu_opts_del(opts); ++ } ++ object_unref(dev); ++out: + hmp_handle_error(mon, err); + } + +-- +2.39.3 + diff --git a/kvm-tests-avocado-hotplug_blk-Fix-addr-in-device_add-com.patch b/kvm-tests-avocado-hotplug_blk-Fix-addr-in-device_add-com.patch new file mode 100644 index 0000000..232d6a5 --- /dev/null +++ b/kvm-tests-avocado-hotplug_blk-Fix-addr-in-device_add-com.patch @@ -0,0 +1,51 @@ +From 6b2d9104632c948f37e640d471765f049cf01d57 Mon Sep 17 00:00:00 2001 +From: Kevin Wolf +Date: Fri, 22 Nov 2024 23:40:42 +0100 +Subject: [PATCH 2/4] tests/avocado/hotplug_blk: Fix addr in device_add command + +RH-Author: Stefan Hajnoczi +RH-MergeRequest: 316: qdev-monitor: avoid QemuOpts in QMP device_add +RH-Jira: RHEL-39948 +RH-Acked-by: Kevin Wolf +RH-Acked-by: Hanna Czenczek +RH-Commit: [2/4] 5848a0ec61904674659ef5f1f19e29065c5c8271 (stefanha/centos-stream-qemu-kvm) + +pci_devfn properties accept both integer and string values, but +integer 1 and string '1' have different meanings: The integer value +means device 0, function 1 whereas the string value '1' is short for +'1.0' and means device 1, function 0. + +This test wants the string version so that the device actually becomes +visible for the guest. device_add hides the problem because it goes +through QemuOpts, which turns all properties into strings - this is a +QEMU bug that we want to fix, but that cancelled out the bug in this +test. + +Fix the test first so that device_add can be fixed afterwards. + +Signed-off-by: Kevin Wolf +Message-ID: <20241122224042.149258-1-kwolf@redhat.com> +Reviewed-by: Markus Armbruster +Signed-off-by: Kevin Wolf +(cherry picked from commit 770de685353e8c495ad4773fbd4bc0db997e4dfd) +Signed-off-by: Stefan Hajnoczi +--- + tests/avocado/hotplug_blk.py | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/tests/avocado/hotplug_blk.py b/tests/avocado/hotplug_blk.py +index d55ded1c1d..b36bca02ec 100644 +--- a/tests/avocado/hotplug_blk.py ++++ b/tests/avocado/hotplug_blk.py +@@ -33,7 +33,7 @@ def plug(self) -> None: + 'drive': 'disk', + 'id': 'virtio-disk0', + 'bus': 'pci.1', +- 'addr': 1 ++ 'addr': '1', + } + + self.assert_no_vda() +-- +2.39.3 + diff --git a/kvm-vl-use-qmp_device_add-in-qemu_create_cli_devices.patch b/kvm-vl-use-qmp_device_add-in-qemu_create_cli_devices.patch new file mode 100644 index 0000000..c0fc2c4 --- /dev/null +++ b/kvm-vl-use-qmp_device_add-in-qemu_create_cli_devices.patch @@ -0,0 +1,65 @@ +From 621bad5cbfd7585f7bcef8bb4fcb17ab7ba52baa Mon Sep 17 00:00:00 2001 +From: Stefan Hajnoczi +Date: Tue, 27 Aug 2024 15:27:51 -0400 +Subject: [PATCH 4/4] vl: use qmp_device_add() in qemu_create_cli_devices() +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +RH-Author: Stefan Hajnoczi +RH-MergeRequest: 316: qdev-monitor: avoid QemuOpts in QMP device_add +RH-Jira: RHEL-39948 +RH-Acked-by: Kevin Wolf +RH-Acked-by: Hanna Czenczek +RH-Commit: [4/4] e653aca40742b262e36e4d8dc554a54235e40e99 (stefanha/centos-stream-qemu-kvm) + +qemu_create_cli_devices() should use qmp_device_add() to match the +behavior of the QMP monitor. A comment explained that libvirt changes +implementing strict CLI syntax were needed. + +Peter Krempa has confirmed that modern libvirt uses +the same JSON for -device (CLI) and device_add (QMP). Go ahead and use +qmp_device_add(). + +Cc: Peter Krempa +Reviewed-by: Markus Armbruster +Signed-off-by: Stefan Hajnoczi +Message-ID: <20240827192751.948633-3-stefanha@redhat.com> +Reviewed-by: Daniel P. Berrangé +Reviewed-by: Kevin Wolf +Signed-off-by: Kevin Wolf +(cherry picked from commit 11bf1d6aa06138e93b274e942d6992af63ffc510) +Signed-off-by: Stefan Hajnoczi +--- + system/vl.c | 14 ++++---------- + 1 file changed, 4 insertions(+), 10 deletions(-) + +diff --git a/system/vl.c b/system/vl.c +index 5359231bf5..900d471f5e 100644 +--- a/system/vl.c ++++ b/system/vl.c +@@ -2661,17 +2661,11 @@ 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; ++ QObject *ret_data = NULL; ++ + loc_push_restore(&opt->loc); +- /* +- * TODO Eventually we should call qmp_device_add() here to make sure it +- * behaves the same, but QMP still has to accept incorrectly typed +- * options until libvirt is fixed and we want to be strict on the CLI +- * from the start, so call qdev_device_add_from_qdict() directly for +- * now. +- */ +- dev = qdev_device_add_from_qdict(opt->opts, true, &error_fatal); +- object_unref(OBJECT(dev)); ++ qmp_device_add(opt->opts, &ret_data, &error_fatal); ++ assert(ret_data == NULL); /* error_fatal aborts */ + loc_pop(&opt->loc); + } + rom_reset_order_override(); +-- +2.39.3 + diff --git a/qemu-kvm.spec b/qemu-kvm.spec index b7d6c75..c18d1b0 100644 --- a/qemu-kvm.spec +++ b/qemu-kvm.spec @@ -149,7 +149,7 @@ Obsoletes: %{name}-block-ssh <= %{epoch}:%{version} \ Summary: QEMU is a machine emulator and virtualizer Name: qemu-kvm Version: 9.1.0 -Release: 8%{?rcrel}%{?dist}%{?cc_suffix} +Release: 9%{?rcrel}%{?dist}%{?cc_suffix} # Epoch because we pushed a qemu-1.0 package. AIUI this can't ever be dropped # Epoch 15 used for RHEL 8 # Epoch 17 used for RHEL 9 (due to release versioning offset in RHEL 8.5) @@ -334,6 +334,14 @@ Patch96: kvm-qga-skip-bind-mounts-in-fs-list.patch Patch97: kvm-vhost-fail-device-start-if-iotlb-update-fails.patch # For RHEL-67107 - [aarch64] [rhel-9.6] Backport some important post 9.1 qemu fixes Patch98: kvm-hw-char-pl011-Use-correct-masks-for-IBRD-and-FBRD.patch +# For RHEL-39948 - qom-get iothread-vq-mapping is empty on new hotplug disk [rhel-9.5] +Patch99: kvm-qdev-Fix-set_pci_devfn-to-visit-option-only-once.patch +# For RHEL-39948 - qom-get iothread-vq-mapping is empty on new hotplug disk [rhel-9.5] +Patch100: kvm-tests-avocado-hotplug_blk-Fix-addr-in-device_add-com.patch +# For RHEL-39948 - qom-get iothread-vq-mapping is empty on new hotplug disk [rhel-9.5] +Patch101: kvm-qdev-monitor-avoid-QemuOpts-in-QMP-device_add.patch +# For RHEL-39948 - qom-get iothread-vq-mapping is empty on new hotplug disk [rhel-9.5] +Patch102: kvm-vl-use-qmp_device_add-in-qemu_create_cli_devices.patch %if %{have_clang} BuildRequires: clang @@ -1400,6 +1408,14 @@ useradd -r -u 107 -g qemu -G kvm -d / -s /sbin/nologin \ %endif %changelog +* Mon Jan 13 2025 Miroslav Rezanina - 9.1.0-9 +- kvm-qdev-Fix-set_pci_devfn-to-visit-option-only-once.patch [RHEL-39948] +- kvm-tests-avocado-hotplug_blk-Fix-addr-in-device_add-com.patch [RHEL-39948] +- kvm-qdev-monitor-avoid-QemuOpts-in-QMP-device_add.patch [RHEL-39948] +- kvm-vl-use-qmp_device_add-in-qemu_create_cli_devices.patch [RHEL-39948] +- Resolves: RHEL-39948 + (qom-get iothread-vq-mapping is empty on new hotplug disk [rhel-9.5]) + * Mon Jan 06 2025 Miroslav Rezanina - 9.1.0-8 - kvm-linux-headers-Update-to-Linux-v6.12-rc5.patch [RHEL-50212] - kvm-s390x-cpumodel-add-msa10-subfunctions.patch [RHEL-50212]