On Mon, Sep 9, 2024 at 2:56 AM Kevin Wolf <kw...@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
>
