5:29 p.m.
On 12/16/2011 09:58 AM, shaohef(a)linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Basically, this feature can go along with qemu monitor passthrough.
That way, if we use new commands in the monitor that generate new events, we want some
way to receive those new events too.
Signed-off-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
---
include/libvirt/libvirt-qemu.h | 27 ++++
include/libvirt/libvirt.h.in | 2 +-
src/conf/domain_event.c | 293 ++++++++++++++++++++++++++++++++++++++--
src/conf/domain_event.h | 50 ++++++-
src/driver.h | 14 ++
src/libvirt-qemu.c | 189 ++++++++++++++++++++++++++
src/libvirt_private.syms | 6 +
src/libvirt_qemu.syms | 5 +
8 files changed, 571 insertions(+), 15 deletions(-)
+/**
+ * virConnectDomainQemuEventCallback:
+ * @conn: connection object
+ * @dom: domain on which the event occurred
+ * @eventName : the name of the unknow or un-implementation event
s/unknow/unknown/
except that I think this interface should be usable to get the JSON
argument string of _any_ qemu monitor event, even if libvirt is already
handling it. That is, in qemu, I think we want to parse every incoming
event, and do two things with it:
1. if the event name has been registered with the qemu-specific event
registration, send a libvirt-qemu event
2. if the event name is known and has a libvirt callback, then do the
libvirt callback (which might send a libvirt event).
So UNKNOWN is really not the right word to use. Rather, it is an
arbitrary qemu event, whether or not libvirt knows about it.
+ * @eventArgs: the content of the unknow or un-implementation event
+ *
+ * The callback signature to use when registering for an event of type
+ * VIR_QEMU_DOMAIN_EVENT_ID_UNKNOWN with virConnectDomainQemuEventRegister()
This sentence would then be:
The callback signature to use when registering for an arbitrary qemu
event with virConnectDomainQemuEventRegister().
+ */
+typedef void (*virConnectDomainQemuEventCallback)(virConnectPtr conn,
+ virDomainPtr dom,
+ const char *eventName, /* The JSON
event name */
+ const char *eventArgs, /* The JSON
string of args */
+ void *opaque);
This signature looks reasonable.
+
+int
+virConnectDomainQemuEventRegister(virConnectPtr conn,
+ virDomainPtr dom, /* option to filter */
+ const char *eventName, /* JSON event name */
+ virConnectDomainQemuEventCallback cb,
+ void *opaque,
+ virFreeCallback freecb);
As does this API.
+int
+virConnectDomainQemuEventDeregister(virConnectPtr conn,
+ int callbackID);
+
# ifdef __cplusplus
}
# endif
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 2480add..9fcb400 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -3207,7 +3207,6 @@ typedef void (*virConnectDomainEventBlockJobCallback)(virConnectPtr
conn,
int type,
int status,
void *opaque);
-
/**
* virConnectDomainEventDiskChangeReason:
*
@@ -3263,6 +3262,7 @@ typedef enum {
VIR_DOMAIN_EVENT_ID_CONTROL_ERROR = 7, /* virConnectDomainEventGenericCallback */
VIR_DOMAIN_EVENT_ID_BLOCK_JOB = 8, /* virConnectDomainEventBlockJobCallback
*/
VIR_DOMAIN_EVENT_ID_DISK_CHANGE = 9, /* virConnectDomainEventDiskChangeCallback
*/
+ VIR_QEMU_DOMAIN_EVENT_ID_UNKNOWN = 10, /* virConnectDomainEventDefaultCallback */
No. We do not want to be adding this event type to libvirt.so. Qemu
events should be sent on a different channel of the RPC protocol, and
never interfere with libvirt events.
/*
* NB: this enum value will increase over time as new events are
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 614ab97..0388a66 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -45,7 +45,9 @@ typedef virDomainMeta *virDomainMetaPtr;
struct _virDomainEventCallback {
int callbackID;
+ int qemuCallbackID;
int eventID;
+ char *eventName;
Since we don't want to piggy-back main libvirt events, but instead send
this directly from qemu, I'm wondering if it's better to have a
qemu-specific structure in src/qemu rather than trying to combine it
into conf/domain_event.c.
virConnectPtr conn;
virDomainMetaPtr dom;
virConnectDomainEventGenericCallback cb;
@@ -94,6 +96,10 @@ struct _virDomainEvent {
char *devAlias;
int reason;
} diskChange;
+ struct {
+ char *eventName;
+ char *eventArgs;
+ }qemuUnknownEvent;
Style: space after }.
+/**
+ * virDomainEventCallbackListAddName:
+ * @conn: pointer to the connection
+ * @cbList: the list
+ * @eventName: the event eventName
+ * @callback: the callback to add
+ * @eventID: the specific eventID
+ * @opaque: opaque data tio pass to callback
s/tio/to/
but again, I'm not sure we want this function in the generic libvirt
code; it may fit better in the qemu-specific code.
+ *
+ * Internal function to add a callback from a virDomainEventCallbackListPtr
+ */
+int
+virDomainEventCallbackListAddName(virConnectPtr conn,
+ virDomainEventCallbackListPtr cbList,
+ virDomainPtr dom,
+ const char* eventName,
+ int eventID,
+ virConnectDomainEventGenericCallback callback,
+ void *opaque,
+ virFreeCallback freecb)
+{
+ virDomainEventCallbackPtr event;
+ int i;
+
+ /* Check incoming */
+ if ( !cbList ) {
+ return -1;
+ }
+
+ /* check if we already have this callback on our list */
+ for (i = 0 ; i < cbList->count ; i++) {
+ if (cbList->callbacks[i]->cb == VIR_DOMAIN_EVENT_CALLBACK(callback)
&&
+ STREQ(cbList->callbacks[i]->eventName, eventName) &&
Is a linear search wise? Or should we be setting up a hashtable of
event names to the appropriate struct. After all, we don't know how
many event names qemu might support, so the goal of O(1) hash lookup
will make it more efficient.
+++ b/src/libvirt-qemu.c
@@ -36,6 +36,77 @@
virReportErrorHelper(VIR_FROM_DOM, error, NULL, __FUNCTION__, \
__LINE__, info)
+/* Helper macros to implement VIR_DOMAIN_DEBUG using just C99. This
+ * assumes you pass fewer than 15 arguments to VIR_DOMAIN_DEBUG, but
+ * can easily be expanded if needed.
Why are we copying this pre-processor magic? It seems like it should be
coming from a common header, and not something repeated in this .c file.
If we need to move it into a common location in order to reuse it, then
that should be a pre-requisite patch (don't mix code motion and new code
in the same patch).
/**
* virDomainQemuMonitorCommand:
* @domain: a domain object
@@ -178,3 +249,121 @@ error:
virDispatchError(conn);
return NULL;
}
+
+/**
+ * virConnectDomainQemuEventRegister:
+ * @conn: pointer to the connection
+ * @dom: pointer to the domain
+ * @eventName: the event Name to receive
+ * @cb: callback to the function handling domain events
+ * @opaque: opaque data to pass on to the callback
+ * @freecb: optional function to deallocate opaque when not used anymore
+ *
+ * Adds a callback to receive notifications of arbitrary qemu domain events
+ * occurring on a domain.
+ *
+ * If dom is NULL, then events will be monitored for any domain. If dom
+ * is non-NULL, then only the specific domain will be monitored
+ *
+ * Most types of event have a callback providing a custom set of parameters
+ * for the event. When registering an event, it is thus neccessary to use
+ * the VIR_DOMAIN_EVENT_CALLBACK() macro to cast the supplied function pointer
+ * to match the signature of this method.
This paragraph is not needed - we only support one type of event
callback (return the entire JSON event details, no additional structure
imposed by libvirt-qemu), so no casting is needed.
+++ b/src/libvirt_qemu.syms
@@ -19,3 +19,8 @@ LIBVIRT_QEMU_0.9.4 {
global:
virDomainQemuAttach;
} LIBVIRT_QEMU_0.8.3;
+LIBVIRT_QEMU_0.9.9 {
+ global:
+ virConnectDomainQemuEventRegister;
+ virConnectDomainQemuEventDeregister;
+} LIBVIRT_QEMU_0.9.4;
We'd want this to be LIBVIRT_QEMU_0.9.10.
Overall, I think the signature looks reasonable, and I'm half inclined
to split this patch into the new API and commit that pre-freeze, then
worry about polishing up the implementation even if that is post-freeze
(that is, the implementation would always fail, but we'd be committing
to the API signature). I'll see what I can come up with, because I'm
interested in getting this patch series in.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org