Re: [PATCH v2 3/7] migration: Enable precopy initial data capability

[Avihai Horon]

On 17/05/2023 19:07, Peter Xu wrote:
> External email: Use caution opening links or attachments
>
>
> On Wed, May 17, 2023 at 06:52:15PM +0300, Avihai Horon wrote:
> > Now that precopy initial data logic has been implemented, enable the
> > capability.
> >
> > Signed-off-by: Avihai Horon <avihaih@nvidia.com>
> > Reviewed-by: Juan Quintela <quintela@redhat.com>
> > ---
> >   migration/options.c | 4 ----
> >   1 file changed, 4 deletions(-)
> >
> > diff --git a/migration/options.c b/migration/options.c
> > index 0a31921a7a..3449ce4f14 100644
> > --- a/migration/options.c
> > +++ b/migration/options.c
> > @@ -561,10 +561,6 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, 
> > Error **errp)
> >                                "capability 'return-path'");
> >               return false;
> >           }
> > -
> > -        /* Disable this capability until it's implemented */
> > -        error_setg(errp, "'precopy-initial-data' is not implemented yet");
> > -        return false;
> >       }
> I'm always confused why we need this and not having this squashed into
> patch 1 (or say, never have these lines).
>
> The only thing it matters is when someone backports patch 1 but not
> backport the rest of the patches.  But that's really, really weird already
> as a backporter doing that, and I doubt its happening.
>
> Neither should we merge patch 1 without merging follow up patches to
> master, as we should just always merge the whole feature or just keep
> reworking on the list.
>
> I'd like to know if I missed something else..

There are also git bisect considerations.
This practice is useful for git bisect for features that are enabled by 
default, so you won't mistakenly run "half a feature" if you do bisect.
But here the capability must be manually enabled, so maybe it's not that 
useful in this case.

I like it for the sake of good order, so this capability can't be 
enabled before it's fully implemented (even if it's unlikely that 
someone would do that).

> PS: sorry to be late on replying to your email for previous version due to
> travelling last week, I'll reply to your series instead.  Actually I was
> just writting up the reply to your previous version when receiving this
> one. :)

No worries, thanks :)