On 06/14/2016 01:41 AM, Peter Krempa wrote: > 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 > Strange. Tried figuring out why I don't see this. If I do cd src; make check-protocol I get: $ make check-protocol GEN remote_protocol-struct GEN lxc_protocol-struct GEN qemu_protocol-struct GEN virnetprotocol-struct GEN virkeepaliveprotocol-struct GEN admin_protocol-struct GEN lxc_monitor_protocol-struct GEN lock_protocol-struct WARNING: pdwtags appears broken; skipping the qemu_protocol-struct test WARNING: pdwtags appears broken; skipping the remote_protocol-struct test WARNING: pdwtags appears broken; skipping the lxc_protocol-struct test WARNING: pdwtags appears broken; skipping the lock_protocol-struct test WARNING: pdwtags appears broken; skipping the virnetprotocol-struct test WARNING: pdwtags appears broken; skipping the virkeepaliveprotocol-struct test WARNING: pdwtags appears broken; skipping the admin_protocol-struct test WARNING: pdwtags appears broken; skipping the lxc_monitor_protocol-struct test So I guess that's why I don't see the error. Makefile.am has some notes about handling different pdwtags output formats, but the last commit is from 2011 from Eric. Maybe the format changed again: $ rpm -qf `which pdwtags` dwarves-1.10-9.fc24.x86_64 Thanks, Cole -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list