See comments below.
----- "Dor Laor" <dlaor@xxxxxxxxxx> wrote:
> On 10/15/2009 11:48 AM, Amos Kong wrote:
> >
> > Test 802.1Q vlan of nic, config it by vconfig command.
> > 1) Create two VMs
> > 2) Setup guests in different vlan by vconfig and test
> communication by ping
> > using hard-coded ip address
> > 3) Setup guests in same vlan and test communication by ping
> > 4) Recover the vlan config
> >
> > Signed-off-by: Amos Kong<akong@xxxxxxxxxx>
> > ---
> > client/tests/kvm/kvm_tests.cfg.sample | 6 +++
> > client/tests/kvm/tests/vlan_tag.py | 73
> +++++++++++++++++++++++++++++++++
> > 2 files changed, 79 insertions(+), 0 deletions(-)
> > mode change 100644 => 100755 client/tests/kvm/scripts/qemu-ifup
>
> In general the above should come as an independent patch.
>
> > create mode 100644 client/tests/kvm/tests/vlan_tag.py
> >
> > diff --git a/client/tests/kvm/kvm_tests.cfg.sample
> b/client/tests/kvm/kvm_tests.cfg.sample
> > index 9ccc9b5..4e47767 100644
> > --- a/client/tests/kvm/kvm_tests.cfg.sample
> > +++ b/client/tests/kvm/kvm_tests.cfg.sample
> > @@ -166,6 +166,12 @@ variants:
> > used_cpus = 5
> > used_mem = 2560
> >
> > + - vlan_tag: install setup
> > + type = vlan_tag
> > + subnet2 = 192.168.123
> > + vlans = "10 20"
>
> If we want to be fanatic and safe we should dynamically choose subnet
> and vlans numbers that are not used on the host instead of hard code
> it.
For the sake of safety maybe we should start both VMs with -snapshot.
Dor, what do you think? Is it safe to start 2 VMs with the same disk image
when only one of them uses -snapshot?
> > + nic_mode = tap
> > + nic_model = e1000
>
> Why only e1000? Let's test virtio and rtl8139 as well. Can't you
> inherit the nic model from the config?
It's not just inherited, it's overwritten, because nic_model is defined
later in the file in a variants block. So this nic_model line has no
effect.
> >
> > - autoit: install setup
> > type = autoit
> > diff --git a/client/tests/kvm/scripts/qemu-ifup
> b/client/tests/kvm/scripts/qemu-ifup
> > old mode 100644
> > new mode 100755
> > diff --git a/client/tests/kvm/tests/vlan_tag.py
> b/client/tests/kvm/tests/vlan_tag.py
> > new file mode 100644
> > index 0000000..15e763f
> > --- /dev/null
> > +++ b/client/tests/kvm/tests/vlan_tag.py
> > @@ -0,0 +1,73 @@
> > +import logging, time
> > +from autotest_lib.client.common_lib import error
> > +import kvm_subprocess, kvm_test_utils, kvm_utils
> > +
> > +def run_vlan_tag(test, params, env):
> > + """
> > + Test 802.1Q vlan of nic, config it by vconfig command.
> > +
> > + 1) Create two VMs
> > + 2) Setup guests in different vlan by vconfig and test
> communication by ping
> > + using hard-coded ip address
> > + 3) Setup guests in same vlan and test communication by ping
> > + 4) Recover the vlan config
> > +
> > + @param test: Kvm test object
> > + @param params: Dictionary with the test parameters.
> > + @param env: Dictionary with test environment.
> > + """
> > +
> > + vm = []
> > + session = []
> > + subnet2 = params.get("subnet2")
> > + vlans = params.get("vlans").split()
> > +
> > + vm.append(kvm_test_utils.get_living_vm(env, "%s" % params.get("main_vm")))
There's no need for the "%s" here.
...get_living_vm(env, params.get("main_vm"))) should work.
> > + params_vm2 = params.copy()
> > + params_vm2['image_snapshot'] = "yes"
> > + params_vm2['kill_vm_gracefully'] = "no"
> > + params_vm2["address_index"] = int(params.get("address_index", 0))+1
> > + vm.append(vm[0].clone("vm2", params_vm2))
> > + kvm_utils.env_register_vm(env, "vm2", vm[1])
> > + if not vm[1].create():
> > + raise error.TestError("VM 1 create faild")
>
>
> The whole 7-8 lines above should be grouped as a function to clone
> existing VM. It should be part of kvm autotest infrastructure.
> Besides that, it looks good.
There's already a clone function and it's being used here.
Instead of those 7-8 lines, why not just define the VM in the config file?
It looks like you're always using 2 VMs so there's no reason to do this in
test code. This should do what you want:
- vlan_tag: install setup
type = vlan_tag
subnet2 = 192.168.123
vlans = "10 20"
nic_mode = tap
vms += " vm2"
extra_params_vm2 += " -snapshot"
kill_vm_gracefully_vm2 = no
address_index_vm2 = 1
The preprocessor then automatically creates vm2 and registers it in env.
To use it in the test just do:
vm.append(kvm_test_utils.get_living_vm(env, "vm2"))
You can also use a parameter that tells the test which VM to use if you don't
want the name "vm2" hardcoded into the test.
Add something like this to the config file:
2nd_vm = vm2
and in the test use params.get("2nd_vm") instead of "vm2" (just like you use
"main_vm").
> > +
> > + for i in range(2):
> > + session.append(kvm_test_utils.wait_for_login(vm[i]))
> > +
> > + try:
> > + vconfig_cmd = "vconfig add eth0 %s;ifconfig eth0.%s %s.%s"
> > + # Attempt to configure IPs for the VMs and record the
> results in
> > + # boolean variables
> > + # Make vm1 and vm2 in the different vlan
> > +
> > + ip_config_vm1_ok =
> (session[0].get_command_status(vconfig_cmd
> > + % (vlans[0], vlans[0], subnet2,
> "11")) == 0)
> > + ip_config_vm2_ok =
> (session[1].get_command_status(vconfig_cmd
> > + % (vlans[1], vlans[1], subnet2,
> "12")) == 0)
> > + if not ip_config_vm1_ok or not ip_config_vm2_ok:
> > + raise error.TestError, "Fail to config VMs ip address"
> > + ping_diff_vlan_ok = (session[0].get_command_status(
> > + "ping -c 2 %s.12" % subnet2) == 0)
> > +
> > + if ping_diff_vlan_ok:
> > + raise error.TestFail("VM 2 is unexpectedly pingable in
> different "
> > + "vlan")
> > + # Make vm2 in the same vlan with vm1
> > + vlan_config_vm2_ok = (session[1].get_command_status(
> > + "vconfig rem eth0.%s;vconfig add eth0
> %s;"
> > + "ifconfig eth0.%s %s.12" %
> > + (vlans[1], vlans[0], vlans[0],
> subnet2)) == 0)
> > + if not vlan_config_vm2_ok:
> > + raise error.TestError, "Fail to config ip address of VM
> 2"
> > +
> > + ping_same_vlan_ok = (session[0].get_command_status(
> > + "ping -c 2 %s.12" % subnet2) == 0)
> > + if not ping_same_vlan_ok:
> > + raise error.TestFail("Fail to ping the guest in same
> vlan")
> > + finally:
> > + # Clean the vlan config
> > + for i in range(2):
> > + session[i].sendline("vconfig rem eth0.%s" % vlans[0])
> > + session[i].close()
Please use get_command_output() or get_command_status() instead of
sendline() when possible. get_command_output() waits for the shell prompt
to return so we know the guest received the command. sendline() just sends
a string to the session without waiting, so the close() that follows might
kill the session before it receives or processes the command.
Other than these minor issues the test looks good.
>
> _______________________________________________
> Autotest mailing list
> Autotest@xxxxxxxxxxxxxxx
> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
--
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
