Re: [Qemu-devel] [PATCH] migration: notify runstate immediately before vcpu stops

[Peter Xu <zhexu@redhat.com>]

On Tue, Jul 16, 2019 at 03:57:49AM -0400, Yan Zhao wrote:
> On Tue, Jul 16, 2019 at 03:50:25PM +0800, Peter Xu wrote:
> > On Tue, Jul 16, 2019 at 03:29:19AM -0400, Yan Zhao wrote:
> > > On Tue, Jul 16, 2019 at 03:23:16PM +0800, Peter Xu wrote:
> > > > On Tue, Jul 16, 2019 at 03:10:42PM +0800, Yan Zhao wrote:
> > > > > for some devices to do live migration, it is needed to do something
> > > > > immediately before vcpu stops. add a notification here.
> > > > 
> > > > Hi, Yan,
> > > > 
> > > > Could I ask for a more detailed commit message here?  E.g., what is
> > > > "some devices"?  And, what's the problem behind?
> > > >
> > > hi Peter,
> > > 
> > > Some devices refer to assigned devices, like NICs.
> > > For assigned devices to do live migration, it is sometimes required that
> > > source device is stopped before stopping source vcpus. vcpus can do some
> > > final cleanups (like handling interrupt) in that case.
> > 
> > I see, so this is a prerequisite of another work?
> 
> Yes.
> > 
> > IMHO it would make more sense to have this patch to be with that
> > patchset, then it'll justify itself with reasoning.  Unless I
> > misunderstood - this single patch seems to help nothing if as a
> > standalone one.
> 
> It would be better. but this patch alone is also somewhat general,
> as it only adds an extra notification and wouldn't impact others.
> 
> Only after this patch is upstreamed, can VFIO live migration have a
> second choice for those devices of special requirements.
> 
> Hope it can get understanding from you:)

No you don't need my understanding, as long as the maintainer likes it
it's good enough, so you probably only need to persuade the
maintainers. :)

But again for me I would prefer patch like this to simply be the first
patch of your live migration series.  There can be some exceptions
like that when the prerequisite is too big so we'd better split them
out to do things step by step, but this patch (which is a oneliner)
should not be the case.

Thanks,

> 
> Or do you think I need to change the commit message a little to unbind
> from migration?
> 
> Thanks
> Yan
> 

-- 
Peter Xu