[libvirt] [PATCH v2] logical: Check for existing vgname before building

[Ján Tomko]

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 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161116/2aeed317/attachment-0001.sig>