qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH RESEND V3 4/6] migration: add postcopy downtime


From: Dr. David Alan Gilbert
Subject: Re: [Qemu-devel] [PATCH RESEND V3 4/6] migration: add postcopy downtime into MigrationIncommingState
Date: Fri, 28 Apr 2017 17:22:05 +0100
User-agent: Mutt/1.8.0 (2017-02-23)

* Peter Xu (address@hidden) wrote:
> On Fri, Apr 28, 2017 at 01:03:45PM +0300, Alexey Perevalov wrote:
> 
> [...]
> 
> > >>diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > >>index 21e7150..f3688f5 100644
> > >>--- a/migration/postcopy-ram.c
> > >>+++ b/migration/postcopy-ram.c
> > >>@@ -132,6 +132,14 @@ static bool ufd_version_check(int ufd, 
> > >>MigrationIncomingState *mis)
> > >>          return false;
> > >>      }
> > >>+#ifdef UFFD_FEATURE_THREAD_ID
> > >>+    if (mis && UFFD_FEATURE_THREAD_ID & supported_features) {
> > >>+        /* kernel supports that feature */
> > >>+        mis->downtime_ctx = downtime_context_new();
> > >>+        new_features |= UFFD_FEATURE_THREAD_ID;
> > >So here I know why in patch 2 new_features == 0...
> > >
> > >If I were you, I would like the series be done in below 4 patches:
> > >
> > >1. update header
> > >2. introduce THREAD_ID feature, and enable it conditionally
> > >3. squash all the downtime thing (downtime context, calculation) in
> > >    one patch here
> > >4. introduce trace
> > >
> > >IMHO that's clearer and easier for review. But I'm okay with current
> > >as well as long as the maintainers (Dave/Juan) won't disagree. :)
> > In previous series, David asked me to split one patch into 2
> > [Qemu-devel] [PATCH 3/6] migration: add UFFD_FEATURE_THREAD_ID feature
> > support
> > 
> > >There seem to be two parts to this:
> > >  a) Adding the mis parameter to ufd_version_check
> > >  b) Asking for the feature
> > 
> > >Please split it into two patches.
> > 
> > So in current patch set, I also added re-factoring, which was missed before
> > "migration: split ufd_version_check onto receive/request features part"
> 
> Sure. As long as Dave agrees, I'm okay with either way.

I'm OK with the split, it pretty much matches what I asked last time I think.

The question I still have is how is this memory-expensive feature turned
on and off by the user?
Also I think Peter had some ideas for simpler data structures, how did
that play out?

Dave


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



reply via email to

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