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

[Liran Schour]

address@hidden wrote on 25/11/2009 00:55:57:


> trying to understand the code and fixing some cosmetic issues around
> progress reporting, one potentially performance-relevant question popped
up:
>
> address@hidden 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?
>

You are right no need to figure it out once again. We can use the old
total_sectors value.

Thanks,
Liran