On 25.11.2015 20:11, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=830056
Add flags handling to the virStoragePoolCreate and virStoragePoolCreateXML
API's which will allow the caller to provide the capability for the storage
pool create API's to also perform a pool build during creation rather than
requiring the additional buildPool step. This will allow transient pools
to be defined, built, and started.
The new flags are:
* VIR_STORAGE_POOL_CREATE_WITH_BUILD
Perform buildPool without any flags passed.
* VIR_STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE
Perform buildPool using VIR_STORAGE_POOL_BUILD_OVERWRITE flag.
* VIR_STORAGE_POOL_CREATE_WITH_BUILD_NO_OVERWRITE
Perform buildPool using VIR_STORAGE_POOL_BUILD_NO_OVERWRITE flag.
It is up to the backend to handle the processing of build flags. The
overwrite and no-overwrite flags are mutually exclusive.
NB:
This patch is loosely based upon code originally authored by Osier
Yang that were not reviewed and pushed, see:
https://www.redhat.com/archives/libvir-list/2012-July/msg01328.html
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
include/libvirt/libvirt-storage.h | 16 ++++++++++++++-
src/libvirt-storage.c | 4 ++--
src/storage/storage_driver.c | 42 +++++++++++++++++++++++++++++++++++++--
3 files changed, 57 insertions(+), 5 deletions(-)
diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h
index 9fc3c2d..996a726 100644
--- a/include/libvirt/libvirt-storage.h
+++ b/include/libvirt/libvirt-storage.h
@@ -57,7 +57,6 @@ typedef enum {
# endif
} virStoragePoolState;
-
typedef enum {
VIR_STORAGE_POOL_BUILD_NEW = 0, /* Regular build from scratch */
VIR_STORAGE_POOL_BUILD_REPAIR = (1 << 0), /* Repair / reinitialize */
@@ -71,6 +70,21 @@ typedef enum {
VIR_STORAGE_POOL_DELETE_ZEROED = 1 << 0, /* Clear all data to zeros (slow)
*/
} virStoragePoolDeleteFlags;
+typedef enum {
+ VIR_STORAGE_POOL_CREATE_NORMAL = 0,
+
+ /* Create the pool and perform pool build without any flags */
+ VIR_STORAGE_POOL_CREATE_WITH_BUILD = 1 << 0,
+
+ /* Create the pool and perform pool build using the
+ * VIR_STORAGE_POOL_BUILD_OVERWRITE flag */
+ VIR_STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE = 1 << 1,
+
+ /* Create the pool and perform pool build using the
+ * VIR_STORAGE_POOL_BUILD_NO_OVERWRITE flag */
+ VIR_STORAGE_POOL_CREATE_WITH_BUILD_NO_OVERWRITE = 1 << 2,
So _OVERWRITE and _NO_OVERWRITE flags are mutually exclusive then,
right? Probably worth noting.
+} virStoragePoolCreateFlags;
+
typedef struct _virStoragePoolInfo virStoragePoolInfo;
struct _virStoragePoolInfo {
diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c
index 66dd9f0..238a6cd 100644
--- a/src/libvirt-storage.c
+++ b/src/libvirt-storage.c
@@ -505,7 +505,7 @@ virStoragePoolLookupByVolume(virStorageVolPtr vol)
* virStoragePoolCreateXML:
* @conn: pointer to hypervisor connection
* @xmlDesc: XML description for new pool
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of virStoragePoolCreateFlags
*
* Create a new storage based on its XML description. The
* pool is not persistent, so its definition will disappear
@@ -670,7 +670,7 @@ virStoragePoolUndefine(virStoragePoolPtr pool)
/**
* virStoragePoolCreate:
* @pool: pointer to storage pool
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of virStoragePoolCreateFlags
*
* Starts an inactive storage pool
*
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index bbf21f6..59a18bf 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -671,8 +671,11 @@ storagePoolCreateXML(virConnectPtr conn,
virStoragePoolPtr ret = NULL;
virStorageBackendPtr backend;
char *stateFile = NULL;
+ unsigned int build_flags = 0;
- virCheckFlags(0, NULL);
+ virCheckFlags(VIR_STORAGE_POOL_CREATE_WITH_BUILD |
+ VIR_STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE |
+ VIR_STORAGE_POOL_CREATE_WITH_BUILD_NO_OVERWRITE, NULL);
How about VIR_EXCLUSIVE_FLAGS_RET(_OVERWRITE, _NO_OVERWRITE, NULL);
storageDriverLock();
if (!(def = virStoragePoolDefParseString(xml)))
@@ -694,6 +697,22 @@ storagePoolCreateXML(virConnectPtr conn,
goto cleanup;
def = NULL;
+ if (backend->buildPool) {
+ if (flags & VIR_STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE)
+ build_flags |= VIR_STORAGE_POOL_BUILD_OVERWRITE;
+ else if (flags & VIR_STORAGE_POOL_CREATE_WITH_BUILD_NO_OVERWRITE)
+ build_flags |= VIR_STORAGE_POOL_BUILD_NO_OVERWRITE;
+
+ if (build_flags ||
+ (flags & VIR_STORAGE_POOL_CREATE_WITH_BUILD)) {
a-ha! So I need to pass _BUILD flag, and optionally one of _OVERWRITE
and _NO_OVERWRITE too. Okay.
+ if (backend->buildPool(conn, pool, build_flags) <
0) {
+ virStoragePoolObjRemove(&driver->pools, pool);
+ pool = NULL;
+ goto cleanup;
+ }
+ }
+ }
+
if (backend->startPool &&
backend->startPool(conn, pool) < 0) {
virStoragePoolObjRemove(&driver->pools, pool);
@@ -845,8 +864,11 @@ storagePoolCreate(virStoragePoolPtr obj,
virStorageBackendPtr backend;
int ret = -1;
char *stateFile = NULL;
+ unsigned int build_flags = 0;
- virCheckFlags(0, -1);
+ virCheckFlags(VIR_STORAGE_POOL_CREATE_WITH_BUILD |
+ VIR_STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE |
+ VIR_STORAGE_POOL_CREATE_WITH_BUILD_NO_OVERWRITE, NULL);
s/NULL/-1/. The function is returning an integer, not a pointer.
Plus it would be nice if we check the mutually exclusive flags here too.
if (!(pool = virStoragePoolObjFromStoragePool(obj)))
return -1;
@@ -864,6 +886,22 @@ storagePoolCreate(virStoragePoolPtr obj,
goto cleanup;
}
+ if (backend->buildPool) {
+ if (flags & VIR_STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE)
+ build_flags |= VIR_STORAGE_POOL_BUILD_OVERWRITE;
+ else if (flags & VIR_STORAGE_POOL_CREATE_WITH_BUILD_NO_OVERWRITE)
+ build_flags |= VIR_STORAGE_POOL_BUILD_NO_OVERWRITE;
+
+ if (build_flags ||
+ (flags & VIR_STORAGE_POOL_CREATE_WITH_BUILD)) {
+ if (backend->buildPool(obj->conn, pool, build_flags) < 0) {
+ virStoragePoolObjRemove(&driver->pools, pool);
+ pool = NULL;
+ goto cleanup;
+ }
+ }
+ }
+
VIR_INFO("Starting up storage pool '%s'", pool->def->name);
if (backend->startPool &&
backend->startPool(obj->conn, pool) < 0)
Otherwise looking good. ACK.
Michal