On 12/16/2011 09:58 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
Pretty light on the commit message.
Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- daemon/libvirtd.h | 10 +++ daemon/remote.c | 172 +++++++++++++++++++++++++++++++++++++++++- src/remote/qemu_protocol.x | 29 +++++++- src/remote/remote_driver.c | 162 ++++++++++++++++++++++++++++++++++++++- src/remote/remote_protocol.x | 6 ++ src/remote_protocol-structs | 5 + 6 files changed, 376 insertions(+), 8 deletions(-)
diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index c8d3ca2..fab7290 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -38,6 +38,15 @@ # endif # include "virnetserverprogram.h"
+/* limit the number unknow event of an conncet can register */
s/unknow/unknown/ s/conncet/connection/ But based on my review of 1/4, this has several problems. They aren't unknown events, so much as qemu events, which might be sent in parallel to a libvirt event. Is the goal to hard-code the number of event names we allow to be registered in order to avoid a DoS where someone just registers an indefinite stream of arbitrary event names to exhaust libvirtd's memory? But since we don't limit the number of event registration ids you can create, I think we're better off tracking this as a growable hash table, rather than capping it to a hard limit in the number of names.
+++ b/daemon/remote.c @@ -421,6 +421,53 @@ mem_error: return -1; }
+static int remoteRelayDomainEventUnknown(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + const char *eventName, /* The JSON event name */ + const char *eventArgs, /* The JSON string of args */ + void *opaque)
static int remoteRelayDomainEventControlError(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainPtr dom, @@ -509,6 +556,7 @@ static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventControlError), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventBlockJob), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventDiskChange), + VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventUnknown), };
I think you want to separate things; since registering a qemu callback should not interfere with libvirt events, this will not be in the table of domainEventCallbacks. I think there's going to be a bit of work to refactor this so that it is triggered in the right code paths for libvirt-qemu.so, but doesn't impact libvirt.so.
+++ b/src/remote/qemu_protocol.x @@ -47,6 +47,30 @@ struct qemu_domain_attach_ret { remote_nonnull_domain dom; };
+struct qemu_domain_events_register_args { + remote_nonnull_string eventName; +}; + +struct qemu_domain_events_deregister_args { + remote_nonnull_string eventName;
Shouldn't need the eventName on deregister.
+ int callbackID; +}; + +struct qemu_domain_events_register_ret { + int callbackID; +};
I'd stick the related args and ret structs next to one another.
+ +struct qemu_domain_events_deregister_ret { + int callbackID; +};
Deregistration doesn't need a _ret.
+ +struct qemu_domain_events_unknown_event_msg { + remote_nonnull_domain dom; + remote_nonnull_string eventName; + remote_nonnull_string eventArgs; +}; + + /* Define the program number, protocol version and procedure numbers here. */ const QEMU_PROGRAM = 0x20008087; const QEMU_PROTOCOL_VERSION = 1; @@ -61,5 +85,8 @@ enum qemu_procedure { * are some exceptions to this rule, e.g. domainDestroy. Other APIs MAY * be marked as high priority. If in doubt, it's safe to choose low. */ QEMU_PROC_MONITOR_COMMAND = 1, /* skipgen skipgen priority:low */ - QEMU_PROC_DOMAIN_ATTACH = 2 /* autogen autogen priority:low */ + QEMU_PROC_DOMAIN_ATTACH = 2, /* autogen autogen priority:low */ + QEMU_PROC_DOMAIN_EVENTS_REGISTER = 3, /* skipgen skipgen priority:low */ + QEMU_PROC_DOMAIN_EVENTS_DEREGISTER = 4, /* skipgen skipgen priority:low */ + QEMU_PROC_DOMAIN_EVENTS_UNKNOWN_EVENT = 5 /* skipgen skipgen */
I still don't like the name UNKNOWN everywhere.
@@ -4627,6 +4777,8 @@ static virDriver remote_driver = { .nodeGetFreeMemory = remoteNodeGetFreeMemory, /* 0.3.3 */ .domainEventRegister = remoteDomainEventRegister, /* 0.5.0 */ .domainEventDeregister = remoteDomainEventDeregister, /* 0.5.0 */ + .qemuDomainQemuEventRegister = remoteDomainQemuEventRegister, /* 0.9.9 */ + .qemuDomainQemuEventDeregister = remoteDomainQemuEventDeregister, /* 0.9.9 */
0.9.10, now. It looks like a lot of this will be copy and paste from existing event codes, but I didn't review it closely.
+++ b/src/remote/remote_protocol.x @@ -2049,6 +2049,12 @@ struct remote_domain_event_disk_change_msg { int reason; };
+struct remote_domain_event_default_event_msg { + remote_nonnull_domain dom; + remote_nonnull_string eventName; + remote_nonnull_string eventArgs; +};
No. We should _not_ be sending any qemu-specific event over remote_protocol.x. It should all be handled solely by qemu_protocol.x. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org