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

[Eric Blake]

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.

> +
> +
> +##
>  # @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.

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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature