From: Alistair Francis <alistair23@gmail.com>
To: Damien Hedde <damien.hedde@greensocs.com>
Cc: "qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>,
"Peter Maydell" <peter.maydell@linaro.org>,
"Michael S. Tsirkin" <mst@redhat.com>,
"David Hildenbrand" <david@redhat.com>,
"Peter Xu" <peterx@redhat.com>,
mirela.grujic@greensocs.com,
"Alistair Francis" <Alistair.Francis@wdc.com>,
"Gerd Hoffmann" <kraxel@redhat.com>,
"Ani Sinha" <ani@anisinha.ca>, "Eric Blake" <eblake@redhat.com>,
"Stefano Stabellini" <sstabellini@kernel.org>,
"open list:X86" <xen-devel@lists.xenproject.org>,
"Paul Durrant" <paul@xen.org>,
"Markus Armbruster" <armbru@redhat.com>,
"Anthony Perard" <anthony.perard@citrix.com>,
"Marc-André Lureau" <marcandre.lureau@redhat.com>,
"Eduardo Habkost" <ehabkost@redhat.com>,
"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
"Eric Auger" <eric.auger@redhat.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"open list:RISC-V" <qemu-riscv@nongnu.org>,
"Daniel P. Berrangé" <berrange@redhat.com>,
"Mark Burton" <mark.burton@greensocs.com>,
"Edgar Iglesias" <edgari@xilinx.com>,
"Igor Mammedov" <imammedo@redhat.com>
Subject: Re: [RFC PATCH v2 04/16] softmmu/qdev-monitor: add error handling in qdev_set_id
Date: Wed, 13 Oct 2021 17:10:35 +1000 [thread overview]
Message-ID: <CAKmqyKNWBYpq8CbhtGyw9=1Gt_LujZZrtD7J_Ye2FyyVA6eqcw@mail.gmail.com> (raw)
In-Reply-To: <20210922161405.140018-5-damien.hedde@greensocs.com>
On Thu, Sep 23, 2021 at 2:29 AM Damien Hedde <damien.hedde@greensocs.com> wrote:
>
> qdev_set_id() is mostly used when the user adds a device (using
> -device cli option or device_add qmp command). This commit adds
> an error parameter to handle the case where the given id is
> already taken.
>
> Also document the function and add a return value in order to
> be able to capture success/failure: the function now returns the
> id in case of success, or NULL in case of failure.
>
> The commit modifies the 2 calling places (qdev-monitor and
> xen-legacy-backend) to add the error object parameter.
>
> Note that the id is, right now, guaranteed to be unique because
> all ids came from the "device" QemuOptsList where the id is used
> as key. This addition is a preparation for a future commit which
> will relax the uniqueness.
>
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> include/monitor/qdev.h | 25 +++++++++++++++++++++++-
> hw/xen/xen-legacy-backend.c | 3 ++-
> softmmu/qdev-monitor.c | 38 +++++++++++++++++++++++++++----------
> 3 files changed, 54 insertions(+), 12 deletions(-)
>
> diff --git a/include/monitor/qdev.h b/include/monitor/qdev.h
> index eaa947d73a..23c31f5296 100644
> --- a/include/monitor/qdev.h
> +++ b/include/monitor/qdev.h
> @@ -9,6 +9,29 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp);
>
> int qdev_device_help(QemuOpts *opts);
> DeviceState *qdev_device_add(QemuOpts *opts, Error **errp);
> -void qdev_set_id(DeviceState *dev, const char *id);
> +
> +/**
> + * qdev_set_id: parent the device and set its id if provided.
> + * @dev: device to handle
> + * @id: id to be given to the device, or NULL.
> + *
> + * Returns: the id of the device in case of success; otherwise NULL.
> + *
> + * @dev must be unrealized, unparented and must not have an id.
> + *
> + * If @id is non-NULL, this function tries to setup @dev qom path as
> + * "/peripheral/id". If @id is already taken, it fails. If it succeeds,
> + * the id field of @dev is set to @id (@dev now owns the given @id
> + * parameter).
> + *
> + * If @id is NULL, this function generates a unique name and setups @dev
> + * qom path as "/peripheral-anon/name". This name is not set as the id
> + * of @dev.
> + *
> + * Upon success, it returns the id/name (generated or provided). The
> + * returned string is owned by the corresponding child property and must
> + * not be freed by the caller.
> + */
> +const char *qdev_set_id(DeviceState *dev, const char *id, Error **errp);
>
> #endif
> diff --git a/hw/xen/xen-legacy-backend.c b/hw/xen/xen-legacy-backend.c
> index dd8ae1452d..f541cfa0e9 100644
> --- a/hw/xen/xen-legacy-backend.c
> +++ b/hw/xen/xen-legacy-backend.c
> @@ -276,7 +276,8 @@ static struct XenLegacyDevice *xen_be_get_xendev(const char *type, int dom,
> xendev = g_malloc0(ops->size);
> object_initialize(&xendev->qdev, ops->size, TYPE_XENBACKEND);
> OBJECT(xendev)->free = g_free;
> - qdev_set_id(DEVICE(xendev), g_strdup_printf("xen-%s-%d", type, dev));
> + qdev_set_id(DEVICE(xendev), g_strdup_printf("xen-%s-%d", type, dev),
> + &error_fatal);
> qdev_realize(DEVICE(xendev), xen_sysbus, &error_fatal);
> object_unref(OBJECT(xendev));
>
> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> index 25275984bd..0007698ff3 100644
> --- a/softmmu/qdev-monitor.c
> +++ b/softmmu/qdev-monitor.c
> @@ -578,22 +578,34 @@ static BusState *qbus_find(const char *path, Error **errp)
> return bus;
> }
>
> -void qdev_set_id(DeviceState *dev, const char *id)
> +const char *qdev_set_id(DeviceState *dev, const char *id, Error **errp)
> {
> + ObjectProperty *prop;
> +
> + assert(!dev->id && !dev->realized);
> +
> + /*
> + * object_property_[try_]add_child() below will assert the device
> + * has no parent
> + */
> if (id) {
> - dev->id = id;
> - }
> -
> - if (dev->id) {
> - object_property_add_child(qdev_get_peripheral(), dev->id,
> - OBJECT(dev));
> + prop = object_property_try_add_child(qdev_get_peripheral(), id,
> + OBJECT(dev), NULL);
> + if (prop) {
> + dev->id = id;
> + } else {
> + error_setg(errp, "Duplicate device ID '%s'", id);
> + return NULL;
> + }
> } else {
> static int anon_count;
> gchar *name = g_strdup_printf("device[%d]", anon_count++);
> - object_property_add_child(qdev_get_peripheral_anon(), name,
> - OBJECT(dev));
> + prop = object_property_add_child(qdev_get_peripheral_anon(), name,
> + OBJECT(dev));
> g_free(name);
> }
> +
> + return prop->name;
> }
>
> DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
> @@ -677,7 +689,13 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
> }
> }
>
> - qdev_set_id(dev, qemu_opts_id(opts));
> + /*
> + * set dev's parent and register its id.
> + * If it fails it means the id is already taken.
> + */
> + if (!qdev_set_id(dev, qemu_opts_id(opts), errp)) {
> + goto err_del_dev;
> + }
>
> /* set properties */
> if (qemu_opt_foreach(opts, set_property, dev, errp)) {
> --
> 2.33.0
>
>
next prev parent reply other threads:[~2021-10-13 7:11 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-22 16:13 [RFC PATCH v2 00/16] Initial support for machine creation via QMP Damien Hedde
2021-09-22 16:13 ` [RFC PATCH v2 01/16] rename MachineInitPhase enum constants for QAPI compatibility Damien Hedde
2021-09-22 16:13 ` [RFC PATCH v2 02/16] qapi: Implement query-machine-phase QMP command Damien Hedde
2021-09-22 17:37 ` Philippe Mathieu-Daudé
2021-09-23 12:43 ` Damien Hedde
2021-09-23 13:46 ` Ani Sinha
2021-10-12 22:08 ` Alistair Francis
2021-09-22 16:13 ` [RFC PATCH v2 03/16] qapi: Implement x-machine-init " Damien Hedde
2021-10-12 22:19 ` Alistair Francis
2021-09-22 16:13 ` [RFC PATCH v2 04/16] softmmu/qdev-monitor: add error handling in qdev_set_id Damien Hedde
2021-10-13 7:10 ` Alistair Francis [this message]
2021-10-13 14:30 ` Damien Hedde
2021-09-22 16:13 ` [RFC PATCH v2 05/16] qdev-monitor: prevent conflicts between qmp/device_add and cli/-device Damien Hedde
2021-09-22 16:13 ` [RFC PATCH v2 06/16] qapi: Allow device_add to execute in machine initialized phase Damien Hedde
2021-10-12 22:25 ` Alistair Francis
2021-09-22 16:13 ` [RFC PATCH v2 07/16] hw/core/machine: add machine_class_is_dynamic_sysbus_dev_allowed Damien Hedde
2021-09-23 10:51 ` Ani Sinha
2021-09-23 13:07 ` Damien Hedde
2021-09-22 16:13 ` [RFC PATCH v2 08/16] qdev-monitor: Check sysbus device type before creating it Damien Hedde
2021-10-12 22:42 ` Alistair Francis
2021-09-22 16:13 ` [RFC PATCH v2 09/16] hw/core/machine: Remove the dynamic sysbus devices type check Damien Hedde
2021-10-12 23:07 ` Alistair Francis
2021-09-22 16:13 ` [RFC PATCH v2 10/16] qdev-monitor: allow adding any sysbus device before machine is ready Damien Hedde
2021-09-23 11:04 ` Ani Sinha
2021-09-23 11:55 ` Ani Sinha
2021-09-23 14:04 ` Damien Hedde
2021-09-22 16:14 ` [RFC PATCH v2 11/16] softmmu/memory: add memory_region_try_add_subregion function Damien Hedde
2021-09-22 17:42 ` Philippe Mathieu-Daudé
2021-09-22 17:49 ` David Hildenbrand
2021-10-13 7:12 ` Alistair Francis
2021-09-22 16:14 ` [RFC PATCH v2 12/16] add x-sysbus-mmio-map qmp command Damien Hedde
2021-10-13 7:16 ` Alistair Francis
2021-09-22 16:14 ` [RFC PATCH v2 13/16] hw/mem/system-memory: add a memory sysbus device Damien Hedde
2021-09-22 16:14 ` [RFC PATCH v2 14/16] docs/system: add doc about the initialized machine phase and an example Damien Hedde
2021-09-22 16:14 ` [RFC PATCH v2 15/16] hw/char/ibex_uart: set user_creatable Damien Hedde
2021-09-22 16:14 ` [RFC PATCH v2 16/16] hw/intc/ibex_plic: " Damien Hedde
2021-09-22 17:50 ` [RFC PATCH v2 00/16] Initial support for machine creation via QMP Philippe Mathieu-Daudé
2021-09-23 11:08 ` Damien Hedde
2021-10-04 15:56 ` Damien Hedde
2021-10-12 22:16 ` Alistair Francis
2021-10-13 5:56 ` Mark Burton
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='CAKmqyKNWBYpq8CbhtGyw9=1Gt_LujZZrtD7J_Ye2FyyVA6eqcw@mail.gmail.com' \
--to=alistair23@gmail.com \
--cc=Alistair.Francis@wdc.com \
--cc=ani@anisinha.ca \
--cc=anthony.perard@citrix.com \
--cc=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=damien.hedde@greensocs.com \
--cc=david@redhat.com \
--cc=dgilbert@redhat.com \
--cc=eblake@redhat.com \
--cc=edgari@xilinx.com \
--cc=ehabkost@redhat.com \
--cc=eric.auger@redhat.com \
--cc=imammedo@redhat.com \
--cc=kraxel@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=mark.burton@greensocs.com \
--cc=mirela.grujic@greensocs.com \
--cc=mst@redhat.com \
--cc=paul@xen.org \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=peterx@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).