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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux