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

[Pavel Hrdina]

On Wed, Jan 27, 2016 at 10:05:36AM -0500, John Ferlan wrote:
> 
> 
> 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 ;-)

Yes, you're right and I did a bad job during review.  The segtype is required
and we should always consider it, there is like 20 segtypes right now and some
of them could also use 'stripes'.

> 
> 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
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list