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

[Pavel Hrdina]

On Wed, Jan 27, 2016 at 03:26:12PM -0500, John Ferlan wrote:
> ...
> >>
> >> 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'.
> > 
> 
> The 'segtype' is currently only required because commit id '82c1740a'
> added 'segtype' and 'stripes' to resolve a bz (or more than one
> depending on how far you chase) by using 'segtype' to determine whether
> more than one existed. It also did quite a few things in one step which
> by today's review standards is a bad thing ;-).
> 
> However, that only worked for "striped" lv's. If there was a "mirror"
> lv, then while the mirror could be found, the vol-dumpxml output is
> wrong because it only parsed 1 extent (incorrectly at that):

That leads me to conclusion, that we should parse only the segtypes that we know
how to parse.  The rest should be ignored.

> 
>   <source>
>     <device path='lv_test_mirror_rimage_0(0),lv_test_mirror_rimage_1'>
>       <extent start='0' end='33554432'/>
>     </device>
>   </source>
> 
> instead of something like (if using 'stripes' to get nsegments):
> 
>   <source>
>     <device path='lv_test_mirror_rimage_0'>
>       <extent start='0' end='33554432'/>
>     </device>
>     <device path='lv_test_mirror_rimage_1'>
>       <extent start='0' end='33554432'/>
>     </device>
>   </source>
> 
> Linking 'nsegments' to 'striped' lv's is a "limitation".
> 
> Anyway, dropping 'segtype' wasn't necessarily bad unless you needed
> "more information", such as perhaps the lv thin pool source when
> nsegments == 0. This becomes obvious once the change to use "(\\S*)"
> instead of "(\\S+)" hits. Now we can "see" thin lv's in a volume group.
> Not sure what else becomes visible, though.  The problem is you then get:

But we need more information and we propagate those information to user via our
API and that's why I think that we should parse only those volume which we know
how to parse to pass correct values to user.

Pavel

> 
>  <source>
>  </source>
> 
> But that's fixable... The interesting part about <source> is that it's
> an output only (virStorageVolDefParseXML doesn't read it). So, by adding
> a new parse field 'pool_lv', we can then check 'segtype' to be "thin"
> and if so, store the new field in a new vol->source.thin_pool field.
> 
> Still cannot create a thin lv pool/volume without using the lvcreate
> command. Nor can one create a mirror or stripe lv using libvirt's
> vol-create command. One just cannot "find" a thin lv right now...
> 
> John
>