From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH OSSTEST] get_hostflags: return an empty list when there is no flight/job. Date: Fri, 31 Jul 2015 16:39:38 +0100 Message-ID: <1438357178.30740.70.camel@citrix.com> References: <1438187304-17039-1-git-send-email-ian.campbell@citrix.com> <21947.38096.288929.877968@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <21947.38096.288929.877968@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Jackson Cc: xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Fri, 2015-07-31 at 16:31 +0100, Ian Jackson wrote: > Ian Campbell writes ("[PATCH OSSTEST] get_hostflags: return an empty list > when there is no flight/job."): > > From: Ian Campbell > > > > Otherwise trying to use mg-hosts mkpxedir fails with: > > I think your proposed fix is incorrect. It is wrong to call > get_hostflags outside the context of a job, because get_hostflags is > supposed to return the job's host flags for that ident. > > The bug was introduced by me in 11e788f7 "JobDB/Executive: Improve an > internal `die' error", where a refactoring meant that we always call > get_hostflags. > > How about this instead ? > > diff --git a/Osstest/JobDB/Executive.pm b/Osstest/JobDB/Executive.pm > index 1ec947e..cc52f57 100644 > --- a/Osstest/JobDB/Executive.pm > +++ b/Osstest/JobDB/Executive.pm > @@ -128,7 +128,7 @@ sub host_check_allocated ($$) { #method > $ho->{Shared} && > $ho->{Shared}{State} eq 'ready'; > my $harness = get_harness_rev(); > - my @flags = get_hostflags($ho->{Ident}); > + my @flags = defined($job) ? get_hostflags($ho->{Ident}) : qw(OUTSIDE > -JOB); > $ho->{SharedReady}= > $ho->{SharedMaybeOthers} && > !! (grep { $_." ".$harness eq "share-".$ho->{Shared}{Type} } > > Signed-off-by: Ian Jackson LGTM, I suppose the use of the OUTSIDE-JOB sentinel value is just for the benefit of the reader of the following die() should it occur. Acked-by: Ian Campbell Ian.