[libvirt] [PATCH v3 4/4] logical: Display thin lv's found in a libvirt managed volume group

[John Ferlan]

On 02/02/2016 08:30 AM, Pavel Hrdina wrote:
> On Tue, Feb 02, 2016 at 09:11:51AM +0100, Ján Tomko wrote:
>> On Mon, Feb 01, 2016 at 03:29:53PM -0500, John Ferlan wrote:
>>> Modify the regex for the 'devices' (a/k/a 'extents') from "(\\S+)"
>>> (e.g., 1 or more) to "(\\S*)" (e.g., zero or more).
>>>
>>> Then for any "thin" lv's found, mark the volume as a sparse volume so
>>> that the volume wipe algorithm doesn't work.
>>>
>>> Since a "thin" segtype has no devices, this will result in any "thin"
>>> lv part of some thin-pool within a volume group used as a libvirt pool
>>> to be displayed as a possible volume to use.
>>>
>>
>> A thin pool is another layer on top of some of the LVs in the VG.
>> I think it deserves a separate pool type.
> 
> I must agree with Jan, and also already wrote the same for v2 patch.  The thin
> pool and thin LV shouldn't be mixed with normal logical pool even they share the
> same VG.
> 

If LVM allows it, then who are we to say it cannot or shouldn't work or
how it should be configured? Thin pools were introduced in LVM2
(2.02.89) according to what I've found. That's a few releases behind the
2.02.132 I have on my f23 laptop. Nothing in the description creating a
thin-pool requires it's own volume group although sure some "best
practices" indicate creating vg's as the container for a thin-pool.

I can certainly understand and agree in principle to creating a "strong
preference" to have a thin-pool be a libvirt pool because it simplifies
things such as allocated/capacity management and physical device source
listing.

So should I assume then the desire from the review viewpoint is to not
support thin lv's at this point in time? And that would be because we
want them to be configured a specific way?  FWIW: if someone did that -
we still won't be able to support it without this patch.

Any thoughts on the VIR_STORAGE_VOL_LOGICAL*_REGEX's? (depending on the
answer to the prior question with the obvious change of using S+ instead
of S* for devices)?

John