11:49 a.m.
This patch adds some new RPC call numbers, but for ease of review,
they sit idle until a later patch adds the client counterpart to
drive the new RPCs. Also for ease of review, I limited this patch
to just the lifecycle event; although converting the remaining
15 domain events will be quite mechanical. On the server side,
we have to have a function per RPC call, largely with duplicated
bodies (the key difference being that we store in our callback
opaque pointer whether events should be fired with old or new
style); meanwhile, a single function can drive multiple RPC
messages, along with a strategic choice of XDR struct layout,
makes the event generation code for both styles fairly compact.
I debated about adding a tri-state witness variable per
connection (values 'unknown', 'legacy', 'modern'). It would
start
as 'unknown', move to 'legacy' if any RPC call is made to a legacy
event call, and move to 'modern' if the feature probe is made;
then the event code could issue an error if the witness state is
incorrect (a legacy RPC call while in 'modern', a modern RPC call
while in 'unknown' or 'legacy', and a feature probe while in
'legacy' or 'modern'). But while it might prevent odd behavior
caused by protocol fuzzing, I don't see that it would prevent
any security holes, so I considered it bloat.
* src/libvirt_internal.h (VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK):
New feature.
* src/remote/remote_protocol.x
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_CALLBACK_REGISTER_ANY)
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_CALLBACK_DEREGISTER_ANY)
(REMOTE_PROC_DOMAIN_EVENT_CALLBACK_LIFECYCLE): New RPCs.
* daemon/remote.c (daemonClientCallback): Add field.
(remoteDispatchConnectDomainEventCallbackRegisterAny)
(remoteDispatchConnectDomainEventCallbackDeregisterAny): New
functions.
(remoteDispatchConnectDomainEventRegisterAny)
(remoteDispatchConnectDomainEventDeregisterAny): Mark legacy use.
(remoteRelayDomainEventLifecycle): Change message based on legacy
or new use.
(remoteDispatchConnectSupportsFeature): Advertise new feature.
* src/remote_protocol-structs: Regenerate.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
daemon/remote.c | 173 ++++++++++++++++++++++++++++++++++++++++---
src/libvirt_internal.h | 7 +-
src/remote/remote_protocol.x | 39 +++++++++-
src/remote_protocol-structs | 17 +++++
4 files changed, 225 insertions(+), 11 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index be0af3d..1dfcb65 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -79,6 +79,7 @@ struct daemonClientEventCallback {
virNetServerClientPtr client;
int eventID;
int callbackID;
+ bool legacy;
};
static virDomainPtr get_nonnull_domain(virConnectPtr conn, remote_nonnull_domain
domain);
@@ -199,8 +200,8 @@ remoteRelayDomainEventLifecycle(virConnectPtr conn,
!remoteRelayDomainEventCheckACL(callback->client, conn, dom))
return -1;
- VIR_DEBUG("Relaying domain lifecycle event %d %d, callback %d",
- event, detail, callback->callbackID);
+ VIR_DEBUG("Relaying domain lifecycle event %d %d, callback %d legacy %d",
+ event, detail, callback->callbackID, callback->legacy);
/* build return data */
memset(&data, 0, sizeof(data));
@@ -208,9 +209,20 @@ remoteRelayDomainEventLifecycle(virConnectPtr conn,
data.event = event;
data.detail = detail;
- remoteDispatchObjectEventSend(callback->client, remoteProgram,
- REMOTE_PROC_DOMAIN_EVENT_LIFECYCLE,
- (xdrproc_t)xdr_remote_domain_event_lifecycle_msg,
&data);
+ if (callback->legacy) {
+ remoteDispatchObjectEventSend(callback->client, remoteProgram,
+ REMOTE_PROC_DOMAIN_EVENT_LIFECYCLE,
+ (xdrproc_t)xdr_remote_domain_event_lifecycle_msg,
+ &data);
+ } else {
+ remote_domain_event_callback_lifecycle_msg msg = { callback->callbackID,
+ data };
+
+ remoteDispatchObjectEventSend(callback->client, remoteProgram,
+ REMOTE_PROC_DOMAIN_EVENT_CALLBACK_LIFECYCLE,
+
(xdrproc_t)xdr_remote_domain_event_callback_lifecycle_msg,
+ &msg);
+ }
return 0;
}
@@ -3281,6 +3293,7 @@ remoteDispatchConnectDomainEventRegister(virNetServerPtr server
ATTRIBUTE_UNUSED
callback->client = client;
callback->eventID = VIR_DOMAIN_EVENT_ID_LIFECYCLE;
callback->callbackID = -1;
+ callback->legacy = true;
ref = callback;
if (VIR_APPEND_ELEMENT(priv->domainEventCallbacks,
priv->ndomainEventCallbacks,
@@ -3469,6 +3482,12 @@ cleanup:
return rv;
}
+
+/* Due to back-compat reasons, two RPC calls map to the same libvirt
+ * API of virConnectDomainEventRegisterAny. A client should only use
+ * the new call if they have probed
+ * VIR_DRV_SUPPORTS_FEATURE(VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK),
+ * and must not mix the two styles. */
static int
remoteDispatchConnectDomainEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED,
virNetServerClientPtr client,
@@ -3490,9 +3509,13 @@ remoteDispatchConnectDomainEventRegisterAny(virNetServerPtr server
ATTRIBUTE_UNU
virMutexLock(&priv->lock);
- if (args->eventID >= VIR_DOMAIN_EVENT_ID_LAST ||
+ /* We intentionally do not use VIR_DOMAIN_EVENT_ID_LAST here; any
+ * new domain events added after this point should only use the
+ * modern callback style of RPC. */
+ if (args->eventID > VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED ||
args->eventID < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported event ID %d"),
args->eventID);
+ virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported event ID %d"),
+ args->eventID);
goto cleanup;
}
@@ -3507,6 +3530,7 @@ remoteDispatchConnectDomainEventRegisterAny(virNetServerPtr server
ATTRIBUTE_UNU
callback->client = client;
callback->eventID = args->eventID;
callback->callbackID = -1;
+ callback->legacy = true;
ref = callback;
if (VIR_APPEND_ELEMENT(priv->domainEventCallbacks,
priv->ndomainEventCallbacks,
@@ -3539,6 +3563,85 @@ cleanup:
static int
+remoteDispatchConnectDomainEventCallbackRegisterAny(virNetServerPtr server
ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client,
+ virNetMessagePtr msg
ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr
ATTRIBUTE_UNUSED,
+
remote_connect_domain_event_callback_register_any_args *args,
+
remote_connect_domain_event_callback_register_any_ret *ret)
+{
+ int callbackID;
+ int rv = -1;
+ daemonClientEventCallbackPtr callback = NULL;
+ daemonClientEventCallbackPtr ref;
+ struct daemonClientPrivate *priv =
+ virNetServerClientGetPrivateData(client);
+ virDomainPtr dom = NULL;
+
+ if (!priv->conn) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
+ goto cleanup;
+ }
+
+ virMutexLock(&priv->lock);
+
+ if (args->dom &&
+ !(dom = get_nonnull_domain(priv->conn, *args->dom)))
+ goto cleanup;
+
+ /* FIXME: support all domain events */
+ if (args->eventID != VIR_DOMAIN_EVENT_ID_LIFECYCLE) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported event ID %d"),
+ args->eventID);
+ goto cleanup;
+ }
+
+ /* If we call register first, we could append a complete callback
+ * to our array, but on OOM append failure, we'd have to then hope
+ * deregister works to undo our register. So instead we append an
+ * incomplete callback to our array, then register, then fix up
+ * our callback; but since VIR_APPEND_ELEMENT clears 'callback' on
+ * success, we use 'ref' to save a copy of the pointer. */
+ if (VIR_ALLOC(callback) < 0)
+ goto cleanup;
+ callback->client = client;
+ callback->eventID = args->eventID;
+ callback->callbackID = -1;
+ ref = callback;
+ if (VIR_APPEND_ELEMENT(priv->domainEventCallbacks,
+ priv->ndomainEventCallbacks,
+ callback) < 0)
+ goto cleanup;
+
+ if ((callbackID = virConnectDomainEventRegisterAny(priv->conn,
+ dom,
+ args->eventID,
+
domainEventCallbacks[args->eventID],
+ ref,
+ remoteEventCallbackFree)) < 0)
{
+ VIR_SHRINK_N(priv->domainEventCallbacks,
+ priv->ndomainEventCallbacks, 1);
+ callback = ref;
+ goto cleanup;
+ }
+
+ ref->callbackID = callbackID;
+ ret->callbackID = callbackID;
+
+ rv = 0;
+
+cleanup:
+ VIR_FREE(callback);
+ if (rv < 0)
+ virNetMessageSaveError(rerr);
+ if (dom)
+ virDomainFree(dom);
+ virMutexUnlock(&priv->lock);
+ return rv;
+}
+
+
+static int
remoteDispatchConnectDomainEventDeregisterAny(virNetServerPtr server ATTRIBUTE_UNUSED,
virNetServerClientPtr client,
virNetMessagePtr msg ATTRIBUTE_UNUSED,
@@ -3558,9 +3661,13 @@ remoteDispatchConnectDomainEventDeregisterAny(virNetServerPtr
server ATTRIBUTE_U
virMutexLock(&priv->lock);
- if (args->eventID >= VIR_DOMAIN_EVENT_ID_LAST ||
+ /* We intentionally do not use VIR_DOMAIN_EVENT_ID_LAST here; any
+ * new domain events added after this point should only use the
+ * modern callback style of RPC. */
+ if (args->eventID > VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED ||
args->eventID < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported event ID %d"),
args->eventID);
+ virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported event ID %d"),
+ args->eventID);
goto cleanup;
}
@@ -3591,6 +3698,53 @@ cleanup:
return rv;
}
+
+static int
+remoteDispatchConnectDomainEventCallbackDeregisterAny(virNetServerPtr server
ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client,
+ virNetMessagePtr msg
ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr
ATTRIBUTE_UNUSED,
+
remote_connect_domain_event_callback_deregister_any_args *args)
+{
+ int rv = -1;
+ size_t i;
+ struct daemonClientPrivate *priv =
+ virNetServerClientGetPrivateData(client);
+
+ if (!priv->conn) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
+ goto cleanup;
+ }
+
+ virMutexLock(&priv->lock);
+
+ for (i = 0; i < priv->ndomainEventCallbacks; i++) {
+ if (priv->domainEventCallbacks[i]->callbackID == args->callbackID)
+ break;
+ }
+ if (i == priv->ndomainEventCallbacks) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("domain event callback %d not registered"),
+ args->callbackID);
+ goto cleanup;
+ }
+
+ if (virConnectDomainEventDeregisterAny(priv->conn, args->callbackID) < 0)
+ goto cleanup;
+
+ VIR_DELETE_ELEMENT(priv->domainEventCallbacks, i,
+ priv->ndomainEventCallbacks);
+
+ rv = 0;
+
+cleanup:
+ if (rv < 0)
+ virNetMessageSaveError(rerr);
+ virMutexUnlock(&priv->lock);
+ return rv;
+}
+
+
static int
qemuDispatchDomainMonitorCommand(virNetServerPtr server ATTRIBUTE_UNUSED,
virNetServerClientPtr client ATTRIBUTE_UNUSED,
@@ -3911,6 +4065,7 @@ static int remoteDispatchConnectSupportsFeature(virNetServerPtr
server ATTRIBUTE
switch (args->feature) {
case VIR_DRV_FEATURE_FD_PASSING:
+ case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK:
supported = 1;
break;
diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h
index 115d8d1..ebf2acf 100644
--- a/src/libvirt_internal.h
+++ b/src/libvirt_internal.h
@@ -1,7 +1,7 @@
/*
* libvirt_internal.h: internally exported APIs, not for public use
*
- * Copyright (C) 2006-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2014 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -115,6 +115,11 @@ enum {
* Support for migration parameters.
*/
VIR_DRV_FEATURE_MIGRATION_PARAMS = 13,
+
+ /*
+ * Support for server-side event filtering via callback ids in events.
+ */
+ VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK = 14,
};
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 790a020..982ab1f 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -1972,6 +1972,10 @@ struct remote_domain_event_lifecycle_msg {
int event;
int detail;
};
+struct remote_domain_event_callback_lifecycle_msg {
+ int callbackID;
+ remote_domain_event_lifecycle_msg msg;
+};
struct remote_connect_domain_xml_from_native_args {
@@ -2248,6 +2252,19 @@ struct remote_connect_domain_event_deregister_any_args {
int eventID;
};
+struct remote_connect_domain_event_callback_register_any_args {
+ int eventID;
+ remote_domain dom;
+};
+
+struct remote_connect_domain_event_callback_register_any_ret {
+ int callbackID;
+};
+
+struct remote_connect_domain_event_callback_deregister_any_args {
+ int callbackID;
+};
+
struct remote_domain_event_reboot_msg {
remote_nonnull_domain dom;
};
@@ -5068,5 +5085,25 @@ enum remote_procedure {
* @generate: both
* @acl: none
*/
- REMOTE_PROC_NETWORK_EVENT_LIFECYCLE = 315
+ REMOTE_PROC_NETWORK_EVENT_LIFECYCLE = 315,
+
+ /**
+ * @generate: none
+ * @priority: high
+ * @acl: none
+ */
+ REMOTE_PROC_CONNECT_DOMAIN_EVENT_CALLBACK_REGISTER_ANY = 316,
+
+ /**
+ * @generate: none
+ * @priority: high
+ * @acl: none
+ */
+ REMOTE_PROC_CONNECT_DOMAIN_EVENT_CALLBACK_DEREGISTER_ANY = 317,
+
+ /**
+ * @generate: both
+ * @acl: none
+ */
+ REMOTE_PROC_DOMAIN_EVENT_CALLBACK_LIFECYCLE = 318
};
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index e58482e..39d3981 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -1491,6 +1491,10 @@ struct remote_domain_event_lifecycle_msg {
int event;
int detail;
};
+struct remote_domain_event_callback_lifecycle_msg {
+ int callbackID;
+ remote_domain_event_lifecycle_msg msg;
+};
struct remote_connect_domain_xml_from_native_args {
remote_nonnull_string nativeFormat;
remote_nonnull_string nativeConfig;
@@ -1707,6 +1711,16 @@ struct remote_connect_domain_event_register_any_args {
struct remote_connect_domain_event_deregister_any_args {
int eventID;
};
+struct remote_connect_domain_event_callback_register_any_args {
+ int eventID;
+ remote_domain dom;
+};
+struct remote_connect_domain_event_callback_register_any_ret {
+ int callbackID;
+};
+struct remote_connect_domain_event_callback_deregister_any_args {
+ int callbackID;
+};
struct remote_domain_event_reboot_msg {
remote_nonnull_domain dom;
};
@@ -2660,4 +2674,7 @@ enum remote_procedure {
REMOTE_PROC_CONNECT_NETWORK_EVENT_REGISTER_ANY = 313,
REMOTE_PROC_CONNECT_NETWORK_EVENT_DEREGISTER_ANY = 314,
REMOTE_PROC_NETWORK_EVENT_LIFECYCLE = 315,
+ REMOTE_PROC_CONNECT_DOMAIN_EVENT_CALLBACK_REGISTER_ANY = 316,
+ REMOTE_PROC_CONNECT_DOMAIN_EVENT_CALLBACK_DEREGISTER_ANY = 317,
+ REMOTE_PROC_DOMAIN_EVENT_CALLBACK_LIFECYCLE = 318,
};
--
1.8.5.3