Re: [Xen-devel] REGRESSION [PATCH v2 08/13] libxc: Check xc_domain_maximum_gpfn for negative return values

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 27/03/15 19:42, Konrad Rzeszutek Wilk wrote:
> On Thu, Mar 26, 2015 at 09:07:05PM +0000, Andrew Cooper wrote:
>> On 20/03/15 17:03, Ian Campbell wrote:
>>> On Fri, 2015-03-20 at 11:45 -0400, Konrad Rzeszutek Wilk wrote:
>>>> From 45bd7cd377b0b8364757cc2bc0bd8d6a13523a97 Mon Sep 17 00:00:00 2001
>>>> From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>>>> Date: Fri, 13 Mar 2015 14:57:44 -0400
>>>> Subject: [PATCH] libxc: Check xc_domain_maximum_gpfn for negative return
>>>>  values
>>>>
>>>> Instead of assuming everything is always OK. We stash
>>>> the gpfns value as an parameter. Since we use it in three
>>>> of places we might as well update xc_domain_maximum_gpfn
>>>> to do the right thing.
>>>>
>>>> Suggested-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
>>>> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>>> Acked + applied along with the rest of the series, thanks,
>> This change as unfortunately causes a regression in migration v2, because
>> the fenceposting has changed and the function no longer returns the maximum
>> gpfn.  It returns one past the maximum gpfn.
>>
>> It would appear that migration v2 was the only consumer which actually want
>> the max gpfn.
>>
>> Can we either rename the function to accurately name the value it returns
>> (although I am out of ideas as to what this might be), or undo the
>> fenceposting change so that it continues to return the value it claims to
>> return.
> This should do it? (I didn't fix the ';' issue in here).
>
> From a7206930f867025234966c7b784bead9b174930b Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Date: Fri, 27 Mar 2015 15:36:02 -0400
> Subject: [PATCH] libxc: Re-institute the old xc_maximum_ram_page and add
>  xc_maximum_gpfn
>
> The commit 1781f00ea62edb72bed4fe1b6959eeed427e988f
> "libxc: Check xc_maximum_ram_page for negative return values."
> broke migrationv2 (out of tree) as it wanted the raw
> value instead of the +1 manipulation the rest of the callers do.
>
> As such we resurrect xc_maximum_ram_page and rename the
> version that was added in the above mention commit to
> xc_maximum_gpfn.
>
> Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>

It is not xc_maximum_ram_page() which was the problem.  It is
xc_domain_maximum_gpfn() which had its fenceposting changed.

The new xc_maximum_ram_page() is fine, and really does want to keep its
new API.  xc_domain_maximum_gpfn() is also fine keeping its new API, but
wants to not adjust max_pfn by 1.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel