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