8:08 a.m.
Consider this round 1 of 2.... The next series will be 18 patches,
but the majority of those deal with change every {pool|obj}->def->X
to use the accessor virStoragePoolObjGetDef.
v1: https://www.redhat.com/archives/libvir-list/2017-May/msg00218.html
Probably not even worth looking at the v1, but this picks up where v1
left off somewhere around patch 10, but adding smaller steps between
patches.
John Ferlan (12):
storage: Create accessor API's for virStoragePoolObj
storage: Introduce virStoragePoolObjNew
storage: Fill in storage pool @active properly
storage: Introduce storage volume add, delete, count APIs
storage: Introduce APIs to search/scan storage pool volumes list
storage: Use virStoragePoolObj{Get|Set}ConfigFile
storage: Use virStoragePoolObjGetAutostartLink
storage: Use virStoragePoolObj{Is|Set}Active
storage: Use virStoragePoolObj{Is|Set}Autostart
storage: Internally represent @autostart to bool
storage: Use virStoragePoolObj{Get|Incr}Decr}Asyncjobs
storage: Use virStoragePoolObjDefUseNewDef
src/conf/virstorageobj.c | 211 +++++++++++++++++++++++++++++++--
src/conf/virstorageobj.h | 84 ++++++++++++-
src/libvirt_private.syms | 20 ++++
src/storage/storage_backend_disk.c | 93 +++++++++------
src/storage/storage_backend_gluster.c | 5 +-
src/storage/storage_backend_logical.c | 4 +-
src/storage/storage_backend_mpath.c | 3 +-
src/storage/storage_backend_rbd.c | 4 +-
src/storage/storage_backend_scsi.c | 4 +-
src/storage/storage_backend_sheepdog.c | 4 +-
src/storage/storage_backend_zfs.c | 6 +-
src/storage/storage_driver.c | 142 +++++++++-------------
src/storage/storage_util.c | 8 +-
src/test/test_driver.c | 54 ++++-----
tests/storagevolxml2argvtest.c | 20 ++--
15 files changed, 465 insertions(+), 197 deletions(-)
--
2.9.5
8:08 a.m.
New subject: [libvirt] [PATCH v2 01/12] storage: Create accessor API's for virStoragePoolObj
In preparation for making a private object, create accessor API's for
consumer storage functions to use:
virStoragePoolObjGetDef
virStoragePoolObjSetDef
virStoragePoolObjGetNewDef
virStoragePoolObjDefUseNewDef
virStoragePoolObjGetConfigFile
virStoragePoolObjSetConfigFile
virStoragePoolObjGetAutostartLink
virStoragePoolObjIsActive
virStoragePoolObjSetActive
virStoragePoolObjIsAutostart
virStoragePoolObjSetAutostart
virStoragePoolObjGetAsyncjobs
virStoragePoolObjIncrAsyncjobs
virStoragePoolObjDecrAsyncjobs
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++
src/conf/virstorageobj.h | 50 +++++++++++++++++++---
src/libvirt_private.syms | 14 ++++++
3 files changed, 168 insertions(+), 5 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index e83044b..ebda9fe 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -37,6 +37,115 @@
VIR_LOG_INIT("conf.virstorageobj");
+virStoragePoolDefPtr
+virStoragePoolObjGetDef(virStoragePoolObjPtr obj)
+{
+ return obj->def;
+}
+
+
+void
+virStoragePoolObjSetDef(virStoragePoolObjPtr obj,
+ virStoragePoolDefPtr def)
+{
+ virStoragePoolDefFree(obj->def);
+ obj->def = def;
+}
+
+
+virStoragePoolDefPtr
+virStoragePoolObjGetNewDef(virStoragePoolObjPtr obj)
+{
+ return obj->newDef;
+}
+
+
+void
+virStoragePoolObjDefUseNewDef(virStoragePoolObjPtr obj)
+{
+ virStoragePoolDefFree(obj->def);
+ obj->def = obj->newDef;
+ obj->newDef = NULL;
+}
+
+
+char *
+virStoragePoolObjGetConfigFile(virStoragePoolObjPtr obj)
+{
+ return obj->configFile;
+}
+
+
+void
+virStoragePoolObjSetConfigFile(virStoragePoolObjPtr obj,
+ char *configFile)
+{
+ VIR_FREE(obj->configFile);
+ obj->configFile = configFile;
+}
+
+
+char *
+virStoragePoolObjGetAutostartLink(virStoragePoolObjPtr obj)
+{
+ return obj->autostartLink;
+}
+
+
+bool
+virStoragePoolObjIsActive(virStoragePoolObjPtr obj)
+{
+ return obj->active;
+}
+
+
+void
+virStoragePoolObjSetActive(virStoragePoolObjPtr obj,
+ bool active)
+{
+ obj->active = active;
+}
+
+
+bool
+virStoragePoolObjIsAutostart(virStoragePoolObjPtr obj)
+{
+ if (!obj->configFile)
+ return 0;
+
+ return obj->autostart == 1;
+}
+
+
+void
+virStoragePoolObjSetAutostart(virStoragePoolObjPtr obj,
+ int autostart)
+{
+ obj->autostart = autostart;
+}
+
+
+unsigned int
+virStoragePoolObjGetAsyncjobs(virStoragePoolObjPtr obj)
+{
+ return obj->asyncjobs;
+}
+
+
+void
+virStoragePoolObjIncrAsyncjobs(virStoragePoolObjPtr obj)
+{
+ obj->asyncjobs++;
+}
+
+
+void
+virStoragePoolObjDecrAsyncjobs(virStoragePoolObjPtr obj)
+{
+ obj->asyncjobs--;
+}
+
+
void
virStoragePoolObjFree(virStoragePoolObjPtr obj)
{
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index ac51b5f..a5ed8f8 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -70,11 +70,51 @@ typedef bool
(*virStoragePoolObjListFilter)(virConnectPtr conn,
virStoragePoolDefPtr def);
-static inline int
-virStoragePoolObjIsActive(virStoragePoolObjPtr obj)
-{
- return obj->active;
-}
+virStoragePoolDefPtr
+virStoragePoolObjGetDef(virStoragePoolObjPtr obj);
+
+void
+virStoragePoolObjSetDef(virStoragePoolObjPtr obj,
+ virStoragePoolDefPtr def);
+
+virStoragePoolDefPtr
+virStoragePoolObjGetNewDef(virStoragePoolObjPtr obj);
+
+void
+virStoragePoolObjDefUseNewDef(virStoragePoolObjPtr obj);
+
+char *
+virStoragePoolObjGetConfigFile(virStoragePoolObjPtr obj);
+
+void
+virStoragePoolObjSetConfigFile(virStoragePoolObjPtr obj,
+ char *configFile);
+
+char *
+virStoragePoolObjGetAutostartLink(virStoragePoolObjPtr obj);
+
+bool
+virStoragePoolObjIsActive(virStoragePoolObjPtr obj);
+
+void
+virStoragePoolObjSetActive(virStoragePoolObjPtr obj,
+ bool active);
+
+bool
+virStoragePoolObjIsAutostart(virStoragePoolObjPtr obj);
+
+void
+virStoragePoolObjSetAutostart(virStoragePoolObjPtr obj,
+ int autostart);
+
+unsigned int
+virStoragePoolObjGetAsyncjobs(virStoragePoolObjPtr obj);
+
+void
+virStoragePoolObjIncrAsyncjobs(virStoragePoolObjPtr obj);
+
+void
+virStoragePoolObjDecrAsyncjobs(virStoragePoolObjPtr obj);
int
virStoragePoolObjLoadAllConfigs(virStoragePoolObjListPtr pools,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2149b11..8bb1cbc 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1051,10 +1051,20 @@ virSecretObjSetValueSize;
# conf/virstorageobj.h
virStoragePoolObjAssignDef;
virStoragePoolObjClearVols;
+virStoragePoolObjDecrAsyncjobs;
+virStoragePoolObjDefUseNewDef;
virStoragePoolObjDeleteDef;
virStoragePoolObjFindByName;
virStoragePoolObjFindByUUID;
+virStoragePoolObjGetAsyncjobs;
+virStoragePoolObjGetAutostartLink;
+virStoragePoolObjGetConfigFile;
+virStoragePoolObjGetDef;
virStoragePoolObjGetNames;
+virStoragePoolObjGetNewDef;
+virStoragePoolObjIncrAsyncjobs;
+virStoragePoolObjIsActive;
+virStoragePoolObjIsAutostart;
virStoragePoolObjIsDuplicate;
virStoragePoolObjListExport;
virStoragePoolObjListFree;
@@ -1065,6 +1075,10 @@ virStoragePoolObjNumOfStoragePools;
virStoragePoolObjNumOfVolumes;
virStoragePoolObjRemove;
virStoragePoolObjSaveDef;
+virStoragePoolObjSetActive;
+virStoragePoolObjSetAutostart;
+virStoragePoolObjSetConfigFile;
+virStoragePoolObjSetDef;
virStoragePoolObjSourceFindDuplicate;
virStoragePoolObjUnlock;
virStoragePoolObjVolumeGetNames;
--
2.9.5
8:08 a.m.
New subject: [libvirt] [PATCH v2 02/12] storage: Introduce virStoragePoolObjNew
Create/use a helper to perform object allocation.
Adjust storagevolxml2argvtest.c in order to use the allocator and
setting of the obj->def.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.c | 31 +++++++++++++++++++++----------
src/conf/virstorageobj.h | 3 +++
src/libvirt_private.syms | 1 +
tests/storagevolxml2argvtest.c | 20 +++++++++++---------
4 files changed, 36 insertions(+), 19 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index ebda9fe..eb7664c 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -37,6 +37,26 @@
VIR_LOG_INIT("conf.virstorageobj");
+virStoragePoolObjPtr
+virStoragePoolObjNew(void)
+{
+ virStoragePoolObjPtr obj;
+
+ if (VIR_ALLOC(obj) < 0)
+ return NULL;
+
+ if (virMutexInit(&obj->lock) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot initialize mutex"));
+ VIR_FREE(obj);
+ return NULL;
+ }
+ virStoragePoolObjLock(obj);
+ obj->active = 0;
+ return obj;
+}
+
+
virStoragePoolDefPtr
virStoragePoolObjGetDef(virStoragePoolObjPtr obj)
{
@@ -421,17 +441,8 @@ virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
return obj;
}
- if (VIR_ALLOC(obj) < 0)
- return NULL;
-
- if (virMutexInit(&obj->lock) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("cannot initialize mutex"));
- VIR_FREE(obj);
+ if (!(obj = virStoragePoolObjNew()))
return NULL;
- }
- virStoragePoolObjLock(obj);
- obj->active = 0;
if (VIR_APPEND_ELEMENT_COPY(pools->objs, pools->count, obj) < 0) {
virStoragePoolObjUnlock(obj);
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index a5ed8f8..401c4d5 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -70,6 +70,9 @@ typedef bool
(*virStoragePoolObjListFilter)(virConnectPtr conn,
virStoragePoolDefPtr def);
+virStoragePoolObjPtr
+virStoragePoolObjNew(void);
+
virStoragePoolDefPtr
virStoragePoolObjGetDef(virStoragePoolObjPtr obj);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8bb1cbc..fc884fa 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1071,6 +1071,7 @@ virStoragePoolObjListFree;
virStoragePoolObjLoadAllConfigs;
virStoragePoolObjLoadAllState;
virStoragePoolObjLock;
+virStoragePoolObjNew;
virStoragePoolObjNumOfStoragePools;
virStoragePoolObjNumOfVolumes;
virStoragePoolObjRemove;
diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c
index 9e43204..2ee3dba 100644
--- a/tests/storagevolxml2argvtest.c
+++ b/tests/storagevolxml2argvtest.c
@@ -50,18 +50,19 @@ testCompareXMLToArgvFiles(bool shouldFail,
virConnectPtr conn;
virStorageVolDefPtr vol = NULL, inputvol = NULL;
- virStoragePoolDefPtr pool = NULL;
+ virStoragePoolDefPtr def = NULL;
virStoragePoolDefPtr inputpool = NULL;
- virStoragePoolObj poolobj = {.def = NULL };
-
+ virStoragePoolObjPtr obj = NULL;
if (!(conn = virGetConnect()))
goto cleanup;
- if (!(pool = virStoragePoolDefParseFile(poolxml)))
+ if (!(def = virStoragePoolDefParseFile(poolxml)))
goto cleanup;
- poolobj.def = pool;
+ if (!(obj = virStoragePoolObjNew()))
+ goto cleanup;
+ virStoragePoolObjSetDef(obj, def);
if (inputpoolxml) {
if (!(inputpool = virStoragePoolDefParseFile(inputpoolxml)))
@@ -71,17 +72,17 @@ testCompareXMLToArgvFiles(bool shouldFail,
if (inputvolxml)
parse_flags |= VIR_VOL_XML_PARSE_NO_CAPACITY;
- if (!(vol = virStorageVolDefParseFile(pool, volxml, parse_flags)))
+ if (!(vol = virStorageVolDefParseFile(def, volxml, parse_flags)))
goto cleanup;
if (inputvolxml &&
!(inputvol = virStorageVolDefParseFile(inputpool, inputvolxml, 0)))
goto cleanup;
- testSetVolumeType(vol, pool);
+ testSetVolumeType(vol, def);
testSetVolumeType(inputvol, inputpool);
- cmd = virStorageBackendCreateQemuImgCmdFromVol(conn, &poolobj, vol,
+ cmd = virStorageBackendCreateQemuImgCmdFromVol(conn, obj, vol,
inputvol, flags,
create_tool, imgformat,
NULL);
@@ -102,12 +103,13 @@ testCompareXMLToArgvFiles(bool shouldFail,
ret = 0;
cleanup:
- virStoragePoolDefFree(pool);
+ virStoragePoolDefFree(def);
virStoragePoolDefFree(inputpool);
virStorageVolDefFree(vol);
virStorageVolDefFree(inputvol);
virCommandFree(cmd);
VIR_FREE(actualCmdline);
+ virStoragePoolObjUnlock(obj);
virObjectUnref(conn);
return ret;
}
--
2.9.5
1:39 a.m.
New subject: [libvirt] [PATCH v2 02/12] storage: Introduce virStoragePoolObjNew
On 08/24/2017 03:08 PM, John Ferlan wrote:
Create/use a helper to perform object allocation.
Adjust storagevolxml2argvtest.c in order to use the allocator and
setting of the obj->def.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.c | 31 +++++++++++++++++++++----------
src/conf/virstorageobj.h | 3 +++
src/libvirt_private.syms | 1 +
tests/storagevolxml2argvtest.c | 20 +++++++++++---------
4 files changed, 36 insertions(+), 19 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index ebda9fe..eb7664c 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -37,6 +37,26 @@
VIR_LOG_INIT("conf.virstorageobj");
+virStoragePoolObjPtr
+virStoragePoolObjNew(void)
+{
+ virStoragePoolObjPtr obj;
+
+ if (VIR_ALLOC(obj) < 0)
+ return NULL;
+
+ if (virMutexInit(&obj->lock) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot initialize mutex"));
+ VIR_FREE(obj);
+ return NULL;
+ }
+ virStoragePoolObjLock(obj);
+ obj->active = 0;
+ return obj;
+}
+
+
virStoragePoolDefPtr
virStoragePoolObjGetDef(virStoragePoolObjPtr obj)
{
@@ -421,17 +441,8 @@ virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
return obj;
}
- if (VIR_ALLOC(obj) < 0)
- return NULL;
-
- if (virMutexInit(&obj->lock) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("cannot initialize mutex"));
- VIR_FREE(obj);
+ if (!(obj = virStoragePoolObjNew()))
return NULL;
- }
- virStoragePoolObjLock(obj);
- obj->active = 0;
if (VIR_APPEND_ELEMENT_COPY(pools->objs, pools->count, obj) < 0) {
virStoragePoolObjUnlock(obj);
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index a5ed8f8..401c4d5 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -70,6 +70,9 @@ typedef bool
(*virStoragePoolObjListFilter)(virConnectPtr conn,
virStoragePoolDefPtr def);
+virStoragePoolObjPtr
+virStoragePoolObjNew(void);
+
virStoragePoolDefPtr
virStoragePoolObjGetDef(virStoragePoolObjPtr obj);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8bb1cbc..fc884fa 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1071,6 +1071,7 @@ virStoragePoolObjListFree;
virStoragePoolObjLoadAllConfigs;
virStoragePoolObjLoadAllState;
virStoragePoolObjLock;
+virStoragePoolObjNew;
virStoragePoolObjNumOfStoragePools;
virStoragePoolObjNumOfVolumes;
virStoragePoolObjRemove;
diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c
index 9e43204..2ee3dba 100644
--- a/tests/storagevolxml2argvtest.c
+++ b/tests/storagevolxml2argvtest.c
@@ -50,18 +50,19 @@ testCompareXMLToArgvFiles(bool shouldFail,
virConnectPtr conn;
virStorageVolDefPtr vol = NULL, inputvol = NULL;
- virStoragePoolDefPtr pool = NULL;
+ virStoragePoolDefPtr def = NULL;
virStoragePoolDefPtr inputpool = NULL;
- virStoragePoolObj poolobj = {.def = NULL };
-
+ virStoragePoolObjPtr obj = NULL;
if (!(conn = virGetConnect()))
goto cleanup;
- if (!(pool = virStoragePoolDefParseFile(poolxml)))
+ if (!(def = virStoragePoolDefParseFile(poolxml)))
goto cleanup;
- poolobj.def = pool;
+ if (!(obj = virStoragePoolObjNew()))
+ goto cleanup;
+ virStoragePoolObjSetDef(obj, def);
if (inputpoolxml) {
if (!(inputpool = virStoragePoolDefParseFile(inputpoolxml)))
@@ -71,17 +72,17 @@ testCompareXMLToArgvFiles(bool shouldFail,
if (inputvolxml)
parse_flags |= VIR_VOL_XML_PARSE_NO_CAPACITY;
- if (!(vol = virStorageVolDefParseFile(pool, volxml, parse_flags)))
+ if (!(vol = virStorageVolDefParseFile(def, volxml, parse_flags)))
goto cleanup;
if (inputvolxml &&
!(inputvol = virStorageVolDefParseFile(inputpool, inputvolxml, 0)))
goto cleanup;
- testSetVolumeType(vol, pool);
+ testSetVolumeType(vol, def);
testSetVolumeType(inputvol, inputpool);
- cmd = virStorageBackendCreateQemuImgCmdFromVol(conn, &poolobj, vol,
+ cmd = virStorageBackendCreateQemuImgCmdFromVol(conn, obj, vol,
inputvol, flags,
create_tool, imgformat,
NULL);
@@ -102,12 +103,13 @@ testCompareXMLToArgvFiles(bool shouldFail,
ret = 0;
cleanup:
- virStoragePoolDefFree(pool);
+ virStoragePoolDefFree(def);
virStoragePoolDefFree(inputpool);
virStorageVolDefFree(vol);
virStorageVolDefFree(inputvol);
virCommandFree(cmd);
VIR_FREE(actualCmdline);
+ virStoragePoolObjUnlock(obj);
Almost. Firstly, @def is now part of @obj so it shouldn't be freed
separately. Secondly, @obj is leaked. ACK if you squash this in:
diff --git i/tests/Makefile.am w/tests/Makefile.am
index 1f8c4cd42..813888575 100644
--- i/tests/Makefile.am
+++ w/tests/Makefile.am
@@ -885,7 +885,10 @@ storagevolxml2argvtest_SOURCES = \
testutils.c testutils.h
storagevolxml2argvtest_LDADD = \
$(LIBXML_LIBS) \
- ../src/libvirt_driver_storage_impl.la $(LDADDS)
+ ../src/libvirt_driver_storage_impl.la \
+ ../src/libvirt_conf.la \
+ ../src/libvirt_util.la \
+ $(LDADDS)
else ! WITH_STORAGE
EXTRA_DIST += storagevolxml2argvtest.c
diff --git i/tests/storagevolxml2argvtest.c w/tests/storagevolxml2argvtest.c
index 2ee3dbad9..1b3003216 100644
--- i/tests/storagevolxml2argvtest.c
+++ w/tests/storagevolxml2argvtest.c
@@ -60,8 +60,10 @@ testCompareXMLToArgvFiles(bool shouldFail,
if (!(def = virStoragePoolDefParseFile(poolxml)))
goto cleanup;
- if (!(obj = virStoragePoolObjNew()))
+ if (!(obj = virStoragePoolObjNew())) {
+ virStoragePoolDefFree(def);
goto cleanup;
+ }
virStoragePoolObjSetDef(obj, def);
if (inputpoolxml) {
@@ -103,13 +105,13 @@ testCompareXMLToArgvFiles(bool shouldFail,
ret = 0;
cleanup:
- virStoragePoolDefFree(def);
virStoragePoolDefFree(inputpool);
virStorageVolDefFree(vol);
virStorageVolDefFree(inputvol);
virCommandFree(cmd);
VIR_FREE(actualCmdline);
virStoragePoolObjUnlock(obj);
+ virStoragePoolObjFree(obj);
virObjectUnref(conn);
return ret;
}
Alternatively, you can leave the virStoragePoolDefFree() call as is and
just set @def = NULL right after virStoragePoolObjSetDef(). I'll leave it
up to you which one you prefer.
Michal
8:08 a.m.
New subject: [libvirt] [PATCH v2 03/12] storage: Fill in storage pool @active properly
It's a bool not an int, so use true/false and not 1/0
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.c | 4 ++--
src/test/test_driver.c | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index eb7664c..a9fa190 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -52,7 +52,7 @@ virStoragePoolObjNew(void)
return NULL;
}
virStoragePoolObjLock(obj);
- obj->active = 0;
+ obj->active = false;
return obj;
}
@@ -544,7 +544,7 @@ virStoragePoolObjLoadState(virStoragePoolObjListPtr pools,
* as active
*/
- obj->active = 1;
+ obj->active = true;
cleanup:
VIR_FREE(stateFile);
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index aa38f54..989c3a8 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1130,7 +1130,7 @@ testParseStorage(testDriverPtr privconn,
virStoragePoolObjUnlock(obj);
goto error;
}
- obj->active = 1;
+ obj->active = true;
/* Find storage volumes */
if (testOpenVolumesForPool(file, ctxt, obj, i+1) < 0) {
@@ -4322,7 +4322,7 @@ testStoragePoolCreate(virStoragePoolPtr pool,
if (!(obj = testStoragePoolObjFindInactiveByName(privconn, pool->name)))
return -1;
- obj->active = 1;
+ obj->active = true;
event = virStoragePoolEventLifecycleNew(pool->name, pool->uuid,
VIR_STORAGE_POOL_EVENT_STARTED,
@@ -4470,7 +4470,7 @@ testStoragePoolCreateXML(virConnectPtr conn,
* code will not Remove the pool */
VIR_FREE(obj->configFile);
- obj->active = 1;
+ obj->active = true;
event = virStoragePoolEventLifecycleNew(obj->def->name, obj->def->uuid,
VIR_STORAGE_POOL_EVENT_STARTED,
@@ -4618,7 +4618,7 @@ testStoragePoolDestroy(virStoragePoolPtr pool)
if (!(obj = testStoragePoolObjFindActiveByName(privconn, pool->name)))
return -1;
- obj->active = 0;
+ obj->active = false;
if (obj->def->source.adapter.type ==
VIR_STORAGE_ADAPTER_TYPE_FC_HOST) {
--
2.9.5
8:08 a.m.
New subject: [libvirt] [PATCH v2 04/12] storage: Introduce storage volume add, delete, count APIs
Create/use virStoragePoolObjAddVol in order to add volumes onto list.
Create/use virStoragePoolObjRemoveVol in order to remove volumes from list.
Create/use virStoragePoolObjGetVolumesCount to get count of volumes on list.
For the storage driver, the logic alters when the volumes.obj list grows
to after we've fetched the volobj. This is an optimization of sorts, but
also doesn't "needlessly" grow the volumes.objs list and then just decr
the count if the virGetStorageVol fails.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.c | 37 ++++++++++++++++++++++++
src/conf/virstorageobj.h | 11 +++++++
src/libvirt_private.syms | 3 ++
src/storage/storage_backend_disk.c | 5 ++--
src/storage/storage_backend_gluster.c | 3 +-
src/storage/storage_backend_logical.c | 4 +--
src/storage/storage_backend_mpath.c | 3 +-
src/storage/storage_backend_rbd.c | 4 +--
src/storage/storage_backend_sheepdog.c | 4 +--
src/storage/storage_backend_zfs.c | 6 ++--
src/storage/storage_driver.c | 53 +++++++++-------------------------
src/storage/storage_util.c | 8 ++---
src/test/test_driver.c | 18 ++++--------
13 files changed, 85 insertions(+), 74 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index a9fa190..912c27a 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -282,6 +282,43 @@ virStoragePoolObjClearVols(virStoragePoolObjPtr obj)
}
+int
+virStoragePoolObjAddVol(virStoragePoolObjPtr obj,
+ virStorageVolDefPtr voldef)
+{
+ if (VIR_APPEND_ELEMENT(obj->volumes.objs, obj->volumes.count, voldef) < 0)
+ return -1;
+ return 0;
+}
+
+
+void
+virStoragePoolObjRemoveVol(virStoragePoolObjPtr obj,
+ virStorageVolDefPtr voldef)
+{
+ virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj);
+ size_t i;
+
+ for (i = 0; i < obj->volumes.count; i++) {
+ if (obj->volumes.objs[i] == voldef) {
+ VIR_INFO("Deleting volume '%s' from storage pool
'%s'",
+ voldef->name, def->name);
+ virStorageVolDefFree(voldef);
+
+ VIR_DELETE_ELEMENT(obj->volumes.objs, i, obj->volumes.count);
+ return;
+ }
+ }
+}
+
+
+size_t
+virStoragePoolObjGetVolumesCount(virStoragePoolObjPtr obj)
+{
+ return obj->volumes.count;
+}
+
+
virStorageVolDefPtr
virStorageVolDefFindByKey(virStoragePoolObjPtr obj,
const char *key)
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index 401c4d5..d1a1247 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -136,6 +136,17 @@ virStoragePoolObjPtr
virStoragePoolObjFindByName(virStoragePoolObjListPtr pools,
const char *name);
+int
+virStoragePoolObjAddVol(virStoragePoolObjPtr obj,
+ virStorageVolDefPtr voldef);
+
+void
+virStoragePoolObjRemoveVol(virStoragePoolObjPtr obj,
+ virStorageVolDefPtr voldef);
+
+size_t
+virStoragePoolObjGetVolumesCount(virStoragePoolObjPtr obj);
+
virStorageVolDefPtr
virStorageVolDefFindByKey(virStoragePoolObjPtr obj,
const char *key);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fc884fa..a35299b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1049,6 +1049,7 @@ virSecretObjSetValueSize;
# conf/virstorageobj.h
+virStoragePoolObjAddVol;
virStoragePoolObjAssignDef;
virStoragePoolObjClearVols;
virStoragePoolObjDecrAsyncjobs;
@@ -1062,6 +1063,7 @@ virStoragePoolObjGetConfigFile;
virStoragePoolObjGetDef;
virStoragePoolObjGetNames;
virStoragePoolObjGetNewDef;
+virStoragePoolObjGetVolumesCount;
virStoragePoolObjIncrAsyncjobs;
virStoragePoolObjIsActive;
virStoragePoolObjIsAutostart;
@@ -1075,6 +1077,7 @@ virStoragePoolObjNew;
virStoragePoolObjNumOfStoragePools;
virStoragePoolObjNumOfVolumes;
virStoragePoolObjRemove;
+virStoragePoolObjRemoveVol;
virStoragePoolObjSaveDef;
virStoragePoolObjSetActive;
virStoragePoolObjSetAutostart;
diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index e8f67bb..0bf5567 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -65,8 +65,7 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool,
if (VIR_ALLOC(vol) < 0)
return -1;
if (VIR_STRDUP(vol->name, partname) < 0 ||
- VIR_APPEND_ELEMENT_COPY(pool->volumes.objs,
- pool->volumes.count, vol) < 0) {
+ virStoragePoolObjAddVol(pool, vol) < 0) {
virStorageVolDefFree(vol);
return -1;
}
@@ -595,7 +594,7 @@ virStorageBackendDiskPartFormat(virStoragePoolObjPtr pool,
break;
}
}
- if (i == pool->volumes.count) {
+ if (i == virStoragePoolObjGetVolumesCount(pool)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("no extended partition found
and no primary partition available"));
return -1;
diff --git a/src/storage/storage_backend_gluster.c
b/src/storage/storage_backend_gluster.c
index 92038c1..90f31b0 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -386,8 +386,7 @@ virStorageBackendGlusterRefreshPool(virConnectPtr conn
ATTRIBUTE_UNUSED,
if (okay < 0)
goto cleanup;
- if (vol && VIR_APPEND_ELEMENT(pool->volumes.objs,
pool->volumes.count,
- vol) < 0)
+ if (vol && virStoragePoolObjAddVol(pool, vol) < 0)
goto cleanup;
}
if (errno) {
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index 67f70e5..7bfe211 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -356,9 +356,9 @@ virStorageBackendLogicalMakeVol(char **const groups,
if (virStorageBackendLogicalParseVolExtents(vol, groups) < 0)
goto cleanup;
- if (is_new_vol &&
- VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) < 0)
+ if (is_new_vol && virStoragePoolObjAddVol(pool, vol) < 0)
goto cleanup;
+ vol = NULL;
ret = 0;
diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c
index 434a477..46818ef 100644
--- a/src/storage/storage_backend_mpath.c
+++ b/src/storage/storage_backend_mpath.c
@@ -71,8 +71,9 @@ virStorageBackendMpathNewVol(virStoragePoolObjPtr pool,
if (VIR_STRDUP(vol->key, vol->target.path) < 0)
goto cleanup;
- if (VIR_APPEND_ELEMENT_COPY(pool->volumes.objs, pool->volumes.count, vol) <
0)
+ if (virStoragePoolObjAddVol(pool, vol) < 0)
goto cleanup;
+
pool->def->capacity += vol->target.capacity;
pool->def->allocation += vol->target.allocation;
ret = 0;
diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
index 7b8887b..6731677 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -506,7 +506,7 @@ virStorageBackendRBDRefreshPool(virConnectPtr conn,
goto cleanup;
}
- if (VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) <
0) {
+ if (virStoragePoolObjAddVol(pool, vol) < 0) {
virStorageVolDefFree(vol);
virStoragePoolObjClearVols(pool);
goto cleanup;
@@ -514,7 +514,7 @@ virStorageBackendRBDRefreshPool(virConnectPtr conn,
}
VIR_DEBUG("Found %zu images in RBD pool %s",
- pool->volumes.count, pool->def->source.name);
+ virStoragePoolObjGetVolumesCount(pool), pool->def->source.name);
ret = 0;
diff --git a/src/storage/storage_backend_sheepdog.c
b/src/storage/storage_backend_sheepdog.c
index b55d96a..e72dcda 100644
--- a/src/storage/storage_backend_sheepdog.c
+++ b/src/storage/storage_backend_sheepdog.c
@@ -130,11 +130,9 @@ virStorageBackendSheepdogAddVolume(virConnectPtr conn
ATTRIBUTE_UNUSED,
if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0)
goto error;
- if (VIR_EXPAND_N(pool->volumes.objs, pool->volumes.count, 1) < 0)
+ if (virStoragePoolObjAddVol(pool, vol) < 0)
goto error;
- pool->volumes.objs[pool->volumes.count - 1] = vol;
-
return 0;
error:
diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backend_zfs.c
index c6dae71..c266281 100644
--- a/src/storage/storage_backend_zfs.c
+++ b/src/storage/storage_backend_zfs.c
@@ -161,11 +161,9 @@ virStorageBackendZFSParseVol(virStoragePoolObjPtr pool,
if (volume->target.allocation < volume->target.capacity)
volume->target.sparse = true;
- if (is_new_vol &&
- VIR_APPEND_ELEMENT(pool->volumes.objs,
- pool->volumes.count,
- volume) < 0)
+ if (is_new_vol && virStoragePoolObjAddVol(pool, volume) < 0)
goto cleanup;
+ volume = NULL;
ret = 0;
cleanup:
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 8552120..b780f9a 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1626,25 +1626,6 @@ storagePoolLookupByTargetPath(virConnectPtr conn,
}
-static void
-storageVolRemoveFromPool(virStoragePoolObjPtr obj,
- virStorageVolDefPtr voldef)
-{
- size_t i;
-
- for (i = 0; i < obj->volumes.count; i++) {
- if (obj->volumes.objs[i] == voldef) {
- VIR_INFO("Deleting volume '%s' from storage pool
'%s'",
- voldef->name, obj->def->name);
- virStorageVolDefFree(voldef);
-
- VIR_DELETE_ELEMENT(obj->volumes.objs, i, obj->volumes.count);
- break;
- }
- }
-}
-
-
static int
storageVolDeleteInternal(virStorageVolPtr vol,
virStorageBackendPtr backend,
@@ -1676,7 +1657,7 @@ storageVolDeleteInternal(virStorageVolPtr vol,
}
}
- storageVolRemoveFromPool(obj, voldef);
+ virStoragePoolObjRemoveVol(obj, voldef);
ret = 0;
cleanup:
@@ -1815,24 +1796,19 @@ storageVolCreateXML(virStoragePoolPtr pool,
goto cleanup;
}
- if (VIR_REALLOC_N(obj->volumes.objs,
- obj->volumes.count + 1) < 0)
- goto cleanup;
-
/* Wipe any key the user may have suggested, as volume creation
* will generate the canonical key. */
VIR_FREE(voldef->key);
if (backend->createVol(pool->conn, obj, voldef) < 0)
goto cleanup;
- obj->volumes.objs[obj->volumes.count++] = voldef;
- newvol = virGetStorageVol(pool->conn, obj->def->name, voldef->name,
- voldef->key, NULL, NULL);
- if (!newvol) {
- obj->volumes.count--;
+ if (!(newvol = virGetStorageVol(pool->conn, obj->def->name,
voldef->name,
+ voldef->key, NULL, NULL)))
goto cleanup;
- }
+ /* NB: Upon success voldef "owned" by storage pool for deletion purposes
*/
+ if (virStoragePoolObjAddVol(obj, voldef) < 0)
+ goto cleanup;
if (backend->buildVol) {
int buildret;
@@ -1867,7 +1843,7 @@ storageVolCreateXML(virStoragePoolPtr pool,
if (buildret < 0) {
/* buildVol handles deleting volume on failure */
- storageVolRemoveFromPool(obj, voldef);
+ virStoragePoolObjRemoveVol(obj, voldef);
voldef = NULL;
goto cleanup;
}
@@ -2018,9 +1994,6 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool,
backend->refreshVol(pool->conn, obj, voldefsrc) < 0)
goto cleanup;
- if (VIR_REALLOC_N(obj->volumes.objs, obj->volumes.count + 1) < 0)
- goto cleanup;
-
/* 'Define' the new volume so we get async progress reporting.
* Wipe any key the user may have suggested, as volume creation
* will generate the canonical key. */
@@ -2037,13 +2010,13 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool,
memcpy(shadowvol, voldef, sizeof(*voldef));
- obj->volumes.objs[obj->volumes.count++] = voldef;
- newvol = virGetStorageVol(pool->conn, obj->def->name, voldef->name,
- voldef->key, NULL, NULL);
- if (!newvol) {
- obj->volumes.count--;
+ if (!(newvol = virGetStorageVol(pool->conn, obj->def->name,
voldef->name,
+ voldef->key, NULL, NULL)))
+ goto cleanup;
+
+ /* NB: Upon success voldef "owned" by storage pool for deletion purposes
*/
+ if (virStoragePoolObjAddVol(obj, voldef) < 0)
goto cleanup;
- }
/* Drop the pool lock during volume allocation */
obj->asyncjobs++;
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index e1fe162..3efa181 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -3597,13 +3597,13 @@ virStorageBackendRefreshLocal(virConnectPtr conn
ATTRIBUTE_UNUSED,
* An error message was raised, but we just continue. */
}
- if (VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) <
0)
+ if (virStoragePoolObjAddVol(pool, vol) < 0)
goto cleanup;
+ vol = NULL;
}
if (direrr < 0)
goto cleanup;
VIR_DIR_CLOSE(dir);
- vol = NULL;
if (VIR_ALLOC(target))
goto cleanup;
@@ -3785,10 +3785,10 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
pool->def->capacity += vol->target.capacity;
pool->def->allocation += vol->target.allocation;
- if (VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) < 0)
+ if (virStoragePoolObjAddVol(pool, vol) < 0)
goto cleanup;
-
vol = NULL;
+
retval = 0;
cleanup:
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 989c3a8..67b115f 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1079,7 +1079,8 @@ testOpenVolumesForPool(const char *file,
if (!def->key && VIR_STRDUP(def->key, def->target.path) < 0)
goto error;
- if (VIR_APPEND_ELEMENT_COPY(obj->volumes.objs, obj->volumes.count, def)
< 0)
+
+ if (virStoragePoolObjAddVol(obj, def) < 0)
goto error;
obj->def->allocation += def->target.allocation;
@@ -4995,8 +4996,7 @@ testStorageVolCreateXML(virStoragePoolPtr pool,
goto cleanup;
if (VIR_STRDUP(privvol->key, privvol->target.path) < 0 ||
- VIR_APPEND_ELEMENT_COPY(obj->volumes.objs,
- obj->volumes.count, privvol) < 0)
+ virStoragePoolObjAddVol(obj, privvol) < 0)
goto cleanup;
obj->def->allocation += privvol->target.allocation;
@@ -5063,8 +5063,7 @@ testStorageVolCreateXMLFrom(virStoragePoolPtr pool,
goto cleanup;
if (VIR_STRDUP(privvol->key, privvol->target.path) < 0 ||
- VIR_APPEND_ELEMENT_COPY(obj->volumes.objs,
- obj->volumes.count, privvol) < 0)
+ virStoragePoolObjAddVol(obj, privvol) < 0)
goto cleanup;
obj->def->allocation += privvol->target.allocation;
@@ -5089,7 +5088,6 @@ testStorageVolDelete(virStorageVolPtr vol,
testDriverPtr privconn = vol->conn->privateData;
virStoragePoolObjPtr obj;
virStorageVolDefPtr privvol;
- size_t i;
int ret = -1;
virCheckFlags(0, -1);
@@ -5103,14 +5101,8 @@ testStorageVolDelete(virStorageVolPtr vol,
obj->def->allocation -= privvol->target.allocation;
obj->def->available = (obj->def->capacity - obj->def->allocation);
- for (i = 0; i < obj->volumes.count; i++) {
- if (obj->volumes.objs[i] == privvol) {
- virStorageVolDefFree(privvol);
+ virStoragePoolObjRemoveVol(obj, privvol);
- VIR_DELETE_ELEMENT(obj->volumes.objs, i, obj->volumes.count);
- break;
- }
- }
ret = 0;
cleanup:
--
2.9.5
8:08 a.m.
New subject: [libvirt] [PATCH v2 05/12] storage: Introduce APIs to search/scan storage pool volumes list
Introduce virStoragePoolObjForEachVolume to scan each volume
calling the passed callback function until all volumes have been
processed in the storage pool volume list, unless the callback
function returns an error.
Introduce virStoragePoolObjSearchVolume to search each volume
calling the passed callback function until it returns true
indicating that the desired volume was found.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.c | 32 ++++++++++++++
src/conf/virstorageobj.h | 18 ++++++++
src/libvirt_private.syms | 2 +
src/storage/storage_backend_disk.c | 90 +++++++++++++++++++++++---------------
4 files changed, 107 insertions(+), 35 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 912c27a..8ee40e2 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -319,6 +319,38 @@ virStoragePoolObjGetVolumesCount(virStoragePoolObjPtr obj)
}
+int
+virStoragePoolObjForEachVolume(virStoragePoolObjPtr obj,
+ virStorageVolObjListIterator iter,
+ const void *opaque)
+{
+ size_t i;
+
+ for (i = 0; i < obj->volumes.count; i++) {
+ if (iter(obj->volumes.objs[i], opaque) < 0)
+ return -1;
+ }
+
+ return 0;
+}
+
+
+virStorageVolDefPtr
+virStoragePoolObjSearchVolume(virStoragePoolObjPtr obj,
+ virStorageVolObjListSearcher iter,
+ const void *opaque)
+{
+ size_t i;
+
+ for (i = 0; i < obj->volumes.count; i++) {
+ if (iter(obj->volumes.objs[i], opaque))
+ return obj->volumes.objs[i];
+ }
+
+ return NULL;
+}
+
+
virStorageVolDefPtr
virStorageVolDefFindByKey(virStoragePoolObjPtr obj,
const char *key)
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index d1a1247..c2f3f23 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -147,6 +147,24 @@ virStoragePoolObjRemoveVol(virStoragePoolObjPtr obj,
size_t
virStoragePoolObjGetVolumesCount(virStoragePoolObjPtr obj);
+typedef int
+(*virStorageVolObjListIterator)(virStorageVolDefPtr voldef,
+ const void *opaque);
+
+int
+virStoragePoolObjForEachVolume(virStoragePoolObjPtr obj,
+ virStorageVolObjListIterator iter,
+ const void *opaque);
+
+typedef bool
+(*virStorageVolObjListSearcher)(virStorageVolDefPtr voldef,
+ const void *opaque);
+
+virStorageVolDefPtr
+virStoragePoolObjSearchVolume(virStoragePoolObjPtr obj,
+ virStorageVolObjListSearcher iter,
+ const void *opaque);
+
virStorageVolDefPtr
virStorageVolDefFindByKey(virStoragePoolObjPtr obj,
const char *key);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a35299b..67b531f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1057,6 +1057,7 @@ virStoragePoolObjDefUseNewDef;
virStoragePoolObjDeleteDef;
virStoragePoolObjFindByName;
virStoragePoolObjFindByUUID;
+virStoragePoolObjForEachVolume;
virStoragePoolObjGetAsyncjobs;
virStoragePoolObjGetAutostartLink;
virStoragePoolObjGetConfigFile;
@@ -1079,6 +1080,7 @@ virStoragePoolObjNumOfVolumes;
virStoragePoolObjRemove;
virStoragePoolObjRemoveVol;
virStoragePoolObjSaveDef;
+virStoragePoolObjSearchVolume;
virStoragePoolObjSetActive;
virStoragePoolObjSetAutostart;
virStoragePoolObjSetConfigFile;
diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index 0bf5567..dfe9938 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -43,6 +43,17 @@ VIR_LOG_INIT("storage.storage_backend_disk");
#define SECTOR_SIZE 512
+static bool
+virStorageVolPartFindExtended(virStorageVolDefPtr def,
+ const void *opaque ATTRIBUTE_UNUSED)
+{
+ if (def->source.partType == VIR_STORAGE_VOL_DISK_TYPE_EXTENDED)
+ return true;
+
+ return false;
+}
+
+
static int
virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool,
char **const groups,
@@ -191,16 +202,13 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool,
/* Find the extended partition and increase the allocation value */
if (vol->source.partType == VIR_STORAGE_VOL_DISK_TYPE_LOGICAL) {
- size_t i;
+ virStorageVolDefPtr voldef;
- for (i = 0; i < pool->volumes.count; i++) {
- if (pool->volumes.objs[i]->source.partType ==
- VIR_STORAGE_VOL_DISK_TYPE_EXTENDED) {
- pool->volumes.objs[i]->target.allocation +=
- vol->target.allocation;
- break;
- }
- }
+ voldef = virStoragePoolObjSearchVolume(pool,
+ virStorageVolPartFindExtended,
+ NULL);
+ if (voldef)
+ voldef->target.allocation += vol->target.allocation;
}
if (STRNEQ(groups[2], "metadata"))
@@ -521,6 +529,25 @@ virStorageBackendDiskBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED,
return ret;
}
+
+struct virStorageVolNumData {
+ int count;
+};
+
+static int
+virStorageVolNumOfPartTypes(virStorageVolDefPtr def,
+ const void *opaque)
+{
+ struct virStorageVolNumData *data = (struct virStorageVolNumData *)opaque;
+
+ if (def->source.partType == VIR_STORAGE_VOL_DISK_TYPE_PRIMARY ||
+ def->source.partType == VIR_STORAGE_VOL_DISK_TYPE_EXTENDED)
+ data->count++;
+
+ return 0;
+}
+
+
/**
* Decides what kind of partition type that should be created.
* Important when the partition table is of msdos type
@@ -528,19 +555,16 @@ virStorageBackendDiskBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED,
static int
virStorageBackendDiskPartTypeToCreate(virStoragePoolObjPtr pool)
{
+ struct virStorageVolNumData data = { .count = 0 };
+
if (pool->def->source.format == VIR_STORAGE_POOL_DISK_DOS) {
/* count primary and extended partitions,
can't be more than 3 to create a new primary partition */
- size_t i;
- int count = 0;
- for (i = 0; i < pool->volumes.count; i++) {
- int partType = pool->volumes.objs[i]->source.partType;
- if (partType == VIR_STORAGE_VOL_DISK_TYPE_PRIMARY ||
- partType == VIR_STORAGE_VOL_DISK_TYPE_EXTENDED)
- count++;
+ if (virStoragePoolObjForEachVolume(pool, virStorageVolNumOfPartTypes,
+ &data) == 0) {
+ if (data.count >= 4)
+ return VIR_STORAGE_VOL_DISK_TYPE_LOGICAL;
}
- if (count >= 4)
- return VIR_STORAGE_VOL_DISK_TYPE_LOGICAL;
}
/* for all other cases, all partitions are primary */
@@ -552,7 +576,6 @@ virStorageBackendDiskPartFormat(virStoragePoolObjPtr pool,
virStorageVolDefPtr vol,
char** partFormat)
{
- size_t i;
if (pool->def->source.format == VIR_STORAGE_POOL_DISK_DOS) {
const char *partedFormat;
partedFormat = virStoragePartedFsTypeToString(vol->target.format);
@@ -563,13 +586,12 @@ virStorageBackendDiskPartFormat(virStoragePoolObjPtr pool,
}
if (vol->target.format == VIR_STORAGE_VOL_DISK_EXTENDED) {
/* make sure we don't have an extended partition already */
- for (i = 0; i < pool->volumes.count; i++) {
- if (pool->volumes.objs[i]->source.partType ==
- VIR_STORAGE_VOL_DISK_TYPE_EXTENDED) {
+ if (virStoragePoolObjSearchVolume(pool,
+ virStorageVolPartFindExtended,
+ NULL)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("extended partition already exists"));
return -1;
- }
}
if (VIR_STRDUP(*partFormat, partedFormat) < 0)
return -1;
@@ -585,18 +607,16 @@ virStorageBackendDiskPartFormat(virStoragePoolObjPtr pool,
break;
case VIR_STORAGE_VOL_DISK_TYPE_LOGICAL:
/* make sure we have an extended partition */
- for (i = 0; i < pool->volumes.count; i++) {
- if (pool->volumes.objs[i]->source.partType ==
- VIR_STORAGE_VOL_DISK_TYPE_EXTENDED) {
- if (virAsprintf(partFormat, "logical %s",
- partedFormat) < 0)
- return -1;
- break;
- }
- }
- if (i == virStoragePoolObjGetVolumesCount(pool)) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("no extended partition found
and no primary partition available"));
+ if (virStoragePoolObjSearchVolume(pool,
+ virStorageVolPartFindExtended,
+ NULL)) {
+ if (virAsprintf(partFormat, "logical %s",
+ partedFormat) < 0)
+ return -1;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("no extended partition found and no "
+ "primary partition available"));
return -1;
}
break;
--
2.9.5
8:08 a.m.
New subject: [libvirt] [PATCH v2 06/12] storage: Use virStoragePoolObj{Get|Set}ConfigFile
Use the new accessor APIs for storage_driver and test_driver.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend_scsi.c | 4 +++-
src/storage/storage_driver.c | 14 +++++++-------
src/test/test_driver.c | 20 ++++++++++++--------
3 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index 575e6a6..02fd4b6 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -443,8 +443,10 @@ static int
virStorageBackendSCSIStartPool(virConnectPtr conn,
virStoragePoolObjPtr pool)
{
+ const char *configFile = virStoragePoolObjGetConfigFile(pool);
+
if (pool->def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST)
- return createVport(conn, pool->def, pool->configFile,
+ return createVport(conn, pool->def, configFile,
&pool->def->source.adapter.data.fchost);
return 0;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index b780f9a..7b1396f 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -91,7 +91,7 @@ virStoragePoolUpdateInactive(virStoragePoolObjPtr *objptr)
{
virStoragePoolObjPtr obj = *objptr;
- if (obj->configFile == NULL) {
+ if (!virStoragePoolObjGetConfigFile(obj)) {
virStoragePoolObjRemove(&driver->pools, obj);
*objptr = NULL;
} else if (obj->newDef) {
@@ -643,7 +643,7 @@ storagePoolIsPersistent(virStoragePoolPtr pool)
if (virStoragePoolIsPersistentEnsureACL(pool->conn, obj->def) < 0)
goto cleanup;
- ret = obj->configFile ? 1 : 0;
+ ret = virStoragePoolObjGetConfigFile(obj) ? 1 : 0;
cleanup:
virStoragePoolObjUnlock(obj);
@@ -849,7 +849,6 @@ storagePoolUndefine(virStoragePoolPtr pool)
obj->autostartLink, virStrerror(errno, ebuf, sizeof(ebuf)));
}
- VIR_FREE(obj->configFile);
VIR_FREE(obj->autostartLink);
event = virStoragePoolEventLifecycleNew(obj->def->name,
@@ -1250,7 +1249,7 @@ storagePoolGetAutostart(virStoragePoolPtr pool,
if (virStoragePoolGetAutostartEnsureACL(pool->conn, obj->def) < 0)
goto cleanup;
- if (!obj->configFile) {
+ if (!virStoragePoolObjGetConfigFile(obj)) {
*autostart = 0;
} else {
*autostart = obj->autostart;
@@ -1268,6 +1267,7 @@ storagePoolSetAutostart(virStoragePoolPtr pool,
int autostart)
{
virStoragePoolObjPtr obj;
+ const char *configFile;
int ret = -1;
storageDriverLock();
@@ -1277,7 +1277,7 @@ storagePoolSetAutostart(virStoragePoolPtr pool,
if (virStoragePoolSetAutostartEnsureACL(pool->conn, obj->def) < 0)
goto cleanup;
- if (!obj->configFile) {
+ if (!(configFile = virStoragePoolObjGetConfigFile(obj))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("pool has no config file"));
goto cleanup;
@@ -1294,10 +1294,10 @@ storagePoolSetAutostart(virStoragePoolPtr pool,
goto cleanup;
}
- if (symlink(obj->configFile, obj->autostartLink) < 0) {
+ if (symlink(configFile, obj->autostartLink) < 0) {
virReportSystemError(errno,
_("Failed to create symlink '%s' to
'%s'"),
- obj->autostartLink, obj->configFile);
+ obj->autostartLink, configFile);
goto cleanup;
}
} else {
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 67b115f..36e5ba1 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4056,12 +4056,17 @@ testInterfaceDestroy(virInterfacePtr iface,
static int
testStoragePoolObjSetDefaults(virStoragePoolObjPtr obj)
{
+ char *configFile;
obj->def->capacity = defaultPoolCap;
obj->def->allocation = defaultPoolAlloc;
obj->def->available = defaultPoolCap - defaultPoolAlloc;
- return VIR_STRDUP(obj->configFile, "");
+ if (VIR_STRDUP(configFile, "") < 0)
+ return -1;
+
+ virStoragePoolObjSetConfigFile(obj, configFile);
+ return 0;
}
@@ -4303,7 +4308,7 @@ testStoragePoolIsPersistent(virStoragePoolPtr pool)
if (!(obj = testStoragePoolObjFindByUUID(privconn, pool->uuid)))
return -1;
- ret = obj->configFile ? 1 : 0;
+ ret = virStoragePoolObjGetConfigFile(obj) ? 1 : 0;
virStoragePoolObjUnlock(obj);
return ret;
@@ -4469,7 +4474,7 @@ testStoragePoolCreateXML(virConnectPtr conn,
/* *SetDefaults fills this in for the persistent pools, but this
* would be a transient pool so remove it; otherwise, the Destroy
* code will not Remove the pool */
- VIR_FREE(obj->configFile);
+ virStoragePoolObjSetConfigFile(obj, NULL);
obj->active = true;
@@ -4634,7 +4639,7 @@ testStoragePoolDestroy(virStoragePoolPtr pool)
VIR_STORAGE_POOL_EVENT_STOPPED,
0);
- if (obj->configFile == NULL) {
+ if (!(virStoragePoolObjGetConfigFile(obj))) {
virStoragePoolObjRemove(&privconn->pools, obj);
obj = NULL;
}
@@ -4740,11 +4745,10 @@ testStoragePoolGetAutostart(virStoragePoolPtr pool,
if (!(obj = testStoragePoolObjFindByName(privconn, pool->name)))
return -1;
- if (!obj->configFile) {
+ if (!virStoragePoolObjGetConfigFile(obj))
*autostart = 0;
- } else {
+ else
*autostart = obj->autostart;
- }
virStoragePoolObjUnlock(obj);
return 0;
@@ -4762,7 +4766,7 @@ testStoragePoolSetAutostart(virStoragePoolPtr pool,
if (!(obj = testStoragePoolObjFindByName(privconn, pool->name)))
return -1;
- if (!obj->configFile) {
+ if (!virStoragePoolObjGetConfigFile(obj)) {
virReportError(VIR_ERR_INVALID_ARG,
"%s", _("pool has no config file"));
goto cleanup;
--
2.9.5
8:09 a.m.
New subject: [libvirt] [PATCH v2 07/12] storage: Use virStoragePoolObjGetAutostartLink
Use the new accessor API for storage_driver.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_driver.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 7b1396f..a7a77ba 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -814,6 +814,7 @@ static int
storagePoolUndefine(virStoragePoolPtr pool)
{
virStoragePoolObjPtr obj;
+ const char *autostartLink;
virObjectEventPtr event = NULL;
int ret = -1;
@@ -838,18 +839,17 @@ storagePoolUndefine(virStoragePoolPtr pool)
goto cleanup;
}
+ autostartLink = virStoragePoolObjGetAutostartLink(obj);
if (virStoragePoolObjDeleteDef(obj) < 0)
goto cleanup;
- if (unlink(obj->autostartLink) < 0 &&
- errno != ENOENT &&
- errno != ENOTDIR) {
+ if (autostartLink && unlink(autostartLink) < 0 &&
+ errno != ENOENT && errno != ENOTDIR) {
char ebuf[1024];
VIR_ERROR(_("Failed to delete autostart link '%s': %s"),
- obj->autostartLink, virStrerror(errno, ebuf, sizeof(ebuf)));
+ autostartLink, virStrerror(errno, ebuf, sizeof(ebuf)));
}
- VIR_FREE(obj->autostartLink);
event = virStoragePoolEventLifecycleNew(obj->def->name,
obj->def->uuid,
@@ -1268,6 +1268,7 @@ storagePoolSetAutostart(virStoragePoolPtr pool,
{
virStoragePoolObjPtr obj;
const char *configFile;
+ const char *autostartLink;
int ret = -1;
storageDriverLock();
@@ -1283,6 +1284,8 @@ storagePoolSetAutostart(virStoragePoolPtr pool,
goto cleanup;
}
+ autostartLink = virStoragePoolObjGetAutostartLink(obj);
+
autostart = (autostart != 0);
if (obj->autostart != autostart) {
@@ -1294,18 +1297,18 @@ storagePoolSetAutostart(virStoragePoolPtr pool,
goto cleanup;
}
- if (symlink(configFile, obj->autostartLink) < 0) {
+ if (symlink(configFile, autostartLink) < 0) {
virReportSystemError(errno,
_("Failed to create symlink '%s' to
'%s'"),
- obj->autostartLink, configFile);
+ autostartLink, configFile);
goto cleanup;
}
} else {
- if (unlink(obj->autostartLink) < 0 &&
+ if (autostartLink && unlink(autostartLink) < 0 &&
errno != ENOENT && errno != ENOTDIR) {
virReportSystemError(errno,
_("Failed to delete symlink
'%s'"),
- obj->autostartLink);
+ autostartLink);
goto cleanup;
}
}
--
2.9.5
1:38 a.m.
New subject: [libvirt] [PATCH v2 07/12] storage: Use virStoragePoolObjGetAutostartLink
On 08/24/2017 03:09 PM, John Ferlan wrote:
Use the new accessor API for storage_driver.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_driver.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 7b1396f..a7a77ba 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -814,6 +814,7 @@ static int
storagePoolUndefine(virStoragePoolPtr pool)
{
virStoragePoolObjPtr obj;
+ const char *autostartLink;
virObjectEventPtr event = NULL;
int ret = -1;
@@ -838,18 +839,17 @@ storagePoolUndefine(virStoragePoolPtr pool)
goto cleanup;
}
+ autostartLink = virStoragePoolObjGetAutostartLink(obj);
virStoragePoolObjGetAutostartLink() should look a bit different. Now
it's returning char * which suggests that caller is supposed to free the
retval. But in fact they shouldn't. const char * would be better. But
lets save that for a follow up patch.
Michal
8:09 a.m.
New subject: [libvirt] [PATCH v2 08/12] storage: Use virStoragePoolObj{Is|Set}Active
Use the new accessor APIs for storage_driver, test_driver, and
gluster backend.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend_gluster.c | 2 +-
src/storage/storage_driver.c | 16 ++++++++--------
src/test/test_driver.c | 10 +++++-----
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/src/storage/storage_backend_gluster.c
b/src/storage/storage_backend_gluster.c
index 90f31b0..05e7bff 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -544,7 +544,7 @@ virStorageBackendGlusterCheckPool(virStoragePoolObjPtr pool,
/* Return previous state remembered by the status XML. If the pool is not
* available we will fail to refresh it and end up in the same situation.
* This will save one attempt to open the connection to the remote server */
- *active = pool->active;
+ *active = virStoragePoolObjIsActive(pool);
return 0;
}
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index a7a77ba..89eaee9 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -145,9 +145,9 @@ storagePoolUpdateState(virStoragePoolObjPtr obj)
}
}
- obj->active = active;
+ virStoragePoolObjSetActive(obj, active);
- if (!obj->active)
+ if (!virStoragePoolObjIsActive(obj))
virStoragePoolUpdateInactive(&obj);
cleanup:
@@ -226,7 +226,7 @@ storageDriverAutostart(void)
_("Failed to autostart storage pool '%s':
%s"),
obj->def->name, virGetLastErrorMessage());
} else {
- obj->active = true;
+ virStoragePoolObjSetActive(obj, true);
}
VIR_FREE(stateFile);
}
@@ -735,7 +735,7 @@ storagePoolCreateXML(virConnectPtr conn,
0);
VIR_INFO("Creating storage pool '%s'", obj->def->name);
- obj->active = true;
+ virStoragePoolObjSetActive(obj, true);
pool = virGetStoragePool(conn, obj->def->name, obj->def->uuid, NULL,
NULL);
@@ -940,7 +940,7 @@ storagePoolCreate(virStoragePoolPtr pool,
VIR_STORAGE_POOL_EVENT_STARTED,
0);
- obj->active = true;
+ virStoragePoolObjSetActive(obj, true);
ret = 0;
cleanup:
@@ -1040,7 +1040,7 @@ storagePoolDestroy(virStoragePoolPtr pool)
VIR_STORAGE_POOL_EVENT_STOPPED,
0);
- obj->active = false;
+ virStoragePoolObjSetActive(obj, false);
virStoragePoolUpdateInactive(&obj);
@@ -1156,7 +1156,7 @@ storagePoolRefresh(virStoragePoolPtr pool,
obj->def->uuid,
VIR_STORAGE_POOL_EVENT_STOPPED,
0);
- obj->active = false;
+ virStoragePoolObjSetActive(obj, false);
virStoragePoolUpdateInactive(&obj);
@@ -1194,7 +1194,7 @@ storagePoolGetInfo(virStoragePoolPtr pool,
goto cleanup;
memset(info, 0, sizeof(virStoragePoolInfo));
- if (obj->active)
+ if (virStoragePoolObjIsActive(obj))
info->state = VIR_STORAGE_POOL_RUNNING;
else
info->state = VIR_STORAGE_POOL_INACTIVE;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 36e5ba1..33ce9e4 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1131,7 +1131,7 @@ testParseStorage(testDriverPtr privconn,
virStoragePoolObjUnlock(obj);
goto error;
}
- obj->active = true;
+ virStoragePoolObjSetActive(obj, true);
/* Find storage volumes */
if (testOpenVolumesForPool(file, ctxt, obj, i+1) < 0) {
@@ -4328,7 +4328,7 @@ testStoragePoolCreate(virStoragePoolPtr pool,
if (!(obj = testStoragePoolObjFindInactiveByName(privconn, pool->name)))
return -1;
- obj->active = true;
+ virStoragePoolObjSetActive(obj, true);
event = virStoragePoolEventLifecycleNew(pool->name, pool->uuid,
VIR_STORAGE_POOL_EVENT_STARTED,
@@ -4476,7 +4476,7 @@ testStoragePoolCreateXML(virConnectPtr conn,
* code will not Remove the pool */
virStoragePoolObjSetConfigFile(obj, NULL);
- obj->active = true;
+ virStoragePoolObjSetActive(obj, true);
event = virStoragePoolEventLifecycleNew(obj->def->name, obj->def->uuid,
VIR_STORAGE_POOL_EVENT_STARTED,
@@ -4624,7 +4624,7 @@ testStoragePoolDestroy(virStoragePoolPtr pool)
if (!(obj = testStoragePoolObjFindActiveByName(privconn, pool->name)))
return -1;
- obj->active = false;
+ virStoragePoolObjSetActive(obj, false);
if (obj->def->source.adapter.type ==
VIR_STORAGE_ADAPTER_TYPE_FC_HOST) {
@@ -4702,7 +4702,7 @@ testStoragePoolGetInfo(virStoragePoolPtr pool,
return -1;
memset(info, 0, sizeof(virStoragePoolInfo));
- if (obj->active)
+ if (virStoragePoolObjIsActive(obj))
info->state = VIR_STORAGE_POOL_RUNNING;
else
info->state = VIR_STORAGE_POOL_INACTIVE;
--
2.9.5
8:09 a.m.
New subject: [libvirt] [PATCH v2 09/12] storage: Use virStoragePoolObj{Is|Set}Autostart
Use the new accessor APIs for storage_driver and test_driver.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_driver.c | 20 +++++++++-----------
src/test/test_driver.c | 4 ++--
2 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 89eaee9..c9156e6 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -196,7 +196,7 @@ storageDriverAutostart(void)
continue;
}
- if (obj->autostart &&
+ if (virStoragePoolObjIsAutostart(obj) &&
!virStoragePoolObjIsActive(obj)) {
if (backend->startPool &&
backend->startPool(conn, obj) < 0) {
@@ -1249,11 +1249,7 @@ storagePoolGetAutostart(virStoragePoolPtr pool,
if (virStoragePoolGetAutostartEnsureACL(pool->conn, obj->def) < 0)
goto cleanup;
- if (!virStoragePoolObjGetConfigFile(obj)) {
- *autostart = 0;
- } else {
- *autostart = obj->autostart;
- }
+ *autostart = virStoragePoolObjIsAutostart(obj) ? 1 : 0;
ret = 0;
@@ -1269,6 +1265,8 @@ storagePoolSetAutostart(virStoragePoolPtr pool,
virStoragePoolObjPtr obj;
const char *configFile;
const char *autostartLink;
+ bool new_autostart;
+ bool cur_autostart;
int ret = -1;
storageDriverLock();
@@ -1286,10 +1284,10 @@ storagePoolSetAutostart(virStoragePoolPtr pool,
autostartLink = virStoragePoolObjGetAutostartLink(obj);
- autostart = (autostart != 0);
-
- if (obj->autostart != autostart) {
- if (autostart) {
+ new_autostart = (autostart != 0);
+ cur_autostart = virStoragePoolObjIsAutostart(obj);
+ if (cur_autostart != new_autostart) {
+ if (new_autostart) {
if (virFileMakePath(driver->autostartDir) < 0) {
virReportSystemError(errno,
_("cannot create autostart directory
%s"),
@@ -1312,7 +1310,7 @@ storagePoolSetAutostart(virStoragePoolPtr pool,
goto cleanup;
}
}
- obj->autostart = autostart;
+ virStoragePoolObjSetAutostart(obj, autostart);
}
ret = 0;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 33ce9e4..b793187 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4748,7 +4748,7 @@ testStoragePoolGetAutostart(virStoragePoolPtr pool,
if (!virStoragePoolObjGetConfigFile(obj))
*autostart = 0;
else
- *autostart = obj->autostart;
+ *autostart = virStoragePoolObjIsAutostart(obj) ? 1 : 0;
virStoragePoolObjUnlock(obj);
return 0;
@@ -4773,7 +4773,7 @@ testStoragePoolSetAutostart(virStoragePoolPtr pool,
}
autostart = (autostart != 0);
- obj->autostart = autostart;
+ virStoragePoolObjSetAutostart(obj, autostart);
ret = 0;
cleanup:
--
2.9.5
8:09 a.m.
New subject: [libvirt] [PATCH v2 10/12] storage: Internally represent @autostart to bool
Since it's been used that way anyway, let's just convert it to a bool
and only make the external representation be an int.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.c | 4 ++--
src/conf/virstorageobj.h | 4 ++--
src/storage/storage_driver.c | 2 +-
src/test/test_driver.c | 4 ++--
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 8ee40e2..1364bdd 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -133,13 +133,13 @@ virStoragePoolObjIsAutostart(virStoragePoolObjPtr obj)
if (!obj->configFile)
return 0;
- return obj->autostart == 1;
+ return obj->autostart;
}
void
virStoragePoolObjSetAutostart(virStoragePoolObjPtr obj,
- int autostart)
+ bool autostart)
{
obj->autostart = autostart;
}
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index c2f3f23..b65b160 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -33,7 +33,7 @@ struct _virStoragePoolObj {
char *configFile;
char *autostartLink;
bool active;
- int autostart;
+ bool autostart;
unsigned int asyncjobs;
virStoragePoolDefPtr def;
@@ -108,7 +108,7 @@ virStoragePoolObjIsAutostart(virStoragePoolObjPtr obj);
void
virStoragePoolObjSetAutostart(virStoragePoolObjPtr obj,
- int autostart);
+ bool autostart);
unsigned int
virStoragePoolObjGetAsyncjobs(virStoragePoolObjPtr obj);
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index c9156e6..6fae6f4 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1310,7 +1310,7 @@ storagePoolSetAutostart(virStoragePoolPtr pool,
goto cleanup;
}
}
- virStoragePoolObjSetAutostart(obj, autostart);
+ virStoragePoolObjSetAutostart(obj, new_autostart);
}
ret = 0;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index b793187..69d61b5 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4761,6 +4761,7 @@ testStoragePoolSetAutostart(virStoragePoolPtr pool,
{
testDriverPtr privconn = pool->conn->privateData;
virStoragePoolObjPtr obj;
+ bool new_autostart = (autostart != 0);
int ret = -1;
if (!(obj = testStoragePoolObjFindByName(privconn, pool->name)))
@@ -4772,8 +4773,7 @@ testStoragePoolSetAutostart(virStoragePoolPtr pool,
goto cleanup;
}
- autostart = (autostart != 0);
- virStoragePoolObjSetAutostart(obj, autostart);
+ virStoragePoolObjSetAutostart(obj, new_autostart);
ret = 0;
cleanup:
--
2.9.5
1:38 a.m.
New subject: [libvirt] [PATCH v2 10/12] storage: Internally represent @autostart to bool
On 08/24/2017 03:09 PM, John Ferlan wrote:
Since it's been used that way anyway, let's just convert it
to a bool
and only make the external representation be an int.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.c | 4 ++--
src/conf/virstorageobj.h | 4 ++--
src/storage/storage_driver.c | 2 +-
src/test/test_driver.c | 4 ++--
4 files changed, 7 insertions(+), 7 deletions(-)
I'm no native speaker but probably s/to bool/as bool/ in $SUBJ?
Michal
8:09 a.m.
New subject: [libvirt] [PATCH v2 11/12] storage: Use virStoragePoolObj{Get|Incr}Decr}Asyncjobs
Use the new accessor APIs for storage_driver.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_driver.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 6fae6f4..83d9ce3 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -832,7 +832,7 @@ storagePoolUndefine(virStoragePoolPtr pool)
goto cleanup;
}
- if (obj->asyncjobs > 0) {
+ if (virStoragePoolObjGetAsyncjobs(obj) > 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("pool '%s' has asynchronous jobs running."),
obj->def->name);
@@ -1014,7 +1014,7 @@ storagePoolDestroy(virStoragePoolPtr pool)
goto cleanup;
}
- if (obj->asyncjobs > 0) {
+ if (virStoragePoolObjGetAsyncjobs(obj) > 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("pool '%s' has asynchronous jobs running."),
obj->def->name);
@@ -1082,7 +1082,7 @@ storagePoolDelete(virStoragePoolPtr pool,
goto cleanup;
}
- if (obj->asyncjobs > 0) {
+ if (virStoragePoolObjGetAsyncjobs(obj) > 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("pool '%s' has asynchronous jobs running."),
obj->def->name);
@@ -1140,7 +1140,7 @@ storagePoolRefresh(virStoragePoolPtr pool,
goto cleanup;
}
- if (obj->asyncjobs > 0) {
+ if (virStoragePoolObjGetAsyncjobs(obj) > 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("pool '%s' has asynchronous jobs running."),
obj->def->name);
@@ -1827,7 +1827,7 @@ storageVolCreateXML(virStoragePoolPtr pool,
memcpy(buildvoldef, voldef, sizeof(*voldef));
/* Drop the pool lock during volume allocation */
- obj->asyncjobs++;
+ virStoragePoolObjIncrAsyncjobs(obj);
voldef->building = true;
virStoragePoolObjUnlock(obj);
@@ -1840,7 +1840,7 @@ storageVolCreateXML(virStoragePoolPtr pool,
storageDriverUnlock();
voldef->building = false;
- obj->asyncjobs--;
+ virStoragePoolObjDecrAsyncjobs(obj);
if (buildret < 0) {
/* buildVol handles deleting volume on failure */
@@ -2020,13 +2020,13 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool,
goto cleanup;
/* Drop the pool lock during volume allocation */
- obj->asyncjobs++;
+ virStoragePoolObjIncrAsyncjobs(obj);
voldef->building = true;
voldefsrc->in_use++;
virStoragePoolObjUnlock(obj);
if (objsrc) {
- objsrc->asyncjobs++;
+ virStoragePoolObjIncrAsyncjobs(objsrc);
virStoragePoolObjUnlock(objsrc);
}
@@ -2040,10 +2040,10 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool,
voldefsrc->in_use--;
voldef->building = false;
- obj->asyncjobs--;
+ virStoragePoolObjDecrAsyncjobs(obj);
if (objsrc) {
- objsrc->asyncjobs--;
+ virStoragePoolObjDecrAsyncjobs(objsrc);
virStoragePoolObjUnlock(objsrc);
objsrc = NULL;
}
--
2.9.5
1:38 a.m.
New subject: [libvirt] [PATCH v2 11/12] storage: Use virStoragePoolObj{Get|Incr}Decr}Asyncjobs
On 08/24/2017 03:09 PM, John Ferlan wrote:
Use the new accessor APIs for storage_driver.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_driver.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
s/Incr}Decr/Incr|Decr/ in $SUBJ.
Michal
8:09 a.m.
New subject: [libvirt] [PATCH v2 12/12] storage: Use virStoragePoolObjDefUseNewDef
Use the new accessor API for storage_driver.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_driver.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 83d9ce3..90d4bf3 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -94,10 +94,8 @@ virStoragePoolUpdateInactive(virStoragePoolObjPtr *objptr)
if (!virStoragePoolObjGetConfigFile(obj)) {
virStoragePoolObjRemove(&driver->pools, obj);
*objptr = NULL;
- } else if (obj->newDef) {
- virStoragePoolDefFree(obj->def);
- obj->def = obj->newDef;
- obj->newDef = NULL;
+ } else if (virStoragePoolObjGetNewDef(obj)) {
+ virStoragePoolObjDefUseNewDef(obj);
}
}
--
2.9.5
9:10 p.m.
ping?
Would be nice to make a bit more progress here.
Tks,
John
On 08/24/2017 09:08 AM, John Ferlan wrote:
Consider this round 1 of 2.... The next series will be 18 patches,
but the majority of those deal with change every {pool|obj}->def->X
to use the accessor virStoragePoolObjGetDef.
v1: https://www.redhat.com/archives/libvir-list/2017-May/msg00218.html
Probably not even worth looking at the v1, but this picks up where v1
left off somewhere around patch 10, but adding smaller steps between
patches.
John Ferlan (12):
storage: Create accessor API's for virStoragePoolObj
storage: Introduce virStoragePoolObjNew
storage: Fill in storage pool @active properly
storage: Introduce storage volume add, delete, count APIs
storage: Introduce APIs to search/scan storage pool volumes list
storage: Use virStoragePoolObj{Get|Set}ConfigFile
storage: Use virStoragePoolObjGetAutostartLink
storage: Use virStoragePoolObj{Is|Set}Active
storage: Use virStoragePoolObj{Is|Set}Autostart
storage: Internally represent @autostart to bool
storage: Use virStoragePoolObj{Get|Incr}Decr}Asyncjobs
storage: Use virStoragePoolObjDefUseNewDef
src/conf/virstorageobj.c | 211 +++++++++++++++++++++++++++++++--
src/conf/virstorageobj.h | 84 ++++++++++++-
src/libvirt_private.syms | 20 ++++
src/storage/storage_backend_disk.c | 93 +++++++++------
src/storage/storage_backend_gluster.c | 5 +-
src/storage/storage_backend_logical.c | 4 +-
src/storage/storage_backend_mpath.c | 3 +-
src/storage/storage_backend_rbd.c | 4 +-
src/storage/storage_backend_scsi.c | 4 +-
src/storage/storage_backend_sheepdog.c | 4 +-
src/storage/storage_backend_zfs.c | 6 +-
src/storage/storage_driver.c | 142 +++++++++-------------
src/storage/storage_util.c | 8 +-
src/test/test_driver.c | 54 ++++-----
tests/storagevolxml2argvtest.c | 20 ++--
15 files changed, 465 insertions(+), 197 deletions(-)
1:39 a.m.
On 08/24/2017 03:08 PM, John Ferlan wrote:
Consider this round 1 of 2.... The next series will be 18 patches,
but the majority of those deal with change every {pool|obj}->def->X
to use the accessor virStoragePoolObjGetDef.
v1: https://www.redhat.com/archives/libvir-list/2017-May/msg00218.html
Probably not even worth looking at the v1, but this picks up where v1
left off somewhere around patch 10, but adding smaller steps between
patches.
John Ferlan (12):
storage: Create accessor API's for virStoragePoolObj
storage: Introduce virStoragePoolObjNew
storage: Fill in storage pool @active properly
storage: Introduce storage volume add, delete, count APIs
storage: Introduce APIs to search/scan storage pool volumes list
storage: Use virStoragePoolObj{Get|Set}ConfigFile
storage: Use virStoragePoolObjGetAutostartLink
storage: Use virStoragePoolObj{Is|Set}Active
storage: Use virStoragePoolObj{Is|Set}Autostart
storage: Internally represent @autostart to bool
storage: Use virStoragePoolObj{Get|Incr}Decr}Asyncjobs
storage: Use virStoragePoolObjDefUseNewDef
src/conf/virstorageobj.c | 211 +++++++++++++++++++++++++++++++--
src/conf/virstorageobj.h | 84 ++++++++++++-
src/libvirt_private.syms | 20 ++++
src/storage/storage_backend_disk.c | 93 +++++++++------
src/storage/storage_backend_gluster.c | 5 +-
src/storage/storage_backend_logical.c | 4 +-
src/storage/storage_backend_mpath.c | 3 +-
src/storage/storage_backend_rbd.c | 4 +-
src/storage/storage_backend_scsi.c | 4 +-
src/storage/storage_backend_sheepdog.c | 4 +-
src/storage/storage_backend_zfs.c | 6 +-
src/storage/storage_driver.c | 142 +++++++++-------------
src/storage/storage_util.c | 8 +-
src/test/test_driver.c | 54 ++++-----
tests/storagevolxml2argvtest.c | 20 ++--
15 files changed, 465 insertions(+), 197 deletions(-)
ACK series, but please see my comments before pushing.
Michal
7:36 a.m.
On 09/19/2017 02:39 AM, Michal Privoznik wrote:
On 08/24/2017 03:08 PM, John Ferlan wrote:
> Consider this round 1 of 2.... The next series will be 18 patches,
> but the majority of those deal with change every {pool|obj}->def->X
> to use the accessor virStoragePoolObjGetDef.
>
> v1: https://www.redhat.com/archives/libvir-list/2017-May/msg00218.html
>
> Probably not even worth looking at the v1, but this picks up where v1
> left off somewhere around patch 10, but adding smaller steps between
> patches.
>
>
> John Ferlan (12):
> storage: Create accessor API's for virStoragePoolObj
> storage: Introduce virStoragePoolObjNew
> storage: Fill in storage pool @active properly
> storage: Introduce storage volume add, delete, count APIs
> storage: Introduce APIs to search/scan storage pool volumes list
> storage: Use virStoragePoolObj{Get|Set}ConfigFile
> storage: Use virStoragePoolObjGetAutostartLink
> storage: Use virStoragePoolObj{Is|Set}Active
> storage: Use virStoragePoolObj{Is|Set}Autostart
> storage: Internally represent @autostart to bool
> storage: Use virStoragePoolObj{Get|Incr}Decr}Asyncjobs
> storage: Use virStoragePoolObjDefUseNewDef
>
> src/conf/virstorageobj.c | 211 +++++++++++++++++++++++++++++++--
> src/conf/virstorageobj.h | 84 ++++++++++++-
> src/libvirt_private.syms | 20 ++++
> src/storage/storage_backend_disk.c | 93 +++++++++------
> src/storage/storage_backend_gluster.c | 5 +-
> src/storage/storage_backend_logical.c | 4 +-
> src/storage/storage_backend_mpath.c | 3 +-
> src/storage/storage_backend_rbd.c | 4 +-
> src/storage/storage_backend_scsi.c | 4 +-
> src/storage/storage_backend_sheepdog.c | 4 +-
> src/storage/storage_backend_zfs.c | 6 +-
> src/storage/storage_driver.c | 142 +++++++++-------------
> src/storage/storage_util.c | 8 +-
> src/test/test_driver.c | 54 ++++-----
> tests/storagevolxml2argvtest.c | 20 ++--
> 15 files changed, 465 insertions(+), 197 deletions(-)
>
ACK series, but please see my comments before pushing.
Michal
Adjusted patch 2 as recommended and fixed the commit messages in patches
10 and 11. I'll push shortly and follow-up with alteration of
virStoragePoolObjGetAutostartLink (and virStoragePoolObjGetConfigFile)
to change the prototype return to be const char *
Thanks for the review!
John
2502
days inactive
2528
days old
19 comments
2 participants
participants (2)
-
John Ferlan -
Michal Privoznik