[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4] monitor: allow device_del to accept QOM path
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v4] monitor: allow device_del to accept QOM paths |
Date: |
Mon, 14 Sep 2015 17:53:47 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
"Daniel P. Berrange" <address@hidden> writes:
> Currently device_del requires that the client provide the
> device short ID. device_add allows devices may be created
"allows devices to be created"
Could perhaps be touched up on commit.
> without giving an ID, at which point there is no way to
> delete them with device_del. The QOM object path, however,
> provides an alternative way to identify the devices.
>
> Allowing device_del to accept an object path ensures all
> devices are deletable regardless of whether they have an
> ID.
>
> (qemu) device_add usb-mouse
> (qemu) qom-list /machine/peripheral-anon
> device[0] (child<usb-mouse>)
> type (string)
> (qemu) device_del /machine/peripheral-anon/device[0]
>
> Devices are required to be marked as hotpluggable
> otherwise an error is raised
>
> (qemu) device_del /machine/unattached/device[4]
> Device 'PIIX3' does not support hotplugging
>
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
>
> Changes in v4:
>
> - Drop additions to object_del, since that can be misused
> to delete internally created devices, and object_create
> mandates an ID parameter.
>
> hmp-commands.hx | 3 ++-
> qapi-schema.json | 2 +-
> qdev-monitor.c | 19 ++++++++++++++-----
> qmp-commands.hx | 7 ++++++-
> 4 files changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 286dcc7..e56373b 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -676,7 +676,8 @@ ETEXI
> STEXI
> @item device_del @var{id}
> @findex device_del
> -Remove device @var{id}.
> +Remove device @var{id}. @var{id} may be a short ID
> +or a QOM object path.
> ETEXI
>
> {
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 67fef37..273a906 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1950,7 +1950,7 @@
> #
> # Remove a device from a guest
> #
> -# @id: the name of the device
> +# @id: the name or QOM path of the device
> #
> # Returns: Nothing on success
> # If @id is not a valid device, DeviceNotFound
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index f9e2d62..295441b 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -785,19 +785,28 @@ void qmp_device_add(QDict *qdict, QObject **ret_data,
> Error **errp)
> void qmp_device_del(const char *id, Error **errp)
> {
> Object *obj;
> - char *root_path = object_get_canonical_path(qdev_get_peripheral());
> - char *path = g_strdup_printf("%s/%s", root_path, id);
>
> - g_free(root_path);
> - obj = object_resolve_path_type(path, TYPE_DEVICE, NULL);
> - g_free(path);
> + if (id[0] == '/') {
> + obj = object_resolve_path(id, NULL);
> + } else {
> + char *root_path = object_get_canonical_path(qdev_get_peripheral());
> + char *path = g_strdup_printf("%s/%s", root_path, id);
>
> + g_free(root_path);
> + obj = object_resolve_path_type(path, TYPE_DEVICE, NULL);
> + g_free(path);
> + }
Blank line deleted right here, could perhaps be fixed up on commit.
> if (!obj) {
> error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> "Device '%s' not found", id);
> return;
> }
>
> + if (!object_dynamic_cast(obj, TYPE_DEVICE)) {
> + error_setg(errp, "%s is not a hotpluggable device", id);
> + return;
> + }
> +
This error can happen only with a path, because with an ID, we use
obj = object_resolve_path_type(path, TYPE_DEVICE, NULL);
We could do similarly for paths, but doing it this way results in a
better error message.
> qdev_unplug(DEVICE(obj), errp);
> }
>
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 9848fd8..d3b4c7f 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -321,13 +321,18 @@ Remove a device.
>
> Arguments:
>
> -- "id": the device's ID (json-string)
> +- "id": the device's ID or QOM path (json-string)
>
> Example:
>
> -> { "execute": "device_del", "arguments": { "id": "net1" } }
> <- { "return": {} }
>
> +Example:
> +
> +-> { "execute": "device_del", "arguments": { "id":
> "/machine/peripheral-anon/device[0]" } }
> +<- { "return": {} }
> +
> EQMP
>
> {
Reviewed-by: Markus Armbruster <address@hidden>
If neither Andreas nor Paolo objects, I'd be willing to take this
through my tree.