Re: [PATCH 2/2] Storage : Fix cloning of raw, sparse volumes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]