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

[Peter Xu <peterx@redhat.com>]

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.

-- 
Peter Xu