[libvirt] [PATCH 2/2] storage: Forbid wiping formatted volume types that are not supported

[Martin Kletzander]

On Fri, Jul 15, 2016 at 09:46:50AM +0200, Ján Tomko wrote:
>On Thu, Jul 14, 2016 at 02:27:41PM +0200, Martin Kletzander wrote:
>>Until now we allowed that to happen, however the only thing we supported
>>was either rewiting the file or truncating it.  That however doesn't
>>keep the format of that file, so QCOWs, VDIs and all others just became
>>RAW with arbitrary size.
>
>Yes, wiping wipes the format as well. Nothing wrong with that.
>

It is not?  Even though QEMU will not start?

Also consider this:

$ virsh vol-info asdf.img default
Name:           asdf.img
Type:           file
Capacity:       10.00 GiB
Allocation:     196.00 KiB

$ virsh vol-wipe asdf.img default
Vol asdf.img wiped

$ virsh vol-info asdf.img default
Name:           asdf.img
Type:           file
Capacity:       196.00 KiB
Allocation:     196.00 KiB

Does that seem right?

>> Not to mention any domain using such volume
>>could not start anymore.  Instead of dealing with the recreation of
>>every single possible file that we have (and possibly failing due to
>>create_tool capabilities) just forbid it for now.
>
>> We even state in our
>>documentation that it has no value for file-backed volumes.
>>
>
>Where?
>

OK, my bad, can't find it.  Anyway it should be there.  "Ensure data
previously on a volume is not accessible to future reads." for me,
personally, means that you cannot get any data back by issuing read() on
the volume.  So if it is a file, I think:

  truncate(fd, 0);
  truncate(fd, size);

is enough.  Algorithm makes sense for partitions and other block (or
basically non-file backed) storage.

>  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 network file systems are known to be problematic.
>
>http://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolWipe
>
>This only says that it might not work, not that it's completely useless.
>
>I think we have a precedent for supporting marginally useful features by
>still supporting qcow encryption.
>
>>Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=868771
>>
>
>NACK to breaking functionality in order to resolve a 4-year-old
>synthetic QE-filed bug.
>

How about the fact that it boils my blood (I have no idea how to say
"pisses me off" politely) when I see that we have an API not doing it's
one job correctly and we can fix it with almost trivial patch?  Could
you elaborate on what functionality is being broken here?

>I suggest WONTFIX or NOTABUG.
>

No, it is a bug and I don't see a reason why we wouldn't fix it.

>Jan
-------------- 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/20160715/d66aa2b7/attachment-0001.sig>