5:50 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>
Pretty light on the commit message.
Signed-off-by: ShaoHe Feng <shaohef(a)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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org