提交 c4ff1a18 编写于 作者: J John Ferlan

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: NJohn Ferlan <jferlan@redhat.com>
上级 87e50c9c
...@@ -4550,7 +4550,6 @@ testDestroyVport(testDriverPtr privconn, ...@@ -4550,7 +4550,6 @@ testDestroyVport(testDriverPtr privconn,
const char *wwnn ATTRIBUTE_UNUSED, const char *wwnn ATTRIBUTE_UNUSED,
const char *wwpn ATTRIBUTE_UNUSED) const char *wwpn ATTRIBUTE_UNUSED)
{ {
int ret = -1;
virNodeDeviceObjPtr obj = NULL; virNodeDeviceObjPtr obj = NULL;
virObjectEventPtr event = NULL; virObjectEventPtr event = NULL;
...@@ -4564,7 +4563,7 @@ testDestroyVport(testDriverPtr privconn, ...@@ -4564,7 +4563,7 @@ testDestroyVport(testDriverPtr privconn,
if (!(obj = virNodeDeviceObjFindByName(&privconn->devs, "scsi_host12"))) { if (!(obj = virNodeDeviceObjFindByName(&privconn->devs, "scsi_host12"))) {
virReportError(VIR_ERR_NO_NODE_DEVICE, "%s", virReportError(VIR_ERR_NO_NODE_DEVICE, "%s",
_("no node device with matching name 'scsi_host12'")); _("no node device with matching name 'scsi_host12'"));
goto cleanup; return -1;
} }
event = virNodeDeviceEventLifecycleNew("scsi_host12", event = virNodeDeviceEventLifecycleNew("scsi_host12",
...@@ -4573,15 +4572,9 @@ testDestroyVport(testDriverPtr privconn, ...@@ -4573,15 +4572,9 @@ testDestroyVport(testDriverPtr privconn,
virNodeDeviceObjRemove(&privconn->devs, obj); virNodeDeviceObjRemove(&privconn->devs, obj);
virNodeDeviceObjFree(obj); virNodeDeviceObjFree(obj);
obj = NULL;
ret = 0;
cleanup:
if (obj)
virNodeDeviceObjUnlock(obj);
testObjectEventQueue(privconn, event); testObjectEventQueue(privconn, event);
return ret; return 0;
} }
...@@ -4594,7 +4587,7 @@ testStoragePoolDestroy(virStoragePoolPtr pool) ...@@ -4594,7 +4587,7 @@ testStoragePoolDestroy(virStoragePoolPtr pool)
virObjectEventPtr event = NULL; virObjectEventPtr event = NULL;
if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name))) if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name)))
goto cleanup; return -1;
if (!virStoragePoolObjIsActive(privpool)) { if (!virStoragePoolObjIsActive(privpool)) {
virReportError(VIR_ERR_OPERATION_INVALID, virReportError(VIR_ERR_OPERATION_INVALID,
...@@ -5369,7 +5362,7 @@ testNodeDeviceLookupByName(virConnectPtr conn, const char *name) ...@@ -5369,7 +5362,7 @@ testNodeDeviceLookupByName(virConnectPtr conn, const char *name)
virNodeDevicePtr ret = NULL; virNodeDevicePtr ret = NULL;
if (!(obj = testNodeDeviceObjFindByName(driver, name))) if (!(obj = testNodeDeviceObjFindByName(driver, name)))
goto cleanup; return NULL;
def = virNodeDeviceObjGetDef(obj); def = virNodeDeviceObjGetDef(obj);
if ((ret = virGetNodeDevice(conn, name))) { if ((ret = virGetNodeDevice(conn, name))) {
...@@ -5379,8 +5372,6 @@ testNodeDeviceLookupByName(virConnectPtr conn, const char *name) ...@@ -5379,8 +5372,6 @@ testNodeDeviceLookupByName(virConnectPtr conn, const char *name)
} }
} }
cleanup:
if (obj)
virNodeDeviceObjUnlock(obj); virNodeDeviceObjUnlock(obj);
return ret; return ret;
} }
...@@ -5396,12 +5387,10 @@ testNodeDeviceGetXMLDesc(virNodeDevicePtr dev, ...@@ -5396,12 +5387,10 @@ testNodeDeviceGetXMLDesc(virNodeDevicePtr dev,
virCheckFlags(0, NULL); virCheckFlags(0, NULL);
if (!(obj = testNodeDeviceObjFindByName(driver, dev->name))) if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
goto cleanup; return NULL;
ret = virNodeDeviceDefFormat(virNodeDeviceObjGetDef(obj)); ret = virNodeDeviceDefFormat(virNodeDeviceObjGetDef(obj));
cleanup:
if (obj)
virNodeDeviceObjUnlock(obj); virNodeDeviceObjUnlock(obj);
return ret; return ret;
} }
...@@ -5415,7 +5404,7 @@ testNodeDeviceGetParent(virNodeDevicePtr dev) ...@@ -5415,7 +5404,7 @@ testNodeDeviceGetParent(virNodeDevicePtr dev)
char *ret = NULL; char *ret = NULL;
if (!(obj = testNodeDeviceObjFindByName(driver, dev->name))) if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
goto cleanup; return NULL;
def = virNodeDeviceObjGetDef(obj); def = virNodeDeviceObjGetDef(obj);
if (def->parent) { if (def->parent) {
...@@ -5425,8 +5414,6 @@ testNodeDeviceGetParent(virNodeDevicePtr dev) ...@@ -5425,8 +5414,6 @@ testNodeDeviceGetParent(virNodeDevicePtr dev)
"%s", _("no parent for this device")); "%s", _("no parent for this device"));
} }
cleanup:
if (obj)
virNodeDeviceObjUnlock(obj); virNodeDeviceObjUnlock(obj);
return ret; return ret;
} }
...@@ -5440,20 +5427,16 @@ testNodeDeviceNumOfCaps(virNodeDevicePtr dev) ...@@ -5440,20 +5427,16 @@ testNodeDeviceNumOfCaps(virNodeDevicePtr dev)
virNodeDeviceDefPtr def; virNodeDeviceDefPtr def;
virNodeDevCapsDefPtr caps; virNodeDevCapsDefPtr caps;
int ncaps = 0; int ncaps = 0;
int ret = -1;
if (!(obj = testNodeDeviceObjFindByName(driver, dev->name))) if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
goto cleanup; return -1;
def = virNodeDeviceObjGetDef(obj); def = virNodeDeviceObjGetDef(obj);
for (caps = def->caps; caps; caps = caps->next) for (caps = def->caps; caps; caps = caps->next)
++ncaps; ++ncaps;
ret = ncaps;
cleanup:
if (obj)
virNodeDeviceObjUnlock(obj); virNodeDeviceObjUnlock(obj);
return ret; return ncaps;
} }
...@@ -5465,27 +5448,26 @@ testNodeDeviceListCaps(virNodeDevicePtr dev, char **const names, int maxnames) ...@@ -5465,27 +5448,26 @@ testNodeDeviceListCaps(virNodeDevicePtr dev, char **const names, int maxnames)
virNodeDeviceDefPtr def; virNodeDeviceDefPtr def;
virNodeDevCapsDefPtr caps; virNodeDevCapsDefPtr caps;
int ncaps = 0; int ncaps = 0;
int ret = -1;
if (!(obj = testNodeDeviceObjFindByName(driver, dev->name))) if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
goto cleanup; return -1;
def = virNodeDeviceObjGetDef(obj); def = virNodeDeviceObjGetDef(obj);
for (caps = def->caps; caps && ncaps < maxnames; caps = caps->next) { for (caps = def->caps; caps && ncaps < maxnames; caps = caps->next) {
if (VIR_STRDUP(names[ncaps++], virNodeDevCapTypeToString(caps->data.type)) < 0) if (VIR_STRDUP(names[ncaps],
goto cleanup; virNodeDevCapTypeToString(caps->data.type)) < 0)
goto error;
ncaps++;
} }
ret = ncaps;
cleanup:
if (obj)
virNodeDeviceObjUnlock(obj); virNodeDeviceObjUnlock(obj);
if (ret == -1) { return ncaps;
--ncaps;
error:
while (--ncaps >= 0) while (--ncaps >= 0)
VIR_FREE(names[ncaps]); VIR_FREE(names[ncaps]);
} virNodeDeviceObjUnlock(obj);
return ret; return -1;
} }
...@@ -5639,14 +5621,14 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) ...@@ -5639,14 +5621,14 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
virObjectEventPtr event = NULL; virObjectEventPtr event = NULL;
if (!(obj = testNodeDeviceObjFindByName(driver, dev->name))) if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
goto out; return -1;
def = virNodeDeviceObjGetDef(obj); def = virNodeDeviceObjGetDef(obj);
if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) == -1) if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) == -1)
goto out; goto cleanup;
if (VIR_STRDUP(parent_name, def->parent) < 0) if (VIR_STRDUP(parent_name, def->parent) < 0)
goto out; goto cleanup;
/* virNodeDeviceGetParentHost will cause the device object's lock to be /* virNodeDeviceGetParentHost will cause the device object's lock to be
* taken, so we have to dup the parent's name and drop the lock * taken, so we have to dup the parent's name and drop the lock
...@@ -5659,7 +5641,7 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) ...@@ -5659,7 +5641,7 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
if (virNodeDeviceObjGetParentHost(&driver->devs, def, if (virNodeDeviceObjGetParentHost(&driver->devs, def,
EXISTING_DEVICE) < 0) { EXISTING_DEVICE) < 0) {
obj = NULL; obj = NULL;
goto out; goto cleanup;
} }
event = virNodeDeviceEventLifecycleNew(dev->name, event = virNodeDeviceEventLifecycleNew(dev->name,
...@@ -5671,7 +5653,7 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) ...@@ -5671,7 +5653,7 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
virNodeDeviceObjFree(obj); virNodeDeviceObjFree(obj);
obj = NULL; obj = NULL;
out: cleanup:
if (obj) if (obj)
virNodeDeviceObjUnlock(obj); virNodeDeviceObjUnlock(obj);
testObjectEventQueue(driver, event); testObjectEventQueue(driver, event);
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册