On 2014-12-05 at 14:14, Eric Blake wrote:
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.
Yes, right.
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.
Hm, really?
HMP:
(qemu) change fdd ""
Could not open image: No such file or directory
QMP:
{'execute':'change','arguments':{'device':'fdd','target':''}}
{"timestamp": {"seconds": 1417785755, "microseconds": 519130}, "event":
"DEVICE_TRAY_MOVED", "data": {"device": "fdd", "tray-open": true}}
{"error": {"class": "GenericError", "desc": "Could not open image: No
such file or directory"}}
Technically, the medium is now empty, but that wasn't a successful
execution.
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).
If it was indeed possible somehow, I'd vote for disallowing empty file
names for blockdev-change-medium and instead invoke 'eject' from the
'change' commands (and point to 'eject' in the 'change' documentation).
Max
