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

[Luiz Capitulino]

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).

That said, I'm not necessarily against covering all cases. It just doesn't
seem necessary to me.

> Also, I seem to remember that once we had discussed some kind of a "tray
> status (open/closed) changed" event, which would be more generic.

Yes, but my old series got complex and way beyond the original event. If we
decide to cover all eject scenarios, I'd like to do it w/o adding new commands.

> 
> >> Signed-off-by: Luiz Capitulino <address@hidden>
> > 
> > Reviewed-by: Eric Blake <address@hidden>
> > 
> >>  
> >> +BLOCK_MEDIUM_EJECT
> >> +------------------
> >> +
> >> +Emitted when the guest succeeds ejecting a medium. If the device has a 
> >> tray,
> > 
> > s/succeeds ejecting/succeeds at ejecting/
> > 
> > Since libvirt is also tracking whether it makes 'eject' and 'change'
> > monitor commands, and can also do a query when reconnecting to the
> > monitor after a libvirtd restart to see if state changed in the meantime
> > (when an event was lost), this should be sufficient for libvirt to have
> > an accurate picture of the device state.
> > 
> > I do have to wonder, however, if we also need an event for when the
> > guest initiates a tray lock or tray unlock event, in order to track
> > whether plain eject will work or whether a forced eject to override the
> > tray lock would be required.
> 
> The tray is usually locked for a reason, so I would vote against libvirt
> automagically overriding it. Note that you don't really need a
> lock/unlock event for implementing it, you could just always pass
> force=true (or do it after you got QERR_DEVICE_LOCKED).

Yes, libvirt will get the device locked error and can just retry.

> 
> The only reasonable thing for a management tool to do with a lock/unlock
> event would be updating some icon in a GUI or something like that.
> 
> Kevin
>