Re: [Qemu-devel] [PATCH v2 0/4] blockdev: Add blockdev-change-medium with read-only option

[Eric Blake]

On 12/05/2014 03:08 AM, Max Reitz wrote:
> The 'change' QMP and HMP command allows replacing the medium in drives
> which support this, e.g. floppy disk drives. For some drives, the medium
> carries information about whether it can be written to or not (again,
> floppy drives). Therefore, it should be possible to change the read-only
> state of block devices when changing the loaded medium.
> 
> Following a suggestion from Eric, this series first introduces a
> 'blockdev-change-medium' QMP command which is intended to replace the
> 'change' command for block devices. Then, an optional additional
> 'read-only' parameter is added which allows chaning the read-only state

s/chaning/changing/ (I first read it as chaining) - of course, typos in
cover letters don't really matter in the long run :)

> in three ways:
> 
> - 'retain': Just keep the status as it was before; this is the current
>   behavior and thus this will be the default.
> - 'ro': Force read-only access
> - 'rw': Force writable access
> 
> Finally, that 'read-only' parameter is added to the HMP 'change'
> command. This series does not add a 'blockdev-change-medium' QMP command

I assume you meant HMP in this line.

> because 'change' being overloaded for VNC and block devices is not too
> bad for HMP (while it is for QMP).

I agree with that approach.

> 
> 
> v2:
> - basically completely rewritten
> - Dropped 'auto' [Kevin and Markus]
> - Introduced blockdev-change-medium [Eric]
> 
> - Patch 1 introduces the new QMP command 'blockdev-change-medium'; there
>   are (at least) two questionable design choices which I want to explain
>   here:
>   - The name is rather long; furthermore, the name 'change-blockdev' was
>     already suggested by the existing code. I used such a long name
>     because (1) there are no *-blockdev commands, but there are
>     blockdev-* commands, so "blockdev" should be the prefix, not the
>     suffix, and (2) "blockdev-change" could mean anything, so I wanted
>     to be as clear as possible.

That's actually a good explanation; I'm fine with the name you ended up
with, even if it feels long.

>   - The 'format' argument is optional; this is because by making it
>     mandatory, it would have been difficult for the 'change' QMP and HMP
>     commands to retain their 'format' argument optional as well (which
>     we have to do thanks to compatibility)

Yep, I can see that.  On the other hand, the other questionable feature
of 'change' was that it required a filename, but then allowed "" as the
filename that meant no new medium.  In patch 1/4, I wonder if we should
make the new command a bit stricter, by having 'filename' be optional,
and by forbidding "" as a filename (it's still a 1:1 mapping to the old
semantics, and while it would require more HMP glue, it would feel a bit
cleaner from the interface side).

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

signature.asc
Description: OpenPGP digital signature