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

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Fri, Mar 27, 2015 at 10:52:07AM +0000, Ian Campbell wrote:
> On Thu, 2015-03-26 at 21:07 +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.
> 
> Oops, so it does, I noticed that but didn't fully think through the
> naming implications once I noticed all the callers being correctly
> adjusted, sorry.
> 
> > 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.
> 
> Putting the fence posting back, which will involve adjusting the other
> callers, is probably best and reflects the underlying hypercall.
> 
> I think maximum_gpfn+1 could be described as nr_gpfns if we wanted to go
> the renaming route, or make a wrapper which did the +1, returning the
> original to the expect semantics.
> 
> Konrad, will you take care of this one way or another?

Yes. I will just add an xc_nr_gpfns and conver the in-tree callers to use
that. And naturally revert the xc_domain_maximum_gpfn.

Andrew, thank you for noticing this!
> 
> Ian.
> 

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