Re: [PATCH for 7.1 1/1] block: add 'force' parameter to 'blockdev-change-medium' command

[Vladimir Sementsov-Ogievskiy]

07.04.2022 23:48, Denis V. Lunev wrote:
> 'blockdev-change-medium' is a convinient wrapper for the following
> sequence of commands:
>   * blockdev-open-tray
>   * blockdev-remove-medium
>   * blockdev-insert-medium
>   * blockdev-close-tray
> and should be used f.e. to change ISO image inside the CD-ROM tray.
> Though the guest could lock the tray and some linux guests like
> CentOS 8.5 actually does that. In this case the execution if this
> command results in the error like the following:
>    Device 'scsi0-0-1-0' is locked and force was not specified,
>    wait for tray to open and try again.
>
> This situation is could be resolved 'blockdev-open-tray' by passing
> flag 'force' inside. Thus is seems reasonable to add the same
> capability for 'blockdev-change-medium' too.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Hanna Reitz <hreitz@redhat.com>
> CC: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> ---
>   block/qapi-sysemu.c | 3 ++-
>   monitor/hmp-cmds.c  | 4 +++-
>   qapi/block.json     | 6 ++++++
>   3 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/block/qapi-sysemu.c b/block/qapi-sysemu.c
> index 8498402ad4..5b4fb75787 100644
> --- a/block/qapi-sysemu.c
> +++ b/block/qapi-sysemu.c
> @@ -318,6 +318,7 @@ void qmp_blockdev_change_medium(bool has_device, const char 
> *device,
>                                   bool has_id, const char *id,
>                                   const char *filename,
>                                   bool has_format, const char *format,
> +                                bool has_force, bool force,
>                                   bool has_read_only,
>                                   BlockdevChangeReadOnlyMode read_only,
>                                   Error **errp)
> @@ -380,7 +381,7 @@ void qmp_blockdev_change_medium(bool has_device, const char 
> *device,
>   
>       rc = do_open_tray(has_device ? device : NULL,
>                         has_id ? id : NULL,
> -                      false, &err);
> +                      has_force ? force : false, &err);

It's guaranteed for force to be false when has_force is false (ans similarly 
for pointers), so that can be written as

  rc = do_open_tray(device, id, force, &err);

>       if (rc && rc != -ENOSYS) {
>           error_propagate(errp, err);
>           goto fail;
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 634968498b..d8b98bed6c 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1472,6 +1472,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
>       const char *target = qdict_get_str(qdict, "target");
>       const char *arg = qdict_get_try_str(qdict, "arg");
>       const char *read_only = qdict_get_try_str(qdict, "read-only-mode");
> +    bool force = qdict_get_try_bool(qdict, "force", false);
>       BlockdevChangeReadOnlyMode read_only_mode = 0;
>       Error *err = NULL;
>   
> @@ -1508,7 +1509,8 @@ void hmp_change(Monitor *mon, const QDict *qdict)
>           }
>   
>           qmp_blockdev_change_medium(true, device, false, NULL, target,
> -                                   !!arg, arg, !!read_only, read_only_mode,
> +                                   !!arg, arg, true, force,
> +                                   !!read_only, read_only_mode,
>                                      &err);
>       }
>   

Should we also update hmp-commands.hx ? Or you just can pass "false, false" if 
you don't really need hmp interface for new feature.

Also, I don't know what ui/cocoa.m is, but seems it has call to 
qmp_blockdev_change_medium(), which most probably should be updated too.

> diff --git a/qapi/block.json b/qapi/block.json
> index 82fcf2c914..3f100d4887 100644
> --- a/qapi/block.json
> +++ b/qapi/block.json
> @@ -326,6 +326,11 @@
>   # @read-only-mode: change the read-only mode of the device; defaults
>   #                  to 'retain'
>   #
> +# @force: if false (the default), an eject request through blockdev-open-tray
> +#         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. (since 7.1)
> +#
>   # Features:
>   # @deprecated: Member @device is deprecated.  Use @id instead.
>   #
> @@ -367,6 +372,7 @@
>               '*id': 'str',
>               'filename': 'str',
>               '*format': 'str',
> +            '*force': 'bool',
>               '*read-only-mode': 'BlockdevChangeReadOnlyMode' } }
>   
>   


--
Best regards,
Vladimir