Re: [Qemu-devel] [PATCHv2 13/20] block: introduce bdrv_zeroize

[Peter Lieven]

Am 19.09.2013 um 22:26 schrieb Eric Blake <address@hidden>:

> On 09/17/2013 07:48 AM, Peter Lieven wrote:
>> this patch adds a call to completely zero out a block device.
>> the operation is sped up by checking the block status and
>> only writing zeroes to the device if they currently do not
>> return zeroes. optionally the zero writing can be sped up
>> by setting the flag BDRV_REQ_MAY_UNMAP to emulate the zero
>> write by unmapping if the driver supports it.
>> 
>> Signed-off-by: Peter Lieven <address@hidden>
>> ---
>> block.c               |   30 ++++++++++++++++++++++++++++++
>> include/block/block.h |    1 +
>> 2 files changed, 31 insertions(+)
>> 
>> diff --git a/block.c b/block.c
>> index ecc5be4..88b137c 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -2342,6 +2342,36 @@ int bdrv_write_zeroes(BlockDriverState *bs, int64_t 
>> sector_num,
>>                       BDRV_REQ_ZERO_WRITE | flags);
>> }
>> 
>> +int bdrv_zeroize(BlockDriverState *bs, BdrvRequestFlags flags)
> 
> Please add documentation in the code base about what this function does,
> and what return values mean.  (Bad practice in the past doesn't excuse
> new patches from being more maintainer-friendly)

ok ;-)

> 
>> +{
>> +    int64_t target_size = bdrv_getlength(bs) / BDRV_SECTOR_SIZE;
>> +    int64_t ret, nb_sectors, sector_num = 0;
>> +    int n;
>> +    /* split the write requests into 1MB chunks if the driver
>> +     * does not return a maximal size via bdi */
>> +    for (;;) {
>> +        nb_sectors = target_size - sector_num;
>> +        if (nb_sectors <= 0) {
>> +            return 0;
>> +        }
>> +        if (nb_sectors > INT_MAX) {
>> +            nb_sectors = INT_MAX;
>> +        }
>> +        ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &n);
>> +        if (ret & BDRV_BLOCK_ZERO) {
>> +            sector_num += n;
>> +            continue;
>> +        }
>> +        ret = bdrv_write_zeroes(bs, sector_num, n, flags & 
>> BDRV_REQ_MAY_UNMAP);
> 
> Is this intentionally throwing away all other flags except
> BDRV_REQ_MAY_UNMAP?

This is the only option that bdrv_write_zeroes currently expects, but as this 
might
change some day I don't mind to pass all flags.

Peter