On 06/09/2016 02:25 PM, Jovanka Gulicoska wrote:
> Implement storage pool event callbacks for START, STOP, DEFINE, UNDEFINED
> and REFRESHED in functions when a storage pool is created/started/stopped
> etc. accordingly.
>
I say just make the above chunk the commit message, drop the bits below
> virStoragePoolEventRegisterAny:
> Adds a callback to receive notifications of arbitrary storage pool events
> occurring on a storage pool.
>
> virStoragePoolDeregisterAny:
> Removes an eventcallback
> ---
> src/storage/storage_driver.c | 118 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 118 insertions(+)
>
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index fb1b1a2..76fab6a 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -41,6 +41,7 @@
> #include "driver.h"
> #include "storage_driver.h"
> #include "storage_conf.h"
> +#include "storage_event.h"
> #include "viralloc.h"
> #include "storage_backend.h"
> #include "virlog.h"
> @@ -57,6 +58,12 @@ VIR_LOG_INIT("storage.storage_driver");
>
> static virStorageDriverStatePtr driver;
>
> +static virStorageDriverStatePtr
> +storageGetDriver(void)
> +{
> + return driver;
> +}
> +
This isn't needed, just access the global 'driver' variable directly.
> static int storageStateCleanup(void);
>
> typedef struct _virStorageVolStreamInfo virStorageVolStreamInfo;
> @@ -276,6 +283,8 @@ storageStateInitialize(bool privileged,
>
> storagePoolUpdateAllState();
>
> + driver->storageEventState = virObjectEventStateNew();
> +
> storageDriverUnlock();
>
> ret = 0;
> @@ -344,6 +353,8 @@ storageStateCleanup(void)
>
> storageDriverLock();
>
> + virObjectEventStateFree(driver->storageEventState);
> +
> /* free inactive pools */
> virStoragePoolObjListFree(&driver->pools);
>
> @@ -668,6 +679,7 @@ storagePoolCreateXML(virConnectPtr conn,
> virStoragePoolObjPtr pool = NULL;
> virStoragePoolPtr ret = NULL;
> virStorageBackendPtr backend;
> + virObjectEventPtr event = NULL;
> char *stateFile = NULL;
> unsigned int build_flags = 0;
>
> @@ -735,6 +747,12 @@ storagePoolCreateXML(virConnectPtr conn,
> pool = NULL;
> goto cleanup;
> }
> +
> + event = virStoragePoolEventLifecycleNew(pool->def->name,
> + pool->def->uuid,
> + VIR_STORAGE_POOL_EVENT_STARTED,
> + 0);
> +
> VIR_INFO("Creating storage pool '%s'", pool->def->name);
> pool->active = true;
>
> @@ -744,6 +762,8 @@ storagePoolCreateXML(virConnectPtr conn,
> cleanup:
> VIR_FREE(stateFile);
> virStoragePoolDefFree(def);
> + if (event)
> + virObjectEventStateQueue(driver->storageEventState, event);
> if (pool)
> virStoragePoolObjUnlock(pool);
> storageDriverUnlock();
> @@ -758,6 +778,7 @@ storagePoolDefineXML(virConnectPtr conn,
> virStoragePoolDefPtr def;
> virStoragePoolObjPtr pool = NULL;
> virStoragePoolPtr ret = NULL;
> + virObjectEventPtr event = NULL;
>
> virCheckFlags(0, NULL);
>
> @@ -786,6 +807,11 @@ storagePoolDefineXML(virConnectPtr conn,
> pool = NULL;
> goto cleanup;
> }
> +
> + event = virStoragePoolEventLifecycleNew(def->name, def->uuid,
> + VIR_STORAGE_POOL_EVENT_DEFINED,
> + 0);
> +
> def = NULL;
>
> VIR_INFO("Defining storage pool '%s'", pool->def->name);
> @@ -793,6 +819,8 @@ storagePoolDefineXML(virConnectPtr conn,
> NULL, NULL);
>
> cleanup:
> + if (event)
> + virObjectEventStateQueue(driver->storageEventState, event);
> virStoragePoolDefFree(def);
> if (pool)
> virStoragePoolObjUnlock(pool);
> @@ -804,6 +832,7 @@ static int
> storagePoolUndefine(virStoragePoolPtr obj)
> {
> virStoragePoolObjPtr pool;
> + virObjectEventPtr event = NULL;
> int ret = -1;
>
> storageDriverLock();
> @@ -847,12 +876,19 @@ storagePoolUndefine(virStoragePoolPtr obj)
> VIR_FREE(pool->configFile);
> VIR_FREE(pool->autostartLink);
>
> + event = virStoragePoolEventLifecycleNew(pool->def->name,
> + pool->def->uuid,
> + VIR_STORAGE_POOL_EVENT_UNDEFINED,
> + 0);
> +
> VIR_INFO("Undefining storage pool '%s'", pool->def->name);
> virStoragePoolObjRemove(&driver->pools, pool);
> pool = NULL;
> ret = 0;
>
> cleanup:
> + if (event)
> + virObjectEventStateQueue(driver->storageEventState, event);
> if (pool)
> virStoragePoolObjUnlock(pool);
> storageDriverUnlock();
> @@ -865,6 +901,7 @@ storagePoolCreate(virStoragePoolPtr obj,
> {
> virStoragePoolObjPtr pool;
> virStorageBackendPtr backend;
> + virObjectEventPtr event = NULL;
> int ret = -1;
> char *stateFile = NULL;
> unsigned int build_flags = 0;
> @@ -926,11 +963,18 @@ storagePoolCreate(virStoragePoolPtr obj,
> goto cleanup;
> }
>
> + event = virStoragePoolEventLifecycleNew(pool->def->name,
> + pool->def->uuid,
> + VIR_STORAGE_POOL_EVENT_STARTED,
> + 0);
> +
> pool->active = true;
> ret = 0;
>
> cleanup:
> VIR_FREE(stateFile);
> + if (event)
> + virObjectEventStateQueue(driver->storageEventState, event);
> if (pool)
> virStoragePoolObjUnlock(pool);
> return ret;
> @@ -976,6 +1020,7 @@ storagePoolDestroy(virStoragePoolPtr obj)
> {
> virStoragePoolObjPtr pool;
> virStorageBackendPtr backend;
> + virObjectEventPtr event = NULL;
> char *stateFile = NULL;
> int ret = -1;
>
> @@ -1024,6 +1069,11 @@ storagePoolDestroy(virStoragePoolPtr obj)
>
> virStoragePoolObjClearVols(pool);
>
> + event = virStoragePoolEventLifecycleNew(pool->def->name,
> + pool->def->uuid,
> + VIR_STORAGE_POOL_EVENT_STOPPED,
> + 0);
> +
> pool->active = false;
>
> if (pool->configFile == NULL) {
> @@ -1038,6 +1088,8 @@ storagePoolDestroy(virStoragePoolPtr obj)
> ret = 0;
>
> cleanup:
> + if (event)
> + virObjectEventStateQueue(driver->storageEventState, event);
> if (pool)
> virStoragePoolObjUnlock(pool);
> storageDriverUnlock();
> @@ -1109,6 +1161,7 @@ storagePoolRefresh(virStoragePoolPtr obj,
> virStoragePoolObjPtr pool;
> virStorageBackendPtr backend;
> int ret = -1;
> + virObjectEventPtr event = NULL;
>
> virCheckFlags(0, -1);
>
> @@ -1146,6 +1199,10 @@ storagePoolRefresh(virStoragePoolPtr obj,
> if (backend->stopPool)
> backend->stopPool(obj->conn, pool);
>
> + event = virStoragePoolEventLifecycleNew(pool->def->name,
> + pool->def->uuid,
> + VIR_STORAGE_POOL_EVENT_STOPPED,
> + 0);
> pool->active = false;
>
> if (pool->configFile == NULL) {
> @@ -1154,9 +1211,16 @@ storagePoolRefresh(virStoragePoolPtr obj,
> }
> goto cleanup;
> }
> +
> + event = virStoragePoolEventLifecycleNew(pool->def->name,
> + pool->def->uuid,
> + VIR_STORAGE_POOL_EVENT_REFRESHED,
> + 0);
> ret = 0;
>
> cleanup:
> + if (event)
> + virObjectEventStateQueue(driver->storageEventState, event);
> if (pool)
> virStoragePoolObjUnlock(pool);
> storageDriverUnlock();
> @@ -2266,6 +2330,7 @@ virStorageVolPoolRefreshThread(void *opaque)
> virStorageVolStreamInfoPtr cbdata = opaque;
> virStoragePoolObjPtr pool = NULL;
> virStorageBackendPtr backend;
> + virObjectEventPtr event = NULL;
>
> storageDriverLock();
> if (cbdata->vol_path) {
> @@ -2283,7 +2348,14 @@ virStorageVolPoolRefreshThread(void *opaque)
> if (backend->refreshPool(NULL, pool) < 0)
> VIR_DEBUG("Failed to refresh storage pool");
>
> + event = virStoragePoolEventLifecycleNew(pool->def->name,
> + pool->def->uuid,
> + VIR_STORAGE_POOL_EVENT_REFRESHED,
> + 0);
> +
> cleanup:
> + if (event)
> + virObjectEventStateQueue(driver->storageEventState, event);
> if (pool)
> virStoragePoolObjUnlock(pool);
> storageDriverUnlock();
> @@ -2662,6 +2734,50 @@ storageConnectListAllStoragePools(virConnectPtr conn,
> return ret;
> }
>
> +static int
> +storageConnectStoragePoolEventRegisterAny(virConnectPtr conn,
> + virStoragePoolPtr pool,
> + int eventID,
> + virConnectStoragePoolEventGenericCallback callback,
> + void *opaque,
> + virFreeCallback freecb)
> +{
> + virStorageDriverStatePtr storage_driver = storageGetDriver();
So this can be dropped, and just use 'driver' below
> + int ret = -1;
> +
> + if (virConnectStoragePoolEventRegisterAnyEnsureACL(conn) < 0)
> + goto cleanup;
> +
> + if (virStoragePoolEventStateRegisterID(conn, storage_driver->storageEventState,
> + pool, eventID, callback,
> + opaque, freecb, &ret) < 0)
> + ret = -1;
> + cleanup:
> + return ret;
> +}
> +
> +static int
> +storageConnectStoragePoolEventDeregisterAny(virConnectPtr conn,
> + int callbackID)
> +{
> + virStorageDriverStatePtr storage_driver = storageGetDriver();
> + int ret = -1;
> +
> + if (virConnectStoragePoolEventDeregisterAnyEnsureACL(conn) < 0)
> + goto cleanup;
> +
> + if (virObjectEventStateDeregisterID(conn,
> + storage_driver->storageEventState,
> + callbackID) < 0)
> + goto cleanup;
> +
> + ret = 0;
> +
> + cleanup:
> + return ret;
> +}
> +
> +
>
> static virStorageDriver storageDriver = {
> .name = "storage",
> @@ -2670,6 +2786,8 @@ static virStorageDriver storageDriver = {
> .connectNumOfDefinedStoragePools = storageConnectNumOfDefinedStoragePools, /* 0.4.0 */
> .connectListDefinedStoragePools = storageConnectListDefinedStoragePools, /* 0.4.0 */
> .connectListAllStoragePools = storageConnectListAllStoragePools, /* 0.10.2 */
> + .connectStoragePoolEventRegisterAny = storageConnectStoragePoolEventRegisterAny, /* 1.2.1 */
> + .connectStoragePoolEventDeregisterAny = storageConnectStoragePoolEventDeregisterAny, /* 1.2.1 */
The comment at the end is meant to signify the version number the API was
added in, which if the patches land for the next release will be 1.3.6. The
other virStorageDriver changes in previous patches probably need to be updated
as well
Otherwise this looks good to me!
Thanks,
Cole
> .connectFindStoragePoolSources = storageConnectFindStoragePoolSources, /* 0.4.0 */
> .storagePoolLookupByName = storagePoolLookupByName, /* 0.4.0 */
> .storagePoolLookupByUUID = storagePoolLookupByUUID, /* 0.4.0 */
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list