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