Re: [Qemu-devel] [PATCH] qmp: add BLOCK_MEDIUM_EJECT event

[Luiz Capitulino]

On Wed, 25 Jan 2012 10:42:04 -0200
Luiz Capitulino <address@hidden> wrote:

> On Wed, 25 Jan 2012 09:41:20 +0100
> Kevin Wolf <address@hidden> wrote:
> 
> > Am 24.01.2012 21:03, schrieb Eric Blake:
> > > On 01/24/2012 11:16 AM, Luiz Capitulino wrote:
> > >> Libvirt wants to be notified when the guest ejects a medium, so that
> > >> it can update its view of the guest.
> > >>
> > >> This code has been originally written by Daniel Berrange. It adds
> > >> the event to IDE and SCSI emulation.
> > >>
> > >> Please, note that this only covers guest initiated ejects, that's,
> > >> the QMP/HMP commands 'eject' and 'change' are not covered.
> > 
> > What's the reason for this behaviour? It feels inconsistent.
> 
> I don't think it's inconsistent because we're limiting it to guest initiated
> actions. Also, the mngt app knows when it sends a 'eject' or 'change' command.
> The exception is if it allows HMP to run in parallel with QMP, but I don't 
> think
> this is recommended (at least not for commands that change any VM state).

Let me elaborate more. We have two options:

 1. Emit the event for guest initiated ejects (this patch, although I think
    the event should be renamed to GUEST_MEDIUM_EJECT)

 2. Emit the event for guest & QMP initiated ejects, that's, also emit the
    event for the eject and change commands

The first thing to note is that, they're not mutually exclusive. If we do
item 1 now, we can add 2 later (as a different event).

But my point is that doing 2 is problematic and we should avoid it. The problem
is that the semantics of eject and change are complex and/or buggy. And 
something
I've learned about confusing semantics in QMP is that, they will give you 
headaches
soon or later.

The main problem with eject is that it's inconsistent with the handling of
the tray status. Try this:

1. Eject with no medium inserted

(qemu) info block
ide1-cd0: removable=1 locked=0 tray-open=0 io-status=ok [not inserted]
(qemu) eject ide1-cd0
(qemu) info block
ide1-cd0: removable=1 locked=0 tray-open=0 io-status=ok [not inserted]
(qemu) 

Conclusion: the tray didn't move, we shouldn't emit the event.

2. Eject with medium inserted

(qemu) info block
ide1-cd0: removable=1 locked=0 tray-open=0 io-status=ok file=/tmp/vl.iKvBAF 
backing_file=/mnt/fernando/isos/linux/Fedora-14-x86_64-DVD.iso ro=0 drv=qcow2 
encrypted=0 bps=0 bps_rd=0 bps_wr=0 iops=0 iops_rd=0 iops_wr=0
(qemu) eject ide1-cd0
(qemu) info block
ide1-cd0: removable=1 locked=0 tray-open=1 io-status=ok [not inserted]
(qemu) 

Conclusion: the tray opened and the media was purged. We should emit the event.

3. Eject with medium inserted and locked

(qemu) info block
ide1-cd0: removable=1 locked=1 tray-open=0 io-status=ok file=/tmp/vl.2LHApn 
backing_file=/mnt/fernando/isos/linux/Fedora-16-x86_64-DVD.iso ro=0 drv=qcow2 
encrypted=0 bps=0 bps_rd=0 bps_wr=0 iops=0 iops_rd=0 iops_wr=0
(qemu) eject ide1-cd0
Device 'ide1-cd0' is locked
(qemu) info block
ide1-cd0: removable=1 locked=0 tray-open=1 io-status=ok file=/tmp/vl.2LHApn 
backing_file=/mnt/fernando/isos/linux/Fedora-16-x86_64-DVD.iso ro=0 drv=qcow2 
encrypted=0 bps=0 bps_rd=0 bps_wr=0 iops=0 iops_rd=0 iops_wr=0
(qemu) 

Conclusion: eject returned an error, but a few seconds later the tray opened and
            the media wasn't purged. What happened here is that, the _guest_
            opened the tray. The code in this patch would trigger the event, but
            we shouldn't emit it twice if we cover eject & change (ie. special 
case)

Each of the three cases has different semantics wrt to the tray. We could try
to fix them, but this seems to require some surgery... Is it worth it?

Now, the change command. Basically, it opens the tray, purges the medium,
inserts another medium and closes the tray. Should we generate the event twice?

So, I'd just avoid all this and have only GUEST_MEDIUM_EJECT, which has clear
semantics and satisfies libvirt needs...