On 02/01/2016 07:09 AM, Pavel Hrdina wrote:
> On Thu, Jan 28, 2016 at 05:44:06PM -0500, John Ferlan wrote:
>> For any "thin" lv's, mark the volume as a sparse volume so that the
>> volume wipe algorithm doesn't work. Currently thin lv's are ignored
>> because the regex requires 1 or more 'devices' listed in order to
>> process. However, a future patch will be changing this.
>>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>> src/storage/storage_backend_logical.c | 9 +++++++--
>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
>> index 3232c08..a9c6309 100644
>> --- a/src/storage/storage_backend_logical.c
>> +++ b/src/storage/storage_backend_logical.c
>> @@ -64,6 +64,8 @@ virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool,
>> }
>>
>>
>> +#define VIR_STORAGE_VOL_LOGICAL_SEGTYPE_THIN "thin"
>> +
>> struct virStorageBackendLogicalPoolVolData {
>> virStoragePoolObjPtr pool;
>> virStorageVolDefPtr vol;
>> @@ -201,12 +203,15 @@ virStorageBackendLogicalMakeVol(char **const groups,
>> }
>>
>> /* Mark the (s) sparse/snapshot lv, e.g. the lv created using
>> - * the --virtualsize/-V option. We've already ignored the (t)hin
>> + * the --virtualsize/-V option or a thin segtype as sparse. This
>> + * will make sure the volume wipe algorithm doesn't overwrite
>> + * a sparse/thin volumes. We've already ignored the (t)hin
>> * pool definition. In the manner libvirt defines these, the
>> * thin pool is hidden to the lvs output, except as the name
>> * in brackets [] described for the groups[1] (backingStore).
>> */
>> - if (attrs[0] == 's')
>> + if (attrs[0] == 's' ||
>> + STREQ(groups[4], VIR_STORAGE_VOL_LOGICAL_SEGTYPE_THIN))
>> vol->target.sparse = true;
>
> Well, I'm not sure about this code. Based on my research, the 's' means
> snapshot volume and there is another flag 'V' which is thin volume. The comment
> doesn't seems to be correct. By using --virtualsize/-V it will create snapshot
> or thin volume based on 'sparse_segtype_default' configuration option from
> /etc/lvm/lvm.conf. It would be nice to clarify this in the comment for future
> review. One more thing, why don't use || attrs[0] == 'V' ?
Yes, 's' is a 'thin snapshot volume', which is different than a 'thin
volume in a thin pool'.
The '-V'/'--virtualsize' are command line options to lvcreate to
generate a sparse volume. Because 'thin pools' became the default at
some point in time the libvirt code added a "--type snapshot" to the
command line to indicate we wanted what we had been using.
Check out commit id 'cafb934d' for some history as well as the details
left when I tried to add code to handle 'thin volume as part of a
thin_pool':
http://www.redhat.com/archives/libvir-list/2014-December/msg00706.html
Usage of attrs[0] == 'V' and comparing the segname to "thin" are the
same. Without the groups[4] search here though, then 'segname' is
unused and could be removed. Doesn't matter either way to me. Just
seemed more practical to use groups[4] (e.g. segtype) just in case we
needed it in the future.
John
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list