Re: [libvirt] [PATCH v2 4/6] remote: implement storage lifecycle event APIs

On Mon, Jun 13, 2016 at 18:38:41 +0200, Jovanka Gulicoska wrote:a It may be worth noting that along with the lifecycle event this patch also implements all the dispatching stuff for events in the storage driver. It may be also worth splitting those two additions but I'm not going to insist. > --- > daemon/libvirtd.h | 2 + > daemon/remote.c | 201 ++++++++++++++++++++++++++++++++++++++++++- > src/remote/remote_driver.c | 128 +++++++++++++++++++++++++++ > src/remote/remote_protocol.x | 43 ++++++++- > src/remote_protocol-structs | 16 ++++ > 5 files changed, 388 insertions(+), 2 deletions(-) [...] > diff --git a/daemon/remote.c b/daemon/remote.c > index b2a420b..d6f5f1e 100644 > --- a/daemon/remote.c > +++ b/daemon/remote.c > @@ -181,6 +181,32 @@ remoteRelayNetworkEventCheckACL(virNetServerClientPtr client, > return ret; > } > > +static bool > +remoteRelayStoragePoolEventCheckACL(virNetServerClientPtr client, > + virConnectPtr conn, > + virStoragePoolPtr pool) > +{ > + virStoragePoolDef def; > + virIdentityPtr identity = NULL; > + bool ret = false; > + > + /* For now, we just create a virStoragePoolDef with enough contents to > + * * satisfy what viraccessdriverpolkit.c references. This is a bit > + * * fragile, but I don't know of anything better. */ Very broken comment formatting. > + def.name = pool->name; > + memcpy(def.uuid, pool->uuid, VIR_UUID_BUFLEN); > + > + if (!(identity = virNetServerClientGetIdentity(client))) > + goto cleanup; > + if (virIdentitySetCurrent(identity) < 0) > + goto cleanup; > + ret = virConnectStoragePoolEventRegisterAnyCheckACL(conn, &def); > + > + cleanup: > + ignore_value(virIdentitySetCurrent(NULL)); > + virObjectUnref(identity); > + return ret; > +} > > static bool > remoteRelayDomainQemuMonitorEventCheckACL(virNetServerClientPtr client, > @@ -208,7 +234,6 @@ remoteRelayDomainQemuMonitorEventCheckACL(virNetServerClientPtr client, > return ret; > } > > - Spurious whitespace change. > static int > remoteRelayDomainEventLifecycle(virConnectPtr conn, > virDomainPtr dom, [...] > @@ -5434,6 +5512,127 @@ remoteDispatchConnectNetworkEventDeregisterAny(virNetServerPtr server ATTRIBUTE_ > return rv; > } > > +static int > +remoteDispatchConnectStoragePoolEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, > + virNetServerClientPtr client, > + virNetMessagePtr msg ATTRIBUTE_UNUSED, > + virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, > + remote_connect_storage_pool_event_register_any_args *args, > + remote_connect_storage_pool_event_register_any_ret *ret) > +{ > + int callbackID; > + int rv = -1; > + daemonClientEventCallbackPtr callback = NULL; > + daemonClientEventCallbackPtr ref; > + struct daemonClientPrivate *priv = > + virNetServerClientGetPrivateData(client); > + virStoragePoolPtr pool = NULL; > + > + if (!priv->conn) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); > + goto cleanup; > + } > + > + virMutexLock(&priv->lock); > + > + if (args->pool && > + !(pool = get_nonnull_storage_pool(priv->conn, *args->pool))) > + goto cleanup; > + > + if (args->eventID >= VIR_STORAGE_POOL_EVENT_ID_LAST || args->eventID < 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("unsupported storage pool 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 Or you can use VIR_APPEND_ELEMENT_COPY to avoid clearing 'callback' and having to juggle the pointer between 'ref' and 'callback' > + * 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->storageEventCallbacks, > + priv->nstorageEventCallbacks, > + callback) < 0) > + goto cleanup; > + > + if ((callbackID = virConnectStoragePoolEventRegisterAny(priv->conn, > + pool, > + args->eventID, > + storageEventCallbacks[args->eventID], > + ref, > + remoteEventCallbackFree)) < 0) { > + VIR_SHRINK_N(priv->storageEventCallbacks, > + priv->nstorageEventCallbacks, 1); > + callback = ref; > + goto cleanup; > + } > + > + ref->callbackID = callbackID; > + ret->callbackID = callbackID; > + > + rv = 0; > + > + cleanup: > + VIR_FREE(callback); > + if (rv < 0) > + virNetMessageSaveError(rerr); > + virObjectUnref(pool); > + virMutexUnlock(&priv->lock); > + return rv; > +} > + [...] > diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c > index f494cbf..6fae72e 100644 > --- a/src/remote/remote_driver.c > +++ b/src/remote/remote_driver.c [...] > @@ -438,6 +444,10 @@ static virNetClientProgramEvent remoteEvents[] = { > remoteNetworkBuildEventLifecycle, > sizeof(remote_network_event_lifecycle_msg), > (xdrproc_t)xdr_remote_network_event_lifecycle_msg }, > + { REMOTE_PROC_STORAGE_POOL_EVENT_LIFECYCLE, > + remoteStoragePoolBuildEventLifecycle, > + sizeof(remote_storage_pool_event_lifecycle_msg), > + (xdrproc_t)xdr_remote_storage_pool_event_lifecycle_msg }, We are usually adding at the tail of the list. > { REMOTE_PROC_DOMAIN_EVENT_CALLBACK_LIFECYCLE, > remoteDomainBuildEventCallbackLifecycle, > sizeof(remote_domain_event_callback_lifecycle_msg), [...] > diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs > index fe1b8a8..85bc62d 100644 > --- a/src/remote_protocol-structs > +++ b/src/remote_protocol-structs > @@ -2551,6 +2551,22 @@ struct remote_network_event_lifecycle_msg { > int event; > int detail; > }; > +struct remote_connect_storage_pool_event_register_any_args { > + int eventID; > + remote_storage_pool pool; > +}; > +struct remote_connect_storage_pool_event_register_any_ret { > + int callbackID; > +}; > +struct remote_connect_storage_pool_event_deregister_any_args { > + int callbackID; > +}; > +struct remote_storage_pool_event_lifecycle_msg { > + int callbackID; > + remote_nonnull_storage_pool pool; > + int event; > + int detail; > +}; Fails make check: --- remote_protocol-structs 2016-06-14 07:01:24.847495995 +0200 +++ remote_protocol-struct-t3 2016-06-14 07:01:59.265494534 +0200 @@ -2552,8 +2552,8 @@ int detail; }; struct remote_connect_storage_pool_event_register_any_args { - int eventID; - remote_storage_pool pool; + int eventID; + remote_storage_pool pool; }; struct remote_connect_storage_pool_event_register_any_ret { int callbackID; @@ -2562,10 +2562,10 @@ int callbackID; }; struct remote_storage_pool_event_lifecycle_msg { - int callbackID; - remote_nonnull_storage_pool pool; - int event; - int detail; + int callbackID; + remote_nonnull_storage_pool pool; + int event; + int detail; }; struct remote_domain_fsfreeze_args { remote_nonnull_domain dom; @@ -3119,4 +3119,7 @@ REMOTE_PROC_DOMAIN_GET_PERF_EVENTS = 365, REMOTE_PROC_DOMAIN_SET_PERF_EVENTS = 366, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVAL_FAILED = 367, + REMOTE_PROC_CONNECT_STORAGE_POOL_EVENT_REGISTER_ANY = 368, + REMOTE_PROC_CONNECT_STORAGE_POOL_EVENT_DEREGISTER_ANY = 369, + REMOTE_PROC_STORAGE_POOL_EVENT_LIFECYCLE = 370, }; make[3]: *** [Makefile:10977: remote_protocol-struct] Error 1