Bug 868771 - The virtual size of the vol should not be reduced after wiped
The virtual size of the vol should not be reduced after wiped
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: libvirt (Show other bugs)
7.0
Unspecified Unspecified
medium Severity medium
: rc
: ---
Assigned To: Martin Kletzander
Virtualization Bugs
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-22 01:55 EDT by Xuesong Zhang
Modified: 2016-11-03 14:05 EDT (History)
8 users (show)

See Also:
Fixed In Version: libvirt-2.0.0-4.el7
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2016-11-03 14:05:32 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Xuesong Zhang 2012-10-22 01:55:56 EDT
Description of problem:
The virtual size of the vol should not be reduced after wiped.

Version-Release number of selected component (if applicable):
libvirt-0.10.2-4.el6
qemu-kvm-rhev-0.12.1.2-2.317.el6
kernel-2.6.32-309.el6

How reproducible:
100%

Steps to Reproduce:
1. create a vol, which format is qcow2:
#virsh vol-create-as default test2.img 512k --format qcow2
Vol test2.img created
2. make sure the vol info is same as expected in the 1st step.
#qemu-img info test2.img 
image: test2.img
file format: qcow2
virtual size: 512K (524288 bytes)
disk size: 136K
cluster_size: 65536
3. wipe the vol 
#virsh vol-wipe test2.img default
Vol test2.img wiped
4. check the info of the vol
qemu-img info test2.img 
image: test2.img
file format: raw
virtual size: 256K (262144 bytes)
disk size: 0
  
Actual results:
As you can see in step 2 and step 4, the following info has been changed after wiped:
file format: changed from qcow2 to raw (this is as expected)
virtual size: changed from 512k to 256k (The virtual size should not be reduced after the vol is wiped)
disk size: from 512k to 0 (this is as expected)

Expected results:
The virtual size in step2 and step4 should be same

Additional info:
Comment 2 Martin Kletzander 2013-02-05 09:49:16 EST
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.
Comment 7 Jiri Denemark 2014-04-04 17:36:47 EDT
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.
Comment 12 Martin Kletzander 2016-07-14 08:28:29 EDT
Patches proposed upstream:

https://www.redhat.com/archives/libvir-list/2016-July/msg00482.html
Comment 13 Martin Kletzander 2016-07-14 08:32:06 EDT
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.
Comment 14 Martin Kletzander 2016-08-02 01:07:50 EDT
Fixed upstream by further documenting the behaviour in commit v2.1.0-rc1-22-gfa4eea8063b9:

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

    storage: Document wiping formatted volume types
Comment 17 yisun 2016-08-10 01:44:01 EDT
Hi Martin, 
The only thing I found in downstream git log is:
===================== git log =====================
# git show d47f085cfcfc3913f6a3091488073609af4db61a
commit d47f085cfcfc3913f6a3091488073609af4db61a
Author: Martin Kletzander <mkletzan@redhat.com>
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@redhat.com>
    (cherry picked from commit fa4eea8063b972a600903e3441bf04415529a7e9)
    Signed-off-by: Martin Kletzander <mkletzan@redhat.com>

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?
Comment 18 Martin Kletzander 2016-08-10 04:18:38 EDT
(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.
Comment 19 yisun 2016-08-10 05:03:45 EDT
(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)
Comment 21 errata-xmlrpc 2016-11-03 14:05:32 EDT
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

Note You need to log in before you can comment on or make changes to this bug.