qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] ping RE: question: I found a qemu crash about migration


From: Kevin Wolf
Subject: Re: [Qemu-devel] ping RE: question: I found a qemu crash about migration
Date: Fri, 29 Sep 2017 11:35:47 +0200
User-agent: Mutt/1.9.0 (2017-09-02)

Am 28.09.2017 um 19:01 hat Dr. David Alan Gilbert geschrieben:
> Hi,
>   This is a 'fun' bug;  I had a good chat to kwolf about it earlier.
> A proper fix really needs to be done together with libvirt so that we
> can sequence:
>    a) The stopping of the CPU on the source
>    b) The termination of the mirroring block job
>    c) The inactivation of the block devices on the source
>        (bdrv_inactivate_all)
>    d) The activation of the block devices on the destination
>        (bdrv_invalidate_cache_all)
>    e) The start of the CPU on the destination
> 
> It looks like you're hitting a race between b/c;  we've had races
> between c/d in the past and moved the bdrv_inactivate_all.
> 
> During the discussion we ended up with two proposed solutions;
> both of them require one extra command and one extra migration
> capability.
> 
> The block way
> -------------
>    1) Add a new migration capability pause-at-complete
>    2) Add a new migration state almost-complete
>    3) After saving devices, if pause-at-complete is set,
>       transition to almost-complete
>    4) Add a new command (migration-continue) that 
>       causes the migration to inactivate the devices (c)
>       and send the final EOF to the destination.
> 
> You set pause-at-complete, wait until migrate hits almost-complete;
> cleanup the mirror job, and then do migration-continue.  When it
> completes do 'cont' on the destination.

Especially since you're concerned about the exact position of the pause,
the following would be a little safer (using the opportunity to suggest
slightly different names,too):

1) Add a new migration capability pause-at-complete
2) Add a new migration state ready
3) After migration has converged (i.e. the background phase of the
   migration has completed) and we are ready to actually switch to the
   destination, stop the VM, transition to 'ready' and emit a QMP event
4) Continue processing QMP commands in the 'ready' phase. This is where
   libvirt is supposed to tear down any running non-VM activities like
   block jobs, NBD servers, etc.
5) Add a new command (migration-complete) that does the final part of
   migration, including sending the remaining dirty memory, device state
   and the final EOF that signals the destination to resume the VM if -S
   isn't given.

The main reason why I advocate this "block way" is that it looks a lot
less hacky, but more future-proof to me. Instead of adding a one-off
hack for the problem at hand, it introduces a phase where libvirt can
cleanly tear down any activity it has still running on the source qemu.


We got away without doing this because we never did a clean handover of
resources from the source to the destination. From the perspective of a
management tool, we had these distinct phases during migration:

a) 'migrate' command:
   Source VM is still running and in control of all resources (like disk
   images), migrating some state in the background

b) Migration converges:
   Both VMs are stopped (assuming -S is given on the destination,
   otherwise this phase is skipped), the destination is in control of
   the resources

c) 'cont' command:
   Destination VM is running and in control of the resources

There is an asymmetry here because there is no phase where the VMs are
stopped, but the source is in control of the resources (this is the
additional state that my proposal introduces).

As long as we didn't really manage control of the resources with locks,
we could abuse b) for things where the source still accessed the
resources if we knew that the destination wouldn't use them yet (it's
not an assumption that I'm willing to trust too much) and it would still
kind of work in the common cases. But it's not really the correct thing
to do, it has probably always caused bugs in corner cases, and the
locking mechanisms are pointing that out now.

We're now noticing this with block device because we're using locking,
but I wouldn't be surprised if we found sooner or later that there are
more resources that should really be handed over in a cleanly designed
way and that work only by accident as long as you don't do that. So I
don't think a local hack for mirroring (or even all block jobs) is
sufficient to be future-proof.

> The migration way
> -----------------
>    1) Stop doing (d) when the destination is started with -S
>       since it happens anyway when 'cont' is issued
>    2) Add a new migration capability ext-manage-storage
>    3) When 'ext-manage-storage' is set, we don't bother doing (c)
>    4) Add a new command 'block-inactivate' on the source
> 
> You set ext-manage-storage, do the migrate and when it's finished
> clean up the block job, block-inactivate on the source, and
> then cont on the destination.

You would have to inactivate every single block node, which is not
a thing libvirt can currently do (they don't even really manage
individual nodes yet). Apart from that I believe it is much too
low-level and users shouldn't have to worry about what implications
migration has to block devices. A clean migration phase model should be
much easier to understand.

Kevin



reply via email to

[Prev in Thread] Current Thread [Next in Thread]