[libvirt] [PATCH 4/4] logical: Adjust regex for devices

[John Ferlan]

On 01/26/2016 09:55 AM, Pavel Hrdina wrote:
> On Fri, Jan 22, 2016 at 05:21:06PM -0500, John Ferlan wrote:
>> From: Joe Harvell <joe.harvell at tekcomms.com>
>>
>> Since our 'devices' parsing logic now will use the 'nextents' (or
>> lvs 'stripes' output) to decide whether or not to parse the field,
>> use the regex of "(\\S*)" (e.g. zero or more) instead of "(\\S+)"
>> (1 or more) when grabbing the 'groups[3]' or 'devices' field.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  src/storage/storage_backend_logical.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
>> index 3010f58..2af3e69 100644
>> --- a/src/storage/storage_backend_logical.c
>> +++ b/src/storage/storage_backend_logical.c
>> @@ -333,7 +333,8 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
>>       *    striped, so "," is not a suitable separator either (rhbz 727474).
>>       */
>>      const char *regexes[] = {
>> -       "^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S+)#([0-9]+)#(\\S+)#([0-9]+)#([0-9]+)#(\\S+)#?\\s*$"
>> +/*             name   orig   uuid   devs  stripes  segsz  vgextsz       sz lvattr */
>> +       "^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S*)#([0-9]+)#(\\S+)#([0-9]+)#([0-9]+)#(\\S+)#?\\s*$"
>>      };
>>      int vars[] = {
>>          9
> 
> This could be squashed into the previous commit and if you are creating a
> comment with labels, please use the exact names that lvs uses.
> 

I'll just remove the comment - yes it's shorthand, but "vg_extent_size"
is a bit long for the "([0-9]+)"...

BTW: I went separately mainly to make it more obvious of the change.

Patch 3 was just dealing with removing the need for the 'segtype' field
as it was made obsolete by just using the stripes field to fill in the
'nextents'.

Patch 4 alters the regex to allow 0 or more, rather than at least 1.
Although I do understand why combining them is also perfectly
reasonable. I was trying not to do two things at one time...

However, after some more "investigation" of the environment - I think
perhaps there's still a couple of loose ends. Keeping the 'segtype'
field may be necessary/useful... Details follow if interested ;-)

A 'vol-dumpxml' on a "found" thin lv from a thin-pool will provide an
empty "<source>" and the capacity == allocation; whereas, a
'vol-dumpxml' on a snapshot lv will provide the "<source>...<device
path='/dev/sdX'> with the <extent start='#' end='#'/> where the size of
the extent of course matches the allocation value when seen with vol-info.

The <source> of a thin lv is a lv thin-pool and there is no extents data
(e.g. nextents == 0). However, there is "pool_lv" data for a "thin"
segtype which could then be used for display in the
"<source>...<name>". There's still no way to "create" a thin pool from
<name>, but finding/displaying one is still possible... The details of
which "/dev/sdX" is used is hidden by the lv mgr thin_pool code.

We don't display the thin_pool because by itself since it cannot be used
as a volume, but that pool contains the information about the real
sizes, so it's useful. Right now there's no way to get that useful
information so going with the "size" (e.g. allocation) value would have
to suffice. Future patches can perhaps adjust this.

John