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

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

 



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



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