Re: [Xen-devel] [PATCH OSSTEST v2 09/15] distros: add support for installing Debian PV guests via d-i, flight and jobs

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-05-02 at 12:46 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH OSSTEST v2 09/15] distros: add support for 
> installing Debian PV guests via d-i, flight and jobs"):
> ...
> > +if [ x$buildflight = x ]; then
> > +
> > +  if [ "x$BUILD_LVEXTEND_MAX" != x ]; then
> > +     BUILD_RUNVARS+=" build_lvextend_max=$BUILD_LVEXTEND_MAX "
> > +  fi
> 
> This has been clone-and-hacked from make-flight and presumably belongs
> in mfi-common (perhaps a subroutine defined there).

Ack.

> ...
> > +    if ( $di_ver eq "osstest" ) {
> ...
> > +   store_runvar("$gho->{Guest}_netboot_kernel", $netboot_kernel);
> > +   store_runvar("$gho->{Guest}_netboot_initrd", $netboot_initrd);
> > +    } else {
> ...
> > +   store_runvar("$gho->{Guest}_netboot_kernel", $netboot_kernel);
> > +   store_runvar("$gho->{Guest}_netboot_initrd", $netboot_initrd);
> 
> Had you not noticed that you'd written out the store_runvars lines
> twice ?

The $netboot_foo are local in scope to within the two halves of the
if/else.

> > +    target_cmd_root($ho, <<END);
> > +rm -rf /root/di
> > +mkdir /root/di
> 
> You should put the flight and job in this filename in case of future
> host-sharing.

Ack.

> > +    my $cmdline = join(" ", (
> > +                      "debian-installer/exit/always_halt=true",
> > +                      "--",
> > +                      "console=hvc0",
> > +                      "auto-install/enable=true",
> > +                      "hostname=$gho->{Name}",
> > +                      "domain=$c{TestHostDomain}",
> > +                      "url=$ps_url",
> > +                      "DEBIAN_FRONTEND=text",
> > +                      "netcfg/dhcp_timeout=150",
> > +                      "netcfg/choose_interface=eth0",
> 
> I think you want to use Osstest::Debian::di_installcmdline_core.

I think I do too, thanks for the pointer.

To what extent are $ho and $gho interchangeable? Is it ok to call e.g.
get_host_property on a guest (as that function would do).?

> > +    my $blcfg = <<END;
> > +bootloader = "pygrub"
> > +END
> 
> I'm not sure why you've broken this out.  (I just mention this in case
> it's not deliberate.)

It saved some refactoring noise in a later patch when I add support for
pvgrub.

Ian.


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