I say keep the title like
test: implement storage lifecycle event APIs
and in the body say something about extending objecteventtest
On 06/09/2016 02:25 PM, Jovanka Gulicoska wrote:
> ---
> src/test/test_driver.c | 76 +++++++++++++++++++++
> tests/objecteventtest.c | 176 ++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 252 insertions(+)
>
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 6ab939a..fc8e56b 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -49,6 +49,7 @@
> #include "snapshot_conf.h"
> #include "fdstream.h"
> #include "storage_conf.h"
> +#include "storage_event.h"
> #include "node_device_conf.h"
> #include "virxml.h"
> #include "virthread.h"
> @@ -4114,6 +4115,7 @@ testStoragePoolCreate(virStoragePoolPtr pool,
> testDriverPtr privconn = pool->conn->privateData;
> virStoragePoolObjPtr privpool;
> int ret = -1;
> + virObjectEventPtr event = NULL;
>
> virCheckFlags(0, -1);
>
> @@ -4134,9 +4136,14 @@ testStoragePoolCreate(virStoragePoolPtr pool,
> }
>
> privpool->active = 1;
> +
> + event = virStoragePoolEventLifecycleNew(pool->name, pool->uuid,
> + VIR_STORAGE_POOL_EVENT_STARTED,
> + 0);
> ret = 0;
>
> cleanup:
> + testObjectEventQueue(privconn, event);
> if (privpool)
> virStoragePoolObjUnlock(privpool);
> return ret;
> @@ -4204,6 +4211,7 @@ testStoragePoolCreateXML(virConnectPtr conn,
> virStoragePoolDefPtr def;
> virStoragePoolObjPtr pool = NULL;
> virStoragePoolPtr ret = NULL;
> + virObjectEventPtr event = NULL;
>
> virCheckFlags(0, NULL);
>
> @@ -4231,11 +4239,16 @@ testStoragePoolCreateXML(virConnectPtr conn,
> }
> pool->active = 1;
>
> + event = virStoragePoolEventLifecycleNew(pool->def->name, pool->def->uuid,
> + VIR_STORAGE_POOL_EVENT_STARTED,
> + 0);
> +
> ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid,
> NULL, NULL);
>
> cleanup:
> virStoragePoolDefFree(def);
> + testObjectEventQueue(privconn, event);
> if (pool)
> virStoragePoolObjUnlock(pool);
> testDriverUnlock(privconn);
> @@ -4251,6 +4264,7 @@ testStoragePoolDefineXML(virConnectPtr conn,
> virStoragePoolDefPtr def;
> virStoragePoolObjPtr pool = NULL;
> virStoragePoolPtr ret = NULL;
> + virObjectEventPtr event = NULL;
>
> virCheckFlags(0, NULL);
>
> @@ -4266,6 +4280,10 @@ testStoragePoolDefineXML(virConnectPtr conn,
> goto cleanup;
> def = NULL;
>
> + event = virStoragePoolEventLifecycleNew(pool->def->name, pool->def->uuid,
> + VIR_STORAGE_POOL_EVENT_DEFINED,
> + 0);
> +
> if (testStoragePoolObjSetDefaults(pool) == -1) {
> virStoragePoolObjRemove(&privconn->pools, pool);
> pool = NULL;
> @@ -4277,6 +4295,7 @@ testStoragePoolDefineXML(virConnectPtr conn,
>
> cleanup:
> virStoragePoolDefFree(def);
> + testObjectEventQueue(privconn, event);
> if (pool)
> virStoragePoolObjUnlock(pool);
> testDriverUnlock(privconn);
> @@ -4289,6 +4308,7 @@ testStoragePoolUndefine(virStoragePoolPtr pool)
> testDriverPtr privconn = pool->conn->privateData;
> virStoragePoolObjPtr privpool;
> int ret = -1;
> + virObjectEventPtr event = NULL;
>
> testDriverLock(privconn);
> privpool = virStoragePoolObjFindByName(&privconn->pools,
> @@ -4305,6 +4325,10 @@ testStoragePoolUndefine(virStoragePoolPtr pool)
> goto cleanup;
> }
>
> + event = virStoragePoolEventLifecycleNew(pool->name, pool->uuid,
> + VIR_STORAGE_POOL_EVENT_UNDEFINED,
> + 0);
> +
> virStoragePoolObjRemove(&privconn->pools, privpool);
> privpool = NULL;
> ret = 0;
> @@ -4312,6 +4336,7 @@ testStoragePoolUndefine(virStoragePoolPtr pool)
> cleanup:
> if (privpool)
> virStoragePoolObjUnlock(privpool);
> + testObjectEventQueue(privconn, event);
> testDriverUnlock(privconn);
> return ret;
> }
> @@ -4323,6 +4348,7 @@ testStoragePoolBuild(virStoragePoolPtr pool,
> testDriverPtr privconn = pool->conn->privateData;
> virStoragePoolObjPtr privpool;
> int ret = -1;
> + virObjectEventPtr event = NULL;
>
> virCheckFlags(0, -1);
>
> @@ -4341,9 +4367,11 @@ testStoragePoolBuild(virStoragePoolPtr pool,
> _("storage pool '%s' is already active"), pool->name);
> goto cleanup;
> }
> +
> ret = 0;
>
> cleanup:
> + testObjectEventQueue(privconn, event);
> if (privpool)
> virStoragePoolObjUnlock(privpool);
> return ret;
Looks like something went wrong for PoolBuild, I don't think we should be
emitting any event here, so all these three hunks should be dropped.
> @@ -4356,6 +4384,7 @@ testStoragePoolDestroy(virStoragePoolPtr pool)
> testDriverPtr privconn = pool->conn->privateData;
> virStoragePoolObjPtr privpool;
> int ret = -1;
> + virObjectEventPtr event = NULL;
>
> testDriverLock(privconn);
> privpool = virStoragePoolObjFindByName(&privconn->pools,
> @@ -4373,6 +4402,9 @@ testStoragePoolDestroy(virStoragePoolPtr pool)
> }
>
> privpool->active = 0;
> + event = virStoragePoolEventLifecycleNew(privpool->def->name, privpool->def->uuid,
> + VIR_STORAGE_POOL_EVENT_STOPPED,
> + 0);
>
> if (privpool->configFile == NULL) {
> virStoragePoolObjRemove(&privconn->pools, privpool);
> @@ -4381,6 +4413,7 @@ testStoragePoolDestroy(virStoragePoolPtr pool)
> ret = 0;
>
> cleanup:
> + testObjectEventQueue(privconn, event);
> if (privpool)
> virStoragePoolObjUnlock(privpool);
> testDriverUnlock(privconn);
> @@ -4430,6 +4463,7 @@ testStoragePoolRefresh(virStoragePoolPtr pool,
> testDriverPtr privconn = pool->conn->privateData;
> virStoragePoolObjPtr privpool;
> int ret = -1;
> + virObjectEventPtr event = NULL;
>
> virCheckFlags(0, -1);
>
> @@ -4448,9 +4482,15 @@ testStoragePoolRefresh(virStoragePoolPtr pool,
> _("storage pool '%s' is not active"), pool->name);
> goto cleanup;
> }
> +
> + event = virStoragePoolEventLifecycleNew(pool->name, pool->uuid,
> + VIR_STORAGE_POOL_EVENT_REFRESHED,
> + 0);
> +
In the early case like this, you didn't add a newline between the LifecycleNew
call, and the ret = 0; ... I say remove the newline here to be consistent.
> ret = 0;
>
> cleanup:
> + testObjectEventQueue(privconn, event);
> if (privpool)
> virStoragePoolObjUnlock(privpool);
> return ret;
> @@ -5644,6 +5684,39 @@ testConnectNetworkEventDeregisterAny(virConnectPtr conn,
> return ret;
> }
>
> +static int
> +testConnectStoragePoolEventRegisterAny(virConnectPtr conn,
> + virStoragePoolPtr pool,
> + int eventID,
> + virConnectStoragePoolEventGenericCallback callback,
> + void *opaque,
> + virFreeCallback freecb)
> +{
> + testDriverPtr driver = conn->privateData;
> + int ret;
> +
> + if (virStoragePoolEventStateRegisterID(conn, driver->eventState,
> + pool, eventID, callback,
> + opaque, freecb, &ret) < 0)
> + ret = -1;
> +
> + return ret;
> +}
> +
> +static int
> +testConnectStoragePoolEventDeregisterAny(virConnectPtr conn,
> + int callbackID)
> +{
> + testDriverPtr driver = conn->privateData;
> + int ret = 0;
> +
> + if (virObjectEventStateDeregisterID(conn, driver->eventState,
> + callbackID) < 0)
> + ret = -1;
> +
> + return ret;
> +}
> +
> static int testConnectListAllDomains(virConnectPtr conn,
> virDomainPtr **domains,
> unsigned int flags)
> @@ -6781,6 +6854,9 @@ static virStorageDriver testStorageDriver = {
> .storageVolGetPath = testStorageVolGetPath, /* 0.5.0 */
> .storagePoolIsActive = testStoragePoolIsActive, /* 0.7.3 */
> .storagePoolIsPersistent = testStoragePoolIsPersistent, /* 0.7.3 */
> +
There aren't any newlines anywhere else in the structure, so I say drop this...
> + .connectStoragePoolEventRegisterAny = testConnectStoragePoolEventRegisterAny, /*1.3.6*/
> + .connectStoragePoolEventDeregisterAny = testConnectStoragePoolEventDeregisterAny, /*1.3.6*/
And move these two up above, after the other collection of methods that start
with 'connect'
> };
>
> static virNodeDeviceDriver testNodeDeviceDriver = {
> diff --git a/tests/objecteventtest.c b/tests/objecteventtest.c
> index c77b0cd..9e53758 100644
> --- a/tests/objecteventtest.c
> +++ b/tests/objecteventtest.c
> @@ -53,6 +53,20 @@ static const char networkDef[] =
> " </ip>\n"
> "</network>\n";
>
> +static const char storagePoolDef[] =
> +"<pool type='dir'>\n"
> +" <name>P</name>\n"
> +" <source>\n"
> +" <host name='src-host'/>\n"
> +" <dir path='/src/path'/>\n"
> +" <device path='/src/dev'/>\n"
> +" <name>test-storage_pool</name>\n"
You can drop the whole <source> block, it shouldn't matter for type='dir'
> +" </source>\n"
> +" <target>\n"
> +" <path>/target-path</path>\n"
> +" </target>\n"
> +"</pool>\n";
> +
> typedef struct {
> int startEvents;
> int stopEvents;
> @@ -74,6 +88,7 @@ lifecycleEventCounter_reset(lifecycleEventCounter *counter)
> typedef struct {
> virConnectPtr conn;
> virNetworkPtr net;
> + virStoragePoolPtr pool;
> } objecteventTest;
>
>
> @@ -125,6 +140,24 @@ networkLifecycleCb(virConnectPtr conn ATTRIBUTE_UNUSED,
> counter->undefineEvents++;
> }
>
> +static void
> +storagePoolLifecycleCb(virConnectPtr conn ATTRIBUTE_UNUSED,
> + virStoragePoolPtr pool ATTRIBUTE_UNUSED,
> + int event,
> + int detail ATTRIBUTE_UNUSED,
> + void* opaque)
> +{
> + lifecycleEventCounter *counter = opaque;
> +
> + if (event == VIR_STORAGE_POOL_EVENT_STARTED)
> + counter->startEvents++;
> + else if (event == VIR_STORAGE_POOL_EVENT_STOPPED)
> + counter->stopEvents++;
> + else if (event == VIR_STORAGE_POOL_EVENT_DEFINED)
> + counter->defineEvents++;
> + else if (event == VIR_STORAGE_POOL_EVENT_UNDEFINED)
> + counter->undefineEvents++;
> +}
This doesn't track REFRESHED events, so the lifecycleEventCounter needs to be
extended, and some of the below code as well.
>
> static int
> testDomainCreateXMLOld(const void *data)
> @@ -523,6 +556,130 @@ testNetworkStartStopEvent(const void *data)
> return ret;
> }
>
> +static int
> +testStoragePoolCreateXML(const void *data)
> +{
> + const objecteventTest *test = data;
> + lifecycleEventCounter counter;
> + virStoragePoolPtr pool;
> + int id;
> + int ret = 0;
> +
> + lifecycleEventCounter_reset(&counter);
> +
> + id = virConnectStoragePoolEventRegisterAny(test->conn, NULL,
> + VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE,
> + VIR_STORAGE_POOL_EVENT_CALLBACK(&storagePoolLifecycleCb),
> + &counter, NULL);
This line is over 80 columns, so maybe unindent these three lines so it fits.
> + pool = virStoragePoolCreateXML(test->conn, storagePoolDef, 0);
> +
> + if (!pool || virEventRunDefaultImpl() < 0) {
> + ret = -1;
> + goto cleanup;
> + }
> +
> + if (counter.startEvents != 1 || counter.unexpectedEvents > 0) {
> + ret = -1;
> + goto cleanup;
> + }
> +
> + cleanup:
> + virConnectStoragePoolEventDeregisterAny(test->conn, id);
> + if (pool) {
> + virStoragePoolDestroy(pool);
> + virStoragePoolFree(pool);
> + }
> + return ret;
> +}
> +
> +static int
> +testStoragePoolDefine(const void *data)
> +{
> + const objecteventTest *test = data;
> + lifecycleEventCounter counter;
> + virStoragePoolPtr pool;
> + int id;
> + int ret = 0;
> +
> + lifecycleEventCounter_reset(&counter);
> +
> + id = virConnectStoragePoolEventRegisterAny(test->conn, NULL,
> + VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE,
> + VIR_STORAGE_POOL_EVENT_CALLBACK(&storagePoolLifecycleCb),
> + &counter, NULL);
> +
same
> + /* Make sure the define event is triggered */
> + pool = virStoragePoolDefineXML(test->conn, storagePoolDef, 0);
> +
> + if (!pool || virEventRunDefaultImpl() < 0) {
> + ret = -1;
> + goto cleanup;
> + }
> +
> + if (counter.defineEvents != 1 || counter.unexpectedEvents > 0) {
> + ret = -1;
> + goto cleanup;
> + }
> +
> + /* Make sure the undefine event is triggered */
> + virStoragePoolUndefine(pool);
> +
> + if (virEventRunDefaultImpl() < 0) {
> + ret = -1;
> + goto cleanup;
> + }
> +
> + if (counter.undefineEvents != 1 || counter.unexpectedEvents > 0) {
> + ret = -1;
> + goto cleanup;
> + }
> +
> +
> + cleanup:
> + virConnectStoragePoolEventDeregisterAny(test->conn, id);
> + if (pool)
> + virStoragePoolFree(pool);
> +
> + return ret;
> +}
> +
> +static int
> +testStoragePoolStartStopEvent(const void *data)
> +{
> + const objecteventTest *test = data;
> + lifecycleEventCounter counter;
> + int id;
> + int ret = 0;
> +
> + if (!test->pool)
> + return -1;
> +
> + lifecycleEventCounter_reset(&counter);
> +
> + id = virConnectStoragePoolEventRegisterAny(test->conn, test->pool,
> + VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE,
> + VIR_STORAGE_POOL_EVENT_CALLBACK(&storagePoolLifecycleCb),
> + &counter, NULL);
same here
> + virStoragePoolCreate(test->pool, 0);
> + virStoragePoolRefresh(test->pool, 0);
> + virStoragePoolDestroy(test->pool);
> +
> + if (virEventRunDefaultImpl() < 0) {
> + ret = -1;
> + goto cleanup;
> + }
> +
> + if (counter.startEvents != 1 || counter.stopEvents != 1 ||
> + counter.unexpectedEvents > 0) {
> + ret = -1;
> + goto cleanup;
> + }
> + cleanup:
> + virConnectStoragePoolEventDeregisterAny(test->conn, id);
> +
> + return ret;
> +}
> +
> static void
> timeout(int id ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED)
> {
> @@ -581,6 +738,25 @@ mymain(void)
> virNetworkUndefine(test.net);
> virNetworkFree(test.net);
> }
> +
> + /* Storage pool event tests */
> + if (virTestRun("Storage pool createXML start event ", testStoragePoolCreateXML, &test) < 0)
> + ret = EXIT_FAILURE;
Break this long line
> + if (virTestRun("Storage pool (un)define events", testStoragePoolDefine, &test) < 0)
> + ret = EXIT_FAILURE;
> +
> + /* Define a test storage pool */
> + if (!(test.pool = virStoragePoolDefineXML(test.conn, storagePoolDef, 0)))
> + ret = EXIT_FAILURE;
> + if (virTestRun("Storage pool start stop events ", testStoragePoolStartStopEvent, &test) < 0)
Break this long line
> + ret = EXIT_FAILURE;
> +
> + /* Cleanup */
> + if (test.pool) {
> + virStoragePoolUndefine(test.pool);
> + virStoragePoolFree(test.pool);
> + }
> +
> virConnectClose(test.conn);
> virEventRemoveTimeout(timer);
>
Thanks,
Cole
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list