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

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Denis V. Lunev (den@openvz.org) 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.

For HMP:

Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

(Although I'd be pretty careful with this; a guest OS might feel like
it could ignore anything else that was going on and keep it's data
cached if it had it's drive locked).

Dave

> 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>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
> ---
>  block/qapi-sysemu.c |  3 ++-
>  hmp-commands.hx     | 11 +++++++----
>  monitor/hmp-cmds.c  |  4 +++-
>  qapi/block.json     |  6 ++++++
>  ui/cocoa.m          |  1 +
>  5 files changed, 19 insertions(+), 6 deletions(-)
> 
> Changes from v1:
> - added kludge to Objective C code
> - simplified a bit call of do_open_tray() (thanks, Vova!)
> - added record to hmp-command.hx
> 
> diff --git a/block/qapi-sysemu.c b/block/qapi-sysemu.c
> index 8498402ad4..680c7ee342 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);
> +                      force, &err);
>      if (rc && rc != -ENOSYS) {
>          error_propagate(errp, err);
>          goto fail;
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 8476277aa9..6ec593ea08 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -202,9 +202,9 @@ ERST
>  
>      {
>          .name       = "change",
> -        .args_type  = "device:B,target:F,arg:s?,read-only-mode:s?",
> -        .params     = "device filename [format [read-only-mode]]",
> -        .help       = "change a removable medium, optional format",
> +        .args_type  = "device:B,force:-f,target:F,arg:s?,read-only-mode:s?",
> +        .params     = "device [-f] filename [format [read-only-mode]]",
> +        .help       = "change a removable medium, optional format, use -f to force the operation",
>          .cmd        = hmp_change,
>      },
>  
> @@ -212,11 +212,14 @@ SRST
>  ``change`` *device* *setting*
>    Change the configuration of a device.
>  
> -  ``change`` *diskdevice* *filename* [*format* [*read-only-mode*]]
> +  ``change`` *diskdevice* [-f] *filename* [*format* [*read-only-mode*]]
>      Change the medium for a removable disk device to point to *filename*. eg::
>  
>        (qemu) change ide1-cd0 /path/to/some.iso
>  
> +    ``-f``
> +      forces the operation even if the guest has locked the tray.
> +
>      *format* is optional.
>  
>      *read-only-mode* may be used to change the read-only status of the device.
> 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);
>      }
>  
> 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' } }
>  
>  
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index c4e5468f9e..a931174397 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -1529,6 +1529,7 @@ - (void)changeDeviceMedia:(id)sender
>                                         false, NULL,
>                                         [file cStringUsingEncoding:
>                                                   NSASCIIStringEncoding],
> +                                       true, false,
>                                         true, "raw",
>                                         false, 0,
>                                         &err);
> -- 
> 2.32.0
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK