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@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