Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> wrote:
> Postcopy migration of dirty bitmaps. Only named dirty bitmaps,
> associated with root nodes and non-root named nodes are migrated.
>
> If destination qemu is already containing a dirty bitmap with the same name
> as a migrated bitmap (for the same node), than, if their granularities are
> the same the migration will be done, otherwise the error will be generated.
>
> If destination qemu doesn't contain such bitmap it will be created.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
> diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
> new file mode 100644
> index 0000000..c668d02
> --- /dev/null
> +++ b/migration/block-dirty-bitmap.c
> @@ -0,0 +1,699 @@
> +/*
> + * QEMU dirty bitmap migration
> + *
> + * Postcopy migration of dirty bitmaps. Only named dirty bitmaps, associated
> + * with root nodes and non-root named nodes are migrated.
> + *
> + * If destination qemu is already containing a dirty bitmap with the same
> name
> + * as a migrated bitmap (for the same node), than, if their granularities are
> + * the same the migration will be done, otherwise the error will be
> generated.
> + *
> + * If destination qemu doesn't contain such bitmap it will be created.
> + *
> + * format of migration:
> + *
> + * # Header (shared for different chunk types)
> + * 1, 2 or 4 bytes: flags (see qemu_{put,put}_flags)
> + * [ 1 byte: node name size ] \ flags & DEVICE_NAME
> + * [ n bytes: node name ] /
> + * [ 1 byte: bitmap name size ] \ flags & BITMAP_NAME
> + * [ n bytes: bitmap name ] /
> + *
> + * # Start of bitmap migration (flags & START)
> + * header
> + * be64: granularity
> + * 1 byte: bitmap enabled flag
> + *
> + * # Complete of bitmap migration (flags & COMPLETE)
> + * header
> + *
> + * # Data chunk of bitmap migration
> + * header
> + * be64: start sector
> + * be32: number of sectors
> + * [ be64: buffer size ] \ ! (flags & ZEROES)
> + * [ n bytes: buffer ] /
> + *
> + * The last chunk in stream should contain flags & EOS. The chunk may skip
> + * device and/or bitmap names, assuming them to be the same with the previous
> + * chunk.
> + *
> + *
> + * This file is derived from migration/block.c
> + *
> + * Author:
> + * Vladimir Sementsov-Ogievskiy <vsement...@parallels.com>
> + *
> + * original copyright message:
> + * =====================================================================
> + * Copyright IBM, Corp. 2009
> + *
> + * Authors:
> + * Liran Schour <lir...@il.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2. See
> + * the COPYING file in the top-level directory.
> + *
> + * Contributions after 2012-01-13 are licensed under the terms of the
> + * GNU GPL, version 2 or (at your option) any later version.
> + * =====================================================================
> + */
I think that the normal practice is putting first the copyright and then
the comment of the file.
> +static void qemu_put_bitmap_flags(QEMUFile *f, uint32_t flags)
> +{
> + if (!(flags & 0xffffff00)) {
> + qemu_put_byte(f, flags);
> + return;
> + }
> +
> + if (!(flags & 0xffff0000)) {
> + qemu_put_be16(f, flags | DIRTY_BITMAP_MIG_FLAGS_SIZE_16);
> + return;
> + }
> +
> + qemu_put_be32(f, flags | DIRTY_BITMAP_MIG_FLAGS_SIZE_32);
> +}
Do need flags so many times to be a good idea to spend two flags and
make the code more complex? Couldn't just sent always the 32bit word
and call it a day?
I have only looked at the stuff quickly from the migration point of
view, not about the bitmap stuff.
> +static void send_bitmap_complete(QEMUFile *f, DirtyBitmapMigBitmapState
> *dbms)
> +{
> + send_bitmap_header(f, dbms, DIRTY_BITMAP_MIG_FLAG_COMPLETE);
> +}
> +
> +static void send_bitmap_bits(QEMUFile *f, DirtyBitmapMigBitmapState *dbms,
> + uint64_t start_sector, uint32_t nr_sectors)
> +{
> + /* align for buffer_is_zero() */
> + uint64_t align = 4 * sizeof(long);
> + uint64_t unaligned_size =
> + bdrv_dirty_bitmap_serialization_size(dbms->bitmap,
> + start_sector, nr_sectors);
> + uint64_t buf_size = (unaligned_size + align - 1) & ~(align - 1);
> + uint8_t *buf = g_malloc0(buf_size);
> + uint32_t flags = DIRTY_BITMAP_MIG_FLAG_BITS;
> +
> + bdrv_dirty_bitmap_serialize_part(dbms->bitmap, buf,
> + start_sector, nr_sectors);
> +
> + if (buffer_is_zero(buf, buf_size)) {
> + g_free(buf);
> + buf = NULL;
> + flags |= DIRTY_BITMAP_MIG_FLAG_ZEROES;
> + }
> +
> + DPRINTF("parameters:"
> + "\n flags: %x"
> + "\n start_sector: %" PRIu64
> + "\n nr_sectors: %" PRIu32
> + "\n data_size: %" PRIu64 "\n",
> + flags, start_sector, nr_sectors, buf_size);
Now we are adding traces, not DPRINF's to new code in general.
Same for all DPRINTFs
> +
> + send_bitmap_header(f, dbms, flags);
> +
> + qemu_put_be64(f, start_sector);
> + qemu_put_be32(f, nr_sectors);
> +
> + /* 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.
> + * also, skip writing block when migrate only dirty bitmaps. */
> + if (flags & DIRTY_BITMAP_MIG_FLAG_ZEROES) {
> + qemu_fflush(f);
I thought that we were missing the g_free(buf) here, not sure if it is
better to put the return here, or do an else for the rest of the code.
