Re: [PATCH 3/6] unittests and test: implement storage lifecycle event APIs

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

 



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



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