* [PATCH for 7.1 1/1] block: add 'force' parameter to 'blockdev-change-medium' command
@ 2022-04-07 20:48 Denis V. Lunev
2022-04-07 21:51 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 3+ messages in thread
From: Denis V. Lunev @ 2022-04-07 20:48 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: Kevin Wolf, Dr. David Alan Gilbert, Markus Armbruster,
Hanna Reitz, Denis V. Lunev, Eric Blake
'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);
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);
}
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' } }
--
2.32.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH for 7.1 1/1] block: add 'force' parameter to 'blockdev-change-medium' command
2022-04-07 20:48 [PATCH for 7.1 1/1] block: add 'force' parameter to 'blockdev-change-medium' command Denis V. Lunev
@ 2022-04-07 21:51 ` Vladimir Sementsov-Ogievskiy
2022-04-07 21:58 ` Denis V. Lunev
0 siblings, 1 reply; 3+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-04-07 21:51 UTC (permalink / raw)
To: Denis V. Lunev, qemu-block, qemu-devel
Cc: Kevin Wolf, Dr. David Alan Gilbert, Markus Armbruster,
Hanna Reitz, Eric Blake
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH for 7.1 1/1] block: add 'force' parameter to 'blockdev-change-medium' command
2022-04-07 21:51 ` Vladimir Sementsov-Ogievskiy
@ 2022-04-07 21:58 ` Denis V. Lunev
0 siblings, 0 replies; 3+ messages in thread
From: Denis V. Lunev @ 2022-04-07 21:58 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: Kevin Wolf, Dr. David Alan Gilbert, Markus Armbruster,
Hanna Reitz, Eric Blake
On 08.04.2022 00:51, 'Vladimir Sementsov-Ogievskiy' via den wrote:
> 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);
you right :)
>
>> 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.
>
good point, I have added query for the dictionary thus it would be
logical to make an update in .hx file too
> 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.
>
Objective C file, looking like for MacOS UI
>> 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' } }
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-04-07 21:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-07 20:48 [PATCH for 7.1 1/1] block: add 'force' parameter to 'blockdev-change-medium' command Denis V. Lunev
2022-04-07 21:51 ` Vladimir Sementsov-Ogievskiy
2022-04-07 21:58 ` Denis V. Lunev
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.