| Message ID | 1471625435-6190-6-git-send-email-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/19/2016 11:50 AM, Kevin Wolf wrote: > In order to remove the necessity to use BlockBackend names in the > external API, we want to allow qdev device names in all device related > commands. > > This converts x-blockdev-insert-medium to accept a qdev device name. > Since this command is experimental... > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > +++ b/qapi/block-core.json > @@ -2380,14 +2380,17 @@ > # This command is still a work in progress and is considered experimental. > # Stay away from it unless you want to help with its development. > # > -# @device: block device name > +# @device: block device name (deprecated, use @id instead) > +# > +# @id: the name or QOM path of the guest device (since: 2.8) ...why even bother to deprecate 'device'? Can't we just do a whole-sale switch to a required 'id' only? Or should such a wholesale switch be reserved for the day that we remove the x- prefix when promoting the command to stable?
Am 14.09.2016 um 22:57 hat Eric Blake geschrieben: > On 08/19/2016 11:50 AM, Kevin Wolf wrote: > > In order to remove the necessity to use BlockBackend names in the > > external API, we want to allow qdev device names in all device related > > commands. > > > > This converts x-blockdev-insert-medium to accept a qdev device name. > > Since this command is experimental... > > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > > +++ b/qapi/block-core.json > > @@ -2380,14 +2380,17 @@ > > # This command is still a work in progress and is considered experimental. > > # Stay away from it unless you want to help with its development. > > # > > -# @device: block device name > > +# @device: block device name (deprecated, use @id instead) > > +# > > +# @id: the name or QOM path of the guest device (since: 2.8) > > ...why even bother to deprecate 'device'? Can't we just do a whole-sale > switch to a required 'id' only? Or should such a wholesale switch be > reserved for the day that we remove the x- prefix when promoting the > command to stable? I tried this for the experimental commands, but I think it required some more test case rewrites, so I decided to leave it for later. Not necessarily "when promoting to stable" later, but a separate series anyway. Kevin
On 09/15/2016 03:37 AM, Kevin Wolf wrote: > Am 14.09.2016 um 22:57 hat Eric Blake geschrieben: >> On 08/19/2016 11:50 AM, Kevin Wolf wrote: >>> In order to remove the necessity to use BlockBackend names in the >>> external API, we want to allow qdev device names in all device related >>> commands. >>> >>> This converts x-blockdev-insert-medium to accept a qdev device name. >> >> Since this command is experimental... >> >>> Signed-off-by: Kevin Wolf <kwolf@redhat.com> >>> --- >> >>> +++ b/qapi/block-core.json >>> @@ -2380,14 +2380,17 @@ >>> # This command is still a work in progress and is considered experimental. >>> # Stay away from it unless you want to help with its development. >>> # >>> -# @device: block device name >>> +# @device: block device name (deprecated, use @id instead) >>> +# >>> +# @id: the name or QOM path of the guest device (since: 2.8) >> >> ...why even bother to deprecate 'device'? Can't we just do a whole-sale >> switch to a required 'id' only? Or should such a wholesale switch be >> reserved for the day that we remove the x- prefix when promoting the >> command to stable? > > I tried this for the experimental commands, but I think it required some > more test case rewrites, so I decided to leave it for later. Not > necessarily "when promoting to stable" later, but a separate series > anyway. Fair enough. Maybe mention it in the commit message, or even in a comment that part of promoting to stable may include removing the 'driver' parameter. That way, we won't forget the discussion. But in the meantime, the conversion looks sane, so I'm okay with adding: Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/blockdev.c b/blockdev.c index ee3a153..645c639 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2465,34 +2465,26 @@ out: aio_context_release(aio_context); } -static void qmp_blockdev_insert_anon_medium(const char *device, +static void qmp_blockdev_insert_anon_medium(BlockBackend *blk, BlockDriverState *bs, Error **errp) { - BlockBackend *blk; bool has_device; - blk = blk_by_name(device); - if (!blk) { - error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, - "Device '%s' not found", device); - return; - } - /* For BBs without a device, we can exchange the BDS tree at will */ has_device = blk_get_attached_dev(blk); if (has_device && !blk_dev_has_removable_media(blk)) { - error_setg(errp, "Device '%s' is not removable", device); + error_setg(errp, "Device is not removable"); return; } if (has_device && blk_dev_has_tray(blk) && !blk_dev_is_tray_open(blk)) { - error_setg(errp, "Tray of device '%s' is not open", device); + error_setg(errp, "Tray of the device is not open"); return; } if (blk_bs(blk)) { - error_setg(errp, "There already is a medium in device '%s'", device); + error_setg(errp, "There already is a medium in the device"); return; } @@ -2508,11 +2500,20 @@ static void qmp_blockdev_insert_anon_medium(const char *device, } } -void qmp_x_blockdev_insert_medium(const char *device, const char *node_name, - Error **errp) +void qmp_x_blockdev_insert_medium(bool has_device, const char *device, + bool has_id, const char *id, + const char *node_name, Error **errp) { + BlockBackend *blk; BlockDriverState *bs; + blk = qmp_get_blk(has_device ? device : NULL, + has_id ? id : NULL, + errp); + if (!blk) { + return; + } + bs = bdrv_find_node(node_name); if (!bs) { error_setg(errp, "Node '%s' not found", node_name); @@ -2525,7 +2526,7 @@ void qmp_x_blockdev_insert_medium(const char *device, const char *node_name, return; } - qmp_blockdev_insert_anon_medium(device, bs, errp); + qmp_blockdev_insert_anon_medium(blk, bs, errp); } void qmp_blockdev_change_medium(const char *device, const char *filename, @@ -2606,7 +2607,7 @@ void qmp_blockdev_change_medium(const char *device, const char *filename, goto fail; } - qmp_blockdev_insert_anon_medium(device, medium_bs, &err); + qmp_blockdev_insert_anon_medium(blk, medium_bs, &err); if (err) { error_propagate(errp, err); goto fail; diff --git a/qapi/block-core.json b/qapi/block-core.json index 069c699..2625bf8 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2380,14 +2380,17 @@ # This command is still a work in progress and is considered experimental. # Stay away from it unless you want to help with its development. # -# @device: block device name +# @device: block device name (deprecated, use @id instead) +# +# @id: the name or QOM path of the guest device (since: 2.8) # # @node-name: name of a node in the block driver state graph # # Since: 2.5 ## { 'command': 'x-blockdev-insert-medium', - 'data': { 'device': 'str', + 'data': { '*device': 'str', + '*id': 'str', 'node-name': 'str'} } diff --git a/qmp-commands.hx b/qmp-commands.hx index 0fd679c..b3aed7f 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -4412,7 +4412,7 @@ EQMP { .name = "x-blockdev-insert-medium", - .args_type = "device:s,node-name:s", + .args_type = "device:s?,id:s?,node-name:s", .mhandler.cmd_new = qmp_marshal_x_blockdev_insert_medium, }, @@ -4429,7 +4429,9 @@ Stay away from it unless you want to help with its development. Arguments: -- "device": block device name (json-string) +- "device": block device name (deprecated, use @id instead) + (json-string, optional) +- "id": the name or QOM path of the guest device (json-string, optional) - "node-name": root node of the BDS tree to insert into the block device Example:
In order to remove the necessity to use BlockBackend names in the external API, we want to allow qdev device names in all device related commands. This converts x-blockdev-insert-medium to accept a qdev device name. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- blockdev.c | 33 +++++++++++++++++---------------- qapi/block-core.json | 7 +++++-- qmp-commands.hx | 6 ++++-- 3 files changed, 26 insertions(+), 20 deletions(-)