* [PATCH] Test 802.1Q vlan of nic @ 2009-09-23 11:19 Amos Kong 2009-10-14 10:51 ` [Autotest] " Lucas Meneghel Rodrigues 0 siblings, 1 reply; 12+ messages in thread From: Amos Kong @ 2009-09-23 11:19 UTC (permalink / raw) To: autotest, kvm; +Cc: lmr, Amos Kong 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@redhat.com> --- client/tests/kvm/kvm_tests.cfg.sample | 6 +++ client/tests/kvm/tests/vlan_tag.py | 66 +++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 0 deletions(-) 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 285a38f..5a3f97d 100644 --- a/client/tests/kvm/kvm_tests.cfg.sample +++ b/client/tests/kvm/kvm_tests.cfg.sample @@ -145,6 +145,12 @@ variants: kill_vm = yes kill_vm_gracefully = no + - vlan_tag: install setup + type = vlan_tag + subnet2 = 192.168.123 + vlans = "10 20" + nic_mode = tap + nic_model = e1000 # NICs variants: diff --git a/client/tests/kvm/tests/vlan_tag.py b/client/tests/kvm/tests/vlan_tag.py new file mode 100644 index 0000000..2904276 --- /dev/null +++ b/client/tests/kvm/tests/vlan_tag.py @@ -0,0 +1,66 @@ +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"))) + + 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 'vm[1]' create faild" + + 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" + if session[0].get_command_status(vconfig_cmd % (vlans[0], + vlans[0], + subnet2, + "11")) != 0 or \ + session[1].get_command_status(vconfig_cmd % (vlans[1], + vlans[1], + subnet2, + "12")) != 0: + raise error.TestError, "Fail to config VMs ip address" + if session[0].get_command_status("ping -c 2 %s.12" % subnet2) == 0: + raise error.TestFail("Guest is unexpectedly pingable in different " + "vlan") + + if 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: + raise error.TestError, "Fail to config ip address of VM 'vm[1]'" + if session[0].get_command_status("ping -c 2 %s.12" % subnet2) != 0: + raise error.TestFail, "Fail to ping the guest in same vlan" + finally: + for i in range(2): + session[i].sendline("vconfig rem eth0.%s" % vlans[0]) + session[i].close() -- 1.5.5.6 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Autotest] [PATCH] Test 802.1Q vlan of nic 2009-09-23 11:19 [PATCH] Test 802.1Q vlan of nic Amos Kong @ 2009-10-14 10:51 ` Lucas Meneghel Rodrigues 2009-10-15 9:48 ` Amos Kong 0 siblings, 1 reply; 12+ messages in thread From: Lucas Meneghel Rodrigues @ 2009-10-14 10:51 UTC (permalink / raw) To: Amos Kong; +Cc: autotest, kvm Hi Amos, thanks for the patch, here are my comments (pretty much concerning only coding style): On Wed, Sep 23, 2009 at 8:19 AM, Amos Kong <akong@redhat.com> 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@redhat.com> > --- > client/tests/kvm/kvm_tests.cfg.sample | 6 +++ > client/tests/kvm/tests/vlan_tag.py | 66 +++++++++++++++++++++++++++++++++ > 2 files changed, 72 insertions(+), 0 deletions(-) > 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 285a38f..5a3f97d 100644 > --- a/client/tests/kvm/kvm_tests.cfg.sample > +++ b/client/tests/kvm/kvm_tests.cfg.sample > @@ -145,6 +145,12 @@ variants: > kill_vm = yes > kill_vm_gracefully = no > > + - vlan_tag: install setup > + type = vlan_tag > + subnet2 = 192.168.123 > + vlans = "10 20" > + nic_mode = tap > + nic_model = e1000 > > # NICs > variants: > diff --git a/client/tests/kvm/tests/vlan_tag.py b/client/tests/kvm/tests/vlan_tag.py > new file mode 100644 > index 0000000..2904276 > --- /dev/null > +++ b/client/tests/kvm/tests/vlan_tag.py > @@ -0,0 +1,66 @@ > +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"))) > + > + 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 'vm[1]' create faild" In the above exception raise statement, the preferred form to do it is: raise error.TestError("VM 1 create failed") > + 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" > + if session[0].get_command_status(vconfig_cmd % (vlans[0], > + vlans[0], > + subnet2, > + "11")) != 0 or \ > + session[1].get_command_status(vconfig_cmd % (vlans[1], > + vlans[1], > + subnet2, > + "12")) != 0: In the above if statement, I'd assign the comparisons to variables to make the code more readable, like: 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 ip_config_vm1_ok = (session[0].get_command_status( vconfig_cmd % (vlans[0], vlans[0], subnet2, "11")) == 0) ip_config_vm1_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" > + if session[0].get_command_status("ping -c 2 %s.12" % subnet2) == 0: > + raise error.TestFail("Guest is unexpectedly pingable in different " > + "vlan") A similar comment applies to the above block > + if 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: Idem > + raise error.TestError, "Fail to config ip address of VM 'vm[1]'" > + if session[0].get_command_status("ping -c 2 %s.12" % subnet2) != 0: > + raise error.TestFail, "Fail to ping the guest in same vlan" See comment about the preferred form to write raise statements. > + finally: > + for i in range(2): > + session[i].sendline("vconfig rem eth0.%s" % vlans[0]) > + session[i].close() > -- > 1.5.5.6 Please send me an updated version of the patch. Lucas ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Autotest] [PATCH] Test 802.1Q vlan of nic 2009-10-14 10:51 ` [Autotest] " Lucas Meneghel Rodrigues @ 2009-10-15 9:48 ` Amos Kong 2009-10-19 8:22 ` Dor Laor 0 siblings, 1 reply; 12+ messages in thread From: Amos Kong @ 2009-10-15 9:48 UTC (permalink / raw) To: Lucas Meneghel Rodrigues; +Cc: autotest, kvm 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@redhat.com> --- 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 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" + nic_mode = tap + nic_model = e1000 - 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"))) + + 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") + + 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() -- 1.5.5.6 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Autotest] [PATCH] Test 802.1Q vlan of nic 2009-10-15 9:48 ` Amos Kong @ 2009-10-19 8:22 ` Dor Laor 2009-10-21 10:04 ` Amos Kong 0 siblings, 1 reply; 12+ messages in thread From: Dor Laor @ 2009-10-19 8:22 UTC (permalink / raw) To: Amos Kong; +Cc: Lucas Meneghel Rodrigues, autotest, kvm 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@redhat.com> > --- > 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. > + 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? > > - 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"))) > + > + 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. > + > + 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() ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Autotest] [PATCH] Test 802.1Q vlan of nic 2009-10-19 8:22 ` Dor Laor @ 2009-10-21 10:04 ` Amos Kong 0 siblings, 0 replies; 12+ messages in thread From: Amos Kong @ 2009-10-21 10:04 UTC (permalink / raw) To: Dor Laor; +Cc: Lucas Meneghel Rodrigues, autotest, kvm On Mon, Oct 19, 2009 at 10:22:21AM +0200, Dor Laor 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@redhat.com> >> --- >> 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. yes. >> 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. > >> + 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? yes, we should remove 'nic_model = e1000' for testing all kind of nic. It seems that there exists a kvm bug (https://bugzilla.redhat.com/show_bug.cgi?id=516587) It was found by this testcase. >> - 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"))) >> + >> + 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. > >> + >> + 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() -- Amos Kong Quality Engineer Raycom Office(Beijing), Red Hat Inc. Phone: +86-10-62608183 ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <301351519.552421256044394416.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com>]
* Re: [Autotest] [PATCH] Test 802.1Q vlan of nic [not found] <301351519.552421256044394416.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com> @ 2009-10-20 13:19 ` Michael Goldish 2009-10-20 13:38 ` Lucas Meneghel Rodrigues 2009-10-21 10:37 ` Amos Kong 0 siblings, 2 replies; 12+ messages in thread From: Michael Goldish @ 2009-10-20 13:19 UTC (permalink / raw) To: Amos Kong; +Cc: autotest, kvm, dlaor See comments below. ----- "Dor Laor" <dlaor@redhat.com> 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@redhat.com> > > --- > > 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@test.kernel.org > http://test.kernel.org/cgi-bin/mailman/listinfo/autotest ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Autotest] [PATCH] Test 802.1Q vlan of nic 2009-10-20 13:19 ` Michael Goldish @ 2009-10-20 13:38 ` Lucas Meneghel Rodrigues 2009-10-21 10:37 ` Amos Kong 1 sibling, 0 replies; 12+ messages in thread From: Lucas Meneghel Rodrigues @ 2009-10-20 13:38 UTC (permalink / raw) To: Michael Goldish, dlaor; +Cc: Amos Kong, autotest, kvm On Tue, Oct 20, 2009 at 11:19 AM, Michael Goldish <mgoldish@redhat.com> wrote: > See comments below. > > ----- "Dor Laor" <dlaor@redhat.com> 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@redhat.com> >> > --- >> > 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. Ok Michael, I found another small things that I was correcting here, so I will send soon a version addressing your comments and Dor's. >> >> _______________________________________________ >> Autotest mailing list >> Autotest@test.kernel.org >> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest > _______________________________________________ > Autotest mailing list > Autotest@test.kernel.org > http://test.kernel.org/cgi-bin/mailman/listinfo/autotest > -- Lucas ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Autotest] [PATCH] Test 802.1Q vlan of nic 2009-10-20 13:19 ` Michael Goldish 2009-10-20 13:38 ` Lucas Meneghel Rodrigues @ 2009-10-21 10:37 ` Amos Kong 2009-10-21 13:46 ` Uri Lublin 2009-10-27 4:10 ` Amos Kong 1 sibling, 2 replies; 12+ messages in thread From: Amos Kong @ 2009-10-21 10:37 UTC (permalink / raw) To: Michael Goldish; +Cc: autotest, kvm, dlaor On Tue, Oct 20, 2009 at 09:19:50AM -0400, Michael Goldish wrote: > See comments below. Hi all, Thanks for your reply. > ----- "Dor Laor" <dlaor@redhat.com> 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@redhat.com> > > > --- > > > 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? Setup the second VM with -snapshot is enough. The image can only be R/W by 1th VM. > > > + 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. No, this line is effective. If reserve this line, this case just test e1000, not the default 8139. > > > > > > - 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"). Yes, this is better. > > > + > > > + 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. Agree with you. When I test this case, the original get_command_status() always cause special read problem, so I use sendline(). I'll replace sendline() with get_command_status() later. > Other than these minor issues the test looks good. I'll re-send another patch later. Thanks again! -- Amos Kong Quality Engineer Raycom Office(Beijing), Red Hat Inc. Phone: +86-10-62608183 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Autotest] [PATCH] Test 802.1Q vlan of nic 2009-10-21 10:37 ` Amos Kong @ 2009-10-21 13:46 ` Uri Lublin 2009-10-21 15:49 ` Dor Laor 2009-10-27 4:10 ` Amos Kong 1 sibling, 1 reply; 12+ messages in thread From: Uri Lublin @ 2009-10-21 13:46 UTC (permalink / raw) To: Amos Kong; +Cc: Michael Goldish, kvm, dlaor On 10/21/2009 12:37 PM, Amos Kong wrote: > On Tue, Oct 20, 2009 at 09:19:50AM -0400, Michael Goldish wrote: >> ----- "Dor Laor"<dlaor@redhat.com> wrote: >>> On 10/15/2009 11:48 AM, Amos Kong wrote: >> 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? > > Setup the second VM with -snapshot is enough. The image can only be R/W by 1th VM. > Actually, I agree with Michael. If both VMs use the same disk image, it is safer to setup both VMs with "-snapshot". When the first VM writes to the disk-image the second VM may be affected. >>>> + 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. > > No, this line is effective. If reserve this line, this case just test e1000, not the default 8139. It is overwritten for virtio and untouched for rtl8139 (BTW, we need to add rtl8139 definition instead of leaving it empty and use qemu-kvm default nic). If you really want to only test e1000, a filter is more appropriate. Regards, Uri. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Autotest] [PATCH] Test 802.1Q vlan of nic 2009-10-21 13:46 ` Uri Lublin @ 2009-10-21 15:49 ` Dor Laor 0 siblings, 0 replies; 12+ messages in thread From: Dor Laor @ 2009-10-21 15:49 UTC (permalink / raw) To: Uri Lublin; +Cc: Amos Kong, Michael Goldish, kvm On 10/21/2009 03:46 PM, Uri Lublin wrote: > On 10/21/2009 12:37 PM, Amos Kong wrote: >> On Tue, Oct 20, 2009 at 09:19:50AM -0400, Michael Goldish wrote: >>> ----- "Dor Laor"<dlaor@redhat.com> wrote: >>>> On 10/15/2009 11:48 AM, Amos Kong wrote: >>> 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? >> >> Setup the second VM with -snapshot is enough. The image can only be >> R/W by 1th VM. >> > > Actually, I agree with Michael. If both VMs use the same disk image, it > is safer to setup both VMs with "-snapshot". When the first VM writes to > the disk-image the second VM may be affected. That's a must. If only one VM uses -snapshot, its base will get written and the snapshot will get obsolete. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Autotest] [PATCH] Test 802.1Q vlan of nic 2009-10-21 10:37 ` Amos Kong 2009-10-21 13:46 ` Uri Lublin @ 2009-10-27 4:10 ` Amos Kong 1 sibling, 0 replies; 12+ messages in thread From: Amos Kong @ 2009-10-27 4:10 UTC (permalink / raw) To: Michael Goldish; +Cc: autotest, kvm, dlaor On Wed, Oct 21, 2009 at 06:37:56PM +0800, Amos Kong wrote: > On Tue, Oct 20, 2009 at 09:19:50AM -0400, Michael Goldish wrote: > > See comments below. > > Hi all, > Thanks for your reply. > ..... > > Agree with you. > When I test this case, the original get_command_status() always cause special read problem, so I use sendline(). > > I'll replace sendline() with get_command_status() later. > > > Other than these minor issues the test looks good. > > I'll re-send another patch later. Thanks again! Hello all, Execute on VM1 "ping -c 2 -I eth0.10 IP_Address_eth0.10_VM2" We can use -I option to assign the interface of ping, then no need to make eth0.10 and eth0 in different subnet. But eth0 and eth0.10 have the same mac address, so eth0.10 could not get address by DHCP. If we assign it in the code, it's maybe repeat with others. The method is not better than assigning subnet2 in configure file. So I'll send another new version first. Welcome any suggestion :) Best Regards, Amos -- Amos Kong Quality Engineer Raycom Office(Beijing), Red Hat Inc. Phone: +86-10-62608183 ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <702727267.642641256125764995.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com>]
* Re: [Autotest] [PATCH] Test 802.1Q vlan of nic [not found] <702727267.642641256125764995.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com> @ 2009-10-21 11:56 ` Michael Goldish 0 siblings, 0 replies; 12+ messages in thread From: Michael Goldish @ 2009-10-21 11:56 UTC (permalink / raw) To: Amos Kong; +Cc: autotest, kvm, dlaor ----- "Amos Kong" <akong@redhat.com> wrote: > On Tue, Oct 20, 2009 at 09:19:50AM -0400, Michael Goldish wrote: > > See comments below. > > Hi all, > Thanks for your reply. > > > ----- "Dor Laor" <dlaor@redhat.com> 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@redhat.com> > > > > --- > > > > 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? > > Setup the second VM with -snapshot is enough. The image can only be > R/W by 1th VM. > > > > > + 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. > > No, this line is effective. If reserve this line, this case just test > e1000, not the default 8139. OK, you're right in the case of the default rtl8139 variant. Still, we may want to test it sometimes, and if we want e1000 we can use "only e1000" instead of "only rtl8139", so I don't think this line is necessary. > > > > > > > > - 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"). > > Yes, this is better. > > > > > + > > > > + 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. > > Agree with you. > When I test this case, the original get_command_status() always cause > special read problem, so I use sendline(). > > I'll replace sendline() with get_command_status() later. Can you give me some details on that read problem? If it's a bug we should fix it. > > Other than these minor issues the test looks good. > > I'll re-send another patch later. Thanks again! > > -- > Amos Kong > Quality Engineer > Raycom Office(Beijing), Red Hat Inc. > Phone: +86-10-62608183 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2009-10-27 4:14 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-23 11:19 [PATCH] Test 802.1Q vlan of nic Amos Kong
2009-10-14 10:51 ` [Autotest] " Lucas Meneghel Rodrigues
2009-10-15 9:48 ` Amos Kong
2009-10-19 8:22 ` Dor Laor
2009-10-21 10:04 ` Amos Kong
[not found] <301351519.552421256044394416.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com>
2009-10-20 13:19 ` Michael Goldish
2009-10-20 13:38 ` Lucas Meneghel Rodrigues
2009-10-21 10:37 ` Amos Kong
2009-10-21 13:46 ` Uri Lublin
2009-10-21 15:49 ` Dor Laor
2009-10-27 4:10 ` Amos Kong
[not found] <702727267.642641256125764995.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com>
2009-10-21 11:56 ` Michael Goldish
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).