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@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