Re: [PATCH][KVM-AUTOTEST] TAP network support in kvm-autotest

Re: [PATCH][KVM-AUTOTEST] TAP network support in kvm-autotest

[Michael Goldish]

Hi Jason,

We already have patches that implement similar functionality here in
TLV, as mentioned in the to-do list (item #4 under 'Framework').
They're not yet committed upstream because they're still quite fresh.

Still, your patch looks good and is quite similar to mine. The main
difference is that I use MAC/IP address pools specified by the user,
instead of random MACs with arp/nmap to detect the matching IP
addresses.

I will post my patch to the mailing list soon, but it will come
together with quite a few other patches that I haven't posted yet, so
please be patient.

Comments/questions:

Why do you use nmap in addition to arp? In what cases will arp not
suffice? I'm a little put off by the fact that nmap imposes an
additional requirement on the host. Three hosts I've tried don't come
with nmap installed by default.

Please see additional comments below.

----- "Jason Wang" <jasowang@redhat.com> wrote:

> Hi All:
> This patch tries to add tap network support in kvm-autotest. Multiple
> nics connected to different bridges could be achieved through this
> script. Public bridge is important for testing real network traffic
> and migration. The patch gives each nic with randomly generated mac
> address. The ip address required in the test could be dynamically
> probed through nmap/arp. Only the ip address of first NIC is used
> through the test.
> 
> Example:
> nics = nic1 nic2
> network = bridge
> bridge = switch
> ifup =/etc/qemu-ifup-switch
> ifdown =/etc/qemu-ifdown-switch
> 
> This would make the virtual machine have two nics both of which are
> connected to a bridge with the name of 'switch'. Ifup/ifdown scripts
> are also specified.
> 
> Another Example:
> nics = nic1 nic2
> network = bridge
> bridge = switch
> bridge_nic2 = virbr0
> ifup =/etc/qemu-ifup-switch
> ifup_nic2 = /etc/qemu-ifup-virbr0
> 
> This would makes the virtual machine have two nics: nic1 are connected
> to bridge 'switch' and nci2 are connected to bridge 'virbr0'.
> 
> Public mode and user mode nic could also be mixed:
> nics = nic1 nic2
> network = bridge
> network_nic2 = user
> 
> Looking forward for comments and suggestions.
> 
> From: jason <jasowang@redhat.com>
> Date: Wed, 13 May 2009 16:15:28 +0800
> Subject: [PATCH] Add tap networking support.
> 
> ---
>  client/tests/kvm_runtest_2/kvm_utils.py |    7 +++
>  client/tests/kvm_runtest_2/kvm_vm.py    |   74
> ++++++++++++++++++++++++++-----
>  2 files changed, 69 insertions(+), 12 deletions(-)
> 
> diff --git a/client/tests/kvm_runtest_2/kvm_utils.py
> b/client/tests/kvm_runtest_2/kvm_utils.py
> index be8ad95..0d1f7f8 100644
> --- a/client/tests/kvm_runtest_2/kvm_utils.py
> +++ b/client/tests/kvm_runtest_2/kvm_utils.py
> @@ -773,3 +773,10 @@ def md5sum_file(filename, size=None):
>          size -= len(data)
>      f.close()
>      return o.hexdigest()
> +
> +def random_mac():
> +    mac=[0x00,0x16,0x30,
> +         random.randint(0x00,0x09),
> +         random.randint(0x00,0x09),
> +         random.randint(0x00,0x09)]
> +    return ':'.join(map(lambda x: "%02x" %x,mac))

Random MAC addresses will not necessarily work everywhere, as far as
I know. That's why I prefer user specified MAC/IP address ranges.

> diff --git a/client/tests/kvm_runtest_2/kvm_vm.py
> b/client/tests/kvm_runtest_2/kvm_vm.py
> index fab839f..ea7dab6 100644
> --- a/client/tests/kvm_runtest_2/kvm_vm.py
> +++ b/client/tests/kvm_runtest_2/kvm_vm.py
> @@ -105,6 +105,10 @@ class VM:
>          self.qemu_path = qemu_path
>          self.image_dir = image_dir
>          self.iso_dir = iso_dir
> +        self.macaddr = []
> +        for nic_name in kvm_utils.get_sub_dict_names(params,"nics"):
> +            macaddr = kvm_utils.random_mac()
> +            self.macaddr.append(macaddr)
>
>      def verify_process_identity(self):
>          """Make sure .pid really points to the original qemu
> process.
> @@ -189,9 +193,25 @@ class VM:
>          for nic_name in kvm_utils.get_sub_dict_names(params,
> "nics"):
>              nic_params = kvm_utils.get_sub_dict(params, nic_name)
>              qemu_cmd += " -net nic,vlan=%d" % vlan
> +            net = nic_params.get("network")
> +            if net == "bridge":
> +                qemu_cmd += ",macaddr=%s" % self.macaddr[vlan]
>              if nic_params.get("nic_model"):
>                  qemu_cmd += ",model=%s" % nic_params.get("nic_model")
> -            qemu_cmd += " -net user,vlan=%d" % vlan
> +            if net == "bridge":
> +                qemu_cmd += " -net tap,vlan=%d" % vlan
> +                ifup = nic_params.get("ifup")
> +                if ifup:
> +                    qemu_cmd += ",script=%s" % ifup
> +                else:
> +                    qemu_cmd += ",script=/etc/qemu-ifup"

Why not just leave 'script' out if the user doesn't specify 'ifup'?
There's no good reason to prefer /etc/qemu-ifup to /etc/kvm-ifup or
anything else, so I think it's best to leave it up to qemu if the
user has no preference. It's also slightly shorter.

> +                ifdown = nic_params.get("ifdown")
> +                if ifdown:
> +                    qemu_cmd += ",downscript=%s" % ifdown
> +                else:
> +                    qemu_cmd += ",downscript=no"

The same applies here.

This is just my opinion; I'd like to hear your thoughts on this too.

> +            else:
> +                qemu_cmd += " -net user,vlan=%d" % vlan
>              vlan += 1
>  
>          mem = params.get("mem")
> @@ -206,11 +226,11 @@ class VM:
>          extra_params = params.get("extra_params")
>          if extra_params:
>              qemu_cmd += " %s" % extra_params
> -
> +            
>          for redir_name in kvm_utils.get_sub_dict_names(params,
> "redirs"):
>              redir_params = kvm_utils.get_sub_dict(params,
> redir_name)
>              guest_port = int(redir_params.get("guest_port"))
> -            host_port = self.get_port(guest_port)
> +            host_port = self.get_port(guest_port,True)
>              qemu_cmd += " -redir tcp:%s::%s" % (host_port,
> guest_port)
>  
>          if params.get("display") == "vnc":
> @@ -467,27 +487,57 @@ class VM:
>          If port redirection is used, return 'localhost' (the guest
> has no IP
>          address of its own).  Otherwise return the guest's IP
> address.
>          """
> -        # Currently redirection is always used, so return
> 'localhost'
> -        return "localhost"
> +        if self.params.get("network") == "bridge":
> +            # probing ip address through arp
> +            bridge_name = self.params['bridge']
> +            macaddr = self.macaddr[0]

I think VM.get_address() should take an index parameter, instead of
just return the first address. The index parameter can default to 0.

> +            lines = os.popen("arp -a").readlines()
> +            for line in lines:
> +                if macaddr in line:
> +                    return line.split()[1].strip('()')
> +                
> +            # probing ip address through nmap
> +            lines = os.popen("ip route").readlines()
> +            birdge_network = None
> +            for line in lines:
> +                if bridge_name in line:
> +                    bridge_network = line.split()[0]
> +                    break
> +                
> +            if bridge_network != None:
> +                lines = os.popen("nmap -sP -n %s" %
> bridge_network).readlines()
> +                lastline = None
> +                for line in lines:
> +                    if macaddr in line:
> +                        return lastline.split()[1]
> +                    lastline = line
> +
> +            # could not found ip address
> +            return None
> +        else:
> +            return "localhost"
>  
> -    def get_port(self, port):
> +    def get_port(self, port, query = False):
>          """Return the port in host space corresponding to port in
> guest space.
>  
>          If port redirection is used, return the host port redirected
> to guest port port.
>          Otherwise return port.
>          """
> -        # Currently redirection is always used, so use the redirs
> dict
> -        if self.redirs.has_key(port):
> -            return self.redirs[port]
> +
> +        if query == True or self.params.get("network") != "bridge":

Why do we need a 'query' parameter here? It looks to me like
self.params.get("network") != "bridge"
should suffice.

> +            if self.redirs.has_key(port):
> +                return self.redirs[port]
> +            else:
> +                kvm_log.debug("Warning: guest port %s requested but
> not redirected" % port)
> +                return None
>          else:
> -            kvm_log.debug("Warning: guest port %s requested but not
> redirected" % port)
> -            return None
> +            return port
>  
>      def is_sshd_running(self, timeout=10):
>          """Return True iff the guest's SSH port is responsive."""
>          address = self.get_address()
>          port = self.get_port(int(self.params.get("ssh_port")))
> -        if not port:
> +        if not port or not address:
>              return False
>          return kvm_utils.is_sshd_running(address, port,
> timeout=timeout)

Again, I will do my best to post my patches soon.

Thanks,
Michael

Re: [PATCH][KVM-AUTOTEST] TAP network support in kvm-autotest

[jason wang]

Michael Goldish 写道:

Hi Micheal, thanks for your comments.
> Hi Jason,
>
> We already have patches that implement similar functionality here in
> TLV, as mentioned in the to-do list (item #4 under 'Framework').
> They're not yet committed upstream because they're still quite fresh.
>   
OK, I would pay more attention to to-do list.
> Still, your patch looks good and is quite similar to mine. The main
> difference is that I use MAC/IP address pools specified by the user,
> instead of random MACs with arp/nmap to detect the matching IP
> addresses.
>   
We've considers the use of MAC/IP address pools, but this method need to 
handle the cases of multiple kvm-autotest running on multiple guests. 
The MAC pools should not overlapped when using public bridges.
> I will post my patch to the mailing list soon, but it will come
> together with quite a few other patches that I haven't posted yet, so
> please be patient.
>
> Comments/questions:
>
> Why do you use nmap in addition to arp? In what cases will arp not
> suffice? I'm a little put off by the fact that nmap imposes an
> additional requirement on the host. Three hosts I've tried don't come
> with nmap installed by default.
>   
We use nmap to make sure the guest IP could be finally found somehow. 
During our tests, the scripts may fail to get the IP address of guest 
when host iptables is turned on.
> Please see additional comments below.
>
> ----- "Jason Wang" <jasowang@redhat.com> wrote:
>
>   
>> Hi All:
>> This patch tries to add tap network support in kvm-autotest. Multiple
>> nics connected to different bridges could be achieved through this
>> script. Public bridge is important for testing real network traffic
>> and migration. The patch gives each nic with randomly generated mac
>> address. The ip address required in the test could be dynamically
>> probed through nmap/arp. Only the ip address of first NIC is used
>> through the test.
>>
>> Example:
>> nics = nic1 nic2
>> network = bridge
>> bridge = switch
>> ifup =/etc/qemu-ifup-switch
>> ifdown =/etc/qemu-ifdown-switch
>>
>> This would make the virtual machine have two nics both of which are
>> connected to a bridge with the name of 'switch'. Ifup/ifdown scripts
>> are also specified.
>>
>> Another Example:
>> nics = nic1 nic2
>> network = bridge
>> bridge = switch
>> bridge_nic2 = virbr0
>> ifup =/etc/qemu-ifup-switch
>> ifup_nic2 = /etc/qemu-ifup-virbr0
>>
>> This would makes the virtual machine have two nics: nic1 are connected
>> to bridge 'switch' and nci2 are connected to bridge 'virbr0'.
>>
>> Public mode and user mode nic could also be mixed:
>> nics = nic1 nic2
>> network = bridge
>> network_nic2 = user
>>
>> Looking forward for comments and suggestions.
>>
>> From: jason <jasowang@redhat.com>
>> Date: Wed, 13 May 2009 16:15:28 +0800
>> Subject: [PATCH] Add tap networking support.
>>
>> ---
>>  client/tests/kvm_runtest_2/kvm_utils.py |    7 +++
>>  client/tests/kvm_runtest_2/kvm_vm.py    |   74
>> ++++++++++++++++++++++++++-----
>>  2 files changed, 69 insertions(+), 12 deletions(-)
>>
>> diff --git a/client/tests/kvm_runtest_2/kvm_utils.py
>> b/client/tests/kvm_runtest_2/kvm_utils.py
>> index be8ad95..0d1f7f8 100644
>> --- a/client/tests/kvm_runtest_2/kvm_utils.py
>> +++ b/client/tests/kvm_runtest_2/kvm_utils.py
>> @@ -773,3 +773,10 @@ def md5sum_file(filename, size=None):
>>          size -= len(data)
>>      f.close()
>>      return o.hexdigest()
>> +
>> +def random_mac():
>> +    mac=[0x00,0x16,0x30,
>> +         random.randint(0x00,0x09),
>> +         random.randint(0x00,0x09),
>> +         random.randint(0x00,0x09)]
>> +    return ':'.join(map(lambda x: "%02x" %x,mac))
>>     
>
> Random MAC addresses will not necessarily work everywhere, as far as
> I know. That's why I prefer user specified MAC/IP address ranges.
>   
Yes, maybe we could use user specified mac address prefix or more useful 
algorithm to generate mac address.
>> diff --git a/client/tests/kvm_runtest_2/kvm_vm.py
>> b/client/tests/kvm_runtest_2/kvm_vm.py
>> index fab839f..ea7dab6 100644
>> --- a/client/tests/kvm_runtest_2/kvm_vm.py
>> +++ b/client/tests/kvm_runtest_2/kvm_vm.py
>> @@ -105,6 +105,10 @@ class VM:
>>          self.qemu_path = qemu_path
>>          self.image_dir = image_dir
>>          self.iso_dir = iso_dir
>> +        self.macaddr = []
>> +        for nic_name in kvm_utils.get_sub_dict_names(params,"nics"):
>> +            macaddr = kvm_utils.random_mac()
>> +            self.macaddr.append(macaddr)
>>
>>      def verify_process_identity(self):
>>          """Make sure .pid really points to the original qemu
>> process.
>> @@ -189,9 +193,25 @@ class VM:
>>          for nic_name in kvm_utils.get_sub_dict_names(params,
>> "nics"):
>>              nic_params = kvm_utils.get_sub_dict(params, nic_name)
>>              qemu_cmd += " -net nic,vlan=%d" % vlan
>> +            net = nic_params.get("network")
>> +            if net == "bridge":
>> +                qemu_cmd += ",macaddr=%s" % self.macaddr[vlan]
>>              if nic_params.get("nic_model"):
>>                  qemu_cmd += ",model=%s" % nic_params.get("nic_model")
>> -            qemu_cmd += " -net user,vlan=%d" % vlan
>> +            if net == "bridge":
>> +                qemu_cmd += " -net tap,vlan=%d" % vlan
>> +                ifup = nic_params.get("ifup")
>> +                if ifup:
>> +                    qemu_cmd += ",script=%s" % ifup
>> +                else:
>> +                    qemu_cmd += ",script=/etc/qemu-ifup"
>>     
>
> Why not just leave 'script' out if the user doesn't specify 'ifup'?
> There's no good reason to prefer /etc/qemu-ifup to /etc/kvm-ifup or
> anything else, so I think it's best to leave it up to qemu if the
> user has no preference. It's also slightly shorter.
>   
Agree
>   
>> +                ifdown = nic_params.get("ifdown")
>> +                if ifdown:
>> +                    qemu_cmd += ",downscript=%s" % ifdown
>> +                else:
>> +                    qemu_cmd += ",downscript=no"
>>     
>
> The same applies here.
>
> This is just my opinion; I'd like to hear your thoughts on this too.
>   
If no downscript is specified, qemu would give a warning.
>> +            else:
>> +                qemu_cmd += " -net user,vlan=%d" % vlan
>>              vlan += 1
>>  
>>          mem = params.get("mem")
>> @@ -206,11 +226,11 @@ class VM:
>>          extra_params = params.get("extra_params")
>>          if extra_params:
>>              qemu_cmd += " %s" % extra_params
>> -
>> +            
>>          for redir_name in kvm_utils.get_sub_dict_names(params,
>> "redirs"):
>>              redir_params = kvm_utils.get_sub_dict(params,
>> redir_name)
>>              guest_port = int(redir_params.get("guest_port"))
>> -            host_port = self.get_port(guest_port)
>> +            host_port = self.get_port(guest_port,True)
>>              qemu_cmd += " -redir tcp:%s::%s" % (host_port,
>> guest_port)
>>  
>>          if params.get("display") == "vnc":
>> @@ -467,27 +487,57 @@ class VM:
>>          If port redirection is used, return 'localhost' (the guest
>> has no IP
>>          address of its own).  Otherwise return the guest's IP
>> address.
>>          """
>> -        # Currently redirection is always used, so return
>> 'localhost'
>> -        return "localhost"
>> +        if self.params.get("network") == "bridge":
>> +            # probing ip address through arp
>> +            bridge_name = self.params['bridge']
>> +            macaddr = self.macaddr[0]
>>     
>
> I think VM.get_address() should take an index parameter, instead of
> just return the first address. The index parameter can default to 0.
>
>   
Agree.
>> +            lines = os.popen("arp -a").readlines()
>> +            for line in lines:
>> +                if macaddr in line:
>> +                    return line.split()[1].strip('()')
>> +                
>> +            # probing ip address through nmap
>> +            lines = os.popen("ip route").readlines()
>> +            birdge_network = None
>> +            for line in lines:
>> +                if bridge_name in line:
>> +                    bridge_network = line.split()[0]
>> +                    break
>> +                
>> +            if bridge_network != None:
>> +                lines = os.popen("nmap -sP -n %s" %
>> bridge_network).readlines()
>> +                lastline = None
>> +                for line in lines:
>> +                    if macaddr in line:
>> +                        return lastline.split()[1]
>> +                    lastline = line
>> +
>> +            # could not found ip address
>> +            return None
>> +        else:
>> +            return "localhost"
>>  
>> -    def get_port(self, port):
>> +    def get_port(self, port, query = False):
>>          """Return the port in host space corresponding to port in
>> guest space.
>>  
>>          If port redirection is used, return the host port redirected
>> to guest port port.
>>          Otherwise return port.
>>          """
>> -        # Currently redirection is always used, so use the redirs
>> dict
>> -        if self.redirs.has_key(port):
>> -            return self.redirs[port]
>> +
>> +        if query == True or self.params.get("network") != "bridge":
>>     
>
> Why do we need a 'query' parameter here? It looks to me like
> self.params.get("network") != "bridge"
> should suffice.
>   
The query parameter is to make sure the guests nics could work well when 
mixing redirections and public bridges. I don't know if this kind of 
composition is useful.
>> +            if self.redirs.has_key(port):
>> +                return self.redirs[port]
>> +            else:
>> +                kvm_log.debug("Warning: guest port %s requested but
>> not redirected" % port)
>> +                return None
>>          else:
>> -            kvm_log.debug("Warning: guest port %s requested but not
>> redirected" % port)
>> -            return None
>> +            return port
>>  
>>      def is_sshd_running(self, timeout=10):
>>          """Return True iff the guest's SSH port is responsive."""
>>          address = self.get_address()
>>          port = self.get_port(int(self.params.get("ssh_port")))
>> -        if not port:
>> +        if not port or not address:
>>              return False
>>          return kvm_utils.is_sshd_running(address, port,
>> timeout=timeout)
>>     
>
> Again, I will do my best to post my patches soon.
>
>   
OK, wait for your patch.
> Thanks,
> Michael
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>   
Thanks.
Jason

Re: [PATCH][KVM-AUTOTEST] TAP network support in kvm-autotest

[sudhir kumar]

On Thu, May 14, 2009 at 4:10 PM, Michael Goldish <mgoldish@redhat.com> wrote:
> Hi Jason,
>
> We already have patches that implement similar functionality here in
> TLV, as mentioned in the to-do list (item #4 under 'Framework').
> They're not yet committed upstream because they're still quite fresh.
>
> Still, your patch looks good and is quite similar to mine. The main
> difference is that I use MAC/IP address pools specified by the user,
> instead of random MACs with arp/nmap to detect the matching IP
> addresses.
>
> I will post my patch to the mailing list soon, but it will come
> together with quite a few other patches that I haven't posted yet, so
> please be patient.
>
> Comments/questions:
>
> Why do you use nmap in addition to arp? In what cases will arp not
> suffice? I'm a little put off by the fact that nmap imposes an
> additional requirement on the host. Three hosts I've tried don't come
> with nmap installed by default.
Using nmap is very slow. It puts lot of delay in the complete test execution.
Michael,
   How do your patches implement ip search of the guest? system arp
might not be sufficient as it can have only certain fix number of
entries and might not have entry for the newly assigned guest ip. I
would like to have a quick look on your patches.
thanks in advance.
>
> Please see additional comments below.
>
> ----- "Jason Wang" <jasowang@redhat.com> wrote:
>
>> Hi All:
>> This patch tries to add tap network support in kvm-autotest. Multiple
>> nics connected to different bridges could be achieved through this
>> script. Public bridge is important for testing real network traffic
>> and migration. The patch gives each nic with randomly generated mac
>> address. The ip address required in the test could be dynamically
>> probed through nmap/arp. Only the ip address of first NIC is used
>> through the test.
>>
>> Example:
>> nics = nic1 nic2
>> network = bridge
>> bridge = switch
>> ifup =/etc/qemu-ifup-switch
>> ifdown =/etc/qemu-ifdown-switch
>>
>> This would make the virtual machine have two nics both of which are
>> connected to a bridge with the name of 'switch'. Ifup/ifdown scripts
>> are also specified.
>>
>> Another Example:
>> nics = nic1 nic2
>> network = bridge
>> bridge = switch
>> bridge_nic2 = virbr0
>> ifup =/etc/qemu-ifup-switch
>> ifup_nic2 = /etc/qemu-ifup-virbr0
>>
>> This would makes the virtual machine have two nics: nic1 are connected
>> to bridge 'switch' and nci2 are connected to bridge 'virbr0'.
>>
>> Public mode and user mode nic could also be mixed:
>> nics = nic1 nic2
>> network = bridge
>> network_nic2 = user
>>
>> Looking forward for comments and suggestions.
>>
>> From: jason <jasowang@redhat.com>
>> Date: Wed, 13 May 2009 16:15:28 +0800
>> Subject: [PATCH] Add tap networking support.
>>
>> ---
>>  client/tests/kvm_runtest_2/kvm_utils.py |    7 +++
>>  client/tests/kvm_runtest_2/kvm_vm.py    |   74
>> ++++++++++++++++++++++++++-----
>>  2 files changed, 69 insertions(+), 12 deletions(-)
>>
>> diff --git a/client/tests/kvm_runtest_2/kvm_utils.py
>> b/client/tests/kvm_runtest_2/kvm_utils.py
>> index be8ad95..0d1f7f8 100644
>> --- a/client/tests/kvm_runtest_2/kvm_utils.py
>> +++ b/client/tests/kvm_runtest_2/kvm_utils.py
>> @@ -773,3 +773,10 @@ def md5sum_file(filename, size=None):
>>          size -= len(data)
>>      f.close()
>>      return o.hexdigest()
>> +
>> +def random_mac():
>> +    mac=[0x00,0x16,0x30,
>> +         random.randint(0x00,0x09),
>> +         random.randint(0x00,0x09),
>> +         random.randint(0x00,0x09)]
>> +    return ':'.join(map(lambda x: "%02x" %x,mac))
>
> Random MAC addresses will not necessarily work everywhere, as far as
> I know. That's why I prefer user specified MAC/IP address ranges.
>
>> diff --git a/client/tests/kvm_runtest_2/kvm_vm.py
>> b/client/tests/kvm_runtest_2/kvm_vm.py
>> index fab839f..ea7dab6 100644
>> --- a/client/tests/kvm_runtest_2/kvm_vm.py
>> +++ b/client/tests/kvm_runtest_2/kvm_vm.py
>> @@ -105,6 +105,10 @@ class VM:
>>          self.qemu_path = qemu_path
>>          self.image_dir = image_dir
>>          self.iso_dir = iso_dir
>> +        self.macaddr = []
>> +        for nic_name in kvm_utils.get_sub_dict_names(params,"nics"):
>> +            macaddr = kvm_utils.random_mac()
>> +            self.macaddr.append(macaddr)
>>
>>      def verify_process_identity(self):
>>          """Make sure .pid really points to the original qemu
>> process.
>> @@ -189,9 +193,25 @@ class VM:
>>          for nic_name in kvm_utils.get_sub_dict_names(params,
>> "nics"):
>>              nic_params = kvm_utils.get_sub_dict(params, nic_name)
>>              qemu_cmd += " -net nic,vlan=%d" % vlan
>> +            net = nic_params.get("network")
>> +            if net == "bridge":
>> +                qemu_cmd += ",macaddr=%s" % self.macaddr[vlan]
>>              if nic_params.get("nic_model"):
>>                  qemu_cmd += ",model=%s" % nic_params.get("nic_model")
>> -            qemu_cmd += " -net user,vlan=%d" % vlan
>> +            if net == "bridge":
>> +                qemu_cmd += " -net tap,vlan=%d" % vlan
>> +                ifup = nic_params.get("ifup")
>> +                if ifup:
>> +                    qemu_cmd += ",script=%s" % ifup
>> +                else:
>> +                    qemu_cmd += ",script=/etc/qemu-ifup"
>
> Why not just leave 'script' out if the user doesn't specify 'ifup'?
> There's no good reason to prefer /etc/qemu-ifup to /etc/kvm-ifup or
> anything else, so I think it's best to leave it up to qemu if the
> user has no preference. It's also slightly shorter.
>
>> +                ifdown = nic_params.get("ifdown")
>> +                if ifdown:
>> +                    qemu_cmd += ",downscript=%s" % ifdown
>> +                else:
>> +                    qemu_cmd += ",downscript=no"
>
> The same applies here.
>
> This is just my opinion; I'd like to hear your thoughts on this too.
>
>> +            else:
>> +                qemu_cmd += " -net user,vlan=%d" % vlan
>>              vlan += 1
>>
>>          mem = params.get("mem")
>> @@ -206,11 +226,11 @@ class VM:
>>          extra_params = params.get("extra_params")
>>          if extra_params:
>>              qemu_cmd += " %s" % extra_params
>> -
>> +
>>          for redir_name in kvm_utils.get_sub_dict_names(params,
>> "redirs"):
>>              redir_params = kvm_utils.get_sub_dict(params,
>> redir_name)
>>              guest_port = int(redir_params.get("guest_port"))
>> -            host_port = self.get_port(guest_port)
>> +            host_port = self.get_port(guest_port,True)
>>              qemu_cmd += " -redir tcp:%s::%s" % (host_port,
>> guest_port)
>>
>>          if params.get("display") == "vnc":
>> @@ -467,27 +487,57 @@ class VM:
>>          If port redirection is used, return 'localhost' (the guest
>> has no IP
>>          address of its own).  Otherwise return the guest's IP
>> address.
>>          """
>> -        # Currently redirection is always used, so return
>> 'localhost'
>> -        return "localhost"
>> +        if self.params.get("network") == "bridge":
>> +            # probing ip address through arp
>> +            bridge_name = self.params['bridge']
>> +            macaddr = self.macaddr[0]
>
> I think VM.get_address() should take an index parameter, instead of
> just return the first address. The index parameter can default to 0.
>
>> +            lines = os.popen("arp -a").readlines()
>> +            for line in lines:
>> +                if macaddr in line:
>> +                    return line.split()[1].strip('()')
>> +
>> +            # probing ip address through nmap
>> +            lines = os.popen("ip route").readlines()
>> +            birdge_network = None
>> +            for line in lines:
>> +                if bridge_name in line:
>> +                    bridge_network = line.split()[0]
>> +                    break
>> +
>> +            if bridge_network != None:
>> +                lines = os.popen("nmap -sP -n %s" %
>> bridge_network).readlines()
>> +                lastline = None
>> +                for line in lines:
>> +                    if macaddr in line:
>> +                        return lastline.split()[1]
>> +                    lastline = line
>> +
>> +            # could not found ip address
>> +            return None
>> +        else:
>> +            return "localhost"
>>
>> -    def get_port(self, port):
>> +    def get_port(self, port, query = False):
>>          """Return the port in host space corresponding to port in
>> guest space.
>>
>>          If port redirection is used, return the host port redirected
>> to guest port port.
>>          Otherwise return port.
>>          """
>> -        # Currently redirection is always used, so use the redirs
>> dict
>> -        if self.redirs.has_key(port):
>> -            return self.redirs[port]
>> +
>> +        if query == True or self.params.get("network") != "bridge":
>
> Why do we need a 'query' parameter here? It looks to me like
> self.params.get("network") != "bridge"
> should suffice.
>
>> +            if self.redirs.has_key(port):
>> +                return self.redirs[port]
>> +            else:
>> +                kvm_log.debug("Warning: guest port %s requested but
>> not redirected" % port)
>> +                return None
>>          else:
>> -            kvm_log.debug("Warning: guest port %s requested but not
>> redirected" % port)
>> -            return None
>> +            return port
>>
>>      def is_sshd_running(self, timeout=10):
>>          """Return True iff the guest's SSH port is responsive."""
>>          address = self.get_address()
>>          port = self.get_port(int(self.params.get("ssh_port")))
>> -        if not port:
>> +        if not port or not address:
>>              return False
>>          return kvm_utils.is_sshd_running(address, port,
>> timeout=timeout)
>
> Again, I will do my best to post my patches soon.
>
> Thanks,
> Michael
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Sudhir Kumar

[PATCH][KVM-AUTOTEST] TAP network support in kvm-autotest

[Jason Wang]

Hi All:
This patch tries to add tap network support in kvm-autotest. Multiple nics connected to different bridges could be achieved through this script. Public bridge is important for testing real network traffic and migration. The patch gives each nic with randomly generated mac address. The ip address required in the test could be dynamically probed through nmap/arp. Only the ip address of first NIC is used through the test.

Example:
nics = nic1 nic2
network = bridge
bridge = switch
ifup =/etc/qemu-ifup-switch
ifdown =/etc/qemu-ifdown-switch

This would make the virtual machine have two nics both of which are connected to a bridge with the name of 'switch'. Ifup/ifdown scripts are also specified.

Another Example:
nics = nic1 nic2
network = bridge
bridge = switch
bridge_nic2 = virbr0
ifup =/etc/qemu-ifup-switch
ifup_nic2 = /etc/qemu-ifup-virbr0

This would makes the virtual machine have two nics: nic1 are connected to bridge 'switch' and nci2 are connected to bridge 'virbr0'.

Public mode and user mode nic could also be mixed:
nics = nic1 nic2
network = bridge
network_nic2 = user

Looking forward for comments and suggestions.

From: jason <jasowang@redhat.com>
Date: Wed, 13 May 2009 16:15:28 +0800
Subject: [PATCH] Add tap networking support.

---
 client/tests/kvm_runtest_2/kvm_utils.py |    7 +++
 client/tests/kvm_runtest_2/kvm_vm.py    |   74 ++++++++++++++++++++++++++-----
 2 files changed, 69 insertions(+), 12 deletions(-)

diff --git a/client/tests/kvm_runtest_2/kvm_utils.py b/client/tests/kvm_runtest_2/kvm_utils.py
index be8ad95..0d1f7f8 100644
--- a/client/tests/kvm_runtest_2/kvm_utils.py
+++ b/client/tests/kvm_runtest_2/kvm_utils.py
@@ -773,3 +773,10 @@ def md5sum_file(filename, size=None):
         size -= len(data)
     f.close()
     return o.hexdigest()
+
+def random_mac():
+    mac=[0x00,0x16,0x30,
+         random.randint(0x00,0x09),
+         random.randint(0x00,0x09),
+         random.randint(0x00,0x09)]
+    return ':'.join(map(lambda x: "%02x" %x,mac))
diff --git a/client/tests/kvm_runtest_2/kvm_vm.py b/client/tests/kvm_runtest_2/kvm_vm.py
index fab839f..ea7dab6 100644
--- a/client/tests/kvm_runtest_2/kvm_vm.py
+++ b/client/tests/kvm_runtest_2/kvm_vm.py
@@ -105,6 +105,10 @@ class VM:
         self.qemu_path = qemu_path
         self.image_dir = image_dir
         self.iso_dir = iso_dir
+        self.macaddr = []
+        for nic_name in kvm_utils.get_sub_dict_names(params,"nics"):
+            macaddr = kvm_utils.random_mac()
+            self.macaddr.append(macaddr)
 
     def verify_process_identity(self):
         """Make sure .pid really points to the original qemu process.
@@ -189,9 +193,25 @@ class VM:
         for nic_name in kvm_utils.get_sub_dict_names(params, "nics"):
             nic_params = kvm_utils.get_sub_dict(params, nic_name)
             qemu_cmd += " -net nic,vlan=%d" % vlan
+            net = nic_params.get("network")
+            if net == "bridge":
+                qemu_cmd += ",macaddr=%s" % self.macaddr[vlan]
             if nic_params.get("nic_model"):
                 qemu_cmd += ",model=%s" % nic_params.get("nic_model")
-            qemu_cmd += " -net user,vlan=%d" % vlan
+            if net == "bridge":
+                qemu_cmd += " -net tap,vlan=%d" % vlan
+                ifup = nic_params.get("ifup")
+                if ifup:
+                    qemu_cmd += ",script=%s" % ifup
+                else:
+                    qemu_cmd += ",script=/etc/qemu-ifup"
+                ifdown = nic_params.get("ifdown")
+                if ifdown:
+                    qemu_cmd += ",downscript=%s" % ifdown
+                else:
+                    qemu_cmd += ",downscript=no"
+            else:
+                qemu_cmd += " -net user,vlan=%d" % vlan
             vlan += 1
 
         mem = params.get("mem")
@@ -206,11 +226,11 @@ class VM:
         extra_params = params.get("extra_params")
         if extra_params:
             qemu_cmd += " %s" % extra_params
-
+            
         for redir_name in kvm_utils.get_sub_dict_names(params, "redirs"):
             redir_params = kvm_utils.get_sub_dict(params, redir_name)
             guest_port = int(redir_params.get("guest_port"))
-            host_port = self.get_port(guest_port)
+            host_port = self.get_port(guest_port,True)
             qemu_cmd += " -redir tcp:%s::%s" % (host_port, guest_port)
 
         if params.get("display") == "vnc":
@@ -467,27 +487,57 @@ class VM:
         If port redirection is used, return 'localhost' (the guest has no IP
         address of its own).  Otherwise return the guest's IP address.
         """
-        # Currently redirection is always used, so return 'localhost'
-        return "localhost"
+        if self.params.get("network") == "bridge":
+            # probing ip address through arp
+            bridge_name = self.params['bridge']
+            macaddr = self.macaddr[0]
+            lines = os.popen("arp -a").readlines()
+            for line in lines:
+                if macaddr in line:
+                    return line.split()[1].strip('()')
+                
+            # probing ip address through nmap
+            lines = os.popen("ip route").readlines()
+            birdge_network = None
+            for line in lines:
+                if bridge_name in line:
+                    bridge_network = line.split()[0]
+                    break
+                
+            if bridge_network != None:
+                lines = os.popen("nmap -sP -n %s" % bridge_network).readlines()
+                lastline = None
+                for line in lines:
+                    if macaddr in line:
+                        return lastline.split()[1]
+                    lastline = line
+
+            # could not found ip address
+            return None
+        else:
+            return "localhost"
 
-    def get_port(self, port):
+    def get_port(self, port, query = False):
         """Return the port in host space corresponding to port in guest space.
 
         If port redirection is used, return the host port redirected to guest port port.
         Otherwise return port.
         """
-        # Currently redirection is always used, so use the redirs dict
-        if self.redirs.has_key(port):
-            return self.redirs[port]
+
+        if query == True or self.params.get("network") != "bridge":
+            if self.redirs.has_key(port):
+                return self.redirs[port]
+            else:
+                kvm_log.debug("Warning: guest port %s requested but not redirected" % port)
+                return None
         else:
-            kvm_log.debug("Warning: guest port %s requested but not redirected" % port)
-            return None
+            return port
 
     def is_sshd_running(self, timeout=10):
         """Return True iff the guest's SSH port is responsive."""
         address = self.get_address()
         port = self.get_port(int(self.params.get("ssh_port")))
-        if not port:
+        if not port or not address:
             return False
         return kvm_utils.is_sshd_running(address, port, timeout=timeout)
 
-- 
1.5.5.6