| Message ID | 1471625435-6190-5-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 blockdev-open/close-tray to accept a qdev device name. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > blockdev.c | 60 +++++++++++++++++++++++++++++++++++++++------------- > qapi/block-core.json | 14 ++++++++---- > qmp-commands.hx | 12 +++++++---- > 3 files changed, 63 insertions(+), 23 deletions(-) > > +++ b/qapi/block-core.json > @@ -2316,7 +2316,9 @@ > # to it > # - if the guest device does not have an actual tray > # > -# @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) Wish there were an easier way to write mutually-exclusive pairs in JSON, but without that, your approach is fine. > +++ b/qmp-commands.hx > @@ -4277,7 +4277,7 @@ EQMP > > { > .name = "blockdev-open-tray", > - .args_type = "device:s,force:b?", > + .args_type = "device:s?,id:s?,force:b?", > .mhandler.cmd_new = qmp_marshal_blockdev_open_tray, > }, Will conflict with Marc-Andre's work to remove qmp-commands.hx; but we can figure it out based on what merges first. > > @@ -4302,7 +4302,9 @@ which no such event will be generated, these include: > > 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) > - "force": if false (the default), an eject request will be sent to the guest if > it has locked the tray (and the tray will not be opened immediately); > if true, the tray will be opened regardless of whether it is locked Are there any example code snippets that should be updated alongside this? If not, should we be thinking of adding an example? But I can live with this patch as an incremental improvement, even if we decide we want more as a followup based on my question above, so: Reviewed-by: Eric Blake <eblake@redhat.com>
Am 14.09.2016 um 22:49 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 blockdev-open/close-tray to accept a qdev device name. > > > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > blockdev.c | 60 +++++++++++++++++++++++++++++++++++++++------------- > > qapi/block-core.json | 14 ++++++++---- > > qmp-commands.hx | 12 +++++++---- > > 3 files changed, 63 insertions(+), 23 deletions(-) > > > > > +++ b/qapi/block-core.json > > @@ -2316,7 +2316,9 @@ > > # to it > > # - if the guest device does not have an actual tray > > # > > -# @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) > > Wish there were an easier way to write mutually-exclusive pairs in JSON, > but without that, your approach is fine. > > > +++ b/qmp-commands.hx > > @@ -4277,7 +4277,7 @@ EQMP > > > > { > > .name = "blockdev-open-tray", > > - .args_type = "device:s,force:b?", > > + .args_type = "device:s?,id:s?,force:b?", > > .mhandler.cmd_new = qmp_marshal_blockdev_open_tray, > > }, > > Will conflict with Marc-Andre's work to remove qmp-commands.hx; but we > can figure it out based on what merges first. > > > > > @@ -4302,7 +4302,9 @@ which no such event will be generated, these include: > > > > 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) > > - "force": if false (the default), an eject request will be sent to the guest if > > it has locked the tray (and the tray will not be opened immediately); > > if true, the tray will be opened regardless of whether it is locked > > Are there any example code snippets that should be updated alongside > this? If not, should we be thinking of adding an example? We could probably update the examples to avoid deprecated fields in them. Though the old examples still work, so is it worth changing all examples if we're goig to remove qmp-commands.hx anyway? Or will the examples be moved to somewhere else? Kevin
On 09/15/2016 03:35 AM, Kevin Wolf wrote: >>> >>> -- "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) >>> - "force": if false (the default), an eject request will be sent to the guest if >>> it has locked the tray (and the tray will not be opened immediately); >>> if true, the tray will be opened regardless of whether it is locked >> >> Are there any example code snippets that should be updated alongside >> this? If not, should we be thinking of adding an example? > > We could probably update the examples to avoid deprecated fields in > them. Though the old examples still work, so is it worth changing all > examples if we're goig to remove qmp-commands.hx anyway? Or will the > examples be moved to somewhere else? Marc-Andre is moving the examples into qapi-schema.json (and friends), over the course of multiple commits. It's going to be a lot of churn and potential merge conflicts, based on what changes go into qmp-commands.hx after his work starts, but hopefully we can avoid things slipping through the cracks, without too much rebase pain on either Marc-Andre or other developers.
Am 15.09.2016 um 17:51 hat Eric Blake geschrieben: > On 09/15/2016 03:35 AM, Kevin Wolf wrote: > > >>> > >>> -- "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) > >>> - "force": if false (the default), an eject request will be sent to the guest if > >>> it has locked the tray (and the tray will not be opened immediately); > >>> if true, the tray will be opened regardless of whether it is locked > >> > >> Are there any example code snippets that should be updated alongside > >> this? If not, should we be thinking of adding an example? > > > > We could probably update the examples to avoid deprecated fields in > > them. Though the old examples still work, so is it worth changing all > > examples if we're goig to remove qmp-commands.hx anyway? Or will the > > examples be moved to somewhere else? > > Marc-Andre is moving the examples into qapi-schema.json (and friends), > over the course of multiple commits. It's going to be a lot of churn > and potential merge conflicts, based on what changes go into > qmp-commands.hx after his work starts, but hopefully we can avoid things > slipping through the cracks, without too much rebase pain on either > Marc-Andre or other developers. Ok, I'm changing the examples from 'device' to 'id' now. And actually the first one immediately made me notice that the DEVICE_TRAY_MOVED event needs to be changed, too (because its 'device' field can be empty now). Matter for another series, but I put it on my list. Kevin
diff --git a/blockdev.c b/blockdev.c index 97062e3..ee3a153 100644 --- a/blockdev.c +++ b/blockdev.c @@ -56,7 +56,8 @@ static QTAILQ_HEAD(, BlockDriverState) monitor_bdrv_states = QTAILQ_HEAD_INITIALIZER(monitor_bdrv_states); -static int do_open_tray(const char *device, bool force, Error **errp); +static int do_open_tray(const char *blk_name, const char *qdev_id, + bool force, Error **errp); static const char *const if_name[IF_COUNT] = { [IF_NONE] = "none", @@ -1196,6 +1197,29 @@ static BlockDriverState *qmp_get_root_bs(const char *name, Error **errp) return bs; } +static BlockBackend *qmp_get_blk(const char *blk_name, const char *qdev_id, + Error **errp) +{ + BlockBackend *blk; + + if (!blk_name == !qdev_id) { + error_setg(errp, "Need exactly one of 'device' and 'id'"); + return NULL; + } + + if (qdev_id) { + blk = blk_by_qdev_id(qdev_id, errp); + } else { + blk = blk_by_name(blk_name); + if (blk == NULL) { + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, + "Device '%s' not found", blk_name); + } + } + + return blk; +} + void hmp_commit(Monitor *mon, const QDict *qdict) { const char *device = qdict_get_str(qdict, "device"); @@ -2248,7 +2272,7 @@ void qmp_eject(const char *device, bool has_force, bool force, Error **errp) force = false; } - rc = do_open_tray(device, force, &local_err); + rc = do_open_tray(device, NULL, force, &local_err); if (rc && rc != -ENOSYS) { error_propagate(errp, local_err); return; @@ -2293,15 +2317,15 @@ void qmp_block_passwd(bool has_device, const char *device, * If the guest was asked to open the tray, return -EINPROGRESS. * Else, return 0. */ -static int do_open_tray(const char *device, bool force, Error **errp) +static int do_open_tray(const char *blk_name, const char *qdev_id, + bool force, Error **errp) { BlockBackend *blk; + const char *device = qdev_id ?: blk_name; bool locked; - blk = blk_by_name(device); + blk = qmp_get_blk(blk_name, qdev_id, errp); if (!blk) { - error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, - "Device '%s' not found", device); return -ENODEV; } @@ -2337,7 +2361,9 @@ static int do_open_tray(const char *device, bool force, Error **errp) return 0; } -void qmp_blockdev_open_tray(const char *device, bool has_force, bool force, +void qmp_blockdev_open_tray(bool has_device, const char *device, + bool has_id, const char *id, + bool has_force, bool force, Error **errp) { Error *local_err = NULL; @@ -2346,7 +2372,9 @@ void qmp_blockdev_open_tray(const char *device, bool has_force, bool force, if (!has_force) { force = false; } - rc = do_open_tray(device, force, &local_err); + rc = do_open_tray(has_device ? device : NULL, + has_id ? id : NULL, + force, &local_err); if (rc && rc != -ENOSYS && rc != -EINPROGRESS) { error_propagate(errp, local_err); return; @@ -2354,19 +2382,21 @@ void qmp_blockdev_open_tray(const char *device, bool has_force, bool force, error_free(local_err); } -void qmp_blockdev_close_tray(const char *device, Error **errp) +void qmp_blockdev_close_tray(bool has_device, const char *device, + bool has_id, const char *id, + Error **errp) { BlockBackend *blk; - blk = blk_by_name(device); + blk = qmp_get_blk(has_device ? device : NULL, + has_id ? id : NULL, + errp); if (!blk) { - error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, - "Device '%s' not found", device); return; } if (!blk_dev_has_removable_media(blk)) { - error_setg(errp, "Device '%s' is not removable", device); + error_setg(errp, "Device '%s' is not removable", device ?: id); return; } @@ -2562,7 +2592,7 @@ void qmp_blockdev_change_medium(const char *device, const char *filename, goto fail; } - rc = do_open_tray(device, false, &err); + rc = do_open_tray(device, NULL, false, &err); if (rc && rc != -ENOSYS) { error_propagate(errp, err); goto fail; @@ -2584,7 +2614,7 @@ void qmp_blockdev_change_medium(const char *device, const char *filename, blk_apply_root_state(blk, medium_bs); - qmp_blockdev_close_tray(device, errp); + qmp_blockdev_close_tray(true, device, false, NULL, errp); fail: /* If the medium has been inserted, the device has its own reference, so diff --git a/qapi/block-core.json b/qapi/block-core.json index 31f9990..069c699 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2316,7 +2316,9 @@ # to it # - if the guest device does not have an actual tray # -# @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) # # @force: #optional if false (the default), an eject request will be sent to # the guest if it has locked the tray (and the tray will not be opened @@ -2326,7 +2328,8 @@ # Since: 2.5 ## { 'command': 'blockdev-open-tray', - 'data': { 'device': 'str', + 'data': { '*device': 'str', + '*id': 'str', '*force': 'bool' } } ## @@ -2338,12 +2341,15 @@ # # If the tray was already closed before, this will be a no-op. # -# @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) # # Since: 2.5 ## { 'command': 'blockdev-close-tray', - 'data': { 'device': 'str' } } + 'data': { '*device': 'str', + '*id': 'str' } } ## # @x-blockdev-remove-medium: diff --git a/qmp-commands.hx b/qmp-commands.hx index ba2a916..0fd679c 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -4277,7 +4277,7 @@ EQMP { .name = "blockdev-open-tray", - .args_type = "device:s,force:b?", + .args_type = "device:s?,id:s?,force:b?", .mhandler.cmd_new = qmp_marshal_blockdev_open_tray, }, @@ -4302,7 +4302,9 @@ which no such event will be generated, these include: 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) - "force": if false (the default), an eject request will be sent to the guest if it has locked the tray (and the tray will not be opened immediately); if true, the tray will be opened regardless of whether it is locked @@ -4325,7 +4327,7 @@ EQMP { .name = "blockdev-close-tray", - .args_type = "device:s", + .args_type = "device:s?,id:s?", .mhandler.cmd_new = qmp_marshal_blockdev_close_tray, }, @@ -4341,7 +4343,9 @@ If the tray was already closed before, this will be a no-op. 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) 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 blockdev-open/close-tray to accept a qdev device name. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- blockdev.c | 60 +++++++++++++++++++++++++++++++++++++++------------- qapi/block-core.json | 14 ++++++++---- qmp-commands.hx | 12 +++++++---- 3 files changed, 63 insertions(+), 23 deletions(-)