[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: |
Programmingkid |
Subject: |
Re: [Qemu-devel] [PATCH v4] monitor: allow device_del to accept QOM paths |
Date: |
Mon, 14 Sep 2015 12:17:20 -0400 |
On Sep 14, 2015, at 11:53 AM, Markus Armbruster wrote:
> "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.
If you do accept this patch, would you still be willing to
accept an auto-generated ID patch still?