Bug 868771

I agree with you on that, except that file format should not change as well.  We'll probably have to create a new qcow2 volume for this (and qcow/vdi/ for others), as wipe is destroying the format.

This bug was not selected to be addressed in Red Hat Enterprise Linux 6. We will look at it again within the Red Hat Enterprise Linux 7 product.

Patches proposed upstream:

https://www.redhat.com/archives/libvir-list/2016-July/msg00482.html

As explained in the commit messages it does not really make sense to support wipe for file-backed images, so this will be fixed by disabling wiping for volumes with a format that could be destroyed.

Fixed upstream by further documenting the behaviour in commit v2.1.0-rc1-22-gfa4eea8063b9:

commit fa4eea8063b972a600903e3441bf04415529a7e9
Author: Martin Kletzander <mkletzan>
Date:   Thu Jul 14 11:34:14 2016 +0200

    storage: Document wiping formatted volume types

Hi Martin, 
The only thing I found in downstream git log is:
===================== git log =====================
# git show d47f085cfcfc3913f6a3091488073609af4db61a
commit d47f085cfcfc3913f6a3091488073609af4db61a
Author: Martin Kletzander <mkletzan>
Date:   Tue Aug 2 07:10:13 2016 +0200

    storage: Document wiping formatted volume types
    
    When wiping a volume we just rewrite all the data of the volume, not
    only the content.  Since format gets overridden, we need to recreate the
    volume.  However we can't do that for every possible format out there.
    Since it was only coded for the ploop volume type, let's document what
    might be the consequences instead of forbidding it for every other
    format out there.
    
    Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=868771
    
    Signed-off-by: Martin Kletzander <mkletzan>
    (cherry picked from commit fa4eea8063b972a600903e3441bf04415529a7e9)
    Signed-off-by: Martin Kletzander <mkletzan>

diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c
index cebe02f..48996ba 100644
--- a/src/libvirt-storage.c
+++ b/src/libvirt-storage.c
@@ -1725,11 +1725,16 @@ virStorageVolDelete(virStorageVolPtr vol,
  * @vol: pointer to storage volume
  * @flags: extra flags; not used yet, so callers should always pass 0
  *
- * Ensure data previously on a volume is not accessible to future
- * reads. Also note, that depending on the actual volume
- * representation, this call may not really overwrite the
- * physical location of the volume. For instance, files stored
- * journaled, log structured, copy-on-write, versioned, and
+ * Ensure data previously on a volume is not accessible to future reads.
+ *
+ * The data to be wiped may include the format and possibly size information,
+ * so non-raw images might become raw with a different size. It is storage
+ * backend dependent whether the format and size information is regenerated
+ * once the initial volume wipe is completed.
+ *
+ * Depending on the actual volume representation, this call may not
+ * overwrite the physical location of the volume. For instance, files
+ * stored journaled, log structured, copy-on-write, versioned, and
  * network file systems are known to be problematic.
  *
  * Returns 0 on success, or -1 on error
=====================================================


And currently qcow2 file still can be wiped (and metadata lost, format changed to raw). So the only change is downstream commit d47f085cfcfc3913f6a3091488073609af4db61a's document change, and the previously patchs which forbid wiping the images with format other than "raw" not pushed in git. am I correct?

(In reply to yisun from comment #17)
Yes, you're right.  After talking about it we realized we do not want to rebuild the volumes (we can't), but instead of forbidding to do that for volumes that will be screwed, we just document that it might happen.  The reasoning behind that is that we still need to allow for wiping the data even though we don't have enough information to recreate it.  Let me know if that's not clear enough.

(In reply to Martin Kletzander from comment #18)

Thx for your confirmation.

Bug verified with code check

# git branch
* (detached from libvirt-2.0.0-4.el7)

# vim ./src/libvirt-storage.c
/**
 * virStorageVolWipe:
 * @vol: pointer to storage volume
 * @flags: extra flags; not used yet, so callers should always pass 0
 *
 * Ensure data previously on a volume is not accessible to future reads.
 *
 * The data to be wiped may include the format and possibly size information,
 * so non-raw images might become raw with a different size. It is storage
 * backend dependent whether the format and size information is regenerated
 * once the initial volume wipe is completed.
 *
 * Depending on the actual volume representation, this call may not
 * overwrite the physical location of the volume. For instance, files
 * stored journaled, log structured, copy-on-write, versioned, and
 * network file systems are known to be problematic.
 *
 * Returns 0 on success, or -1 on error
 */
int
virStorageVolWipe(virStorageVolPtr vol,
                  unsigned int flags)

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHSA-2016-2577.html