Date: Wed, 7 Mar 2018 11:28:53 +0800
From: Peter Xu <peterx@redhat.com>
Message-ID: <20180307032853.GF17720@xz-mi>
References: <20180305094938.31374-1-peterx@redhat.com>
	<20180306200837.GQ3096@work-vm>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20180306200837.GQ3096@work-vm>
Subject: Re: [Qemu-devel] [PATCH] migration: fix applying wrong capabilities
To: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: qemu-devel@nongnu.org, Laurent Vivier <lvivier@redhat.com>, Juan Quintela <quintela@redhat.com>

On Tue, Mar 06, 2018 at 08:08:37PM +0000, Dr. David Alan Gilbert wrote:
> * Peter Xu (peterx@redhat.com) wrote:
> > When setting migration capabilities via QMP/HMP, we'll apply them even
> > if the capability check failed.  Fix it.
> > 
> > Fixes: 4a84214ebe ("migration: provide migrate_caps_check()", 2017-07-18)
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> 
> OK, yes, that works, so:
> 
> 
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Thanks.

> 
> 
> It is a little odd in a way; 'caps_check' you might expect only checked
> and didn't change anything.   migrate_params is organised a bit
> differently; and somewhat more confusingly.

Indeed.  Maybe the cap_list copy should be within the function, and
then define the function as:

static bool migrate_caps_check(MigrationCapabilityStatusList *params,
                               Error **errp);

Then it at least looks more like the param_check one.

Let me know if you think it's good; I can post another one after
all, and this one would be easy. :)

-- 
Peter Xu