Re: [Qemu-devel] [PATCH RESEND 47/50] blockdev: Add read-only option to blockdev-change-medium

[Max Reitz]

On 2015-01-28 at 16:08, Eric Blake wrote:
> On 01/27/2015 12:46 PM, Max Reitz wrote:
> > Add an option to qmp_blockdev_change_medium() which allows changing the
> > read-only status of the block device whose medium is changed.
> >
> > Some drives do not have a inherently fixed read-only status; for
> > instance, floppy disks can be set read-only or writable independently of
> > the drive. Some users may find it useful to be able to therefore change
> > the read-only status of a block device when changing the medium.
> >
> > Signed-off-by: Max Reitz <address@hidden>
> > ---
> >   blockdev.c           | 25 ++++++++++++++++++++++++-
> >   hmp.c                |  2 +-
> >   qapi/block-core.json | 24 +++++++++++++++++++++++-
> >   qmp-commands.hx      | 24 +++++++++++++++++++++++-
> >   qmp.c                |  3 ++-
> >   5 files changed, 73 insertions(+), 5 deletions(-)
> >
> >   
> >   ##
> > +# @BlockdevChangeReadOnlyMode:
> > +#
> > +# Specifies the new read-only mode of a block device subject to the
> > +# @blockdev-change-medium command.
> > +#
> > +# @retain:  Retains the current read-only mode
> > +#
> > +# @ro:      Makes the device read-only
> > +#
> > +# @rw:      Makes the device writable
> > +#
> > +# Since: 2.3
> > +##
> > +{ 'enum': 'BlockdevChangeReadOnlyMode',
> > +  'data': ['retain', 'ro', 'rw'] }
> Bike-shedding; would 'read-only' and 'read-write' look any better than
> abbreviations?  Doesn't affect functionality, though.

I don't mind either way.

> > +
> > +
> > +##
> >   # @blockdev-change-medium:
> >   #
> >   # Changes the medium inserted into a block device by ejecting the current 
> > medium
> > @@ -1799,12 +1817,16 @@
> >   # @format:      #optional, format to open the new image with (defaults to the
> >   #               probed format)
> >   #
> > +# @read-only:   #optional, change the read-only mode of the device; defaults to
> > +#               'retain'
> "read-only":"rw" looks weird.  Maybe naming this "read-mode" instead of
> "read-only" would help.  Again, bikeshedding that doesn't affect
> functionality, but worth considering for the interface cleanliness.

Well, actually it's write-mode, because reading will always be possible. :-)

"access" would be another possibility, or "read-only-mode".

> So functionally, if nothing changes, you can add:
> Reviewed-by: Eric Blake <address@hidden>
>
> But if you change the interface on a respin, drop my R-b to make sure I
> check and still like the new naming conventions.

Understood.

Max