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@xxxxxxxxxx> > --- > 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list