On 04/29/2016 08:35 AM, John Ferlan wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1318993 > > Commit id 'dd519a294' caused a regression cloning a volume into a > logical pool by removing just the 'allocation' adjustment during > storageVolCreateXMLFrom. Combined with the change to not require the > new volume input XML to have a capacity listed (commit id 'e3f1d2a8') > left the possibility that a copy from a larger volume into a smaller > volume would create a thin/sparse logical volume. If a thin lv becomes > fully populated, then LVM will set the partition 'inactive' and the > subsequent fdatasync() would fail. > > In order to fix this in a backend agnostic manner, only adjust the > new capacity if not provided. Likewise, if the new allocation is not > provided, then use the capacity value as we document that if omitted, > the volume will be fully allocated at time of creation. > > For a logical backend, that creation time is 'createVol', while for a > file backend, creation doesn't set the size, but the 'createRaw' called > during buildVolFrom will decide whether the file is sparse or not based > on the provided capacity and allocation value. > > For volume clones that provide different allocation and capacity values > to allow for sparse files, there is no change. > > Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> > --- > > NOTE: Related bz's to previous changes: > https://bugzilla.redhat.com/show_bug.cgi?id=1130739 > https://bugzilla.redhat.com/show_bug.cgi?id=1298065 > > I have tested using 'virsh vol-clone' and 'virt-clone' with and without > this patch applied. Both tests have the same results for the 'qemu-img info' > on the cloned file. > > src/storage/storage_driver.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c > index fcc0991..4415a4c 100644 > --- a/src/storage/storage_driver.c > +++ b/src/storage/storage_driver.c > @@ -2034,11 +2034,22 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, > goto cleanup; > } > > - /* Use the original volume's capacity in case the new capacity > - * is less than that, or it was omitted */ > - if (newvol->target.capacity < origvol->target.capacity) > + /* Use the original volume's capacity if the new capacity was omitted. > + * If the provided newvol capacity is less than original, we cannot just > + * use the original since the subsequent createVol has "competing needs" > + * for backends. A logical volume backend could then create a thin lv > + * that has different characteristics when copying from the original > + * volume than perhaps a raw disk file. */ > + if (newvol->target.capacity == 0) > newvol->target.capacity = origvol->target.capacity; > > + /* If the new allocation is 0, then use capacity as it's specifically > + * documented "If omitted when creating a volume, the volume will be > + * fully allocated at time of creation.". This is especially important > + * for logical volume creation. */ > + if (newvol->target.allocation == 0) > + newvol->target.allocation = newvol->target.capacity; > + > if (!backend->buildVolFrom) { > virReportError(VIR_ERR_NO_SUPPORT, > "%s", _("storage pool does not support" > ACK - Cole -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list