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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]