Re: [Qemu-devel] [PATCHv2 08/11] block-migration: efficiently encode zero blocks

[Peter Lieven]

Am 01.07.2013 um 16:13 schrieb Stefan Hajnoczi <address@hidden>:

> On Thu, Jun 27, 2013 at 03:11:32PM +0200, Peter Lieven wrote:
> 
> This patch breaks cross-version blog migration.  We need to control
> whether or not to use the new BLK_MIG_FLAG_ZERO_BLOCK flag.
> 
>> diff --git a/block-migration.c b/block-migration.c
>> index 2fd7699..99b3757 100644
>> --- a/block-migration.c
>> +++ b/block-migration.c
>> @@ -29,6 +29,7 @@
>> #define BLK_MIG_FLAG_DEVICE_BLOCK       0x01
>> #define BLK_MIG_FLAG_EOS                0x02
>> #define BLK_MIG_FLAG_PROGRESS           0x04
>> +#define BLK_MIG_FLAG_ZERO_BLOCK         0x08
>> 
>> #define MAX_IS_ALLOCATED_SEARCH 65536
>> 
>> @@ -114,16 +115,29 @@ static void blk_mig_unlock(void)
>> static void blk_send(QEMUFile *f, BlkMigBlock * blk)
>> {
>>     int len;
>> +    int flags = BLK_MIG_FLAG_DEVICE_BLOCK;
>> +    
>> +    if (buffer_is_zero(blk->buf, BLOCK_SIZE)) {
>> +        flags |= BLK_MIG_FLAG_ZERO_BLOCK;
>> +    }
>> 
>>     /* sector number and flags */
>>     qemu_put_be64(f, (blk->sector << BDRV_SECTOR_BITS)
>> -                     | BLK_MIG_FLAG_DEVICE_BLOCK);
>> +                     | flags);
> 
> blk->sector is int64_t and flags is signed int.  This conversion will
> sign-extend from 32-bit flags up to 64-bits.
> 
> Flags should be uint64_t or at least unsigned so that we don't hit
> sign-extension when BLK_MIG_FLAG_x uses the top bit.
> 
>> 
>>     /* device name */
>>     len = strlen(blk->bmds->bs->device_name);
>>     qemu_put_byte(f, len);
>>     qemu_put_buffer(f, (uint8_t *)blk->bmds->bs->device_name, len);
>> 
>> +    /* if a block is zero we need to flush here since the network
>> +     * bandwidth is now a lot higher than the storage device bandwidth.
>> +     * thus if we queue zero blocks we slow down the migration */
>> +    if (flags & BLK_MIG_FLAG_ZERO_BLOCK) {
>> +        qemu_fflush(f);
>> +        return;
>> +    }
> 
> Not sure I understand this.  Is the problem that the other side may
> require an slow writev() to fill zeros?  So you want to tell the
> destination about the zeroes ASAP.

Sorry, missed this question. Yes. If a lot of zero blocks is queued it delays
migration because the target is idle while the source is queuing zero blocks.

Peter