5:02 a.m.
While working through the pool driver, I found a couple of issues in
error paths for the storage driver.
John Ferlan (2):
storage: Fix error path in virStoragePoolObjLoad
storage: Fix error path in storagePoolDefineXML
src/conf/storage_conf.c | 4 ++--
src/storage/storage_driver.c | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)
--
2.5.0
5:02 a.m.
New subject: [libvirt] [PATCH 1/2] storage: Fix error path in virStoragePoolObjLoad
While reviewing how storage driver used ObjListPtr's for reference
in some recent secret driver patches to use the same mechanism, I came
across an instance where the wrong API was called for error paths after
successfully allocating the storage pool pointer and inserting into
the driver pool list.
The path is after virStoragePoolObjAssignDef succeeds - the 'def' passed
in is assigned to pool->def (or newDef) so it shouldn't be the only thing
deleted. The pool is now part of driver->pools.objs, so it would need to
be removed (as happens in the storagePoolCreateXML error paths).
Rather than calling virStoragePoolDefFree to free the def which is now
assigned to the pool, call virStoragePoolObjRemove to ensure the pool
element is removed from the driver list and that anything stored in pool
is properly handled by virStoragePoolObjFree including the call to
virStoragePoolDefFree for the pool->{def|newDef} element.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/storage_conf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 3657dfd..bfba521 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1852,12 +1852,12 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools,
VIR_FREE(pool->configFile); /* for driver reload */
if (VIR_STRDUP(pool->configFile, path) < 0) {
- virStoragePoolDefFree(def);
+ virStoragePoolObjRemove(pools, pool);
return NULL;
}
VIR_FREE(pool->autostartLink); /* for driver reload */
if (VIR_STRDUP(pool->autostartLink, autostartLink) < 0) {
- virStoragePoolDefFree(def);
+ virStoragePoolObjRemove(pools, pool);
return NULL;
}
--
2.5.0
5:02 a.m.
New subject: [libvirt] [PATCH 2/2] storage: Fix error path in storagePoolDefineXML
Found by inspection - after calling virStoragePoolObjAssignDef the
pool is part of the driver->pools.objs list and the failure path
for the virStoragePoolObjSaveDef will use virStoragePoolObjRemove
to remove the pool from the objs list which will unlock and free
the pool pointer (as pools->objs[i] during the loop). Since the call
doesn't clear the pool address from the callee, we need to set it
to NULL; otherwise, the virStoragePoolObjUnlock in the cleanup: code
will fail miserably.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_driver.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index e0ded01..1d96618 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -784,6 +784,7 @@ storagePoolDefineXML(virConnectPtr conn,
if (virStoragePoolObjSaveDef(driver, pool, def) < 0) {
virStoragePoolObjRemove(&driver->pools, pool);
def = NULL;
+ pool = NULL;
goto cleanup;
}
def = NULL;
--
2.5.0
8:13 p.m.
On Thu, Feb 25, 2016 at 04:02:39PM -0500, John Ferlan wrote:
While working through the pool driver, I found a couple of issues in
error paths for the storage driver.
John Ferlan (2):
storage: Fix error path in virStoragePoolObjLoad
storage: Fix error path in storagePoolDefineXML
src/conf/storage_conf.c | 4 ++--
src/storage/storage_driver.c | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)
ACK to both.
Jan
3116
days inactive
3117
days old
3 comments
2 participants
participants (2)
-
John Ferlan -
Ján Tomko