[libvirt] [PATCH 1/9] Add support for event tray moved of removable disks

[Daniel Veillard]

On Fri, Mar 09, 2012 at 06:49:20PM +0800, Osier Yang wrote:
> On 2012年03月09日 16:55, Daniel Veillard wrote:
> >On Mon, Mar 05, 2012 at 06:25:39PM +0800, Osier Yang wrote:
> >>This patch introduces a new event type for the QMP event
> >>DEVICE_TRAY_MOVED, which occurs when the tray of a removable
> >>disk is moved (i.e opened or closed):
> >>
> >>     VIR_DOMAIN_EVENT_ID_TRAY_MOVED
> >>
> >>The event's data includes the device alias and the tray's
> >>status, which indicates whether the tray has been opened
> >>or closed.  Thus the callback definition for the event is:
> >>
> >>typedef void
> >>(*virConnectDomainEventTrayMovedCallback)(virConnectPtr conn,
> >>                                           virDomainPtr dom,
> >>                                           const char *devAlias,
> >>                                           unsigned int trayOpened,
> >>                                           void *opaque);
> >
> >   Hum ... could we make that slightly more generic.
> >Instead of just being able to report on tray opened or not (i.e. closed)
> >Let's report TrayChangeCallback,  and have an 'int reason' instead.
> 
> Hmm, yes, 'int reason' is good idea.
> 
> But for the name, TrayMoved might describe the real action more
> precisely. Unlike DiskChange, it says there was some medium was
> changed, TrayMoved only intends to report the tray status changeing
> event, nothing really changed, or we can rename it to TrayStatusChange
> to indicate the tray status is changed, but IMO it's not much
> difference except getting a longer name. :-)
> 
> >Then for example the API would be able to cope with more set of events,
> >one of the thing I can think of now would be the ability to emulate
> >multiple device in one as disk changers,
> 
> What does "emulate multiple device" mean? is it "s/device/event/"?

  Nahh, I was thinking of thinks like cdrom changers, where the
  enclosure can hold multiple disks and swap them. But in retrospect
  I dould we will have much need to emulate such hardware ever...

> and also possibly be able to
> >provide invormations about the kind of media, i.e. switch from a CD-ROM
> >to a DVD-WR in the tray.
> 
> IMHO we should seperate the events for "tray change" and
> "medium change", the info such as the kind of media should handled
> by DiskChange instead,

  yes, reasonnable.

> How about defining the callback like:
> 
> /**
>  * virConnectDomainEventTrayMovedReason:
>  *
>  * The reason describing why the callback was called
>  */
> enum {
>     VIR_DOMAIN_EVENT_TRAY_MOVED_OPEN = 0,
>     VIR_DOMAIN_EVENT_TRAY_MOVED_CLOSE,
> 
>     /* Something else such as other driver only emits a
>      * event for OPEN+CLOSE.
>      */
> 
> #ifdef VIR_ENUM_SENTINELS
>     VIR_DOMAIN_EVENT_TRAY_MOVED_LAST
> #endif
> } virDomainEventTrayMovedReason;
> 
> 
> /**
>  * virConnectDomainEventTrayMovedCallback:
>  * @conn: connection object
>  * @dom: domain on which the event occurred
>  * @devAlias: device alias
>  * @reason: reason why this callback was called, any of
>             virDomainEventTrayMovedReason
>  * @opaque: application specified data
>  *
>  * This callback occurs when the tray of a removable device is moved.
>  *
>  * The callback signature to use when registering for an event of type
>  * VIR_DOMAIN_EVENT_ID_TRAY_MOVED wit virConnectDomainEventRegisterAny()
>  */
> typedef void
> (*virConnectDomainEventTrayMovedCallback)(virConnectPtr conn,
>                                             virDomainPtr dom,
>                                             const char *devAlias,
>                                             int reason,
>                                             void *opaque);

  I think I would still feel a bit better with Changed rather than Moved
which is more specific, but not a blocker.

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/