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

[John Ferlan]

On 02/03/2016 03:23 AM, Ján Tomko wrote:
> On Tue, Feb 02, 2016 at 11:14:01AM -0500, John Ferlan wrote:
>>
>>
>> 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?
> 
> Even if they are mixed in the same VG, we could show the "thick" LVs
> in the type='logical' pool and the thin ones in type='thin'.
> 

We could if our logic to create/build the pool was overhauled or perhaps
support was added to have two volume groups use the same source device.

Since it's possible to place a thin_pool in the same vg as a snapshot
and a "thick" lv, should we be in the business of splitting hairs and
trying to define how things should be viewed?

Also libvirt doesn't provide the tools to expand the thin pool or add
more devices/extents for the volume group. The management of that is
done via the storage management software - we are hands off. So I think
with respect to defining how a vg can be populated, we should remain
neutral as well.

>> 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.
>>
> 
> This is my reasoning - with all the thick and thin volumes grouped
> together, having free space in one does not mean there is free space for
> another.
> 

I think we allow LVM to take care of space management. We can can
request creation of a specific set of sizes and then display from what
lvs tells us. A thin volume provides its 'data_percent' full, so we
could calculate 'available' based on that. In fact that field is also
useful for snapshot lv's... I think we may be displaying it wrong now.
For capacity, the libvirt output shows a calculated size based on an
lseek of the target device path. The lvs output shows essentially the
virtualsize.

Perhaps what is not logical (sorry) between the 'pool-info' and
'vol-list --details' output is having the calculation of all capacity
values in a vol-list be more than the capacity of the pool-info.

>> So should I assume then the desire from the review viewpoint is to not
>> support thin lv's at this point in time?
> 
> s/time/code/
> But I cannot speak for the silent majority.
> 

If a tree falls in the forest, the only ones that hear it are the ones
listening. So, how do we know who is paying attention to this upstream
discussion if they don't speak?

In this case, is silence golden? Does it mean being complicit or does it
mean indifference?  I'm all for getting a solution, but it doesn't
necessarily help being silent until patches are posted or pushed.

>> 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.
>>
> 
> How so? This patch displays the thin ('V') volumes in type='logical'
> pools. With a new pool type, the 'logical' pool could keep ignoring
> the thin pools and volumes, just like we skip directories in 'fs' pools.
> 

Perhaps a too literal comment - in order to display thin lv's we'd need
the "(\\S*)" if the same exact regex was used to find all lv's in the
pool. Alternatively one could not care about devices in a different/new
regex, but would still need to filter based on "pool_lv".

I don't see the benefit behind requiring the user to decide whether to
have a libvirt pool view either thin lv's or non-thin lv's of their vg
or requiring their vg to essentially be one thin-pool in order to
support thin lv's, when we could support whatever configuration they've
already developed.

If more work is done in LV pools - are we going to create new pool types
for other lvm segtypes ("mirror", "stripe", "raid", etc.)? In order to
display/fetch interesting or specific things about them?

Although there is an RFE/BZ to support pools of thin lvm (1060287 open
since 1/31/14) - it doesn't appear it's been deemed important enough to
get into any recent release.

Also rather than clutter this discussion up (and to not lose some
current thoughts), I updated the bz with lots of thoughts.

>> 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)?
>>
> 
> They do look more readable to me.
> 

I'll post a patch in a bit...

John