On Fri, Jun 26, 2015 at 05:13:26PM +0530, Prerna Saxena wrote: > 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 | 23 ++++++++++------------- > src/storage/storage_driver.c | 5 ----- > 2 files changed, 10 insertions(+), 18 deletions(-) > > diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c > index ce59f63..492b344 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,8 @@ createRawFile(int fd, virStorageVolDefPtr vol, > virStorageVolDefPtr inputvol, > bool reflink_copy) > { > - bool need_alloc = true; > + bool want_sparse = (inputvol && > + (inputvol->target.capacity > vol->target.allocation)); > int ret = 0; > unsigned long long remain; > > @@ -420,10 +421,9 @@ 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 (fallocate(fd, 0, 0, vol->target.allocation) == 0) { > - need_alloc = false; > - } else if (errno != ENOSYS && errno != EOPNOTSUPP) { > + if (vol->target.allocation && !want_sparse) { > + if ((fallocate(fd, 0, 0, vol->target.allocation) != 0) && If fallocate succeeds here, there's no need to copy zero bytes from the source. > + (errno != ENOSYS && errno != EOPNOTSUPP)) { > ret = -errno; > virReportSystemError(errno, > _("cannot allocate %llu bytes in file '%s'"), > @@ -433,22 +433,19 @@ createRawFile(int fd, virStorageVolDefPtr vol, > } > #endif > > - remain = vol->target.allocation; > + remain = vol->target.capacity; > This breaks sparse file creation on systems without fallocate. > 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); > - > + * allocation (allocation < capacity) > + */ > ret = virStorageBackendCopyToFD(vol, inputvol, fd, &remain, > want_sparse, reflink_copy); I'd rahter use !need_alloc instead of want_sparse here, because sparseness only makes sense for cloning volumes, but the bool affects creating new volumes as well. > if (ret < 0) > goto cleanup; > } > > - if (remain && need_alloc) { > + if (remain && !want_sparse) { > if (safezero(fd, vol->target.allocation - remain, remain) < 0) { This calculation assumes that 'remain' contains the bytes remaining to the target allocation, but after this patch it contains the bytes remaining to the target capacity. > ret = -errno; > virReportSystemError(errno, _("cannot fill file '%s'"), I'll send a v4 shortly. Jan
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list