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

[Max Reitz <mreitz@redhat.com>]

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 <mreitz@redhat.com>
>> ---
>>   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 <eblake@redhat.com>
>
> 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