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(-)
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 <j@getutm.app>
---
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'] }
+
##
# @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,
&err);
});
handleAnyDeviceErrors(err);
--
2.41.0Am 09.09.2024 um 03:58 hat Joelle van Dyne geschrieben:
> 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 <j@getutm.app>
I feel once you need to control such details of the backend, you should
really use a separate 'blockdev-add' commannd.
If it feels a bit too cumbersome to send explicit commands to open the
tray, remove the medium, insert the new medium referencing the node you
added with 'blockdev-add' and then close the tray again, I can
understand. Maybe what we should do is extend 'blockdev-change-medium'
so that it doesn't only accept a filename to specify the new images, but
alternatively also a node-name.
> + 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();
> + }
Using "file.locking" makes assumptions about what the passed filename
string would result in. There is nothing that guarantees that the block
driver even has a "file" child, or that the "file" child is referring
to a file-posix driver rather than using a different protocol or being a
filter driver above yet another node. It also doesn't consider backing
files and other non-primary children of the opened node.
So this is not correct, and I don't think there is any realistic way of
making it correct with this approach.
Kevin
On Mon, Sep 9, 2024 at 2:56 AM Kevin Wolf <kwolf@redhat.com> wrote:
>
> Am 09.09.2024 um 03:58 hat Joelle van Dyne geschrieben:
> > 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 <j@getutm.app>
>
> I feel once you need to control such details of the backend, you should
> really use a separate 'blockdev-add' commannd.
>
> If it feels a bit too cumbersome to send explicit commands to open the
> tray, remove the medium, insert the new medium referencing the node you
> added with 'blockdev-add' and then close the tray again, I can
> understand. Maybe what we should do is extend 'blockdev-change-medium'
> so that it doesn't only accept a filename to specify the new images, but
> alternatively also a node-name.
>
> > + 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();
> > + }
>
> Using "file.locking" makes assumptions about what the passed filename
> string would result in. There is nothing that guarantees that the block
> driver even has a "file" child, or that the "file" child is referring
> to a file-posix driver rather than using a different protocol or being a
> filter driver above yet another node. It also doesn't consider backing
> files and other non-primary children of the opened node.
>
> So this is not correct, and I don't think there is any realistic way of
> making it correct with this approach.
The existence of "filename" already makes this assumption that the
input is a file child. While I agree with you that there are better
ways to solve this problem, ultimately "blockdev-change-medium" will
have to be deprecated when this hypothetical "better" way of
referencing a node added with blockdev-add is introduced. Meanwhile
this solves a very real problem on macOS which is that trying to
change medium with an ISO which the OS has already mounted will always
fail even when "read-only-mode" is set.
>
> Kevin
>
Am 09.09.2024 um 16:25 hat Joelle van Dyne geschrieben:
> On Mon, Sep 9, 2024 at 2:56 AM Kevin Wolf <kwolf@redhat.com> wrote:
> >
> > Am 09.09.2024 um 03:58 hat Joelle van Dyne geschrieben:
> > > 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 <j@getutm.app>
> >
> > I feel once you need to control such details of the backend, you should
> > really use a separate 'blockdev-add' commannd.
> >
> > If it feels a bit too cumbersome to send explicit commands to open the
> > tray, remove the medium, insert the new medium referencing the node you
> > added with 'blockdev-add' and then close the tray again, I can
> > understand. Maybe what we should do is extend 'blockdev-change-medium'
> > so that it doesn't only accept a filename to specify the new images, but
> > alternatively also a node-name.
> >
> > > + 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();
> > > + }
> >
> > Using "file.locking" makes assumptions about what the passed filename
> > string would result in. There is nothing that guarantees that the block
> > driver even has a "file" child, or that the "file" child is referring
> > to a file-posix driver rather than using a different protocol or being a
> > filter driver above yet another node. It also doesn't consider backing
> > files and other non-primary children of the opened node.
> >
> > So this is not correct, and I don't think there is any realistic way of
> > making it correct with this approach.
>
> The existence of "filename" already makes this assumption that the
> input is a file child.
No. Try using something like "blkdebug::image.iso" or "nbd://localhost".
In the former case, you get another layer and the "file" child would be
a blkdebug node. To turn off locking on the file-posix block driver
you'd need to set "file.file.locking" in this case.
And the latter doesn't have any file-posix involved, it goes straight to
the NBD block driver.
> While I agree with you that there are better ways to solve this
> problem, ultimately "blockdev-change-medium" will have to be
> deprecated when this hypothetical "better" way of referencing a node
> added with blockdev-add is introduced.
This is not a hypothetical better way. It is how you can achieve this
today.
Extending blockdev-change-medium to alternatively accept a node-name
would just introduce a convenience shortcut (as is the whole command)
that doesn't require you to send four QMP commands (blockdev-open-tray,
blockdev-remove-medium, blockdev-insert-medium and blockdev-close-tray).
There is no need to deprecate blockdev-change-medium with a filename any
more than there is a need to deprecate -hda on the command line. We
could do it because there are alternatives that provide a strict
superset of functionality, but we don't have to.
Kevin
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 <j@getutm.app>
> ---
> 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.
> +
> ##
> # @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.
Regards,
Akihiko Odaki
On Mon, Sep 9, 2024 at 12:36 AM Akihiko Odaki <akihiko.odaki@daynix.com> 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 <j@getutm.app>
> > ---
> > 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
On 2024/09/09 23:18, Joelle van Dyne wrote:
> On Mon, Sep 9, 2024 at 12:36 AM Akihiko Odaki <akihiko.odaki@daynix.com> 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 <j@getutm.app>
>>> ---
>>> 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.
Speaking of semantics, it would be better to use OnOffAuto to match
BlockdevOptionsFile's locking property.
We could also argue that having a dedicated type would make this
consistent with the read-only-mode property, which has such a type, but
there are other properties that use existing types like str and bool so
I think it is fine to use an existing type here too.
>
>>
>>> +
>>> ##
>>> # @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.
I misread the code. I thought of it is a whitespace change for an
existing line but it is adding a line. This change in cocoa.m is fine.
Regards,
Akihiko Odaki
© 2016 - 2024 Red Hat, Inc.