Re: [libvirt] [PATCH 1/2] logical: Use correct syntax for thin/sparse pool creation

Commit id '1ffc78b5' was supposed to support for thin logical volumes; however, instead it added support for thin snapshot volumes. This worked fine for a few releases of LVM; however, more recent versions (not sure exactly which one) made a differentiation between a thin snapshot volume and a thin volume in a thin pool. Furthermore, the creation command used by libvirt: lvcreate --name <name> -L <allocation> --virtualsize <capacity> <VGname> that used to create a thin snapshot volume will now create a thin volume and a thin pool which libvirt doesn't know how to parse, for example: # lvcreate --name test -L 2M -V 5M lvm_test Rounding up size to full physical extent 4.00 MiB Rounding up size to full physical extent 8.00 MiB Logical volume "test" created. # lvs lvm_test LV VG Attr LSize Pool Origin Data% Meta% Move Log Cpy%Sync Convert lvol1 lvm_test twi-a-tz-- 4.00m 0.00 0.98 test lvm_test Vwi-a-tz-- 8.00m lvol1 0.00 compared to the former code which had the following: LV VG Attr LSize Pool Origin Data% Move Log Cpy%Sync Convert test LVM_Test swi-a-s--- 4.00m [test_vorigin] 0.00 When using the virsh vol-create-as or vol-create xmlfile commands, this will cause libvirt to not find the 'test' volume nor mark it as sparse. It cannot find since the command used to find/parse returns a thin volume 'test' with no associated device, for example the output is: lvol1##UgUwkp-fTFP-C0rc-ufue-xrYh-dkPr-FGPFPx#lvol1_tdata(0)#thin-pool#1#4194304#4194304#4194304#twi-a-tz-- test##NcaIoH-4YWJ-QKu3-sJc3-EOcS-goff-cThLIL##thin#0#8388608#4194304#8388608#Vwi-a-tz-- as compared to the former which had the following: test#[test_vorigin]#Dt5Of3-4WE6-buvw-CWJ4-XOiz-ywOU-YULYw6#/dev/sda3(1300)#linear#1#4194304#4194304#4194304#swi-a-s--- The relevant changes being: 1. The field after the UUID and before "thin" is where it's expected to return a device now has nothing. This is used in the vol-dumpxml output; however, it doesn't seem to have other uses 2. The field after "thin" is a count of the number of extents. Which is assumed to be at least 1 by the LV processing code, but is only ever checked for a "striped" LV. 3. The first character of the last field is used to determine LV attributes. A 'V' signifies a "thin Volume", while an 's' signifies a "snapshot" (and in our usage a thin snapshot). 4. The LSize field in the new view is not the 'real' capacity of the 'test' LV - it is now kept in the pool. Formerly (and for the thin snapshot), it's managed by LVM (it can be seen via a 'lvs -a' command). While continuing to use a thin snapshot would be possible by simply changing the "lvcreate" command to add a "--type snapshot" option, it's not clear whether that was the desired result and if libvirt's model is the intended usage for LVM of a thin shapshot. However, going forward the thin volume support is what the following patch will utilize for new LV's created while also still supporting the thin snapshots since it's not clear whether there is a need to convert them and whether it's feasible/desired. Also we have to support the old format for back-compat reasons, so it just seems safer to keep things as they are. This patch will adjust the lvcreate to be a sequence of two commands when it's determined that the allocation and capacity do not match. First a lvcreate --type thin-pool -L <allocation> --thinpool thinpool_<name> <VGname> followed by a lvcreate --name <name> --thin <VGname>/thinpool_<name> \ --type thin -V <capacity> For non thin pools, it'll remain as it was: lvcreate --name <name> -L <capacity> <VGname> (or -s <backingStorePath>) Additionally, a new flag 'thinVolume' will be set to indicate the type of volume. This will be essential during delete and will also be useful for perhaps a new VolumeRefresh option to get the "correct" sizes for thin volumes. The volume processing callback (virStorageBackendLogicalMakeVol) will be changed to handle/recognize the thinVolume. It will ignore the extents and device 'source'. The effect of this is to have an empty source in the vol-dumpxml output. Being able to ignore the devices field also requires a change to the regex since it previously required something in the field. For non thin volumes that don't have the device field, the parsing algorithm already handles with a "malformed volume extent devices value" failure. The volume deletion code will now have to delete not only the LV, but the thin pool associated with the volume. NB: If we didn't provide our own name, LVM would generate one and it's at this point we'd have to figure that out; otherwise, we'd leave around thin pools in the volume group and eventually with enough of them, the VG would be needlessly exhausted.