Re: [Autotest] [PATCH] Test 802.1Q vlan of nic

2009-10-26 Thread Amos Kong
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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Autotest] [PATCH] Test 802.1Q vlan of nic

2009-10-21 Thread Amos Kong
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 Kongak...@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 000..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
 +
 +

Re: [Autotest] [PATCH] Test 802.1Q vlan of nic

2009-10-21 Thread Amos Kong
On Tue, Oct 20, 2009 at 09:19:50AM -0400, Michael Goldish wrote:
 See comments below.

Hi all,
Thanks for your reply.
 
 - Dor Laor dl...@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 Kongak...@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 000..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 

Re: [Autotest] [PATCH] Test 802.1Q vlan of nic

2009-10-21 Thread Michael Goldish

- Amos Kong ak...@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 dl...@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 Kongak...@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 000..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
  

Re: [Autotest] [PATCH] Test 802.1Q vlan of nic

2009-10-21 Thread Uri Lublin

On 10/21/2009 12:37 PM, Amos Kong wrote:

On Tue, Oct 20, 2009 at 09:19:50AM -0400, Michael Goldish wrote:

- Dor Laordl...@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.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Autotest] [PATCH] Test 802.1Q vlan of nic

2009-10-21 Thread Dor Laor

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 Laordl...@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.


--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Autotest] [PATCH] Test 802.1Q vlan of nic

2009-10-20 Thread Michael Goldish
See comments below.

- Dor Laor dl...@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 Kongak...@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 000..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
  +# 

Re: [Autotest] [PATCH] Test 802.1Q vlan of nic

2009-10-20 Thread Lucas Meneghel Rodrigues
On Tue, Oct 20, 2009 at 11:19 AM, Michael Goldish mgold...@redhat.com wrote:
 See comments below.

 - Dor Laor dl...@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 Kongak...@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 000..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 

Re: [Autotest] [PATCH] Test 802.1Q vlan of nic

2009-10-19 Thread Dor Laor

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 Kongak...@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 000..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])
+

Re: [Autotest] [PATCH] Test 802.1Q vlan of nic

2009-10-14 Thread Lucas Meneghel Rodrigues
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 ak...@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 ak...@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 000..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