On Tuesday 23 June 2015 06:29 PM, Ján Tomko wrote: > On Mon, Jun 22, 2015 at 05:09:18PM +0530, Prerna Saxena wrote: >> From: Prerna Saxena <prerna@xxxxxxxxxxxxxxxxxx> >> Date: Mon, 22 Jun 2015 02:54:32 -0500 >> >> When virsh vol-clone is attempted on a raw file where capacity > allocation, >> the resulting cloned volume has a size that matches the virtual-size of >> the parent; in place of matching its actual, disk size. >> This patch fixes the cloned disk to have same _allocated_size_ as >> the parent file from which it was cloned. >> >> Ref: http://www.redhat.com/archives/libvir-list/2015-May/msg00050.html >> >> Also fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1130739 >> >> Signed-off-by: Prerna Saxena <prerna@xxxxxxxxxxxxxxxxxx> >> --- >> src/storage/storage_backend.c | 10 +++++----- >> src/storage/storage_driver.c | 5 ----- >> 2 files changed, 5 insertions(+), 10 deletions(-) >> >> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c >> index ce59f63..c99b718 100644 >> --- a/src/storage/storage_backend.c >> +++ b/src/storage/storage_backend.c >> @@ -342,7 +342,7 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn ATTRIBUTE_UNUSED, >> goto cleanup; >> } >> >> - remain = vol->target.allocation; >> + remain = vol->target.capacity; >> >> if (inputvol) { >> int res = virStorageBackendCopyToFD(vol, inputvol, >> @@ -397,7 +397,7 @@ createRawFile(int fd, virStorageVolDefPtr vol, >> virStorageVolDefPtr inputvol, >> bool reflink_copy) >> { >> - bool need_alloc = true; >> + bool need_alloc = !(inputvol && (inputvol->target.capacity > inputvol->target.allocation)); > Comparing 'inputvol->target.capacity > vol->target.allocation' would > allow creating a sparse volume from a non-sparse one and vice-versa > by specifying the correct allocation. Ok, I was not sure if libvirt wanted to do that. If the parent volume is a sparse volume, I'd expect the cloned volume to be sparse too. Likewise, for a non-sparse parent, the cloned volume should also be non-sparse. Should that not be something we honour in libvirt, when we clone ? >> int ret = 0; >> unsigned long long remain; >> >> @@ -420,7 +420,7 @@ createRawFile(int fd, virStorageVolDefPtr vol, >> * to writing zeroes block by block in case fallocate isn't >> * available, and since we're going to copy data from another >> * file it doesn't make sense to write the file twice. */ >> - if (vol->target.allocation) { >> + if (vol->target.allocation && need_alloc) { >> if (fallocate(fd, 0, 0, vol->target.allocation) == 0) { >> need_alloc = false; >> } else if (errno != ENOSYS && errno != EOPNOTSUPP) { >> @@ -433,14 +433,14 @@ createRawFile(int fd, virStorageVolDefPtr vol, >> } >> #endif >> >> - remain = vol->target.allocation; >> + remain = vol->target.capacity; >> >> if (inputvol) { >> /* allow zero blocks to be skipped if we've requested sparse >> * allocation (allocation < capacity) or we have already >> * been able to allocate the required space. */ >> bool want_sparse = !need_alloc || >> - (vol->target.allocation < inputvol->target.capacity); >> + (inputvol->target.allocation < inputvol->target.capacity); > If allocation < capacity, then need_alloc is already false. > I was trying to accomodate the original usecase of need_alloc, but you are right. This will go away in the next version of this series, which I will post shortly. -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list