From 33607f8bd2e0d56e854131c4e70c770b88fa5441 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 19 Nov 2024 13:03:53 +0100 Subject: [PATCH 1/7] qdev: Fix set_pci_devfn() to visit option only once RH-Author: Stefan Hajnoczi RH-MergeRequest: 312: qdev-monitor: avoid QemuOpts in QMP device_add RH-Jira: RHEL-43412 RH-Acked-by: Kevin Wolf RH-Acked-by: Hanna Czenczek RH-Commit: [1/4] 4d9ce49f16904d34d4f751f1dec3a53abfe8c8a8 (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