From: Ian Campbell <ian.campbell@citrix.com>
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: robert.hu@intel.com, "longtao.pang" <longtaox.pang@intel.com>,
wei.liu2@citrix.com, xen-devel@lists.xen.org
Subject: Re: [OSSTEST Nested PATCH v8 3/7] Edit some APIs in TestSupport.pm for nested test
Date: Tue, 21 Apr 2015 13:53:00 +0100 [thread overview]
Message-ID: <1429620780.4743.67.camel@citrix.com> (raw)
In-Reply-To: <21814.17314.479011.970684@mariner.uk.xensource.com>
On Tue, 2015-04-21 at 13:33 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [OSSTEST Nested PATCH v8 3/7] Edit some APIs in TestSupport.pm for nested test"):
> > On Mon, 2015-04-13 at 17:19 -0400, longtao.pang wrote:
> > > --- a/Osstest/TestSupport.pm
> > > +++ b/Osstest/TestSupport.pm
> ...
> > > + if ( $r{"${name}_ip"} ) {
> > > + $setprop->("IpAddr", $r{"${name}_ip"});
> > > + }
> >
> > This is one for Ian I think, but I suspect this should use ${ident}
> > rather than ${name}.
>
> Yes.
>
> > ${ident} is e.g. 'host' or 'srchost' or 'nestedl1' it is the prefer used
> > on the runvar names. ${name} is the specific value assigned, i.e. an
> > actual host name.
> >
> > For such properties we usually prefer ${ident}_foo. Ian, correct me if
> > I'm wrong please.
>
> Ian C is right.
>
>
> But, I think actually the principle behind this change is wrong.
>
> It seems to be copying information from runvars into the in-memory
> data structure for host properties. But host properties are (by
> definition) matters of (fixed) configuration, not runtime definition.
Remember that here the "host" is actual an L1 virtual machine, whose IP
is not fixed (since it mac address isn't).
I don't know if that affects your opinion at all?
> I haven't looked at the rest of the series, but the same effect should
> be achieved in a different way. Note that a $ho is a hash which
> already contains (after selecthost, say) an element with key `Ip'.
>
> So the right place to do this is indeed probably somewhere around
> selectguest or selecthost, but the information should be put into
> $ho->{Ip} without going via host properties.
>
> Thanks,
> Ian.
next prev parent reply other threads:[~2015-04-21 12:53 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-13 21:19 [OSSTEST Nested PATCH v8 0/7] Introduction of netsted HVM test job longtao.pang
2015-04-13 21:19 ` [OSSTEST Nested PATCH v8 1/7] parsing grub which has 'submenu' primitive longtao.pang
2015-04-21 10:12 ` Ian Campbell
2015-04-13 21:19 ` [OSSTEST Nested PATCH v8 2/7] Changes to support '/boot' leading paths of kernel, xen, in grub longtao.pang
2015-04-21 10:13 ` Ian Campbell
2015-04-13 21:19 ` [OSSTEST Nested PATCH v8 3/7] Edit some APIs in TestSupport.pm for nested test longtao.pang
2015-04-21 10:19 ` Ian Campbell
2015-04-21 12:33 ` Ian Jackson
2015-04-21 12:53 ` Ian Campbell [this message]
2015-04-21 13:28 ` Ian Jackson
2015-04-21 13:41 ` Ian Campbell
2015-04-21 14:30 ` Ian Jackson
2015-04-21 14:43 ` Ian Campbell
2015-04-22 8:25 ` Pang, LongtaoX
2015-04-22 9:48 ` Ian Campbell
2015-04-22 12:50 ` Ian Jackson
2015-04-23 0:34 ` Hu, Robert
2015-04-27 9:36 ` Robert Hu
2015-04-28 7:41 ` Ian Campbell
2015-04-13 21:19 ` [OSSTEST Nested PATCH v8 4/7] Changes on test step of Debian hvm guest install longtao.pang
2015-04-21 10:28 ` Ian Campbell
2015-04-23 5:59 ` Robert Hu
2015-04-23 6:52 ` Ian Campbell
2015-04-23 10:43 ` Hu, Robert
2015-04-23 12:04 ` Ian Campbell
2015-04-23 11:07 ` Ian Jackson
2015-04-13 21:19 ` [OSSTEST Nested PATCH v8 5/7] Add new script to customize nested test configuration longtao.pang
2015-04-21 10:40 ` Ian Campbell
2015-04-22 8:35 ` Pang, LongtaoX
2015-04-22 9:56 ` Ian Campbell
2015-04-23 9:38 ` Robert Hu
2015-04-23 11:30 ` Ian Campbell
2015-04-23 13:05 ` Ian Campbell
2015-04-24 8:45 ` Pang, LongtaoX
2015-04-28 7:39 ` Ian Campbell
2015-04-23 7:27 ` Pang, LongtaoX
2015-04-23 11:35 ` Ian Campbell
2015-04-13 21:19 ` [OSSTEST Nested PATCH v8 6/7] Compose the main recipe of nested test job longtao.pang
2015-04-21 10:48 ` Ian Campbell
2015-04-22 8:38 ` Pang, LongtaoX
2015-04-22 11:04 ` Ian Campbell
2015-04-22 11:23 ` Ian Campbell
2015-04-23 8:08 ` Pang, LongtaoX
2015-04-23 11:05 ` Ian Jackson
2015-04-24 6:31 ` Robert Hu
2015-04-23 10:49 ` Robert Hu
2015-04-13 21:19 ` [OSSTEST Nested PATCH v8 7/7] Add test job for nest test case longtao.pang
2015-04-21 10:51 ` Ian Campbell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1429620780.4743.67.camel@citrix.com \
--to=ian.campbell@citrix.com \
--cc=Ian.Jackson@eu.citrix.com \
--cc=longtaox.pang@intel.com \
--cc=robert.hu@intel.com \
--cc=wei.liu2@citrix.com \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.