[libvirt] [PATCH v2 3/6] logical: Search for a segtype of "thin" and mark lv as sparse

[John Ferlan]

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 at redhat.com>
>> ---
>>  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