Re: [Qemu-devel] [PULL 11/13] migration: add postcopy migration of dirty bitmaps

[Peter Maydell]

On 13 March 2018 at 21:14, John Snow <address@hidden> wrote:
> From: Vladimir Sementsov-Ogievskiy <address@hidden>
>
> Postcopy migration of dirty bitmaps. Only named dirty bitmaps are migrated.
>
> If destination qemu is already containing a dirty bitmap with the same name
> as a migrated bitmap (for the same node), then, 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.

Hi. Coverity complains (CID1390627) about a resource leak in this function;

> +static int dirty_bitmap_load_bits(QEMUFile *f, DirtyBitmapLoadState *s)
> +{
> +    uint64_t first_byte = qemu_get_be64(f) << BDRV_SECTOR_BITS;
> +    uint64_t nr_bytes = (uint64_t)qemu_get_be32(f) << BDRV_SECTOR_BITS;
> +    trace_dirty_bitmap_load_bits_enter(first_byte >> BDRV_SECTOR_BITS,
> +                                       nr_bytes >> BDRV_SECTOR_BITS);
> +
> +    if (s->flags & DIRTY_BITMAP_MIG_FLAG_ZEROES) {
> +        trace_dirty_bitmap_load_bits_zeroes();
> +        bdrv_dirty_bitmap_deserialize_zeroes(s->bitmap, first_byte, nr_bytes,
> +                                             false);
> +    } else {
> +        size_t ret;
> +        uint8_t *buf;
> +        uint64_t buf_size = qemu_get_be64(f);
> +        uint64_t needed_size =
> +            bdrv_dirty_bitmap_serialization_size(s->bitmap,
> +                                                 first_byte, nr_bytes);
> +
> +        if (needed_size > buf_size ||
> +            buf_size > QEMU_ALIGN_UP(needed_size, 4 * sizeof(long))
> +             /* Here used same alignment as in send_bitmap_bits */
> +        ) {
> +            error_report("Migrated bitmap granularity doesn't "
> +                         "match the destination bitmap '%s' granularity",
> +                         bdrv_dirty_bitmap_name(s->bitmap));
> +            return -EINVAL;
> +        }
> +
> +        buf = g_malloc(buf_size);

Here we allocate memory into buf...

> +        ret = qemu_get_buffer(f, buf, buf_size);
> +        if (ret != buf_size) {
> +            error_report("Failed to read bitmap bits");
> +            return -EIO;

...but in this error-exit path we do not free it.

> +        }
> +
> +        bdrv_dirty_bitmap_deserialize_part(s->bitmap, buf, first_byte, 
> nr_bytes,
> +                                           false);
> +        g_free(buf);
> +    }
> +
> +    return 0;
> +}

thanks
-- PMM