[Qemu-devel] Re: [PATCH 2/3 v5] Block live migration

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 1473 bytes --]

Hi Liran,

trying to understand the code and fixing some cosmetic issues around
progress reporting, one potentially performance-relevant question popped up:

lirans@il.ibm.com wrote:
> diff --git a/block-migration.c b/block-migration.c
> new file mode 100644
> index 0000000..4b4eddf
> --- /dev/null
> +++ b/block-migration.c

...

> +static int mig_read_device_bulk(QEMUFile *f, BlkMigDevState *bms)
> +{ 
> +    int nr_sectors;
> +    int64_t total_sectors, cur_sector = 0;
> +    BlockDriverState *bs = bms->bs;
> +    BlkMigBlock *blk;
> +    
> +    blk = qemu_malloc(sizeof(BlkMigBlock));
> +    blk->buf = qemu_malloc(BLOCK_SIZE);
> +    
> +    cur_sector = bms->cur_sector;
> +    total_sectors = bdrv_getlength(bs) >> SECTOR_BITS;

Why re-calculating total_sectors here? Specifically bdrv_getlength() can
be a non-trivial I/O operation.

And what is the difference to bms->total_sectors which looks a lot like...

[...]
> +
> +static void init_blk_migration(QEMUFile *f)
> +{
> +    BlkMigDevState **pbmds, *bmds;
> +    BlockDriverState *bs;
> +    
> +    for (bs = bdrv_first; bs != NULL; bs = bs->next) {
> +        if(bs->type == BDRV_TYPE_HD) {
> +            bmds = qemu_mallocz(sizeof(BlkMigDevState));
> +            bmds->bs = bs;
> +            bmds->bulk_completed = 0;
> +            bmds->total_sectors = bdrv_getlength(bs) >> SECTOR_BITS;

...it already contains all the information we need?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]