1
0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-25 01:34:11 +03:00

test: Adjust cleanup/error paths for nodedev test APIs

- In testDestroyVport rather than use a cleanup label, just return -1
   immediately since nothing else is needed.

 - In testStoragePoolDestroy, if !privpool, then just return -1 since
   nothing else will happen anyway.

 - Rather than "goto cleanup;" on failure to virNodeDeviceObjFindByName
   an @obj, just return directly.  This then allows the cleanup: label code
   to not have to check "if (obj)" before calling virNodeDeviceObjUnlock.
   This also simplifies some exit logic...

 - In testNodeDeviceObjFindByName use an error: label to handle the failure
   and don't do the ncaps++ within the VIR_STRDUP() source target index.
   Only increment ncaps after success. Easier on eyes at error label too.

 - In testNodeDeviceDestroy use "cleanup" rather than "out" for the goto

Signed-off-by: John Ferlan <jferlan@redhat.com>
This commit is contained in:
John Ferlan 2017-05-12 11:08:57 -04:00
parent 87e50c9cea
commit c4ff1a1825

View File

@ -4550,7 +4550,6 @@ testDestroyVport(testDriverPtr privconn,
const char *wwnn ATTRIBUTE_UNUSED,
const char *wwpn ATTRIBUTE_UNUSED)
{
int ret = -1;
virNodeDeviceObjPtr obj = NULL;
virObjectEventPtr event = NULL;
@ -4564,7 +4563,7 @@ testDestroyVport(testDriverPtr privconn,
if (!(obj = virNodeDeviceObjFindByName(&privconn->devs, "scsi_host12"))) {
virReportError(VIR_ERR_NO_NODE_DEVICE, "%s",
_("no node device with matching name 'scsi_host12'"));
goto cleanup;
return -1;
}
event = virNodeDeviceEventLifecycleNew("scsi_host12",
@ -4573,15 +4572,9 @@ testDestroyVport(testDriverPtr privconn,
virNodeDeviceObjRemove(&privconn->devs, obj);
virNodeDeviceObjFree(obj);
obj = NULL;
ret = 0;
cleanup:
if (obj)
virNodeDeviceObjUnlock(obj);
testObjectEventQueue(privconn, event);
return ret;
return 0;
}
@ -4594,7 +4587,7 @@ testStoragePoolDestroy(virStoragePoolPtr pool)
virObjectEventPtr event = NULL;
if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name)))
goto cleanup;
return -1;
if (!virStoragePoolObjIsActive(privpool)) {
virReportError(VIR_ERR_OPERATION_INVALID,
@ -5369,7 +5362,7 @@ testNodeDeviceLookupByName(virConnectPtr conn, const char *name)
virNodeDevicePtr ret = NULL;
if (!(obj = testNodeDeviceObjFindByName(driver, name)))
goto cleanup;
return NULL;
def = virNodeDeviceObjGetDef(obj);
if ((ret = virGetNodeDevice(conn, name))) {
@ -5379,9 +5372,7 @@ testNodeDeviceLookupByName(virConnectPtr conn, const char *name)
}
}
cleanup:
if (obj)
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjUnlock(obj);
return ret;
}
@ -5396,13 +5387,11 @@ testNodeDeviceGetXMLDesc(virNodeDevicePtr dev,
virCheckFlags(0, NULL);
if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
goto cleanup;
return NULL;
ret = virNodeDeviceDefFormat(virNodeDeviceObjGetDef(obj));
cleanup:
if (obj)
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjUnlock(obj);
return ret;
}
@ -5415,7 +5404,7 @@ testNodeDeviceGetParent(virNodeDevicePtr dev)
char *ret = NULL;
if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
goto cleanup;
return NULL;
def = virNodeDeviceObjGetDef(obj);
if (def->parent) {
@ -5425,9 +5414,7 @@ testNodeDeviceGetParent(virNodeDevicePtr dev)
"%s", _("no parent for this device"));
}
cleanup:
if (obj)
virNodeDeviceObjUnlock(obj);
virNodeDeviceObjUnlock(obj);
return ret;
}
@ -5440,20 +5427,16 @@ testNodeDeviceNumOfCaps(virNodeDevicePtr dev)
virNodeDeviceDefPtr def;
virNodeDevCapsDefPtr caps;
int ncaps = 0;
int ret = -1;
if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
goto cleanup;
return -1;
def = virNodeDeviceObjGetDef(obj);
for (caps = def->caps; caps; caps = caps->next)
++ncaps;
ret = ncaps;
cleanup:
if (obj)
virNodeDeviceObjUnlock(obj);
return ret;
virNodeDeviceObjUnlock(obj);
return ncaps;
}
@ -5465,27 +5448,26 @@ testNodeDeviceListCaps(virNodeDevicePtr dev, char **const names, int maxnames)
virNodeDeviceDefPtr def;
virNodeDevCapsDefPtr caps;
int ncaps = 0;
int ret = -1;
if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
goto cleanup;
return -1;
def = virNodeDeviceObjGetDef(obj);
for (caps = def->caps; caps && ncaps < maxnames; caps = caps->next) {
if (VIR_STRDUP(names[ncaps++], virNodeDevCapTypeToString(caps->data.type)) < 0)
goto cleanup;
if (VIR_STRDUP(names[ncaps],
virNodeDevCapTypeToString(caps->data.type)) < 0)
goto error;
ncaps++;
}
ret = ncaps;
cleanup:
if (obj)
virNodeDeviceObjUnlock(obj);
if (ret == -1) {
--ncaps;
while (--ncaps >= 0)
VIR_FREE(names[ncaps]);
}
return ret;
virNodeDeviceObjUnlock(obj);
return ncaps;
error:
while (--ncaps >= 0)
VIR_FREE(names[ncaps]);
virNodeDeviceObjUnlock(obj);
return -1;
}
@ -5639,14 +5621,14 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
virObjectEventPtr event = NULL;
if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
goto out;
return -1;
def = virNodeDeviceObjGetDef(obj);
if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) == -1)
goto out;
goto cleanup;
if (VIR_STRDUP(parent_name, def->parent) < 0)
goto out;
goto cleanup;
/* virNodeDeviceGetParentHost will cause the device object's lock to be
* taken, so we have to dup the parent's name and drop the lock
@ -5659,7 +5641,7 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
if (virNodeDeviceObjGetParentHost(&driver->devs, def,
EXISTING_DEVICE) < 0) {
obj = NULL;
goto out;
goto cleanup;
}
event = virNodeDeviceEventLifecycleNew(dev->name,
@ -5671,7 +5653,7 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
virNodeDeviceObjFree(obj);
obj = NULL;
out:
cleanup:
if (obj)
virNodeDeviceObjUnlock(obj);
testObjectEventQueue(driver, event);