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

[Avihai Horon <avihaih@nvidia.com>]

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 :)