Re: [Qemu-devel] [PATCH v2] monitor: allow object_del & device_del to accept QOM paths

[Programmingkid]

On Aug 27, 2015, at 12:13 PM, Daniel P. Berrange wrote:

> Currently both object_del and device_del require that the
> client provide the object/device short ID. While user
> creatable objects require an ID to be provided at time of
> creation, qdev devices may be created without giving an
> ID. The only unique identifier they would then have is the
> QOM object path.
> 
> 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]
> Although objects require an ID to be provided upfront,
> there may be cases where the client would prefer to
> use QOM paths when deleting.
> 
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
> hmp-commands.hx  |  6 ++++--
> qapi-schema.json |  4 ++--
> qdev-monitor.c   | 14 +++++++++-----
> qmp-commands.hx  | 13 +++++++++++--
> qmp.c            | 10 +++++++---
> 5 files changed, 33 insertions(+), 14 deletions(-)
> 
> Changed in v2:
> 
> - Fix stupid typo in qmp-commands.hx
> - Expand parameter docs in qapi-schema.json
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index d3b7932..c0c113d 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -675,7 +675,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
> 
>     {
> @@ -1279,7 +1280,8 @@ ETEXI
> STEXI
> @item object_del
> @findex object_del
> -Destroy QOM object.
> +Destroy QOM object. @var{id} may be a short ID
> +or a QOM object path.
> ETEXI
> 
> #ifdef CONFIG_SLIRP
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 4342a08..bf9ef3a 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
> @@ -2121,7 +2121,7 @@
> #
> # Remove a QOM object.
> #
> -# @id: the name of the QOM object to remove
> +# @id: the name or path of the QOM object to remove
> #
> # Returns: Nothing on success
> #          Error if @id is not a valid id for a QOM object
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index f9e2d62..894b799 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -785,13 +785,17 @@ 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);
> +    }
>     if (!obj) {
>         error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
>                   "Device '%s' not found", id);
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index ba630b1..09fc206 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
> 
>     {
> @@ -965,7 +970,7 @@ Remove QOM object.
> 
> Arguments:
> 
> -- "id": the object's ID (json-string)
> +- "id": the object's ID or QOM path (json-string)
> 
> Example:
> 
> @@ -973,6 +978,10 @@ Example:
> <- { "return": {} }
> 
> 
> +-> { "execute": "object-del", "arguments": { "id": "/objects/hostmem1" } }
> +<- { "return": {} }
> +
> +
> EQMP
> 
> 
> diff --git a/qmp.c b/qmp.c
> index 403805a..11e9f51 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -678,11 +678,15 @@ void qmp_object_add(QDict *qdict, QObject **ret, Error 
> **errp)
> 
> void qmp_object_del(const char *id, Error **errp)
> {
> -    Object *container;
>     Object *obj;
> 
> -    container = object_get_objects_root();
> -    obj = object_resolve_path_component(container, id);
> +    if (id[0] == '/') {
> +        obj = object_resolve_path(id, NULL);
> +    } else {
> +        Object *container;
> +        container = object_get_objects_root();
> +        obj = object_resolve_path_component(container, id);
> +    }
>     if (!obj) {
>         error_setg(errp, "object id not found");
>         return;
> -- 
> 2.4.3
> 

Your patch do the job, but could be tweaked. Could we make an alias for
"qom-list /machine/peripheral-anon" that is actually easy to remember. Perhaps 
"info devices". 

Also typing "device_del /machine/peripheral-anon/device[0]" seems unnecessary. 
It probably 
could be shortened to just "device_del device[0]". 

Thank you.