From 8d9d098b6dd4c8af6fb2c7c3edfabdb287606322 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 8 Jan 2014 13:24:22 -0700 Subject: [PATCH] event: wire up RPC for server-side network event filtering We haven't had a release with network events yet, so we are free to fix the RPC so that it actually does what we want. Doing client-side filtering of per-network events is inefficient if a connection is only interested in events on a single network out of hundreds available on the server. But to do server-side per-network filtering, the server needs to know which network to filter on - so we need to pass an optional network over on registration. Furthermore, it is possible to have a client with both a global and per-network filter; in the existing code, the server sends only one event and the client replicates to both callbacks. But with server-side filtering, the server will send the event twice, so we need a way for the client to know which callbackID is sending an event, to ensure that the client can filter out events from a registration that does not match the callbackID from the server. Likewise, the existing style of deregistering by eventID alone is fine; but in the new style, we have to remember which callbackID to delete. This patch fixes the RPC wire definition to contain all the needed pieces of information, and hooks into the server and client side improvements of the previous patches, in order to switch over to full server-side filtering of network events. Also, since we fixed this in time, all released versions of libvirtd that support network events also support per-network filtering, so we can hard-code that assumption into network_event.c. Converting domain events to server-side filtering will require the introduction of new RPC numbers, as well as a server feature bit that the client can use to tell whether to use old-style (server only supports global events) or new-style (server supports filtered events), so that is deferred to a later set of patches. * src/conf/network_event.c (virNetworkEventStateRegisterClient): Assume server-side filtering. * src/remote/remote_protocol.x (remote_connect_network_event_register_any_args): Add network argument. (remote_connect_network_event_register_any_ret): Return callbackID instead of count. (remote_connect_network_event_deregister_any_args): Pass callbackID instead of eventID. (remote_connect_network_event_deregister_any_ret): Drop unused type. (remote_network_event_lifecycle_msg): Add callbackID. * daemon/remote.c (remoteDispatchConnectNetworkEventDeregisterAny): Drop unused arg, and deal with callbackID from client. (remoteRelayNetworkEventLifecycle): Pass callbackID. (remoteDispatchConnectNetworkEventRegisterAny): Likewise, and recognize non-NULL network. * src/remote/remote_driver.c (remoteConnectNetworkEventRegisterAny): Pass network, and track server side id. (remoteConnectNetworkEventDeregisterAny): Deregister by callback id. (remoteNetworkBuildEventLifecycle): Pass remote id to event queue. * src/remote_protocol-structs: Regenerate. Signed-off-by: Eric Blake --- daemon/remote.c | 34 +++++++++++++++++----------------- src/conf/network_event.c | 4 +--- src/remote/remote_driver.c | 17 ++++++++++++++--- src/remote/remote_protocol.x | 12 +++++------- src/remote_protocol-structs | 9 ++++----- 5 files changed, 41 insertions(+), 35 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index ae318b24e5..d2aafbe055 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -685,6 +685,7 @@ remoteRelayNetworkEventLifecycle(virConnectPtr conn ATTRIBUTE_UNUSED, /* build return data */ memset(&data, 0, sizeof(data)); make_nonnull_network(&data.net, net); + data.callbackID = callback->callbackID; data.event = event; data.detail = detail; @@ -5285,7 +5286,7 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server ATTRIBUTE_UN virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, remote_connect_network_event_register_any_args *args, - remote_connect_network_event_register_any_ret *ret ATTRIBUTE_UNUSED) + remote_connect_network_event_register_any_ret *ret) { int callbackID; int rv = -1; @@ -5293,6 +5294,7 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server ATTRIBUTE_UN daemonClientEventCallbackPtr ref; struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + virNetworkPtr net = NULL; if (!priv->conn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); @@ -5301,6 +5303,10 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server ATTRIBUTE_UN virMutexLock(&priv->lock); + if (args->net && + !(net = get_nonnull_network(priv->conn, *args->net))) + goto cleanup; + if (args->eventID >= VIR_NETWORK_EVENT_ID_LAST || args->eventID < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported network event ID %d"), args->eventID); @@ -5325,7 +5331,7 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server ATTRIBUTE_UN goto cleanup; if ((callbackID = virConnectNetworkEventRegisterAny(priv->conn, - NULL, + net, args->eventID, networkEventCallbacks[args->eventID], ref, @@ -5337,6 +5343,7 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server ATTRIBUTE_UN } ref->callbackID = callbackID; + ret->callbackID = callbackID; rv = 0; @@ -5344,6 +5351,8 @@ cleanup: VIR_FREE(callback); if (rv < 0) virNetMessageSaveError(rerr); + if (net) + virNetworkFree(net); virMutexUnlock(&priv->lock); return rv; } @@ -5354,10 +5363,8 @@ remoteDispatchConnectNetworkEventDeregisterAny(virNetServerPtr server ATTRIBUTE_ virNetServerClientPtr client, virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, - remote_connect_network_event_deregister_any_args *args, - remote_connect_network_event_deregister_any_ret *ret ATTRIBUTE_UNUSED) + remote_connect_network_event_deregister_any_args *args) { - int callbackID = -1; int rv = -1; size_t i; struct daemonClientPrivate *priv = @@ -5370,25 +5377,18 @@ remoteDispatchConnectNetworkEventDeregisterAny(virNetServerPtr server ATTRIBUTE_ virMutexLock(&priv->lock); - if (args->eventID >= VIR_NETWORK_EVENT_ID_LAST || args->eventID < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported event ID %d"), args->eventID); - goto cleanup; - } - for (i = 0; i < priv->nnetworkEventCallbacks; i++) { - if (priv->networkEventCallbacks[i]->eventID == args->eventID) { - callbackID = priv->networkEventCallbacks[i]->callbackID; + if (priv->networkEventCallbacks[i]->callbackID == args->callbackID) break; - } } - if (callbackID < 0) { + if (i == priv->nnetworkEventCallbacks) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("network event %d not registered"), args->eventID); + _("network event callback %d not registered"), + args->callbackID); goto cleanup; } - if (virConnectNetworkEventDeregisterAny(priv->conn, callbackID) < 0) + if (virConnectNetworkEventDeregisterAny(priv->conn, args->callbackID) < 0) goto cleanup; VIR_DELETE_ELEMENT(priv->networkEventCallbacks, i, diff --git a/src/conf/network_event.c b/src/conf/network_event.c index d9c47b8a22..a0b60a0de0 100644 --- a/src/conf/network_event.c +++ b/src/conf/network_event.c @@ -189,12 +189,10 @@ virNetworkEventStateRegisterClient(virConnectPtr conn, if (virNetworkEventsInitialize() < 0) return -1; - /* FIXME: All servers that support network events should also support - * per-object filtering. */ return virObjectEventStateRegisterID(conn, state, net ? net->uuid : NULL, virNetworkEventClass, eventID, VIR_OBJECT_EVENT_CALLBACK(cb), - opaque, freecb, callbackID, false); + opaque, freecb, callbackID, true); } diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index d719a493fc..18eb454f58 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2923,8 +2923,10 @@ remoteConnectNetworkEventRegisterAny(virConnectPtr conn, int rv = -1; struct private_data *priv = conn->privateData; remote_connect_network_event_register_any_args args; + remote_connect_network_event_register_any_ret ret; int callbackID; int count; + remote_nonnull_network network; remoteDriverLock(priv); @@ -2938,14 +2940,23 @@ remoteConnectNetworkEventRegisterAny(virConnectPtr conn, * events on the server */ if (count == 1) { args.eventID = eventID; + if (net) { + make_nonnull_network(&network, net); + args.net = &network; + } else { + args.net = NULL; + } + memset(&ret, 0, sizeof(ret)); if (call(conn, priv, 0, REMOTE_PROC_CONNECT_NETWORK_EVENT_REGISTER_ANY, (xdrproc_t) xdr_remote_connect_network_event_register_any_args, (char *) &args, - (xdrproc_t) xdr_void, (char *)NULL) == -1) { + (xdrproc_t) xdr_remote_connect_network_event_register_any_ret, (char *) &ret) == -1) { virObjectEventStateDeregisterID(conn, priv->eventState, callbackID); goto done; } + virObjectEventStateSetRemote(conn, priv->eventState, callbackID, + ret.callbackID); } rv = callbackID; @@ -2980,7 +2991,7 @@ remoteConnectNetworkEventDeregisterAny(virConnectPtr conn, /* If that was the last callback for this eventID, we need to disable * events on the server */ if (count == 0) { - args.eventID = eventID; + args.callbackID = remoteID; if (call(conn, priv, 0, REMOTE_PROC_CONNECT_NETWORK_EVENT_DEREGISTER_ANY, (xdrproc_t) xdr_remote_connect_network_event_deregister_any_args, (char *) &args, @@ -4924,7 +4935,7 @@ remoteNetworkBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, msg->detail); virNetworkFree(net); - remoteEventQueue(priv, event, -1); + remoteEventQueue(priv, event, msg->callbackID); } diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index c3d544fe22..ae27a7721f 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3,7 +3,7 @@ * remote_internal driver and libvirtd. This protocol is * internal and may change at any time. * - * 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 @@ -2851,21 +2851,19 @@ struct remote_connect_get_cpu_model_names_ret { struct remote_connect_network_event_register_any_args { int eventID; + remote_network net; }; struct remote_connect_network_event_register_any_ret { - int cb_registered; + int callbackID; }; struct remote_connect_network_event_deregister_any_args { - int eventID; -}; - -struct remote_connect_network_event_deregister_any_ret { - int cb_registered; + int callbackID; }; struct remote_network_event_lifecycle_msg { + int callbackID; remote_nonnull_network net; int event; int detail; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 7373d6573c..e58482e7f4 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2330,17 +2330,16 @@ struct remote_connect_get_cpu_model_names_ret { }; struct remote_connect_network_event_register_any_args { int eventID; + remote_network net; }; struct remote_connect_network_event_register_any_ret { - int cb_registered; + int callbackID; }; struct remote_connect_network_event_deregister_any_args { - int eventID; -}; -struct remote_connect_network_event_deregister_any_ret { - int cb_registered; + int callbackID; }; struct remote_network_event_lifecycle_msg { + int callbackID; remote_nonnull_network net; int event; int detail;