[libvirt] [PATCH] storage: Attempt to refresh volume after successful wipe volume

[Ján Tomko]

On Wed, Dec 02, 2015 at 03:22:38PM -0500, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1270709
> 
> When a volume wipe is successful, a volume refresh should be done afterwards
> to update any volume data that may be used in future volume commands, such as
> volume resize.  For a raw file volume, a wipe would truncate the file and
> a followup volume resize the capacity may fail because the volume target
> allocation isn't updated to reflect the wipe activity.
> 

I would expect that after wiping a 200 MB volume with zeros, it would
contain 200 MB of zeros and it would not be shrinkable to 50 MB, not
even after a volume refresh.

While it seems ftruncate to 0 bytes and back satisfies the documentation
of the virStorageVolWipe API:
  Ensure data previously on a volume is not accessible to future reads
the ALG_ZERO description says:
  VIR_STORAGE_VOL_WIPE_ALG_ZERO   =   0   1-pass, all zeroes

Do we need to update it to reflect that there might not be any pass over
the old data (which might not happen for non-sparse files either,
if the filesystem does not overwrite the same sectors)?

> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/storage/storage_driver.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index bbf21f6..2e59e39 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -2436,7 +2436,19 @@ storageVolWipePattern(virStorageVolPtr obj,
>          goto cleanup;
>      }
>  
> -    ret = backend->wipeVol(obj->conn, pool, vol, algorithm, flags);
> +    if ((ret = backend->wipeVol(obj->conn, pool, vol, algorithm, flags)) < 0)
> +        goto cleanup;

More readable as:
  if (func() < 0)
     goto cleanup;

  ret = 0;
If the return value does not need to be propagated. (Which it should not
here, but it is possible in some cases. I will send a patch)

> +
> +    /* Best effort to refresh the volume data. If unsuccessful, we've already
> +     * wiped the data so there's no going back on that. Best we can do is
> +     * provide some details over what happened and move on
> +     */

The wipe did happen, but if refreshVol fails, there is something
seriously wrong with the volume. I think returning -1 and reporting an
error is reasonable here.

Jan

> +    if (backend->refreshVol &&
> +        backend->refreshVol(obj->conn, pool, vol) < 0) {
> +        VIR_WARN("failed to refresh volume '%s' info after volume wipe",
> +                 vol->name);
> +        virResetLastError();
> +    }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151211/15d80b41/attachment-0001.sig>