Re: [Xen-devel] [PATCH v3 7/8] osstest: introduce a script to create a FreeBSD flight

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

Roger Pau Monne writes ("[PATCH v3 7/8] osstest: introduce a script to create a 
FreeBSD flight"):
> +FreeBSDDist
> +   Path to the folder that contains the FreeBSD install image and
> +   the FreeBSD compressed install sets, together with the MANIFEST
> +   file that holds the checksums. This is required in order to run
> +   a FreeBSD host install if no previous FreeBSD buildjob is
> +   available (ie: for example when running in standalone mode).
> +
> +FreeBSDVersion
> +   Numeric value holding the major FreeBSD version of the media
> +   provided in FreeBSDDist (ie: 12).

This means that a user who is setting these manually needs to supply
both.  Is there a way to avoid that ?

> +get_freebsdjob_flags () {
> +    arch=$1

You repeated use the word "flags" here for things which are runvars.
flags are, in osstest, strictly boolean.

> +flags=`get_freebsdjob_flags $arch`
> +job_create_build build-$arch-freebsd build-freebsd                       \
> +            arch=$arch                                                   \
> +            $RUNVARS $BUILD_RUNVARS $BUILD_FREEBSD_RUNVARS $arch_runvars \
> +            tree_freebsd=$TREE_FREEBSD                                   \
> +            revision_freebsd=$REVISION_FREEBSD                           \
> +            extra_hostflags=arch-$arch,purpose-build                     \
> +            $flags

See my comments about extra_hostflags in the previous patch.  Use
host_hostflags here.

> diff --git a/sg-run-job b/sg-run-job
> index ceb79800..239446b8 100755
> --- a/sg-run-job
> +++ b/sg-run-job

IWBN to split this out.  Firstly, please split the sg-run-job changes
from the make-flight changes.

Secondly, can you split the sg-run-job patch into two ?  1. change the
meaning of need_build_host but not introduce the FreeBSD versions;
2. add the FreeBSD cases.  sg-run-job is a bit fragile to do this kind
of work in.

Also I'm afraid you're going to find some conflicts with my recent
syslog work.  You will probably want to rebase onto pretest, which I
think is very likely to pass and be pushed soon.

> @@ -53,12 +53,15 @@ proc run-job {job} {
>      set skip_globs     [jobdb::read-runvar $flight $job skip_testids]
>  
>      set nh [need-hosts/$jobinfo(recipe)]
> -    if {![string compare $nh BUILD]} {
> +    if {![string compare $nh BUILD_LINUX]} {

How about
       if {[string match $nh BUILD_*]} {
and then set need_build_host to the RHS ?

> -    if {$need_build_host} { catching-otherwise broken prepare-build-host }
> +    if {![string compare $need_build_host LINUX]} {
> +        catching-otherwise broken prepare-build-host-linux
> +    } elseif {![string compare $need_build_host FREEBSD]}  {
> +        catching-otherwise broken prepare-build-host-freebsd
> +    }

Please use switch(3tcl).

> -proc prepare-build-host {} {
> +proc prepare-build-host-linux {} {

This rename should be in the first patch.

Ian.

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