From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lucas Meneghel Rodrigues Subject: Re: [Autotest] [PATCH] Test 802.1Q vlan of nic Date: Wed, 14 Oct 2009 07:51:17 -0300 Message-ID: <6ac58f4f0910140351g62afbe4ak5ada703c598ffbad@mail.gmail.com> References: <1253704740-26370-1-git-send-email-akong@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: autotest@test.kernel.org, kvm@vger.kernel.org To: Amos Kong Return-path: Received: from mail-px0-f180.google.com ([209.85.216.180]:58374 "EHLO mail-px0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756103AbZJNKvx convert rfc822-to-8bit (ORCPT ); Wed, 14 Oct 2009 06:51:53 -0400 Received: by pxi10 with SMTP id 10so111496pxi.4 for ; Wed, 14 Oct 2009 03:51:17 -0700 (PDT) In-Reply-To: <1253704740-26370-1-git-send-email-akong@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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 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 b= y ping > =A0 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 > --- > =A0client/tests/kvm/kvm_tests.cfg.sample | =A0 =A06 +++ > =A0client/tests/kvm/tests/vlan_tag.py =A0 =A0| =A0 66 +++++++++++++++= ++++++++++++++++++ > =A02 files changed, 72 insertions(+), 0 deletions(-) > =A0create 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: > =A0 =A0 =A0 =A0 kill_vm =3D yes > =A0 =A0 =A0 =A0 kill_vm_gracefully =3D no > > + =A0 =A0- vlan_tag: =A0install setup > + =A0 =A0 =A0 =A0type =3D vlan_tag > + =A0 =A0 =A0 =A0subnet2 =3D 192.168.123 > + =A0 =A0 =A0 =A0vlans =3D "10 20" > + =A0 =A0 =A0 =A0nic_mode =3D tap > + =A0 =A0 =A0 =A0nic_model =3D e1000 > > =A0# NICs > =A0variants: > diff --git a/client/tests/kvm/tests/vlan_tag.py b/client/tests/kvm/te= sts/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): > + =A0 =A0""" > + =A0 =A0Test 802.1Q vlan of nic, config it by vconfig command. > + > + =A0 =A01) Create two VMs > + =A0 =A02) Setup guests in different vlan by vconfig and test commun= ication by ping > + =A0 =A0 =A0 using hard-coded ip address > + =A0 =A03) Setup guests in same vlan and test communication by ping > + =A0 =A04) Recover the vlan config > + > + =A0 =A0@param test: Kvm test object > + =A0 =A0@param params: Dictionary with the test parameters. > + =A0 =A0@param env: Dictionary with test environment. > + =A0 =A0""" > + > + =A0 =A0vm =3D [] > + =A0 =A0session =3D [] > + =A0 =A0subnet2 =3D params.get("subnet2") > + =A0 =A0vlans =3D params.get("vlans").split() > + > + =A0 =A0vm.append(kvm_test_utils.get_living_vm(env, "%s" % params.ge= t("main_vm"))) > + > + =A0 =A0params_vm2 =3D params.copy() > + =A0 =A0params_vm2['image_snapshot'] =3D "yes" > + =A0 =A0params_vm2['kill_vm_gracefully'] =3D "no" > + =A0 =A0params_vm2["address_index"] =3D int(params.get("address_inde= x", 0))+1 > + =A0 =A0vm.append(vm[0].clone("vm2", params_vm2)) > + =A0 =A0kvm_utils.env_register_vm(env, "vm2", vm[1]) > + =A0 =A0if not vm[1].create(): > + =A0 =A0 =A0 =A0raise 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") > + =A0 =A0for i in range(2): > + =A0 =A0 =A0 =A0session.append(kvm_test_utils.wait_for_login(vm[i])) > + > + =A0 =A0try: > + =A0 =A0 =A0 =A0vconfig_cmd =3D "vconfig add eth0 %s;ifconfig eth0.%= s %s.%s" > + =A0 =A0 =A0 =A0if session[0].get_command_status(vconfig_cmd % (vlan= s[0], > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0vlans[0], > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0subnet2, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"11")) !=3D 0 or \ > + =A0 =A0 =A0 =A0 =A0 session[1].get_command_status(vconfig_cmd % (vl= ans[1], > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0vlans[1], > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0subnet2, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"12")) !=3D 0: In the above if statement, I'd assign the comparisons to variables to make the code more readable, like: try: vconfig_cmd =3D "vconfig add eth0 %s;ifconfig eth0.%s %s.%s" # Attempt to configure IPs for the VMs and record the results in bo= olean # variables ip_config_vm1_ok =3D (session[0].get_command_status( vconfig_cmd % (vlans[0], vlans[0], subnet2, "11= ")) =3D=3D 0) ip_config_vm1_ok =3D (session[1].get_command_status( vconfig_cmd % (vlans[1], vlans[1], subnet2, "12= ")) =3D=3D 0) if not ip_config_vm1_ok or not ip_config_vm2_ok: > + =A0 =A0 =A0 =A0 =A0 =A0raise error.TestError, "Fail to config VMs i= p address" > + =A0 =A0 =A0 =A0if session[0].get_command_status("ping -c 2 %s.12" %= subnet2) =3D=3D 0: > + =A0 =A0 =A0 =A0 =A0 =A0raise error.TestFail("Guest is unexpectedly = pingable in different " > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "vl= an") A similar comment applies to the above block > + =A0 =A0 =A0 =A0if session[1].get_command_status("vconfig rem eth0.%= s;vconfig add eth0 " > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 "%s;ifconfig eth0.%s %s.12" % > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0(vlans[1], > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 vlans[0], > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 vlans[0], > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 subnet2)) !=3D 0: Idem > + =A0 =A0 =A0 =A0 =A0 =A0raise error.TestError, "Fail to config ip ad= dress of VM 'vm[1]'" > + =A0 =A0 =A0 =A0if session[0].get_command_status("ping -c 2 %s.12" %= subnet2) !=3D 0: > + =A0 =A0 =A0 =A0 =A0 =A0raise error.TestFail, "Fail to ping the gues= t in same vlan" See comment about the preferred form to write raise statements. > + =A0 =A0finally: > + =A0 =A0 =A0 =A0for i in range(2): > + =A0 =A0 =A0 =A0 =A0 =A0session[i].sendline("vconfig rem eth0.%s" % = vlans[0]) > + =A0 =A0 =A0 =A0 =A0 =A0session[i].close() > -- > 1.5.5.6 Please send me an updated version of the patch. Lucas