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

[Daniel P. Berrange]

On Fri, Dec 12, 2014 at 04:14:29PM -0500, John Ferlan wrote:
> 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.

I'm unclear still on what the difference is between a thin snapshot (with
no backing volume) and a thin volume ?

FWIW, the original intent was that this provide a volume that is equivalent
semantically to a sparse file created on a filesystem. ie the LVM equivalent
to  'dd if=/dev/zero of=foo.img seek=1G count=0'


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|