Re: [Xen-devel] [PATCH v3] Set {ident}_suite runvar when install a Debian guest.

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("[PATCH v3] Set {ident}_suite runvar when install a Debian 
guest."):
> Currently those places which want this open code a lookup of the
> {ident}_suite runvar with a fallback to the configuration file.
> 
> However selecthost was missing such a lookup in the case where it is
> constructing a nested L1 host (which begins from the selectguest
> template), which lead to ts-xen-install on Jessie missing the
> installation of libnl-route-3-200.
> 
> Fix this by providing debian_guest_suite($gho) which as well as
> initialising $gho->{Suite} stores an {ident}_suite runvar (taking care
> to handle the case where one is already set by e.g. make-flight). For
> convenience debian_guest_suite() also returns the suite name.
> 
> ts-debian-install, ts-debian-di-install and ts-debian-hvm-install now
> use debian_guest_suite instead of open coding the lookup.
> 
> The final piece of the puzzle is to have selectguest() pickup the
> {ident}_suite runvar (if it is set) and initialise $gho->{Suite} from
> it.

Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> ---
> I've kicked off an adhoc run testing:
> +    test-amd64-amd64-qemuu-nested);; # nested
> +    test-amd64-amd64-xl-qemuu-debianhvm-amd64);; # regular HVM
> +    test-amd64-amd64-xl-qcow2);; # DI install
> +    test-amd64-amd64-xl);; # Normal PV
> with this change.

This looks like you wrote a case statement in sh.  Did you know about
OSSTEST_JOBS_ONLY (see cs-job-create) ?


FFR, I have some style tips:

> +sub debian_guest_suite ($) {
> +    my ($gho) = @_;
> +
> +    return $gho->{Suite} if $gho->{Suite};
> +
> +    $gho->{Suite} = guest_var($gho,'suite',undef);

If you do this instead

  +    $gho->{Suite} //= guest_var($gho,'suite',undef);

the previous line (`return ... if') is not needed.

> +    if ( ! $gho->{Suite} ) {

This is stylistically odd IMO.  Seems to have been written by a
hypervisor maintainer :-).

> +     $gho->{Suite} = $c{GuestDebianSuite};
> +     store_runvar("$gho->{Guest}_suite", $gho->{Suite});
> +    }
> +    return $gho->{Suite};
> +}

But none of that really matters.

Ian.

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