On Mon, Sep 9, 2024 at 12:36 AM Akihiko Odaki <[email protected]> wrote:
>
> On 2024/09/09 10:58, Joelle van Dyne wrote:
> > New optional argument for 'blockdev-change-medium' QAPI command to allow
> > the caller to specify if they wish to enable file locking.
> >
> > Signed-off-by: Joelle van Dyne <[email protected]>
> > ---
> > qapi/block.json | 23 ++++++++++++++++++++++-
> > block/monitor/block-hmp-cmds.c | 2 +-
> > block/qapi-sysemu.c | 22 ++++++++++++++++++++++
> > ui/cocoa.m | 1 +
> > 4 files changed, 46 insertions(+), 2 deletions(-)
> >
> > diff --git a/qapi/block.json b/qapi/block.json
> > index e66666f5c6..35e8e2e191 100644
> > --- a/qapi/block.json
> > +++ b/qapi/block.json
> > @@ -309,6 +309,23 @@
> > { 'enum': 'BlockdevChangeReadOnlyMode',
> > 'data': ['retain', 'read-only', 'read-write'] }
> >
> > +##
> > +# @BlockdevChangeFileLockingMode:
> > +#
> > +# Specifies the new locking mode of a file image passed to the
> > +# @blockdev-change-medium command.
> > +#
> > +# @auto: Use locking if API is available
> > +#
> > +# @off: Disable file image locking
> > +#
> > +# @on: Enable file image locking
> > +#
> > +# Since: 9.2
> > +##
> > +{ 'enum': 'BlockdevChangeFileLockingMode',
> > + 'data': ['auto', 'off', 'on'] }
>
> You can use OnOffAuto type instead of defining your own.
This can be done. I had thought that defining a new type makes the
argument more explicit about the meaning.
>
> > +
> > ##
> > # @blockdev-change-medium:
> > #
> > @@ -330,6 +347,9 @@
> > # @read-only-mode: change the read-only mode of the device; defaults
> > # to 'retain'
> > #
> > +# @file-locking-mode: change the locking mode of the file image; defaults
> > +# to 'auto' (since: 9.2)
> > +#
> > # @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,
> > @@ -378,7 +398,8 @@
> > 'filename': 'str',
> > '*format': 'str',
> > '*force': 'bool',
> > - '*read-only-mode': 'BlockdevChangeReadOnlyMode' } }
> > + '*read-only-mode': 'BlockdevChangeReadOnlyMode',
> > + '*file-locking-mode': 'BlockdevChangeFileLockingMode' } }
> >
> > ##
> > # @DEVICE_TRAY_MOVED:
> > diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
> > index bdf2eb50b6..ff64020a80 100644
> > --- a/block/monitor/block-hmp-cmds.c
> > +++ b/block/monitor/block-hmp-cmds.c
> > @@ -1007,5 +1007,5 @@ void hmp_change_medium(Monitor *mon, const char
> > *device, const char *target,
> > }
> >
> > qmp_blockdev_change_medium(device, NULL, target, arg, true, force,
> > - !!read_only, read_only_mode, errp);
> > + !!read_only, read_only_mode, false, 0,
> > errp);
> > }
> > diff --git a/block/qapi-sysemu.c b/block/qapi-sysemu.c
> > index e4282631d2..8064bdfb3a 100644
> > --- a/block/qapi-sysemu.c
> > +++ b/block/qapi-sysemu.c
> > @@ -311,6 +311,8 @@ void qmp_blockdev_change_medium(const char *device,
> > bool has_force, bool force,
> > bool has_read_only,
> > BlockdevChangeReadOnlyMode read_only,
> > + bool has_file_locking_mode,
> > + BlockdevChangeFileLockingMode
> > file_locking_mode,
> > Error **errp)
> > {
> > BlockBackend *blk;
> > @@ -362,6 +364,26 @@ void qmp_blockdev_change_medium(const char *device,
> > qdict_put_str(options, "driver", format);
> > }
> >
> > + if (!has_file_locking_mode) {
> > + file_locking_mode = BLOCKDEV_CHANGE_FILE_LOCKING_MODE_AUTO;
> > + }
> > +
> > + switch (file_locking_mode) {
> > + case BLOCKDEV_CHANGE_FILE_LOCKING_MODE_AUTO:
> > + break;
> > +
> > + case BLOCKDEV_CHANGE_FILE_LOCKING_MODE_OFF:
> > + qdict_put_str(options, "file.locking", "off");
> > + break;
> > +
> > + case BLOCKDEV_CHANGE_FILE_LOCKING_MODE_ON:
> > + qdict_put_str(options, "file.locking", "on");
> > + break;
> > +
> > + default:
> > + abort();
> > + }
> > +
> > medium_bs = bdrv_open(filename, NULL, options, bdrv_flags, errp);
> >
> > if (!medium_bs) {
> > diff --git a/ui/cocoa.m b/ui/cocoa.m
> > index 4c2dd33532..6e73c6e13e 100644
> > --- a/ui/cocoa.m
> > +++ b/ui/cocoa.m
> > @@ -1611,6 +1611,7 @@ - (void)changeDeviceMedia:(id)sender
> > "raw",
> > true, false,
> > false, 0,
> > + false, 0,
>
> This change is irrelevant.
This change is needed otherwise QEMU will not compile.
>
> Regards,
> Akihiko Odaki
