On Wed, Nov 16, 2016 at 09:28:55AM -0500, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1373711
Add a check for an existing volume group name before trying to build the volume group. Since the process of building a vg involves wiping the first 512 bytes and using pvcreate on each source device path before creating the vg - we could conceivably overwrite something that we shouldn't be.
That sounds like a bug in the pool build process. We should not be overwriting the source devices without the OVERWRITE flag.
Also, once a pv is part of a vg, the pvcreate would fail unless we chose to overwrite the volume.
Signed-off-by: John Ferlan <jferlan@redhat.com> ---
Difference w/ v2 is that I'm taking a different approach - disallow the second pool-build since the vg already exists. NB: libvirt has no API to extend an existing vg - that's left to an admin anyway.
src/storage/storage_backend_logical.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index ca05fe1..b241495 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -682,14 +682,31 @@ virStorageBackendLogicalBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, unsigned int flags) { - virCommandPtr vgcmd; + virCommandPtr vgcmd = NULL; int fd; char zeros[PV_BLANK_SECTOR_SIZE]; int ret = -1; size_t i; + virStoragePoolSourceList sourceList;
virCheckFlags(0, -1);
+ /* Let's make sure the about to be created vg doesn't already exist */ + memset(&sourceList, 0, sizeof(sourceList)); + sourceList.type = VIR_STORAGE_POOL_LOGICAL; + + if (virStorageBackendLogicalGetPoolSources(&sourceList) < 0) + return -1; +
Rather than introducing one more place where we use this lovely function, I'd prefer to stick to non-destructive steps and let the lvm tools error out if the vg already exists. Jan