[libvirt] [PATCH 3/3] storage: logical: Fix flawed logic with (NO)_OVERWRITE flags.

[John Ferlan]

On 01/12/2017 09:24 AM, Peter Krempa wrote:
> Commit f573f84eb7 introduced flawed logic which would cause a regression
> in creating of lvm volumes when neither libblkid nor parted are
> installed.
> 
> Fix the logic so that it triggers only if NO_OVERWRITE was specified
> explicitly.
> ---
>  src/storage/storage_backend_logical.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

So if no flag is provided, this change would essentially allow an
overwrite of data on the target device.

If the target host doesn't have blkid and/or parted, then not only does
this code fail, so does the fs and disk code - even prior to any of the
patches from the series. For both disk/fs - if someone doesn't have
blkid/parted, then they must provide the overwrite flag to indicate they
know what they're doing; otherwise, we just fail because we either
cannot or did not validate that the device had nothing on it and we
don't want to overwrite something that may be important.

What makes logical "special" so that we just ignore the check?  History?
- that is since we've never checked, thus we have to keep that going
forward?  From a data integrity viewpoint, that doesn't feel right.

John

> diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
> index 6a6720e22..77fc0d3c2 100644
> --- a/src/storage/storage_backend_logical.c
> +++ b/src/storage/storage_backend_logical.c
> @@ -778,7 +778,7 @@ virStorageBackendLogicalBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED,
>          /* The blkid FS and Part probing code doesn't know "lvm2" (this
>           * pool's only format type), but it does know "LVM2_member", so
>           * we'll pass that here */
> -        if (!(flags & VIR_STORAGE_POOL_BUILD_OVERWRITE) &&
> +        if ((flags & VIR_STORAGE_POOL_BUILD_NO_OVERWRITE) &&
>              !virStorageBackendDeviceIsEmpty(path, "LVM2_member", true))
>              goto cleanup;
>