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

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/"?

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,

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