[libvirt] [PATCH 2/5] event: server RPC protocol tweaks for domain lifecycle events

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&gt; --- 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 f0b9922..05fb708 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -76,6 +76,7 @@ struct daemonClientEventCallback { virNetServerClientPtr client; int eventID; int callbackID; + bool legacy; }; static virDomainPtr get_nonnull_domain(virConnectPtr conn, remote_nonnull_domain domain); @@ -140,8 +141,8 @@ remoteRelayDomainEventLifecycle(virConnectPtr conn ATTRIBUTE_UNUSED, if (callback->callbackID < 0) 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)); @@ -149,9 +150,20 @@ remoteRelayDomainEventLifecycle(virConnectPtr conn ATTRIBUTE_UNUSED, 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; } @@ -3206,6 +3218,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, @@ -3394,6 +3407,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, @@ -3415,9 +3434,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; } @@ -3432,6 +3455,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, @@ -3464,6 +3488,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, @@ -3483,9 +3586,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; } @@ -3516,6 +3623,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, @@ -3836,6 +3990,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 f94a38a..96838ba 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; }; @@ -5063,5 +5080,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.4.2