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

[Max Reitz <mreitz@redhat.com>]

On 2014-12-05 at 14:31, Markus Armbruster wrote:
> Max Reitz <mreitz@redhat.com> writes:
>
>> 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
>> 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
>> because 'change' being overloaded for VNC and block devices is not too
>> bad for HMP (while it is for QMP).
> Debatable, but let's hash out QMP before we worry about HMP.
>
> I'd like to make sure that new commands to control removable media get
> us closer to a sane set of such commands.  Let's consider states and
> transitions.
>
> If we ignore the tray lock for a moment, we have:
>
>                                      load
>                      tray open  --------------->  tray closed
>                        empty    <---------------    empty
>                        ^   |        eject
>                        |   |
>          remove medium |   | insert medium
>                        |   |
>                        |   v        load
>                      tray open  --------------->  tray closed
>                         full    <---------------    full
>                                      eject
>
> Both the operator and the guest OS can load / eject.
>
> Only the operator can remove / insert medium.
>
> A tray lock complicates things a bit.  Each state above splits into a
> locked and unlocked state, with the obvious lock / unlock state
> transitions.  Only the guest OS can lock / unlock.
>
> When the tray is locked and closed, operator eject merely notifies the
> guest OS (blk_dev_eject_request(blk, false)).
>
> In states tray closed / locked, there's an additional operation "eject
> forcefully".  It notifies the guest OS (blk_dev_eject_request(blk,
> true)), and opens the tray.  Whether unlocks it depends on the device.
>
> Like change, blockdev-change-medium conflates several basic operations.
> Is that what we want, or should we create something that lets us do
> basic operations?

Good question. I don't think it will be bad in practice, though. If you 
split up blockdev-change-medium or really only change the medium without 
loading it, then the only advantage that you get is that you can 
exchange media without loading them; I can't really think of a use case 
for that, so in reality you'll always have blockdev-change-medium 
followed immediately by blockdev-load-medium or blockdev-close-tray or 
whatever.

You could split it up even more of course, then you'd have the following 
order for loading a medium:
(1) 'blockdev-open-tray', if not yet open
(2) 'blockdev-remove-medium', if not yet empty
(3) 'blockdev-insert-medium'
(4) 'blockdev-close-tray

I can't think of any time when you'd want to call insert-medium without 
close-tray or without having called remove-medium before (well, if it's 
empty, you don't have to, but well...).

And 'eject' does blockdev-open-tray plus blockdev-remove-medium, so...

Or better, let's collect use cases:
(1) Insert medium into empty drive and load it: Works with 
'blockdev-change-medium'
(2) Open drive, remove medium: Works with 'eject'
(3) Open drive, remove medium, insert medium, close drive: Works with 
'blockdev-change-medium'
(4) Open drive, remove medium, close drive: Does not work with only 
'eject' and 'blockdev-change-medium', but I can't see a difference 
between an open drive and a closed empty drive
(5) Open drive, repeatedly change medium, close drive: Does not work 
with 'blockdev-change-medium' because the guest will see all the media 
you cycled through; but I don't consider this an important use case

So, of course it may be nice in principle to have broken it down to the 
fundamental operations, but I don't see the practical implication.

Hm, well, there is one. I remember someone complaining that 'eject' 
sometimes removes the medium and sometimes doesn't. It did remove the 
medium when qemu could immediately eject it; but it didn't if the drive 
was locked, the guest was notified and then the guest opened the tray. 
So that is a practical implication, because after calling 
blockdev-open-tray, you'd be sure that the medium is still inserted.

I personally don't have a strong opinion. Introducing more commands 
would be work, but I guess I would have time for that now.

Max