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

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: [PATCH OSSTEST v2 09/15] distros: add support for 
installing Debian PV guests via d-i, flight and jobs"):
> On Fri, 2014-05-02 at 12:46 +0100, Ian Jackson wrote:
> > 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.

You can move declare them (with my) outside the if to fix this
problem.  That's IMO much better than writing out those runvar stores
twice.

> > 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).?

Hrm.  Arguably according to its name, get_host_property should look
into the $ho->{Host} but it doesn't and that's not what you want here.

Is whether we should do that a function of the property, or a function
of the enquiring call site ?  The code at the moment implies the
former: there is explicit machinery for doing the indirection for
(e.g.) DhcpWatchMethod.

I think until we discover otherwise, we can declare that it's fine to
call get_host_property on a guest object, provided that we understand
that the answer is always going to be "no such property set".  Perhaps
this should be documented.

> > > +    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.

So I now see.

Thanks,
Ian.

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