| Message ID | 20220412095048.231030-1-den@openvz.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,for,7.1,1/1] block: add 'force' parameter to 'blockdev-change-medium' command | expand |
* 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 >
12.04.2022 12:50, 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> > 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", The addition should go here, after format options. With that fixed: Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org> > false, 0, > &err);
On 12.04.2022 16:17, Vladimir Sementsov-Ogievskiy wrote: > 12.04.2022 12:50, 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> >> 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", > > The addition should go here, after format options. With that fixed: > > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org> > Nope. Either we should fix prototype in C and JSON all around, i.e. put "'*force': 'bool'," after " '*read-only-mode': 'BlockdevChangeReadOnlyMode' " in JSON, or the code is correct. Den
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);
'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> 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