Re: [Xen-devel] [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Tue, 2015-03-17 at 14:16 -0400, longtao.pang wrote:
> From: "longtao.pang" <longtaox.pang@xxxxxxxxx>
> 
> 1. Designate vif model to 'e1000', otherwise, with default device model,
> the L1 eth0 interface disappear, hence xenbridge cannot work.
> Maybe this limitation can be removed later after some fix it. For now, we
> have to accomodate to it.

You have done this unconditionally, which means it affects all guests.
You need to make this configurable by the caller, probably by plumbing
it through in $xopts (a hash of extra options).

I see now you were told this last time around by Ian J, please don't
just resend such things without change either fix them, make an argument
for doing it your way or ask for clarification if you don't understand
the requested change.

> 2. Since reboot L1 guest VM will take more time to boot up, we increase
> multi-times for reboot-confirm-booted if test nested job, and the multi value 
> is stored as a runvar in 'ts-nested-setup' script. Added another function 
> 'guest_editconfig_cd' and expose it, this function bascically changes guest
> boot device sequence, alter its on_reboot behavior to restart and enabled 
> nestedhvm feature.

This looks like two items run together?

The multi_reboot_time thing sounds ok, but it should be called
reboot_time_factor or something like that. In fact I see that Ian
suggested previously that it should have the host ident in it, that
makes sense to me.

The editconfig_cd thing -- yet another thing which Ian questioned and
which it was agreed you would change but you haven't.

I think perhaps you have accidentally resent an older version of the
series. If not then please go back and ensure you have addressed all of
the feedback given on the last iteration before sending another version.

Ian.



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