Re: [Qemu-devel] [PATCH v3 0/7] migration: pause-before-switchover

[Dr. David Alan Gilbert]

* Peter Xu (address@hidden) wrote:
> On Thu, Oct 19, 2017 at 12:21:23PM +0100, Dr. David Alan Gilbert wrote:
> > * Peter Xu (address@hidden) wrote:
> > > On Wed, Oct 18, 2017 at 06:40:06PM +0100, Dr. David Alan Gilbert (git) 
> > > wrote:
> > > 
> > > [...]
> > > 
> > > > The precopy flow is:
> > > > active->pre-switchover->device->completed
> > > > 
> > > > The postcopy flow is:
> > > > active->pre-switchover->postcopy-active->completed
> > > 
> > > The naming is still slightly confusing to me:
> > > 
> > > (1) we have a capability called "pause-before-switchover", so it feels
> > >     like there is something called "switchover" and if we enable this
> > >     we'll pause before that point;
> > > 
> > > (2) we have a new status "pre-switchover", it feels like that's the
> > >     point before we are in "switchover" state;
> > > 
> > > (3) we don't really have a "switchover" state, but instead it's called
> > >     "device" which is exactly the "switchover" action.
> > > 
> > > Considering (1) and (2), I would prefer "device" state to be just
> > > "switchover"...
> > 
> > Yes I stuck to pause-before-device and device originally; but
> > what we're doing during the 'device' stage is mostly saving device
> > state; the actual switchover occurs at the end.  So hmm.
> 
> That's fine to me.
> 
> > 
> > > Further, not sure we can unify the state transition as well (say, we
> > > add this switchover state even without cap "pause-before-switchover"
> > > set, although it does not make much sense itself). Then, we can also
> > > unify the precopy/postcopy state machine into one:
> > > 
> > > active->
> > >   [pre-switchover->]      (optional, decided by "pause-before-switchover")
> > >     switchover->
> > >       [postcopy-active->] (optional, decided by "postcopy-arm")
> > >         completed
> > 
> > I didn't want to change the state transition behaviour without the
> > capability set, since that could upset an existing libvirt that would
> > get confused by the new state.
> 
> Indeed.  However this (and also Juan's xbzrle cache size series) lets
> me think about whether we should loosen the "compatibility" sometimes.
> 
> For most of the times, we are paying the compatibility bill by
> complicating the code logic.  For this one, we satisfy live block
> migration logic to introduce two new state transition paths (for
> precopy and postcopy). I am just afraid we need to pay a larger bill
> some day.
> 
> But I'd say it's only my worry; maybe it's just too superfluous.

Yes, it's true - almost all the behaviour we have forms part of our
API that we expose to libvirt; we have to be pretty careful.

> (I provided all r-bs, so the series looks good to me after all)

Thanks! One comment fix coming up soon as spotted by Jiri.

Dave

> 
> Thanks,
> 
> -- 
> Peter Xu
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK