Re: [KVM-AUTOTEST PATCH 12/12] KVM test: make stress_boot work properly with TAP networking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Aug 03, 2009 at 04:00:45AM -0400, Michael Goldish wrote:
> 
> ----- "Yolkfull Chow" <yzhou@xxxxxxxxxx> wrote:
> 
> > On Mon, Aug 03, 2009 at 02:45:23AM -0400, Michael Goldish wrote:
> > > 
> > > ----- "Yolkfull Chow" <yzhou@xxxxxxxxxx> wrote:
> > > 
> > > > Hi Michael, I just have some comments on what you changed on
> > > > stress_boot. :-)
> > > > 
> > > > 
> > > > On Mon, Aug 03, 2009 at 02:58:21AM +0300, Michael Goldish wrote:
> > > > > Take an additional parameter 'clone_address_index_base' which
> > > > indicates the
> > > > > initial value for 'address_index' for the cloned VMs.  This
> > value
> > > > is
> > > > > incremented after each clone is created.  I assume the original
> > VM
> > > > has a single
> > > > > NIC; otherwise NICs will end up sharing MAC addresses, which is
> > > > bad.
> > > > > 
> > > > > Also, make a few small corrections:
> > > > > 
> > > > > - Take the params for the clones from the original VM's params,
> > not
> > > > from the
> > > > > test's params, because the test's params contain information
> > about
> > > > several
> > > > > VMs, only one of which is the original VM. The original VM's
> > params,
> > > > on the
> > > > > other hand, describe just a single VM (which we want to clone).
> > > > > 
> > > > > - Change the way kill_vm.* parameters are sent to the
> > > > postprocessor.
> > > > > (The postprocessor doesn't read params from the VM objects but
> > > > rather from the
> > > > > test's params dict.)
> > > > > 
> > > > > - Replace 'if get_command_status(...)' with 'if
> > > > get_command_status(...) != 0'.
> > > > > 
> > > > > - Replace the test command 'ps aux' with 'uname -a'.  The silly
> > > > reason for this
> > > > > is that DSL-4.2.5 doesn't like 'ps aux'.  Since 'uname -a' is
> > just
> > > > as good
> > > > > (AFAIK), use it instead.
> > > > > 
> > > > > Signed-off-by: Michael Goldish <mgoldish@xxxxxxxxxx>
> > > > > ---
> > > > >  client/tests/kvm/kvm_tests.cfg.sample |    7 ++++++-
> > > > >  client/tests/kvm/kvm_tests.py         |   14 ++++++--------
> > > > >  2 files changed, 12 insertions(+), 9 deletions(-)
> > > > > 
> > > > > diff --git a/client/tests/kvm/kvm_tests.cfg.sample
> > > > b/client/tests/kvm/kvm_tests.cfg.sample
> > > > > index 3c23432..3a4bf64 100644
> > > > > --- a/client/tests/kvm/kvm_tests.cfg.sample
> > > > > +++ b/client/tests/kvm/kvm_tests.cfg.sample
> > > > > @@ -117,7 +117,12 @@ variants:
> > > > >      - stress_boot:  install setup
> > > > >          type = stress_boot
> > > > >          max_vms = 5    
> > > > > -        alive_test_cmd = ps aux
> > > > > +        alive_test_cmd = uname -a
> > > > > +        clone_address_index_base = 10
> > > > > +        kill_vm = yes
> > > > > +        kill_vm_vm1 = no
> > > > 
> > > > 
> > > > Actually, there are two methods to deal with started VMs via
> > > > framework, one is as soon
> > > > as the current test finished and the other is just before next
> > test
> > > > going to be started. 
> > > > In this case both methods equal somewhat between each other I
> > think,
> > > > and
> > > > I had chosen the later one. But if you had done some testing and
> > > > proved the previous one is better, I would agree. 
> > > 
> > > It's not that I tested and found the former to be better, it's just
> > that
> > > if a user runs only stress_boot, it makes sense to clean up at the
> > end
> > > of the test, not at the beginning of the next one (because there's
> > no
> > > next one).  Since you're adding VMs to the postprocessor's params
> > > (params['vms'] += " " + vm_name) I thought we might as well use
> > that.
> > > (In order to clean up the VMs automatically when the next test
> > starts,
> > > we don't need to add the VM to those params -- only to 'env'.)
> > 
> > Hmm...yes, if users only want to run stress_boot, cleaning up should
> > be
> > implemented at the end of the test. 
> > 
> > > 
> > > > 
> > > > 
> > > > > +        kill_vm_gracefully = no
> > > > 
> > > > This has been specified within code during cloning. 
> > > 
> > > Right, but the postprocessor is supposed to ignore that, because it
> > doesn't
> > > look at the params inside the VM -- it looks at test params that get
> > to the
> > > postprocessor.
> > > 
> > > > 
> > > > > +        extra_params += " -snapshot"
> > > > 
> > > > If you add this option '-snapshot' here, the first VM will be in
> > > > snapshot
> > > > mode as well which will affect next test.
> > > 
> > > Which is good in my opinion, because I don't want the first VM
> > writing to the
> > > same image that others are reading from.  It sounds like it might
> > > 
> > > Let me know what you think.
> > 
> > My opinion is, since the scenario that the first VM using
> > non-snapshot
> > mode while the others do has not caused any problem, why not just keep
> > the first VM
> > in non-snapshot mode and let it to be used in next test which really
> > save booting time? 
> > 
> > What do you think?  Please let me know if there is a trap in such
> > scenario.
> > Thanks. :-)
> 
> I'm not sure if it really will cause trouble, but it sounds like it might,
> since we're modifying an image that's being used by another VM, if I
> understand correctly.  I don't think we actually have to see a test fail
> before we decide to avoid doing something -- it's enough just to know that
> something is bad practice.  I'm not even sure this is bad practice, but I
> want to be on the safe side anyway.  Can anyone shed some light on this?
> (Is it risky to start a VM without -snapshot, and in parallel start other
> VMs using the same image with -snapshot?)

Hi Michael, I think you are right. If there is a way can avoid
trouble/problem, we should walk along. In respect of snapshot mode, we
haven't encountered such problem though, we should be conscious. :-)

> 
> > > 
> > > Thanks,
> > > Michael
> > > 
> > > > 
> > > > >  
> > > > >      - shutdown:     install setup
> > > > >          type = shutdown
> > > > > diff --git a/client/tests/kvm/kvm_tests.py
> > > > b/client/tests/kvm/kvm_tests.py
> > > > > index 9784ec9..308db97 100644
> > > > > --- a/client/tests/kvm/kvm_tests.py
> > > > > +++ b/client/tests/kvm/kvm_tests.py
> > > > > @@ -541,8 +541,8 @@ def run_stress_boot(tests, params, env):
> > > > >          raise error.TestFail("Could not log into first guest")
> > > > >  
> > > > >      num = 2
> > > > > -    vms = []
> > > > >      sessions = [session]
> > > > > +    address_index = int(params.get("clone_address_index_base",
> > > > 10))
> > > > >  
> > > > >      # boot the VMs
> > > > >      while num <= int(params.get("max_vms")):
> > > > > @@ -550,15 +550,12 @@ def run_stress_boot(tests, params, env):
> > > > >              vm_name = "vm" + str(num)
> > > > >  
> > > > >              # clone vm according to the first one
> > > > > -            vm_params = params.copy()
> > > > > -            vm_params['image_snapshot'] = "yes"
> > > > > -            vm_params['kill_vm'] = "yes"
> > > > > -            vm_params['kill_vm_gracefully'] = "no"
> > > > > +            vm_params = vm.get_params().copy()
> > > > > +            vm_params["address_index"] = str(address_index)
> > > > >              curr_vm = vm.clone(vm_name, vm_params)
> > > > >              kvm_utils.env_register_vm(env, vm_name, curr_vm)
> > > > >              params['vms'] += " " + vm_name
> > > > >  
> > > > > -            #vms.append(curr_vm)
> > > > >              logging.info("Booting guest #%d" % num)
> > > > >              if not curr_vm.create():
> > > > >                  raise error.TestFail("Cannot create VM #%d" %
> > num)
> > > > > @@ -571,10 +568,11 @@ def run_stress_boot(tests, params, env):
> > > > >              sessions.append(curr_vm_session)
> > > > >  
> > > > >              # check whether all previous ssh sessions are
> > > > responsive
> > > > > -            for i, vm_session in enumerate(sessions):
> > > > > -                if
> > > > vm_session.get_command_status(params.get("alive_test_cmd")):
> > > > > +            for i, se in enumerate(sessions):
> > > > > +                if
> > > > se.get_command_status(params.get("alive_test_cmd")) != 0:
> > > > >                      raise error.TestFail("Session #%d is not
> > > > responsive" % i)
> > > > >              num += 1
> > > > > +            address_index += 1
> > > > >  
> > > > >          except (error.TestFail, OSError):
> > > > >              for se in sessions:
> > > > > -- 
> > > > > 1.5.4.1
> > > > > 
> > > > > --
> > > > > To unsubscribe from this list: send the line "unsubscribe kvm"
> > in
> > > > > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > > > > More majordomo info at 
> > http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux