[libvirt] [PATCH 0/2] Alter refresh algorithm for volWipe

[Martin Kletzander]

On Fri, Aug 25, 2017 at 06:11:49PM -0400, John Ferlan wrote:
>
>
>On 08/25/2017 08:28 AM, Martin Kletzander wrote:
>> On Fri, Aug 25, 2017 at 07:52:25AM -0400, John Ferlan wrote:
>>>
>>>
>>> On 08/25/2017 05:44 AM, Martin Kletzander wrote:
>>>> On Thu, Aug 24, 2017 at 06:28:27PM -0400, John Ferlan wrote:
>>>>> Alter wipeVol to do same refresh operation as pool refresh would do.
>>>>>
>>>>
>>>> I think we should rather keep the format as it is.  Did I miss something
>>>> here or isn't the source of the problem just the fact that we wipe the
>>>> volume without keeping the format?
>>>>
>>>
>>> Once you "wipe" the file/image the format that was there (such as qcow2
>>> as noted in the bz from patch2) is no longer there.
>>>
>>> Consider the following sequence:
>>>
>>> virsh vol-create-as default bz 10M --format qcow2
>>>
>>> virsh vol-dumpxml bz default | grep format
>>>    <format type='qcow2'/>
>>>
>>> qemu-img info /home/vm-images/bz
>>> image: /home/vm-images/bz
>>> file format: qcow2
>>> virtual size: 10M (10485760 bytes)
>>> disk size: 196K
>>> cluster_size: 65536
>>> Format specific information:
>>>    compat: 0.10
>>>    refcount bits: 16
>>>
>>> virsh vol-wipe bz default
>>>
>>> qemu-img info /home/vm-images/bz
>>> image: /home/vm-images/bz
>>> file format: raw
>>> virtual size: 196K (200704 bytes)
>>> disk size: 196K
>>>
>>> virsh vol-dumpxml bz default | grep format
>>>    <format type='qcow2'/>
>>>
>>> (without the patch in 2/2 of course)
>>>
>>> BTW: I did consider just changing the format to RAW regardless, but
>>> figured that was just too simple and may not be the right answer for
>>> every case.
>>>
>>
>> Sure, but that's not my point.  My point is that instead of rewriting
>> the data with zeros, we could re-initialize it.  Anyway, from what I see
>> in the docs, we "fixed" it by documenting this behaviour already, so my
>> point is moo:
>>
>> https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolWipe
>>
>
>Doh! I should have looked there too...  Interesting though... So should
>we instead close this bug as documented to work that way?  Or should the
>text there be modified as well since the bz will cause the alteration to
>at least RAW for "most" storage backends - I think it's only the disk
>backend w/ an extended partition and a sparse logical volume that cannot
>be wiped, but those fail the wipe - so we'd never get to the change the
>target.format code.
>

I think that all the problems are essentially caused by people
misunderstanding the API's purpose.  That's why I ACKed it, as we at
least report the right information.  I think your solution is precisely
the midpoint between not doing anything and fixing all corner cases.

>John
>
>
>> (I guess I should know that when I pushed that documentation into the tree)
>>
>> So ACK series.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170829/b8489afa/attachment-0001.sig>