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

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

 



On 06/09/2016 02:25 PM, Jovanka Gulicoska wrote:
> ---
>  daemon/libvirtd.h            |   2 +
>  daemon/remote.c              | 205 +++++++++++++++++++++++++++++++++++++++++++
>  src/remote/remote_driver.c   | 133 ++++++++++++++++++++++++++++
>  src/remote/remote_protocol.x |  43 ++++++++-
>  src/remote_protocol-structs  |  22 ++++-
>  5 files changed, 401 insertions(+), 4 deletions(-)
> 
> diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h
> index 7271b0f..cc91266 100644
> --- a/daemon/libvirtd.h
> +++ b/daemon/libvirtd.h
> @@ -60,6 +60,8 @@ struct daemonClientPrivate {
>      size_t nnetworkEventCallbacks;
>      daemonClientEventCallbackPtr *qemuEventCallbacks;
>      size_t nqemuEventCallbacks;
> +    daemonClientEventCallbackPtr *storageEventCallbacks;
> +    size_t nstorageEventCallbacks;
>      bool closeRegistered;
>  
>  # if WITH_SASL
> diff --git a/daemon/remote.c b/daemon/remote.c
> index b2a420b..25a0fb7 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -183,6 +183,33 @@ remoteRelayNetworkEventCheckACL(virNetServerClientPtr client,
>  
>  
>  static bool
> +remoteRelayStoragePoolEventCheckACL(virNetServerClientPtr client,
> +                                    virConnectPtr conn, virStoragePoolPtr pool)

Split 'virStoragePoolPtr ...' to its own line

> +{
> +    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.  */
> +    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,
>                                            virConnectPtr conn, virDomainPtr dom)
>  {
> @@ -1236,6 +1263,46 @@ static virConnectNetworkEventGenericCallback networkEventCallbacks[] = {
>  
>  verify(ARRAY_CARDINALITY(networkEventCallbacks) == VIR_NETWORK_EVENT_ID_LAST);
>  
> +
> +static int
> +remoteRelayStoragePoolEventLifecycle(virConnectPtr conn,
> +                                     virStoragePoolPtr pool,
> +                                     int event,
> +                                     int detail,
> +                                     void *opaque)
> +{
> +    daemonClientEventCallbackPtr callback = opaque;
> +    remote_storage_pool_event_lifecycle_msg data;
> +
> +    if (callback->callbackID < 0 ||
> +        !remoteRelayStoragePoolEventCheckACL(callback->client, conn, pool))
> +        return -1;
> +
> +    VIR_DEBUG("Relaying storage pool lifecycle event %d, detail %d, callback %d",
> +              event, detail, callback->callbackID);
> +
> +    /* build return data */
> +    memset(&data, 0, sizeof(data));
> +    make_nonnull_storage_pool(&data.pool, pool);
> +    data.callbackID = callback->callbackID;
> +    data.event = event;
> +    data.detail = detail;
> +
> +    remoteDispatchObjectEventSend(callback->client, remoteProgram,
> +                                  REMOTE_PROC_STORAGE_POOL_EVENT_LIFECYCLE,
> +                                  (xdrproc_t)xdr_remote_storage_pool_event_lifecycle_msg,
> +                                  &data);
> +
> +    return 0;
> +}
> +
> +static virConnectStoragePoolEventGenericCallback storageEventCallbacks[] = {
> +    VIR_STORAGE_POOL_EVENT_CALLBACK(remoteRelayStoragePoolEventLifecycle),
> +};
> +
> +verify(ARRAY_CARDINALITY(storageEventCallbacks) == VIR_STORAGE_POOL_EVENT_ID_LAST);
> +
> +
>  static void
>  remoteRelayDomainQemuMonitorEvent(virConnectPtr conn,
>                                    virDomainPtr dom,
> @@ -1343,6 +1410,21 @@ void remoteClientFreeFunc(void *data)
>          }
>          VIR_FREE(priv->networkEventCallbacks);
>  
> +        for (i = 0; i < priv->nstorageEventCallbacks; i++) {
> +            int callbackID = priv->storageEventCallbacks[i]->callbackID;
> +            if (callbackID < 0) {
> +                VIR_WARN("unexpected incomplete storage pool callback %zu", i);
> +                continue;
> +            }
> +            VIR_DEBUG("Deregistering remote storage pool event relay %d",
> +                      callbackID);
> +            priv->storageEventCallbacks[i]->callbackID = -1;
> +            if (virConnectStoragePoolEventDeregisterAny(priv->conn,
> +                                                        callbackID) < 0)
> +                VIR_WARN("unexpected storage pool event deregister failure");
> +        }
> +        VIR_FREE(priv->storageEventCallbacks);
> +
>          for (i = 0; i < priv->nqemuEventCallbacks; i++) {
>              int callbackID = priv->qemuEventCallbacks[i]->callbackID;
>              if (callbackID < 0) {
> @@ -5436,6 +5518,129 @@ remoteDispatchConnectNetworkEventDeregisterAny(virNetServerPtr server ATTRIBUTE_
>  
>  
>  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
> +     * 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;
> +}
> +
> +
> +static int
> +remoteDispatchConnectStoragePoolEventDeregisterAny(virNetServerPtr server ATTRIBUTE_UNUSED,
> +                                               virNetServerClientPtr client,
> +                                               virNetMessagePtr msg ATTRIBUTE_UNUSED,
> +                                               virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED,
> +                                               remote_connect_storage_pool_event_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->nstorageEventCallbacks; i++) {
> +        if (priv->storageEventCallbacks[i]->callbackID == args->callbackID)
> +            break;
> +    }
> +    if (i == priv->nstorageEventCallbacks) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("storage pool event callback %d not registered"),
> +                       args->callbackID);
> +        goto cleanup;
> +    }
> +
> +    if (virConnectStoragePoolEventDeregisterAny(priv->conn, args->callbackID) < 0)
> +        goto cleanup;
> +
> +    VIR_DELETE_ELEMENT(priv->storageEventCallbacks, i,
> +                       priv->nstorageEventCallbacks);
> +
> +    rv = 0;
> +
> + cleanup:
> +    if (rv < 0)
> +        virNetMessageSaveError(rerr);
> +    virMutexUnlock(&priv->lock);
> +    return rv;
> +}
> +
> +
> +static int
>  qemuDispatchConnectDomainMonitorEventRegister(virNetServerPtr server ATTRIBUTE_UNUSED,
>                                                virNetServerClientPtr client,
>                                                virNetMessagePtr msg ATTRIBUTE_UNUSED,
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index f494cbf..daba435 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -34,6 +34,7 @@
>  #include "datatypes.h"
>  #include "domain_event.h"
>  #include "network_event.h"
> +#include "storage_event.h"
>  #include "driver.h"
>  #include "virbuffer.h"
>  #include "remote_driver.h"
> @@ -356,6 +357,11 @@ remoteNetworkBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED,
>                                   void *evdata, void *opaque);
>  
>  static void
> +remoteStoragePoolBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED,
> +                                     virNetClientPtr client ATTRIBUTE_UNUSED,
> +                                     void *evdata, void *opaque);
> +
> +static void
>  remoteConnectNotifyEventConnectionClosed(virNetClientProgramPtr prog ATTRIBUTE_UNUSED,
>                                           virNetClientPtr client ATTRIBUTE_UNUSED,
>                                           void *evdata, void *opaque);
> @@ -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 },
>      { REMOTE_PROC_DOMAIN_EVENT_CALLBACK_LIFECYCLE,
>        remoteDomainBuildEventCallbackLifecycle,
>        sizeof(remote_domain_event_callback_lifecycle_msg),
> @@ -3042,6 +3052,103 @@ remoteConnectNetworkEventDeregisterAny(virConnectPtr conn,
>  
>  
>  static int
> +remoteConnectStoragePoolEventRegisterAny(virConnectPtr conn,
> +                                         virStoragePoolPtr pool,
> +                                         int eventID,
> +                                         virConnectStoragePoolEventGenericCallback callback,
> +                                         void *opaque,
> +                                         virFreeCallback freecb)
> +{
> +    int rv = -1;
> +    struct private_data *priv = conn->privateData;
> +    remote_connect_storage_pool_event_register_any_args args;
> +    remote_connect_storage_pool_event_register_any_ret ret;
> +    int callbackID;
> +    int count;
> +    remote_nonnull_storage_pool storage_pool;
> +
> +    remoteDriverLock(priv);
> +
> +    if ((count = virStoragePoolEventStateRegisterClient(conn, priv->eventState,
> +                                                        pool, eventID, callback,
> +                                                        opaque, freecb,
> +                                                        &callbackID)) < 0)
> +        goto done;
> +
> +    /* If this is the first callback for this eventID, we need to enable
> +     * events on the server */
> +    if (count == 1) {
> +        args.eventID = eventID;
> +        if (pool) {
> +            make_nonnull_storage_pool(&storage_pool, pool);
> +            args.pool = &storage_pool;
> +        } else {
> +            args.pool = NULL;
> +        }
> +
> +        memset(&ret, 0, sizeof(ret));
> +        if (call(conn, priv, 0, REMOTE_PROC_CONNECT_STORAGE_POOL_EVENT_REGISTER_ANY,
> +                 (xdrproc_t) xdr_remote_connect_storage_pool_event_register_any_args, (char *) &args,
> +                 (xdrproc_t) xdr_remote_connect_storage_pool_event_register_any_ret, (char *) &ret) == -1) {
> +            virObjectEventStateDeregisterID(conn, priv->eventState,
> +                                            callbackID);
> +            goto done;
> +        }
> +
> +        virObjectEventStateSetRemote(conn, priv->eventState, callbackID,
> +                                     ret.callbackID);
> +    }
> +
> +    rv = callbackID;
> +
> + done:
> +    remoteDriverUnlock(priv);
> +    return rv;
> +}
> +
> +
> +static int
> +remoteConnectStoragePoolEventDeregisterAny(virConnectPtr conn,
> +                                           int callbackID)
> +{
> +    struct private_data *priv = conn->privateData;
> +    int rv = -1;
> +    remote_connect_storage_pool_event_deregister_any_args args;
> +    int eventID;
> +    int remoteID;
> +    int count;
> +
> +    remoteDriverLock(priv);
> +
> +    if ((eventID = virObjectEventStateEventID(conn, priv->eventState,
> +                                              callbackID, &remoteID)) < 0)
> +        goto done;
> +
> +    if ((count = virObjectEventStateDeregisterID(conn, priv->eventState,
> +                                                 callbackID)) < 0)
> +        goto done;
> +
> +    /* If that was the last callback for this eventID, we need to disable
> +     * events on the server */
> +    if (count == 0) {
> +        args.callbackID = remoteID;
> +
> +        if (call(conn, priv, 0, REMOTE_PROC_CONNECT_STORAGE_POOL_EVENT_DEREGISTER_ANY,
> +                 (xdrproc_t) xdr_remote_connect_storage_pool_event_deregister_any_args, (char *) &args,
> +                 (xdrproc_t) xdr_void, (char *) NULL) == -1)
> +            goto done;
> +
> +    }
> +
> +    rv = 0;
> +
> + done:
> +    remoteDriverUnlock(priv);
> +    return rv;
> +}
> +
> +
> +static int
>  remoteConnectDomainQemuMonitorEventRegister(virConnectPtr conn,
>                                              virDomainPtr dom,
>                                              const char *event,
> @@ -5015,6 +5122,29 @@ remoteNetworkBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED,
>  
>  
>  static void
> +remoteStoragePoolBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED,
> +                                     virNetClientPtr client ATTRIBUTE_UNUSED,
> +                                     void *evdata, void *opaque)
> +{
> +    virConnectPtr conn = opaque;
> +    struct private_data *priv = conn->privateData;
> +    remote_storage_pool_event_lifecycle_msg *msg = evdata;
> +    virStoragePoolPtr pool;
> +    virObjectEventPtr event = NULL;
> +
> +    pool = get_nonnull_storage_pool(conn, msg->pool);
> +    if (!pool)
> +        return;
> +
> +    event = virStoragePoolEventLifecycleNew(pool->name, pool->uuid, msg->event,
> +                                            msg->detail);
> +    virObjectUnref(pool);
> +
> +    remoteEventQueue(priv, event, msg->callbackID);
> +}
> +
> +
> +static void
>  remoteDomainBuildQemuMonitorEvent(virNetClientProgramPtr prog ATTRIBUTE_UNUSED,
>                                    virNetClientPtr client ATTRIBUTE_UNUSED,
>                                    void *evdata, void *opaque)
> @@ -7943,6 +8073,9 @@ static virStorageDriver storage_driver = {
>      .storageVolResize = remoteStorageVolResize, /* 0.9.10 */
>      .storagePoolIsActive = remoteStoragePoolIsActive, /* 0.7.3 */
>      .storagePoolIsPersistent = remoteStoragePoolIsPersistent, /* 0.7.3 */
> +
> +    .connectStoragePoolEventDeregisterAny = remoteConnectStoragePoolEventDeregisterAny, /* 1.2.1 */
> +    .connectStoragePoolEventRegisterAny = remoteConnectStoragePoolEventRegisterAny, /* 1.2.1 */

Similar to the test driver, drop the newline and move this up near the other
'connect' functions

>  };
>  
>  static virSecretDriver secret_driver = {
> diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
> index bab8ef2..0170c0c 100644
> --- a/src/remote/remote_protocol.x
> +++ b/src/remote/remote_protocol.x
> @@ -3098,6 +3098,26 @@ struct remote_network_event_lifecycle_msg {
>      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;
> +};
> +
>  struct remote_domain_fsfreeze_args {
>      remote_nonnull_domain dom;
>      remote_nonnull_string mountpoints<REMOTE_DOMAIN_FSFREEZE_MOUNTPOINTS_MAX>; /* (const char **) */
> @@ -5793,5 +5813,26 @@ enum remote_procedure {
>       * @generate: both
>       * @acl: none
>       */
> -    REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVAL_FAILED = 367
> +    REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVAL_FAILED = 367,
> +
> +    /**
> +     * @generate: none
> +     * @priority: high
> +     * @acl: connect:search_storage_pools
> +     * @aclfilter: storage_pool:getattr
> +     */
> +    REMOTE_PROC_CONNECT_STORAGE_POOL_EVENT_REGISTER_ANY = 368,
> +
> +    /**
> +     * @generate: none
> +     * @priority: high
> +     * @acl: connect:read
> +     */
> +    REMOTE_PROC_CONNECT_STORAGE_POOL_EVENT_DEREGISTER_ANY = 369,
> +
> +    /**
> +     * @generate: both
> +     * @acl: none
> +     */
> +    REMOTE_PROC_STORAGE_POOL_EVENT_LIFECYCLE = 370
>  };
> diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
> index fe1b8a8..3c745de 100644
> --- a/src/remote_protocol-structs
> +++ b/src/remote_protocol-structs
> @@ -2546,10 +2546,26 @@ struct remote_connect_network_event_deregister_any_args {
>          int                        callbackID;
>  };
>  struct remote_network_event_lifecycle_msg {
> +        int                             callbackID;
> +        remote_nonnull_storage_pool     pool;
> +        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;
> -        remote_nonnull_network     net;
> -        int                        event;
> -        int                        detail;

Whoops, something is wrong here. We shouldn't be removing any lines in this chunk

Other than that things look good. I can't comment on all the particulars of
the remote driver stuff but it matches up nicely with the network events impl

Thanks,
Cole

--
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]