Re: [PATCH 1/3] libxl: In domain death search, start search at first domid we want

[Ian Campbell <ian.campbell@citrix.com>]

On Wed, 2015-03-18 at 11:47 -0600, Jim Fehlig wrote:
> Ian Campbell wrote:
> > On Tue, 2015-03-17 at 09:30 -0600, Jim Fehlig wrote:
> >   
> >> From: Ian Jackson <Ian.Jackson@eu.citrix.com>
> >>
> >> From: Ian Jackson <Ian.Jackson@eu.citrix.com>
> >>
> >> When domain_death_xswatch_callback needed a further call to
> >> xc_domain_getinfolist it would restart it with the last domain it
> >> found rather than the first one it wants.
> >>
> >> If it only wants one it will also only ask for one domain.  The result
> >> would then be that it gets the previous domain again (ie, the previous
> >> one to the one it wants), which still doesn't reveal the answer to the
> >> question, and it would therefore loop again.
> >>
> >> It's completely unclear to me why I thought it was a good idea to
> >> start the xc_domain_getinfolist with the last domain previously found
> >> rather than the first one left un-confirmed.  The code has been that
> >> way since it was introduced.
> >>     
> >
> > Is it because the xc_domain_getinfolist will fetch at most:
> >         int nentries = LIBXL_TAILQ_NEXT(evg, entry) ? 200 : 1;
> > entries?
> >
> > After your change then if the domid we are looking for is the 201st
> > domain then won't we just keep going round looking at the first 200
> > (undying) domains?
> >   
> 
> Yes, that theoretically looks to be the case. When all 200 domains have
> been examined the inner loop is terminated
> 
> if (got == gotend) {
> LIBXL__LOG(CTX, LIBXL__LOG_DEBUG, " got==gotend");
> break;
> }
> 
> which will repeat the outer loop with same conditions. But
> xc_domain_getinfolist() is called with first_domain set to the domain ID
> we are looking for. Is it possible for xc_domain_getinfolist() to return
> 200 domains with IDs less than the ID we asked for?

I'm not sure, but it does seem rather unlikely.

However what I'm not sure about is whether this function is intending to
handle multiple domains dying or not. It looks to me like it is (and
that's what I would expect).

Ah, and Ian has pushed the selection of domid from outside the loop to
inside the loop (as evg->domid). Which means that, assuming
xc_domain_getinfolist can't return 200 domains with IDs less that the
one we wanted we will infact make progress, possibly handling up to 200
dead domains, and then go round the loop again and start from the new
smallest domid. (evg is progressed inside domain_death_occurred).

So right, I think I understand what this patch is doing and I think it
is correct:

Acked-by: Ian Campbell <ian.campbell@citrix.com>