[libvirt] [PATCH 0/3] Clean
More "fallout" from the RFC series to make a common pool object: http://www.redhat.com/archives/libvir-list/2017-February/msg00519.html were changes made along the way in the test_driver code to "commonalize" the various vir*ObjFindBy{Name|UUID} callers. This series will do the same for the existing code. Not even attempted was making test_driver to have updated coding style (maybe another day for that)! John Ferlan (3): test: Make common test*ObjFindByName helpers test: Make a common testNetworkObjFindByName test: Make common test*ObjFindByUUID helpers src/test/test_driver.c | 499 ++++++++++++++++--------------------------------- 1 file changed, 166 insertions(+), 333 deletions(-) -- 2.9.3
Rather than have continued repeated sequences of : testDriverLock() xxx = vir*ObjFindByName() testDriverUnlock() if (xxx == NULL) { virReportError goto cleanup; } Make some common helpers which will use the pattern and make a single reference using a single common error message. Altered for Interfaces, Storage Pools, Storage Volumes, and Node Devices. For each the common error message can now also indicate which 'name' was not found. For Storage Volumes, the "new" error will be more specific rather than just invalid argument. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/test/test_driver.c | 346 +++++++++++++------------------------------------ 1 file changed, 90 insertions(+), 256 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 61c82b9..c2536c9 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3637,6 +3637,25 @@ static int testNetworkSetAutostart(virNetworkPtr network, */ +static virInterfaceObjPtr +testInterfaceObjFindByName(testDriverPtr privconn, + const char *name) +{ + virInterfaceObjPtr iface; + + testDriverLock(privconn); + iface = virInterfaceFindByName(&privconn->ifaces, name); + testDriverUnlock(privconn); + + if (!iface) + virReportError(VIR_ERR_NO_INTERFACE, + _("no interface with matching name '%s'"), + name); + + return iface; +} + + static int testConnectNumOfInterfaces(virConnectPtr conn) { testDriverPtr privconn = conn->privateData; @@ -3736,14 +3755,8 @@ static virInterfacePtr testInterfaceLookupByName(virConnectPtr conn, virInterfaceObjPtr iface; virInterfacePtr ret = NULL; - testDriverLock(privconn); - iface = virInterfaceFindByName(&privconn->ifaces, name); - testDriverUnlock(privconn); - - if (iface == NULL) { - virReportError(VIR_ERR_NO_INTERFACE, NULL); + if (!(iface = testInterfaceObjFindByName(privconn, name))) goto cleanup; - } ret = virGetInterface(conn, iface->def->name, iface->def->mac); @@ -3789,13 +3802,9 @@ static int testInterfaceIsActive(virInterfacePtr iface) virInterfaceObjPtr obj; int ret = -1; - testDriverLock(privconn); - obj = virInterfaceFindByName(&privconn->ifaces, iface->name); - testDriverUnlock(privconn); - if (!obj) { - virReportError(VIR_ERR_NO_INTERFACE, NULL); + if (!(obj = testInterfaceObjFindByName(privconn, iface->name))) goto cleanup; - } + ret = virInterfaceObjIsActive(obj); cleanup: @@ -3900,15 +3909,8 @@ static char *testInterfaceGetXMLDesc(virInterfacePtr iface, virCheckFlags(0, NULL); - testDriverLock(privconn); - privinterface = virInterfaceFindByName(&privconn->ifaces, - iface->name); - testDriverUnlock(privconn); - - if (privinterface == NULL) { - virReportError(VIR_ERR_NO_INTERFACE, __FUNCTION__); + if (!(privinterface = testInterfaceObjFindByName(privconn, iface->name))) goto cleanup; - } ret = virInterfaceDefFormat(privinterface->def); @@ -3953,14 +3955,8 @@ static int testInterfaceUndefine(virInterfacePtr iface) virInterfaceObjPtr privinterface; int ret = -1; - testDriverLock(privconn); - privinterface = virInterfaceFindByName(&privconn->ifaces, - iface->name); - - if (privinterface == NULL) { - virReportError(VIR_ERR_NO_INTERFACE, NULL); + if (!(privinterface = testInterfaceObjFindByName(privconn, iface->name))) goto cleanup; - } virInterfaceRemove(&privconn->ifaces, privinterface); @@ -3980,14 +3976,8 @@ static int testInterfaceCreate(virInterfacePtr iface, virCheckFlags(0, -1); - testDriverLock(privconn); - privinterface = virInterfaceFindByName(&privconn->ifaces, - iface->name); - - if (privinterface == NULL) { - virReportError(VIR_ERR_NO_INTERFACE, NULL); + if (!(privinterface = testInterfaceObjFindByName(privconn, iface->name))) goto cleanup; - } if (privinterface->active != 0) { virReportError(VIR_ERR_OPERATION_INVALID, NULL); @@ -4013,14 +4003,8 @@ static int testInterfaceDestroy(virInterfacePtr iface, virCheckFlags(0, -1); - testDriverLock(privconn); - privinterface = virInterfaceFindByName(&privconn->ifaces, - iface->name); - - if (privinterface == NULL) { - virReportError(VIR_ERR_NO_INTERFACE, NULL); + if (!(privinterface = testInterfaceObjFindByName(privconn, iface->name))) goto cleanup; - } if (privinterface->active == 0) { virReportError(VIR_ERR_OPERATION_INVALID, NULL); @@ -4055,6 +4039,25 @@ static int testStoragePoolObjSetDefaults(virStoragePoolObjPtr pool) } +static virStoragePoolObjPtr +testStoragePoolObjFindByName(testDriverPtr privconn, + const char *name) +{ + virStoragePoolObjPtr pool; + + testDriverLock(privconn); + pool = virStoragePoolObjFindByName(&privconn->pools, name); + testDriverUnlock(privconn); + + if (!pool) + virReportError(VIR_ERR_NO_STORAGE_POOL, + _("no storage pool with matching name '%s'"), + name); + + return pool; +} + + static virStoragePoolPtr testStoragePoolLookupByUUID(virConnectPtr conn, const unsigned char *uuid) @@ -4089,14 +4092,8 @@ testStoragePoolLookupByName(virConnectPtr conn, virStoragePoolObjPtr pool; virStoragePoolPtr ret = NULL; - testDriverLock(privconn); - pool = virStoragePoolObjFindByName(&privconn->pools, name); - testDriverUnlock(privconn); - - if (pool == NULL) { - virReportError(VIR_ERR_NO_STORAGE_POOL, NULL); + if (!(pool = testStoragePoolObjFindByName(privconn, name))) goto cleanup; - } ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid, NULL, NULL); @@ -4283,15 +4280,8 @@ testStoragePoolCreate(virStoragePoolPtr pool, virCheckFlags(0, -1); - testDriverLock(privconn); - privpool = virStoragePoolObjFindByName(&privconn->pools, - pool->name); - testDriverUnlock(privconn); - - if (privpool == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name))) goto cleanup; - } if (virStoragePoolObjIsActive(privpool)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -4474,14 +4464,8 @@ testStoragePoolUndefine(virStoragePoolPtr pool) int ret = -1; virObjectEventPtr event = NULL; - testDriverLock(privconn); - privpool = virStoragePoolObjFindByName(&privconn->pools, - pool->name); - - if (privpool == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name))) goto cleanup; - } if (virStoragePoolObjIsActive(privpool)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -4515,15 +4499,8 @@ testStoragePoolBuild(virStoragePoolPtr pool, virCheckFlags(0, -1); - testDriverLock(privconn); - privpool = virStoragePoolObjFindByName(&privconn->pools, - pool->name); - testDriverUnlock(privconn); - - if (privpool == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name))) goto cleanup; - } if (virStoragePoolObjIsActive(privpool)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -4547,14 +4524,8 @@ testStoragePoolDestroy(virStoragePoolPtr pool) int ret = -1; virObjectEventPtr event = NULL; - testDriverLock(privconn); - privpool = virStoragePoolObjFindByName(&privconn->pools, - pool->name); - - if (privpool == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name))) goto cleanup; - } if (!virStoragePoolObjIsActive(privpool)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -4592,15 +4563,8 @@ testStoragePoolDelete(virStoragePoolPtr pool, virCheckFlags(0, -1); - testDriverLock(privconn); - privpool = virStoragePoolObjFindByName(&privconn->pools, - pool->name); - testDriverUnlock(privconn); - - if (privpool == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name))) goto cleanup; - } if (virStoragePoolObjIsActive(privpool)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -4628,15 +4592,8 @@ testStoragePoolRefresh(virStoragePoolPtr pool, virCheckFlags(0, -1); - testDriverLock(privconn); - privpool = virStoragePoolObjFindByName(&privconn->pools, - pool->name); - testDriverUnlock(privconn); - - if (privpool == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name))) goto cleanup; - } if (!virStoragePoolObjIsActive(privpool)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -4663,15 +4620,8 @@ testStoragePoolGetInfo(virStoragePoolPtr pool, virStoragePoolObjPtr privpool; int ret = -1; - testDriverLock(privconn); - privpool = virStoragePoolObjFindByName(&privconn->pools, - pool->name); - testDriverUnlock(privconn); - - if (privpool == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name))) goto cleanup; - } memset(info, 0, sizeof(virStoragePoolInfo)); if (privpool->active) @@ -4699,15 +4649,8 @@ testStoragePoolGetXMLDesc(virStoragePoolPtr pool, virCheckFlags(0, NULL); - testDriverLock(privconn); - privpool = virStoragePoolObjFindByName(&privconn->pools, - pool->name); - testDriverUnlock(privconn); - - if (privpool == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name))) goto cleanup; - } ret = virStoragePoolDefFormat(privpool->def); @@ -4725,15 +4668,8 @@ testStoragePoolGetAutostart(virStoragePoolPtr pool, virStoragePoolObjPtr privpool; int ret = -1; - testDriverLock(privconn); - privpool = virStoragePoolObjFindByName(&privconn->pools, - pool->name); - testDriverUnlock(privconn); - - if (privpool == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name))) goto cleanup; - } if (!privpool->configFile) { *autostart = 0; @@ -4756,15 +4692,8 @@ testStoragePoolSetAutostart(virStoragePoolPtr pool, virStoragePoolObjPtr privpool; int ret = -1; - testDriverLock(privconn); - privpool = virStoragePoolObjFindByName(&privconn->pools, - pool->name); - testDriverUnlock(privconn); - - if (privpool == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name))) goto cleanup; - } if (!privpool->configFile) { virReportError(VIR_ERR_INVALID_ARG, @@ -4790,15 +4719,8 @@ testStoragePoolNumOfVolumes(virStoragePoolPtr pool) virStoragePoolObjPtr privpool; int ret = -1; - testDriverLock(privconn); - privpool = virStoragePoolObjFindByName(&privconn->pools, - pool->name); - testDriverUnlock(privconn); - - if (privpool == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name))) goto cleanup; - } if (!virStoragePoolObjIsActive(privpool)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -4826,16 +4748,8 @@ testStoragePoolListVolumes(virStoragePoolPtr pool, memset(names, 0, maxnames * sizeof(*names)); - testDriverLock(privconn); - privpool = virStoragePoolObjFindByName(&privconn->pools, - pool->name); - testDriverUnlock(privconn); - - if (privpool == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name))) goto cleanup; - } - if (!virStoragePoolObjIsActive(privpool)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -4938,16 +4852,8 @@ testStorageVolLookupByName(virStoragePoolPtr pool, virStorageVolDefPtr privvol; virStorageVolPtr ret = NULL; - testDriverLock(privconn); - privpool = virStoragePoolObjFindByName(&privconn->pools, - pool->name); - testDriverUnlock(privconn); - - if (privpool == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name))) goto cleanup; - } - if (!virStoragePoolObjIsActive(privpool)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -5058,15 +4964,8 @@ testStorageVolCreateXML(virStoragePoolPtr pool, virCheckFlags(0, NULL); - testDriverLock(privconn); - privpool = virStoragePoolObjFindByName(&privconn->pools, - pool->name); - testDriverUnlock(privconn); - - if (privpool == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name))) goto cleanup; - } if (!virStoragePoolObjIsActive(privpool)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -5132,15 +5031,8 @@ testStorageVolCreateXMLFrom(virStoragePoolPtr pool, virCheckFlags(0, NULL); - testDriverLock(privconn); - privpool = virStoragePoolObjFindByName(&privconn->pools, - pool->name); - testDriverUnlock(privconn); - - if (privpool == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name))) goto cleanup; - } if (!virStoragePoolObjIsActive(privpool)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -5215,16 +5107,8 @@ testStorageVolDelete(virStorageVolPtr vol, virCheckFlags(0, -1); - testDriverLock(privconn); - privpool = virStoragePoolObjFindByName(&privconn->pools, - vol->pool); - testDriverUnlock(privconn); - - if (privpool == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privpool = testStoragePoolObjFindByName(privconn, vol->pool))) goto cleanup; - } - privvol = virStorageVolDefFindByName(privpool, vol->name); @@ -5285,15 +5169,8 @@ testStorageVolGetInfo(virStorageVolPtr vol, virStorageVolDefPtr privvol; int ret = -1; - testDriverLock(privconn); - privpool = virStoragePoolObjFindByName(&privconn->pools, - vol->pool); - testDriverUnlock(privconn); - - if (privpool == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privpool = testStoragePoolObjFindByName(privconn, vol->pool))) goto cleanup; - } privvol = virStorageVolDefFindByName(privpool, vol->name); @@ -5333,15 +5210,8 @@ testStorageVolGetXMLDesc(virStorageVolPtr vol, virCheckFlags(0, NULL); - testDriverLock(privconn); - privpool = virStoragePoolObjFindByName(&privconn->pools, - vol->pool); - testDriverUnlock(privconn); - - if (privpool == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privpool = testStoragePoolObjFindByName(privconn, vol->pool))) goto cleanup; - } privvol = virStorageVolDefFindByName(privpool, vol->name); @@ -5374,15 +5244,8 @@ testStorageVolGetPath(virStorageVolPtr vol) virStorageVolDefPtr privvol; char *ret = NULL; - testDriverLock(privconn); - privpool = virStoragePoolObjFindByName(&privconn->pools, - vol->pool); - testDriverUnlock(privconn); - - if (privpool == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privpool = testStoragePoolObjFindByName(privconn, vol->pool))) goto cleanup; - } privvol = virStorageVolDefFindByName(privpool, vol->name); @@ -5410,6 +5273,25 @@ testStorageVolGetPath(virStorageVolPtr vol) /* Node device implementations */ +static virNodeDeviceObjPtr +testNodeDeviceObjFindByName(testDriverPtr driver, + const char *name) +{ + virNodeDeviceObjPtr obj; + + testDriverLock(driver); + obj = virNodeDeviceObjFindByName(&driver->devs, name); + testDriverUnlock(driver); + + if (!obj) + virReportError(VIR_ERR_NO_NODE_DEVICE, + _("no node device with matching name '%s'"), + name); + + return obj; +} + + static int testNodeNumOfDevices(virConnectPtr conn, const char *cap, @@ -5475,16 +5357,8 @@ testNodeDeviceLookupByName(virConnectPtr conn, const char *name) virNodeDeviceObjPtr obj; virNodeDevicePtr ret = NULL; - testDriverLock(driver); - obj = virNodeDeviceObjFindByName(&driver->devs, name); - testDriverUnlock(driver); - - if (!obj) { - virReportError(VIR_ERR_NO_NODE_DEVICE, - _("no node device with matching name '%s'"), - name); + if (!(obj = testNodeDeviceObjFindByName(driver, name))) goto cleanup; - } if ((ret = virGetNodeDevice(conn, name))) { if (VIR_STRDUP(ret->parent, obj->def->parent) < 0) @@ -5507,16 +5381,8 @@ testNodeDeviceGetXMLDesc(virNodeDevicePtr dev, virCheckFlags(0, NULL); - testDriverLock(driver); - obj = virNodeDeviceObjFindByName(&driver->devs, dev->name); - testDriverUnlock(driver); - - if (!obj) { - virReportError(VIR_ERR_NO_NODE_DEVICE, - _("no node device with matching name '%s'"), - dev->name); + if (!(obj = testNodeDeviceObjFindByName(driver, dev->name))) goto cleanup; - } ret = virNodeDeviceDefFormat(obj->def); @@ -5533,16 +5399,8 @@ testNodeDeviceGetParent(virNodeDevicePtr dev) virNodeDeviceObjPtr obj; char *ret = NULL; - testDriverLock(driver); - obj = virNodeDeviceObjFindByName(&driver->devs, dev->name); - testDriverUnlock(driver); - - if (!obj) { - virReportError(VIR_ERR_NO_NODE_DEVICE, - _("no node device with matching name '%s'"), - dev->name); + if (!(obj = testNodeDeviceObjFindByName(driver, dev->name))) goto cleanup; - } if (obj->def->parent) { ignore_value(VIR_STRDUP(ret, obj->def->parent)); @@ -5567,16 +5425,8 @@ testNodeDeviceNumOfCaps(virNodeDevicePtr dev) int ncaps = 0; int ret = -1; - testDriverLock(driver); - obj = virNodeDeviceObjFindByName(&driver->devs, dev->name); - testDriverUnlock(driver); - - if (!obj) { - virReportError(VIR_ERR_NO_NODE_DEVICE, - _("no node device with matching name '%s'"), - dev->name); + if (!(obj = testNodeDeviceObjFindByName(driver, dev->name))) goto cleanup; - } for (caps = obj->def->caps; caps; caps = caps->next) ++ncaps; @@ -5598,16 +5448,8 @@ testNodeDeviceListCaps(virNodeDevicePtr dev, char **const names, int maxnames) int ncaps = 0; int ret = -1; - testDriverLock(driver); - obj = virNodeDeviceObjFindByName(&driver->devs, dev->name); - testDriverUnlock(driver); - - if (!obj) { - virReportError(VIR_ERR_NO_NODE_DEVICE, - _("no node device with matching name '%s'"), - dev->name); + if (!(obj = testNodeDeviceObjFindByName(driver, dev->name))) goto cleanup; - } for (caps = obj->def->caps; caps && ncaps < maxnames; caps = caps->next) { if (VIR_STRDUP(names[ncaps++], virNodeDevCapTypeToString(caps->data.type)) < 0) @@ -5771,16 +5613,8 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) char *parent_name = NULL, *wwnn = NULL, *wwpn = NULL; virObjectEventPtr event = NULL; - testDriverLock(driver); - obj = virNodeDeviceObjFindByName(&driver->devs, dev->name); - testDriverUnlock(driver); - - if (!obj) { - virReportError(VIR_ERR_NO_NODE_DEVICE, - _("no node device with matching name '%s'"), - dev->name); + if (!(obj = testNodeDeviceObjFindByName(driver, dev->name))) goto out; - } if (virNodeDeviceGetWWNs(obj->def, &wwnn, &wwpn) == -1) goto out; -- 2.9.3
Rather than have multiple places using the same pattern to find a network by name using virNetworkObjFindByName, create a common helper which will provide a consistent error message as well. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/test/test_driver.c | 57 +++++++++++++++++++++----------------------------- 1 file changed, 24 insertions(+), 33 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index c2536c9..e4402cc 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3236,6 +3236,22 @@ static virNetworkPtr testNetworkLookupByUUID(virConnectPtr conn, return ret; } + +static virNetworkObjPtr +testNetworkObjFindByName(testDriverPtr privconn, + const char *name) +{ + virNetworkObjPtr net; + + if (!(net = virNetworkObjFindByName(privconn->networks, name))) + virReportError(VIR_ERR_NO_NETWORK, + _("no network with matching name '%s'"), + name); + + return net; +} + + static virNetworkPtr testNetworkLookupByName(virConnectPtr conn, const char *name) { @@ -3243,11 +3259,8 @@ static virNetworkPtr testNetworkLookupByName(virConnectPtr conn, virNetworkObjPtr net; virNetworkPtr ret = NULL; - net = virNetworkObjFindByName(privconn->networks, name); - if (net == NULL) { - virReportError(VIR_ERR_NO_NETWORK, NULL); + if (!(net = testNetworkObjFindByName(privconn, name))) goto cleanup; - } ret = virGetNetwork(conn, net->def->name, net->def->uuid); @@ -3411,12 +3424,8 @@ static int testNetworkUndefine(virNetworkPtr network) int ret = -1; virObjectEventPtr event = NULL; - privnet = virNetworkObjFindByName(privconn->networks, network->name); - - if (privnet == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privnet = testNetworkObjFindByName(privconn, network->name))) goto cleanup; - } if (virNetworkObjIsActive(privnet)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -3490,11 +3499,8 @@ static int testNetworkCreate(virNetworkPtr network) int ret = -1; virObjectEventPtr event = NULL; - privnet = virNetworkObjFindByName(privconn->networks, network->name); - if (privnet == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privnet = testNetworkObjFindByName(privconn, network->name))) goto cleanup; - } if (virNetworkObjIsActive(privnet)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -3521,11 +3527,8 @@ static int testNetworkDestroy(virNetworkPtr network) int ret = -1; virObjectEventPtr event = NULL; - privnet = virNetworkObjFindByName(privconn->networks, network->name); - if (privnet == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privnet = testNetworkObjFindByName(privconn, network->name))) goto cleanup; - } privnet->active = 0; event = virNetworkEventLifecycleNew(privnet->def->name, privnet->def->uuid, @@ -3551,11 +3554,8 @@ static char *testNetworkGetXMLDesc(virNetworkPtr network, virCheckFlags(0, NULL); - privnet = virNetworkObjFindByName(privconn->networks, network->name); - if (privnet == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privnet = testNetworkObjFindByName(privconn, network->name))) goto cleanup; - } ret = virNetworkDefFormat(privnet->def, flags); @@ -3569,11 +3569,8 @@ static char *testNetworkGetBridgeName(virNetworkPtr network) { char *bridge = NULL; virNetworkObjPtr privnet; - privnet = virNetworkObjFindByName(privconn->networks, network->name); - if (privnet == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privnet = testNetworkObjFindByName(privconn, network->name))) goto cleanup; - } if (!(privnet->def->bridge)) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -3596,11 +3593,8 @@ static int testNetworkGetAutostart(virNetworkPtr network, virNetworkObjPtr privnet; int ret = -1; - privnet = virNetworkObjFindByName(privconn->networks, network->name); - if (privnet == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privnet = testNetworkObjFindByName(privconn, network->name))) goto cleanup; - } *autostart = privnet->autostart; ret = 0; @@ -3617,11 +3611,8 @@ static int testNetworkSetAutostart(virNetworkPtr network, virNetworkObjPtr privnet; int ret = -1; - privnet = virNetworkObjFindByName(privconn->networks, network->name); - if (privnet == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!(privnet = testNetworkObjFindByName(privconn, network->name))) goto cleanup; - } privnet->autostart = autostart ? 1 : 0; ret = 0; -- 2.9.3
Make common helpers testNetworkObjFindByUUID and testStoragePoolObjFindByUUID which will replace the repeated patter for each to find objects by UUID. As a bonus, the error message processing will also provide the failed uuidstr rather than a generic error message. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/test/test_driver.c | 96 +++++++++++++++++++++++++++----------------------- 1 file changed, 52 insertions(+), 44 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index e4402cc..1c54069 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3216,6 +3216,24 @@ static int testDomainInterfaceStats(virDomainPtr domain, } +static virNetworkObjPtr +testNetworkObjFindByUUID(testDriverPtr privconn, + const unsigned char *uuid) +{ + virNetworkObjPtr net; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + if (!(net = virNetworkObjFindByUUID(privconn->networks, uuid))) { + virUUIDFormat(uuid, uuidstr); + virReportError(VIR_ERR_NO_NETWORK, + _("no network with matching uuid '%s'"), + uuidstr); + } + + return net; +} + + static virNetworkPtr testNetworkLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { @@ -3223,11 +3241,8 @@ static virNetworkPtr testNetworkLookupByUUID(virConnectPtr conn, virNetworkObjPtr net; virNetworkPtr ret = NULL; - net = virNetworkObjFindByUUID(privconn->networks, uuid); - if (net == NULL) { - virReportError(VIR_ERR_NO_NETWORK, NULL); + if (!(net = testNetworkObjFindByUUID(privconn, uuid))) goto cleanup; - } ret = virGetNetwork(conn, net->def->name, net->def->uuid); @@ -3326,11 +3341,9 @@ static int testNetworkIsActive(virNetworkPtr net) virNetworkObjPtr obj; int ret = -1; - obj = virNetworkObjFindByUUID(privconn->networks, net->uuid); - if (!obj) { - virReportError(VIR_ERR_NO_NETWORK, NULL); + if (!(obj = testNetworkObjFindByUUID(privconn, net->uuid))) goto cleanup; - } + ret = virNetworkObjIsActive(obj); cleanup: @@ -3344,11 +3357,9 @@ static int testNetworkIsPersistent(virNetworkPtr net) virNetworkObjPtr obj; int ret = -1; - obj = virNetworkObjFindByUUID(privconn->networks, net->uuid); - if (!obj) { - virReportError(VIR_ERR_NO_NETWORK, NULL); + if (!(obj = testNetworkObjFindByUUID(privconn, net->uuid))) goto cleanup; - } + ret = obj->persistent; cleanup: @@ -3462,12 +3473,8 @@ testNetworkUpdate(virNetworkPtr net, VIR_NETWORK_UPDATE_AFFECT_CONFIG, -1); - network = virNetworkObjFindByUUID(privconn->networks, net->uuid); - if (!network) { - virReportError(VIR_ERR_NO_NETWORK, - "%s", _("no network with matching uuid")); + if (!(network = testNetworkObjFindByUUID(privconn, net->uuid))) goto cleanup; - } /* VIR_NETWORK_UPDATE_AFFECT_CURRENT means "change LIVE if network * is active, else change CONFIG @@ -4049,6 +4056,28 @@ testStoragePoolObjFindByName(testDriverPtr privconn, } +static virStoragePoolObjPtr +testStoragePoolObjFindByUUID(testDriverPtr privconn, + const unsigned char *uuid) +{ + virStoragePoolObjPtr pool; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + testDriverLock(privconn); + pool = virStoragePoolObjFindByUUID(&privconn->pools, uuid); + testDriverUnlock(privconn); + + if (!pool) { + virUUIDFormat(uuid, uuidstr); + virReportError(VIR_ERR_NO_STORAGE_POOL, + _("no storage pool with matching uuid '%s'"), + uuidstr); + } + + return pool; +} + + static virStoragePoolPtr testStoragePoolLookupByUUID(virConnectPtr conn, const unsigned char *uuid) @@ -4057,14 +4086,8 @@ testStoragePoolLookupByUUID(virConnectPtr conn, virStoragePoolObjPtr pool; virStoragePoolPtr ret = NULL; - testDriverLock(privconn); - pool = virStoragePoolObjFindByUUID(&privconn->pools, uuid); - testDriverUnlock(privconn); - - if (pool == NULL) { - virReportError(VIR_ERR_NO_STORAGE_POOL, NULL); + if (!(pool = testStoragePoolObjFindByUUID(privconn, uuid))) goto cleanup; - } ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid, NULL, NULL); @@ -4222,13 +4245,9 @@ static int testStoragePoolIsActive(virStoragePoolPtr pool) virStoragePoolObjPtr obj; int ret = -1; - testDriverLock(privconn); - obj = virStoragePoolObjFindByUUID(&privconn->pools, pool->uuid); - testDriverUnlock(privconn); - if (!obj) { - virReportError(VIR_ERR_NO_STORAGE_POOL, NULL); + if (!(obj = testStoragePoolObjFindByUUID(privconn, pool->uuid))) goto cleanup; - } + ret = virStoragePoolObjIsActive(obj); cleanup: @@ -4243,13 +4262,9 @@ static int testStoragePoolIsPersistent(virStoragePoolPtr pool) virStoragePoolObjPtr obj; int ret = -1; - testDriverLock(privconn); - obj = virStoragePoolObjFindByUUID(&privconn->pools, pool->uuid); - testDriverUnlock(privconn); - if (!obj) { - virReportError(VIR_ERR_NO_STORAGE_POOL, NULL); + if (!(obj = testStoragePoolObjFindByUUID(privconn, pool->uuid))) goto cleanup; - } + ret = obj->configFile ? 1 : 0; cleanup: @@ -4781,15 +4796,8 @@ testStoragePoolListAllVolumes(virStoragePoolPtr obj, virCheckFlags(0, -1); - testDriverLock(privconn); - pool = virStoragePoolObjFindByUUID(&privconn->pools, obj->uuid); - testDriverUnlock(privconn); - - if (!pool) { - virReportError(VIR_ERR_NO_STORAGE_POOL, "%s", - _("no storage pool with matching uuid")); + if (!(pool = testStoragePoolObjFindByUUID(privconn, obj->uuid))) goto cleanup; - } if (!virStoragePoolObjIsActive(pool)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", -- 2.9.3
Obviously something caused truncation of my subject... Should be "Clean test_driver calls" or something similar. John On 03/04/2017 10:00 AM, John Ferlan wrote:
More "fallout" from the RFC series to make a common pool object:
http://www.redhat.com/archives/libvir-list/2017-February/msg00519.html
were changes made along the way in the test_driver code to "commonalize" the various vir*ObjFindBy{Name|UUID} callers. This series will do the same for the existing code.
Not even attempted was making test_driver to have updated coding style (maybe another day for that)!
John Ferlan (3): test: Make common test*ObjFindByName helpers test: Make a common testNetworkObjFindByName test: Make common test*ObjFindByUUID helpers
src/test/test_driver.c | 499 ++++++++++++++++--------------------------------- 1 file changed, 166 insertions(+), 333 deletions(-)
On 04.03.2017 16:00, John Ferlan wrote:
More "fallout" from the RFC series to make a common pool object:
http://www.redhat.com/archives/libvir-list/2017-February/msg00519.html
were changes made along the way in the test_driver code to "commonalize" the various vir*ObjFindBy{Name|UUID} callers. This series will do the same for the existing code.
Not even attempted was making test_driver to have updated coding style (maybe another day for that)!
John Ferlan (3): test: Make common test*ObjFindByName helpers test: Make a common testNetworkObjFindByName test: Make common test*ObjFindByUUID helpers
src/test/test_driver.c | 499 ++++++++++++++++--------------------------------- 1 file changed, 166 insertions(+), 333 deletions(-)
Nice cleanup. ACK series. Michal
participants (2)
-
John Ferlan -
Michal Privoznik