6:56 a.m.
v1: https://www.redhat.com/archives/libvir-list/2017-December/msg00543.html
Changes since v1...
* Added a patch to handle a NULL return with pool obj lock
* Alter the IsDuplicate API to use a bool parameter
* Use the IsDuplicate API from the test driver. This would have generated
the correct error message about a duplicate UUID instead of the Duplicate
key that was generated. Ran virt-manager tests prior to Cole's fixes and
of course after.
John Ferlan (3):
conf: Need to unlock pools on object allocation failure
conf: Use bool for @check_active parameter
test: Use virStoragePoolObjIsDuplicate for storage define/create
src/conf/virstorageobj.c | 4 ++--
src/conf/virstorageobj.h | 2 +-
src/storage/storage_driver.c | 4 ++--
src/test/test_driver.c | 11 ++++-------
4 files changed, 9 insertions(+), 12 deletions(-)
--
2.13.6
6:56 a.m.
New subject: [libvirt] [PATCH v2 1/3] conf: Need to unlock pools on object allocation failure
The RW pool could be left unlocked if allocation fails.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 49fe24b28..19903b6c5 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -748,7 +748,7 @@ virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
}
if (!(obj = virStoragePoolObjNew()))
- return NULL;
+ goto error;
virUUIDFormat(def->uuid, uuidstr);
if (virHashAddEntry(pools->objs, uuidstr, obj) < 0)
--
2.13.6
6:56 a.m.
New subject: [libvirt] [PATCH v2 2/3] conf: Use bool for @check_active parameter
Use a bool as that's how the variable is used in the function.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.c | 2 +-
src/conf/virstorageobj.h | 2 +-
src/storage/storage_driver.c | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 19903b6c5..1eaa53423 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -1133,7 +1133,7 @@ virStoragePoolObjGetNames(virStoragePoolObjListPtr pools,
int
virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def,
- unsigned int check_active)
+ bool check_active)
{
int ret = -1;
virStoragePoolObjPtr obj = NULL;
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index 4f372386a..dd7001c4b 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -247,7 +247,7 @@ virStoragePoolObjRemove(virStoragePoolObjListPtr pools,
int
virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def,
- unsigned int check_active);
+ bool check_active);
int
virStoragePoolObjSourceFindDuplicate(virConnectPtr conn,
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index f86087fb0..f590f6b9b 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -656,7 +656,7 @@ storagePoolCreateXML(virConnectPtr conn,
if (virStoragePoolCreateXMLEnsureACL(conn, newDef) < 0)
goto cleanup;
- if (virStoragePoolObjIsDuplicate(driver->pools, newDef, 1) < 0)
+ if (virStoragePoolObjIsDuplicate(driver->pools, newDef, true) < 0)
goto cleanup;
if (virStoragePoolObjSourceFindDuplicate(conn, driver->pools, newDef) < 0)
@@ -751,7 +751,7 @@ storagePoolDefineXML(virConnectPtr conn,
if (virStoragePoolDefineXMLEnsureACL(conn, newDef) < 0)
goto cleanup;
- if (virStoragePoolObjIsDuplicate(driver->pools, newDef, 0) < 0)
+ if (virStoragePoolObjIsDuplicate(driver->pools, newDef, false) < 0)
goto cleanup;
if (virStoragePoolObjSourceFindDuplicate(conn, driver->pools, newDef) < 0)
--
2.13.6
6:56 a.m.
New subject: [libvirt] [PATCH v2 3/3] test: Use virStoragePoolObjIsDuplicate for storage define/create
Avoid the chance that there could be a duplicate storage pool UUID
or Name from the test driver storage pool define/create functions.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/test/test_driver.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 8adc2167b..dc743b498 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4440,14 +4440,8 @@ testStoragePoolCreateXML(virConnectPtr conn,
if (!(newDef = virStoragePoolDefParseString(xml)))
goto cleanup;
- obj = virStoragePoolObjFindByUUID(privconn->pools, newDef->uuid);
- if (!obj)
- obj = virStoragePoolObjFindByName(privconn->pools, newDef->name);
- if (obj) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("storage pool already exists"));
+ if (virStoragePoolObjIsDuplicate(privconn->pools, newDef, true) < 0)
goto cleanup;
- }
if (!(obj = virStoragePoolObjAssignDef(privconn->pools, newDef)))
goto cleanup;
@@ -4520,6 +4514,9 @@ testStoragePoolDefineXML(virConnectPtr conn,
newDef->allocation = defaultPoolAlloc;
newDef->available = defaultPoolCap - defaultPoolAlloc;
+ if (virStoragePoolObjIsDuplicate(privconn->pools, newDef, false) < 0)
+ goto cleanup;
+
if (!(obj = virStoragePoolObjAssignDef(privconn->pools, newDef)))
goto cleanup;
newDef = NULL;
--
2.13.6
8:48 a.m.
ping?
Tks -
John
On 12/18/2017 07:56 AM, John Ferlan wrote:
v1:
https://www.redhat.com/archives/libvir-list/2017-December/msg00543.html
Changes since v1...
* Added a patch to handle a NULL return with pool obj lock
* Alter the IsDuplicate API to use a bool parameter
* Use the IsDuplicate API from the test driver. This would have generated
the correct error message about a duplicate UUID instead of the Duplicate
key that was generated. Ran virt-manager tests prior to Cole's fixes and
of course after.
John Ferlan (3):
conf: Need to unlock pools on object allocation failure
conf: Use bool for @check_active parameter
test: Use virStoragePoolObjIsDuplicate for storage define/create
src/conf/virstorageobj.c | 4 ++--
src/conf/virstorageobj.h | 2 +-
src/storage/storage_driver.c | 4 ++--
src/test/test_driver.c | 11 ++++-------
4 files changed, 9 insertions(+), 12 deletions(-)
8:40 a.m.
On Mon, Dec 18, 2017 at 07:56:51AM -0500, John Ferlan wrote:
v1:
https://www.redhat.com/archives/libvir-list/2017-December/msg00543.html
Changes since v1...
* Added a patch to handle a NULL return with pool obj lock
* Alter the IsDuplicate API to use a bool parameter
* Use the IsDuplicate API from the test driver. This would have generated
the correct error message about a duplicate UUID instead of the Duplicate
key that was generated. Ran virt-manager tests prior to Cole's fixes and
of course after.
John Ferlan (3):
conf: Need to unlock pools on object allocation failure
conf: Use bool for @check_active parameter
test: Use virStoragePoolObjIsDuplicate for storage define/create
src/conf/virstorageobj.c | 4 ++--
src/conf/virstorageobj.h | 2 +-
src/storage/storage_driver.c | 4 ++--
src/test/test_driver.c | 11 ++++-------
4 files changed, 9 insertions(+), 12 deletions(-)
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
2514
days inactive
2531
days old
6 comments
2 participants
participants (2)
-
Erik Skultety -
John Ferlan