Re: [Xen-devel] [PATCH OSSTEST 04/11] TestSupport: introduce set_host_prop

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

Roger Pau Monne writes ("Re: [PATCH OSSTEST 04/11] TestSupport: introduce 
set_host_prop"):
> On Tue, Aug 01, 2017 at 02:01:35PM +0100, Ian Jackson wrote:
> > TBH, since this is only being called in the one
> > ts-set-host-properties-from-runvars script (or whatever you're calling
> > it), I think you can use $mjobdb-> directly.  That's not too bad a
> > layer violation.
> 
> In the new version that I've sent I've already added a helper to
> TestSupport, it's just two lines of code so unless you feel really
> annoyed by it I would probably leave it there.

No, sure, the helper is probably a bit better.

> > I think your runvars should probably be named after the ident, not the
> > hostname.  That may involve rethinking your encoding, since idents can
> > contain _ (hostnames can contain - but not _).
> 
> Does it make sense to have the hostname or the ident?
> 
> After all ts-set-host-properties-from-runvars is only going to save
> properties for the host passed as 'host' ident, and I don't see much
> reason for allowing it to support two different idents like src_host
> or dst_host, or in general for having a test script that sets host
> properties for multiple hosts.

I don't think this can be right.

You have a helper function that you pass a $ho, and which records the
runvars, and which therefore pretends to arrange for the properties to
be set (later), for that $ho.  It should do as it implicity promises.

That means the second actually-modifying-the-db ts-* script must
process all of the specified runvars.  So it must call selecthost.  So
really it should have the ident.  So the runvars should be named after
the ident.

Ian.

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