Re: [Qemu-devel] [Qemu-block] migrate -b problems

[Dr. David Alan Gilbert]

* Kevin Wolf (address@hidden) wrote:
> Am 18.04.2017 um 16:47 hat Stefan Hajnoczi geschrieben:
> > On Wed, Apr 12, 2017 at 11:18:19AM +0200, Kevin Wolf wrote:
> > > after getting assertion failure reports for block migration in the last
> > > minute, we just hacked around it by commenting out op blocker assertions
> > > for the 2.9 release, but now we need to see how to fix things properly.
> > > Luckily, get_maintainer.pl doesn't report me, but only you. :-)
> > > 
> > > The main problem I see with the block migration code (on the
> > > destination) is that it abuses the BlockBackend that belongs to the
> > > guest device to make its own writes to the image file. If the guest
> > > isn't allowed to write to the image (which it now isn't during incoming
> > > migration since it would conflict with the newer style of block
> > > migration using an NBD server), writing to this BlockBackend doesn't
> > > work any more.
> > > 
> > > So what should really happen is that incoming block migration creates
> > > its own BlockBackend for writing to the image. Now we don't want to do
> > > this anew for every incoming block, but ideally we'd just create all
> > > necessary BlockBackends upfront and then keep using them throughout the
> > > whole migration. Is there a way to get some setup/teardown callbacks
> > > at the start/end of the migration that could initialise and free such
> > > global data?
> > 
> > It can be done in the beginning of block_load() similar to
> > block_mig_state.bmds_list, which is created in init_blk_migration() at
> > save time.
> 
> The difference is that block_load() is the counterpart for
> block_save_iterate(), not for init_blk_migration(). That is, it is
> called for each chunk of block migration data, which is interleaved with
> normal RAM migration chunks.
> 
> So we can either create each BlockBackend the first time we need it in
> block_load(), or create BlockBackends for all existing device BBs and
> BDSes the first time block_load() is called. We still need some place
> to actually free the BlockBackends again when the migration completes.
> 
> Dave suggested migration state notifiers, which looked like an option,
> but at least the existing migration states aren't enough, because the
> BlockBackends need to go away before blk_resume_after_migration() is
> called, but MIGRATION_STATUS_COMPLETED is set only afterwards.
> 
> > We can also move the if (blk != blk_prev) blk_invalidate_cache() code
> > out of the load loop.  It should be done once when setting up
> > BlockBackends.
> 
> Same problem as above, while saving has setup/cleanup callbacks, we only
> have the iterate callback for loading.


Yes, and while we have the notifier chain for the source on migration state
changes we don't have the notifier on the destination.

If we just add a load_cleanup  member to SaveVMHandlers and call all of them
at the end of an inbound migration would that be enough?
(And define 'end')

Dave

> > > The other problem with block migration is that is uses a BlockBackend
> > > name to identify which device is migrated. However, there can be images
> > > that are not attached to any BlockBackend, or if it is, the BlockBackend
> > > might be anonymous, so this doesn't work. I suppose changing the field
> > > to "device name if available, node-name otherwise" would solve this.
> > 
> > Yes, that sounds good and is backwards compatible.
> 
> Kevin


--
Dr. David Alan Gilbert / address@hidden / Manchester, UK