Re: [Qemu-block] [PATCH v4 19/21] block: Make bdrv_is_allocated() byte-based

[Eric Blake]

On 07/06/2017 11:02 AM, Kevin Wolf wrote:

>> +++ b/qemu-img.c
>> @@ -3229,6 +3229,7 @@ static int img_rebase(int argc, char **argv)
>>          int64_t new_backing_num_sectors = 0;
>>          uint64_t sector;
>>          int n;
>> +        int64_t count;
>>          float local_progress = 0;
>>
>>          buf_old = blk_blockalign(blk, IO_BUF_SIZE);
>> @@ -3276,12 +3277,14 @@ static int img_rebase(int argc, char **argv)
>>              }
>>
>>              /* If the cluster is allocated, we don't need to take action */
>> -            ret = bdrv_is_allocated(bs, sector, n, &n);
>> +            ret = bdrv_is_allocated(bs, sector << BDRV_SECTOR_BITS,
>> +                                    n << BDRV_SECTOR_BITS, &count);
>>              if (ret < 0) {
>>                  error_report("error while reading image metadata: %s",
>>                               strerror(-ret));
>>                  goto out;
>>              }
>> +            n = DIV_ROUND_UP(count, BDRV_SECTOR_SIZE);
>>              if (ret) {
>>                  continue;
>>              }
> 
> Same thing. Sectors that are half allocated and half free must be
> considered completely allocated if the caller can't do byte alignment.
> Just rounding up without looking at ret isn't correct.

In reality, qemu-img rebase should probably be rewritten to exploit
bdrv_get_block_status() instead of the weaker bdrv_is_allocated(). Right
now, it is manually calling buffer_is_zero() on every sector to learn
what sectors are candidates for optimizing, rather than being able to
exploit BDRV_BLOCK_ZERO where possible (yes, we may STILL want to call
buffer_is_zero() on BDRV_BLOCK_DATA when we are trying to squeeze out
all parts of an image that can be compressed, but maybe that should be
an option, just as GNU dd has options for creating as much sparseness as
possible (slow, because it reads data) vs. preserving existing
sparseness (faster, because it relies on hole detection but leaves
explicit written zeroes as non-sparse).  I guess there's still lots of
ground for improving qemu-img, so for THIS series, I'll just leave it at
an assertion that things are sector-aligned.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature