From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <Ian.Campbell@citrix.com>
Subject: Re: [PATCH OSSTEST v5 9/9] mfi-common,
 make-flight: create XSM test jobs
Date: Tue, 3 Feb 2015 12:08:03 +0000
Message-ID: <1422965283.9323.57.camel@citrix.com>
References: <1422959786-22738-1-git-send-email-wei.liu2@citrix.com>
	<1422959786-22738-10-git-send-email-wei.liu2@citrix.com>
	<1422962564.9323.42.camel@citrix.com>
	<20150203120620.GB17714@zion.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <20150203120620.GB17714@zion.uk.xensource.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Wei Liu <wei.liu2@citrix.com>
Cc: ian.jackson@eu.citrix.com, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

On Tue, 2015-02-03 at 12:06 +0000, Wei Liu wrote:
> On Tue, Feb 03, 2015 at 11:22:44AM +0000, Ian Campbell wrote:
> > On Tue, 2015-02-03 at 10:36 +0000, Wei Liu wrote:
> > > Duplicate Debian PV and HVM test jobs for XSM testing.
> > 
> > If ~/.xen-osstest/config contains e.g. "PlatformsArmhf midway cubietruck
> > arndale" (correspodning to the platform-{...} flag being set on some
> > host) then I think this will generate a per platform XSM test, which
> > doesn't seem necessary since XSM isn't platform specific.
> > 
> > So:
> > 
> > > +  for xsm in $test_xsm ; do
> > > +
> > > +    # Basic PV Linux test with xl
> > > +    for platform in '' `getplatforms $xenarch` ; do
> > > +      suffix=${platform:+-$platform}
> > > +      hostflags=${most_hostflags}${platform:+,platform-$platform}
> > > +
> > > +      job_create_test test-$xenarch$kern-$dom0arch-xl$suffix test-debian xl \
> > > +                      $xenarch $dom0arch                                    \
> > > +                      enable_xsm=$xsm                                       \
> > > +                      $debian_runvars all_hostflags=$hostflags
> > > +    done
> > 
> > I think you could pull the for platform loop outside of the for xsm one
> > and remove '' from it, and readd the generic one standalone inside the
> > xsm loop. (You can also simplify the suffix stuff in the platform loop
> > since that is just to handle '' correctly).
> > 
> > Or add "if xsm = true and platform != '' -> continue"? 
> > 
> 
> I use the "continue" trick and now the function looks like:

LGTM. Two nits:

> +      # xsm test is not platform specifc

"specific"

> +      if [ "x$xsm" = "xtrue" -a "x$platform" != "x" ]; then

IIRC Ian previously indicated that he prefers to avoid " when it is
unnecessary, which I think is the case for all 4 words above.

Ian.