[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 0/4] blockdev: Add blockdev-change-medium wit
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 0/4] blockdev: Add blockdev-change-medium with read-only option |
Date: |
Fri, 05 Dec 2014 06:14:10 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 |
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
- Re: [Qemu-devel] [PATCH v2 2/4] hmp: Use blockdev-change-medium for change command, (continued)
[Qemu-devel] [PATCH v2 3/4] blockdev: Add read-only option to blockdev-change-medium, Max Reitz, 2014/12/05
[Qemu-devel] [PATCH v2 1/4] qmp: Introduce blockdev-change-medium, Max Reitz, 2014/12/05
[Qemu-devel] [PATCH v2 4/4] hmp: Add read-only option to change command, Max Reitz, 2014/12/05
Re: [Qemu-devel] [PATCH v2 0/4] blockdev: Add blockdev-change-medium with read-only option,
Eric Blake <=
Re: [Qemu-devel] [PATCH v2 0/4] blockdev: Add blockdev-change-medium with read-only option, Markus Armbruster, 2014/12/05