[PATCH 0/5] Setup private bridge in KVM autotest, get rid of ifup scripts

[PATCH 0/5] Setup private bridge in KVM autotest, get rid of ifup scripts

[Lucas Meneghel Rodrigues]

This patch combines 2 awesome patchsets by Amos and Jason to
single handedly slay 2 long time annoyances for KVM autotest users:

* By default we were using qemu userspace networking, a mode unsupported
by KVM developers and not terribly useful, and many network tests are
not supposed to work properly with it
* Use of ifup scripts that tried to do a very naive form of bridge
autodetection. If we control our bridge creation, we don't need no
autodetection.

Carefully made, verified, but not tested in several setups.
This might need some more testing love before we can get it
upstream once for all.

Changes from v2:
* Try to close tap file descriptors in order to run multiple iterations

Lucas Meneghel Rodrigues (5):
  KVM test: Adding framework code to control bridge creation
  KVM test: Add helpers to control the TAP/bridge
  KVM test: virt_env_process: Setup private bridge during
    postprocessing
  KVM test: setup tap fd and pass it to qemu-kvm v3
  KVM test: Changing KVM autotest default to private bridge

 client/tests/kvm/scripts/qemu-ifup     |   11 --
 client/tests/kvm/tests_base.cfg.sample |   12 +-
 client/virt/kvm_vm.py                  |   59 ++++++---
 client/virt/virt_env_process.py        |    9 ++
 client/virt/virt_test_setup.py         |  103 +++++++++++++++
 client/virt/virt_utils.py              |  226 ++++++++++++++++++++++++++++++++
 6 files changed, 384 insertions(+), 36 deletions(-)
 delete mode 100755 client/tests/kvm/scripts/qemu-ifup

-- 
1.7.5.1

[PATCH 1/5] KVM test: Adding framework code to control bridge creation

[Lucas Meneghel Rodrigues]

Provide in framework utility code to control the creation
of a bridge, in order to provide TAP functionality for
autotest users without relying on previous setup made by
the user.

This is a reimplementation of Amos's code, the differences
are:

 * Implemented as a setup class, taking advantage of object
internal state to use in different places of the code
 * Split up the operations to make it easier to understand
the steps and why we are doing them
 * Use of autotest API instead of commands

Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
Signed-off-by: Amos Kong <akong@redhat.com>
---
 client/virt/virt_test_setup.py |  103 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 103 insertions(+), 0 deletions(-)

diff --git a/client/virt/virt_test_setup.py b/client/virt/virt_test_setup.py
index f915c1b..cdc0e23 100644
--- a/client/virt/virt_test_setup.py
+++ b/client/virt/virt_test_setup.py
@@ -105,3 +105,106 @@ class HugePageConfig(object):
             return
         utils.system("echo 0 > %s" % self.kernel_hp_file)
         logging.debug("Hugepage memory successfuly dealocated")
+
+
+class PrivateBridgeError(Exception):
+    def __init__(self, brname):
+        self.brname = brname
+
+    def __str__(self):
+        return "Bridge %s not available after setup" % self.brname
+
+
+class PrivateBridgeConfig(object):
+    __shared_state = {}
+    def __init__(self, params=None):
+        self.__dict__ = self.__shared_state
+        if params is not None:
+            self.brname = params.get("priv_brname", 'atbr0')
+            self.subnet = params.get("priv_subnet", '192.168.58')
+            self.ip_version = params.get("bridge_ip_version", "ipv4")
+            self.dhcp_server_pid = None
+
+
+    def _add_bridge(self):
+        utils.system("brctl addbr %s" % self.brname)
+        ip_fwd_path = "/proc/sys/net/%s/ip_forward" % self.ip_version
+        ip_fwd = open(ip_fwd_path, "w")
+        ip_fwd.write("1\n")
+        utils.system("brctl stp %s on" % self.brname)
+        utils.system("brctl setfd %s 0" % self.brname)
+
+
+    def _bring_bridge_up(self):
+        utils.system("ifconfig %s %s.1 up" % (self.brname, self.subnet))
+
+
+    def _enable_nat(self):
+        utils.system("iptables -t nat -A POSTROUTING -s %s.254/24 ! "
+                     "-d %s.254/24 -j MASQUERADE" % (self.subnet, self.subnet))
+
+
+    def _start_dhcp_server(self):
+        utils.system("service dnsmasq stop")
+        utils.system("dnsmasq --strict-order --bind-interfaces "
+                     "--listen-address %s.1 --dhcp-range %s.1,%s.254 "
+                     "--pid-file=/tmp/dnsmasq.pid "
+                     "--log-facility=/tmp/dnsmasq.log" %
+                     (self.subnet, self.subnet, self.subnet))
+        try:
+            self.dhcp_server_pid = int(open('/tmp/dnsmasq.pid', 'r').read())
+        except ValueError:
+            raise PrivateBridgeError(self.brname)
+        logging.debug("Started internal DHCP server with PID %s",
+                      self.dhcp_server_pid)
+
+
+    def _verify_bridge(self):
+        brctl_output = utils.system_output("brctl show")
+        if self.brname not in brctl_output:
+            raise PrivateBridgeError(self.brname)
+
+
+    def setup(self):
+        brctl_output = utils.system_output("brctl show")
+        if self.brname not in brctl_output:
+            logging.info("Configuring KVM test private bridge %s", self.brname)
+            self._add_bridge()
+            self._bring_bridge_up()
+            self._enable_nat()
+            self._start_dhcp_server()
+            self._verify_bridge()
+
+
+    def _stop_dhcp_server(self):
+        os.kill(self.dhcp_server_pid, 15)
+
+
+    def _bring_bridge_down(self):
+        utils.system("ifconfig %s down" % self.brname)
+
+
+    def _disable_nat(self):
+        utils.system("iptables -t nat -D POSTROUTING -s %s.254/24 !"
+                     " -d %s.254/24 -j MASQUERADE" % (self.subnet, self.subnet))
+
+
+    def _remove_bridge(self):
+        utils.system("brctl delbr %s" % self.brname)
+
+
+    def cleanup(self):
+        brctl_output = utils.system_output("brctl show")
+        cleanup = False
+        for line in brctl_output:
+            if line.startswith(self.brname):
+                # len == 4 means there is a TAP using the bridge
+                # so don't try to clean up
+                if len(line.split()) < 4:
+                    cleanup = True
+        if cleanup:
+            logging.debug("Cleaning up KVM test private bridge %s", self.brname)
+            self._stop_dhcp_server()
+            self._bring_bridge_down()
+            self._disable_nat()
+            self._remove_bridge()
-- 
1.7.5.1

[PATCH 2/5] KVM test: Add helpers to control the TAP/bridge

[Lucas Meneghel Rodrigues]

This patch adds some helpers to assist virt test to setup the bridge or
macvtap based guest networking.

Changes from v1:
 * Fixed undefined variable errors on the exception class definitions

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
---
 client/virt/virt_utils.py |  222 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 222 insertions(+), 0 deletions(-)

diff --git a/client/virt/virt_utils.py b/client/virt/virt_utils.py
index 5510c89..7eeaf4f 100644
--- a/client/virt/virt_utils.py
+++ b/client/virt/virt_utils.py
@@ -6,6 +6,7 @@ KVM test utility functions.
 
 import time, string, random, socket, os, signal, re, logging, commands, cPickle
 import fcntl, shelve, ConfigParser, threading, sys, UserDict, inspect
+import struct
 from autotest_lib.client.bin import utils, os_dep
 from autotest_lib.client.common_lib import error, logging_config
 import rss_client, aexpect
@@ -15,6 +16,20 @@ try:
 except ImportError:
     KOJI_INSTALLED = False
 
+# From include/linux/sockios.h
+SIOCSIFHWADDR = 0x8924
+SIOCGIFHWADDR = 0x8927
+SIOCSIFFLAGS = 0x8914
+SIOCGIFINDEX = 0x8933
+SIOCBRADDIF = 0x89a2
+# From linux/include/linux/if_tun.h
+TUNSETIFF = 0x400454ca
+TUNGETIFF = 0x800454d2
+TUNGETFEATURES = 0x800454cf
+IFF_UP = 0x1
+IFF_TAP = 0x0002
+IFF_NO_PI = 0x1000
+IFF_VNET_HDR = 0x4000
 
 def _lock_file(filename):
     f = open(filename, "w")
@@ -36,6 +51,80 @@ def is_vm(obj):
     return obj.__class__.__name__ == "VM"
 
 
+class NetError(Exception):
+    pass
+
+
+class TAPModuleError(NetError):
+    def __init__(self, devname):
+        NetError.__init__(self, devname)
+        self.devname = devname
+
+    def __str__(self):
+        return "Can't open %s" % self.devname
+
+class TAPNotExistError(NetError):
+    def __init__(self, ifname):
+        NetError.__init__(self, ifname)
+        self.ifname = ifname
+
+    def __str__(self):
+        return "Interface %s does not exist" % self.ifname
+
+
+class TAPCreationError(NetError):
+    def __init__(self, ifname, details=None):
+        NetError.__init__(self, ifname, details)
+        self.ifname = ifname
+        self.details = details
+
+    def __str__(self):
+        e_msg = "Cannot create TAP device %s" % self.ifname
+        if self.details is not None:
+            e_msg += ": %s" % self.details
+        return e_msg
+
+
+class TAPBringUpError(NetError):
+    def __init__(self, ifname):
+        NetError.__init__(self, ifname)
+        self.ifname = ifname
+
+    def __str__(self):
+        return "Cannot bring up TAP %s" % self.ifname
+
+
+class BRAddIfError(NetError):
+    def __init__(self, ifname, brname, details):
+        NetError.__init__(self, ifname, brname, details)
+        self.ifname = ifname
+        self.brname = brname
+        self.details = details
+
+    def __str__(self):
+        return ("Can not add if %s to bridge %s: %s" %
+                (self.ifname, self.brname, self.details))
+
+
+class HwAddrSetError(NetError):
+    def __init__(self, ifname, mac):
+        NetError.__init__(self, ifname, mac)
+        self.ifname = ifname
+        self.mac = mac
+
+    def __str__(self):
+        return "Can not set mac %s to interface %s" % (self.mac, self.ifname)
+
+
+class HwAddrGetError(NetError):
+    def __init__(self, ifname):
+        NetError.__init__(self, ifname)
+        self.ifname = ifname
+
+    def __str__(self):
+        return "Can not get mac of interface %s" % self.ifname
+
+
 class Env(UserDict.IterableUserDict):
     """
     A dict-like object containing global objects used by tests.
@@ -2307,3 +2396,136 @@ def install_host_kernel(job, params):
     else:
         logging.info('Chose %s, using the current kernel for the host',
                      install_type)
+
+
+def bridge_auto_detect():
+    """
+    Automatically detect a bridge for tap through brctl.
+    """
+    try:
+        brctl_output = utils.system_output("ip route list",
+                                           retain_output=True)
+        brname = re.findall("default.*dev (.*) ", brctl_output)[0]
+    except:
+        raise BRAutoDetectError
+    return brname
+
+
+def if_nametoindex(ifname):
+    """
+    Map an interface name into its corresponding index.
+    Returns 0 on error, as 0 is not a valid index
+
+    @param ifname: interface name
+    """
+    index = 0
+    ctrl_sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
+    ifr = struct.pack("16si", ifname, 0)
+    r = fcntl.ioctl(ctrl_sock, SIOCGIFINDEX, ifr)
+    index = struct.unpack("16si", r)[1]
+    ctrl_sock.close()
+    return index
+
+
+def vnet_hdr_probe(tapfd):
+    """
+    Check if the IFF_VNET_HDR is support by tun.
+
+    @param tapfd: the file descriptor of /dev/net/tun
+    """
+    u = struct.pack("I", 0)
+    r = fcntl.ioctl(tapfd, TUNGETFEATURES, u)
+    flags = struct.unpack("I", r)[0]
+    if flags & IFF_VNET_HDR:
+        return True
+    else:
+        return False
+
+
+def open_tap(devname, ifname, vnet_hdr=True):
+    """
+    Open a tap device and returns its file descriptor which is used by
+    fd=<fd> parameter of qemu-kvm.
+
+    @param ifname: TAP interface name
+    @param vnet_hdr: Whether enable the vnet header
+    """
+    try:
+        tapfd = os.open(devname, os.O_RDWR)
+    except OSError, e:
+        raise TAPModuleError(devname, "open", e)
+    flags = IFF_TAP | IFF_NO_PI
+    if vnet_hdr and vnet_hdr_probe(tapfd):
+        flags |= IFF_VNET_HDR
+
+    ifr = struct.pack("16sh", ifname, flags)
+    try:
+        r = fcntl.ioctl(tapfd, TUNSETIFF, ifr)
+    except IOError, details:
+        raise TAPCreationError(ifname, details)
+    ifname = struct.unpack("16sh", r)[0].strip("\x00")
+    return tapfd
+
+
+def add_to_bridge(ifname, brname):
+    """
+    Add a TAP device to bridge
+
+    @param ifname: Name of TAP device
+    @param brname: Name of the bridge
+    """
+    ctrl_sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
+    index = if_nametoindex(ifname)
+    if index == 0:
+        raise TAPNotExistError(ifname)
+    ifr = struct.pack("16si", brname, index)
+    try:
+        r = fcntl.ioctl(ctrl_sock, SIOCBRADDIF, ifr)
+    except IOError, details:
+        raise BRAddIfError(ifname, brname, details)
+    ctrl_sock.close()
+
+
+def bring_up_ifname(ifname):
+    """
+    Bring up an interface
+
+    @param ifname: Name of the interface
+    """
+    ctrl_sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
+    ifr = struct.pack("16si", ifname, IFF_UP)
+    try:
+        fcntl.ioctl(ctrl_sock, SIOCSIFFLAGS, ifr)
+    except IOError:
+        raise TAPBringUpError(ifname)
+    ctrl_sock.close()
+
+
+def if_set_macaddress(ifname, mac):
+    """
+    Set the mac address for an interface
+
+    @param ifname: Name of the interface
+    @mac: Mac address
+    """
+    ctrl_sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
+
+    ifr = struct.pack("256s", ifname)
+    try:
+        mac_dev = fcntl.ioctl(ctrl_sock, SIOCGIFHWADDR, ifr)[18:24]
+        mac_dev = ":".join(["%02x" % ord(m) for m in mac_dev])
+    except IOError, e:
+        raise HwAddrGetError(ifname)
+
+    if mac_dev.lower() == mac.lower():
+        return
+
+    ifr = struct.pack("16sH14s", ifname, 1,
+                      "".join([chr(int(m, 16)) for m in mac.split(":")]))
+    try:
+        fcntl.ioctl(ctrl_sock, SIOCSIFHWADDR, ifr)
+    except IOError, e:
+        logging.info(e)
+        raise HwAddrSetError(ifname, mac)
+    ctrl_sock.close()
+
-- 
1.7.5.1

[PATCH 3/5] KVM test: virt_env_process: Setup private bridge during postprocessing

[Lucas Meneghel Rodrigues]

Call bridge setup at preprocessing and cleanup at postprocessing.
The bridge can be cleaned up when no tap interfaces are using it.

Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
---
 client/virt/virt_env_process.py |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/client/virt/virt_env_process.py b/client/virt/virt_env_process.py
index 625b422..1742a02 100644
--- a/client/virt/virt_env_process.py
+++ b/client/virt/virt_env_process.py
@@ -196,6 +196,11 @@ def preprocess(test, params, env):
     @param env: The environment (a dict-like object).
     """
     error.context("preprocessing")
+
+    if params.get("bridge") == "private":
+        brcfg = virt_test_setup.PrivateBridgeConfig(params)
+        brcfg.setup()
+
     # Start tcpdump if it isn't already running
     if "address_cache" not in env:
         env["address_cache"] = {}
@@ -365,6 +370,10 @@ def postprocess(test, params, env):
                         int(params.get("post_command_timeout", "600")),
                         params.get("post_command_noncritical") == "yes")
 
+    if params.get("bridge") == "private":
+        brcfg = virt_test_setup.PrivateBridgeConfig()
+        brcfg.cleanup()
+
 
 def postprocess_on_error(test, params, env):
     """
-- 
1.7.5.1

[PATCH 4/5] KVM test: setup tap fd and pass it to qemu-kvm v3

[Lucas Meneghel Rodrigues]

We used to use qemu-ifup to manage the tap which have several
limitations:

1) If we want to specify a bridge, we must create a customized
qemu-ifup file as the default script always match the first bridge.
2) It's hard to add support for macvtap device.

So this patch let kvm subtest control the tap creation and setup then
pass it to qemu-kvm. User could specify the bridge he want to used in
configuration file.

The original autoconfiguration was changed by private bridge setup.

Changes from v1:
* Combine the private bridge config and TAP fd in one patchset,
dropped the "auto" mode
* Close TAP fds on VM.destroy() (thanks to Amos Kong for finding
the problem)

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
---
 client/tests/kvm/scripts/qemu-ifup |   11 -------
 client/virt/kvm_vm.py              |   59 +++++++++++++++++++++++------------
 client/virt/virt_utils.py          |    8 ++++-
 3 files changed, 45 insertions(+), 33 deletions(-)
 delete mode 100755 client/tests/kvm/scripts/qemu-ifup

diff --git a/client/tests/kvm/scripts/qemu-ifup b/client/tests/kvm/scripts/qemu-ifup
deleted file mode 100755
index c4debf5..0000000
--- a/client/tests/kvm/scripts/qemu-ifup
+++ /dev/null
@@ -1,11 +0,0 @@
-#!/bin/sh
-
-# The following expression selects the first bridge listed by 'brctl show'.
-# Modify it to suit your needs.
-switch=$(/usr/sbin/brctl show | awk 'NR==2 { print $1 }')
-
-/bin/echo 1 > /proc/sys/net/ipv6/conf/${switch}/disable_ipv6
-/sbin/ifconfig $1 0.0.0.0 up
-/usr/sbin/brctl addif ${switch} $1
-/usr/sbin/brctl setfd ${switch} 0
-/usr/sbin/brctl stp ${switch} off
diff --git a/client/virt/kvm_vm.py b/client/virt/kvm_vm.py
index c9bb273..9b26dee 100644
--- a/client/virt/kvm_vm.py
+++ b/client/virt/kvm_vm.py
@@ -7,7 +7,7 @@ Utility classes and functions to handle Virtual Machine creation using qemu.
 import time, os, logging, fcntl, re, commands, glob
 from autotest_lib.client.common_lib import error
 from autotest_lib.client.bin import utils
-import virt_utils, virt_vm, kvm_monitor, aexpect
+import virt_utils, virt_vm, virt_test_setup, kvm_monitor, aexpect
 
 
 class VM(virt_vm.BaseVM):
@@ -41,6 +41,7 @@ class VM(virt_vm.BaseVM):
             self.pci_assignable = None
             self.netdev_id = []
             self.device_id = []
+            self.tapfds = []
             self.uuid = None
 
 
@@ -240,19 +241,17 @@ class VM(virt_vm.BaseVM):
                 cmd += ",id='%s'" % device_id
             return cmd
 
-        def add_net(help, vlan, mode, ifname=None, script=None,
-                    downscript=None, tftp=None, bootfile=None, hostfwd=[],
-                    netdev_id=None, netdev_extra_params=None):
+        def add_net(help, vlan, mode, ifname=None, tftp=None, bootfile=None,
+                    hostfwd=[], netdev_id=None, netdev_extra_params=None,
+                    tapfd=None):
             if has_option(help, "netdev"):
                 cmd = " -netdev %s,id=%s" % (mode, netdev_id)
                 if netdev_extra_params:
                     cmd += ",%s" % netdev_extra_params
             else:
                 cmd = " -net %s,vlan=%d" % (mode, vlan)
-            if mode == "tap":
-                if ifname: cmd += ",ifname='%s'" % ifname
-                if script: cmd += ",script='%s'" % script
-                cmd += ",downscript='%s'" % (downscript or "no")
+            if mode == "tap" and tapfd:
+                cmd += ",fd=%d" % tapfd
             elif mode == "user":
                 if tftp and "[,tftp=" in help:
                     cmd += ",tftp='%s'" % tftp
@@ -422,20 +421,22 @@ class VM(virt_vm.BaseVM):
             qemu_cmd += add_nic(help, vlan, nic_params.get("nic_model"), mac,
                                 device_id, netdev_id, nic_params.get("nic_extra_params"))
             # Handle the '-net tap' or '-net user' or '-netdev' part
-            script = nic_params.get("nic_script")
-            downscript = nic_params.get("nic_downscript")
             tftp = nic_params.get("tftp")
-            if script:
-                script = virt_utils.get_path(root_dir, script)
-            if downscript:
-                downscript = virt_utils.get_path(root_dir, downscript)
             if tftp:
                 tftp = virt_utils.get_path(root_dir, tftp)
-            qemu_cmd += add_net(help, vlan, nic_params.get("nic_mode", "user"),
-                                vm.get_ifname(vlan),
-                                script, downscript, tftp,
+            if nic_params.get("nic_mode") == "tap":
+                try:
+                    tapfd = vm.tapfds[vlan]
+                except:
+                    tapfd = None
+            else:
+                tapfd = None
+            qemu_cmd += add_net(help, vlan,
+                                nic_params.get("nic_mode", "user"),
+                                vm.get_ifname(vlan), tftp,
                                 nic_params.get("bootp"), redirs, netdev_id,
-                                nic_params.get("netdev_extra_params"))
+                                nic_params.get("netdev_extra_params"),
+                                tapfd)
             # Proceed to next NIC
             vlan += 1
 
@@ -558,6 +559,10 @@ class VM(virt_vm.BaseVM):
         @raise VMBadPATypeError: If an unsupported PCI assignment type is
                 requested
         @raise VMPAError: If no PCI assignable devices could be assigned
+        @raise TAPCreationError: If fail to create tap fd
+        @raise BRAddIfError: If fail to add a tap to a bridge
+        @raise TAPBringUpError: If fail to bring up a tap
+        @raise PrivateBridgeError: If fail to bring the private bridge
         """
         error.context("creating '%s'" % self.name)
         self.destroy(free_mac_addresses=False)
@@ -621,12 +626,24 @@ class VM(virt_vm.BaseVM):
                 guest_port = int(redir_params.get("guest_port"))
                 self.redirs[guest_port] = host_ports[i]
 
-            # Generate netdev/device IDs for all NICs
+            # Generate netdev IDs for all NICs and create TAP fd
             self.netdev_id = []
-            self.device_id = []
+            self.tapfds = []
+            vlan = 0
             for nic in params.objects("nics"):
                 self.netdev_id.append(virt_utils.generate_random_id())
                 self.device_id.append(virt_utils.generate_random_id())
+                nic_params = params.object_params(nic)
+                if nic_params.get("nic_mode") == "tap":
+                    ifname = self.get_ifname(vlan)
+                    brname = nic_params.get("bridge")
+                    if brname == "private":
+                        brname = virt_test_setup.PrivateBridgeConfig().brname
+                    tapfd = virt_utils.open_tap("/dev/net/tun", ifname)
+                    virt_utils.add_to_bridge(ifname, brname)
+                    virt_utils.bring_up_ifname(ifname)
+                    self.tapfds.append(tapfd)
+                vlan += 1
 
             # Find available VNC port, if needed
             if params.get("display") == "vnc":
@@ -865,6 +882,8 @@ class VM(virt_vm.BaseVM):
                 num_nics = len(self.params.objects("nics"))
                 for vlan in range(num_nics):
                     self.free_mac_address(vlan)
+            for tapfd in self.tapfds:
+                os.close(tapfd)
 
 
     @property
diff --git a/client/virt/virt_utils.py b/client/virt/virt_utils.py
index 7eeaf4f..101427e 100644
--- a/client/virt/virt_utils.py
+++ b/client/virt/virt_utils.py
@@ -56,12 +56,16 @@ class NetError(Exception):
 
 
 class TAPModuleError(NetError):
-    def __init__(self, devname):
+    def __init__(self, devname, action="open", details=None):
         NetError.__init__(self, devname)
         self.devname = devname
 
     def __str__(self):
-        return "Can't open %s" % self.devname
+        e_msg = "Can't %s %s" % (self.action, self.devname)
+        if self.details is not None:
+            e_msg += " : %s" % details
+        return e_msg
+
 
 class TAPNotExistError(NetError):
     def __init__(self, ifname):
-- 
1.7.5.1

[PATCH 5/5] KVM test: Changing KVM autotest default to private bridge

[Lucas Meneghel Rodrigues]

Rather than the unsupported userspace mode, which still is
an option. This way we are giving users a default setup
much closer to a real world usage scenario, and enable
people to run all the network tests that don't work properly
in user mode.

Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
---
 client/tests/kvm/tests_base.cfg.sample |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/client/tests/kvm/tests_base.cfg.sample b/client/tests/kvm/tests_base.cfg.sample
index 2b0d63c..5276d23 100644
--- a/client/tests/kvm/tests_base.cfg.sample
+++ b/client/tests/kvm/tests_base.cfg.sample
@@ -57,10 +57,13 @@ redirs = remote_shell
 guest_port_remote_shell = 22
 
 # NIC parameters
-nic_mode = user
-#nic_mode = tap
-nic_script = scripts/qemu-ifup
-#nic_script = scripts/qemu-ifup-ipv6
+#nic_mode = user
+nic_mode = tap
+bridge = private
+# You can set bridge to
+# be a specific bridge
+# name, such as 'virbr0'
+#bridge = virbr0
 run_tcpdump = yes
 
 # Misc
@@ -117,7 +120,6 @@ variants:
             # Install guest from cdrom 
             - cdrom:
                 medium = cdrom
-                nic_mode = user
                 redirs += " unattended_install"
             # Install guest from http/ftp url
             - url:
-- 
1.7.5.1

[PATCH 1/5] KVM test: Adding framework code to control bridge creation

[Lucas Meneghel Rodrigues]

Provide in framework utility code to control the creation
of a bridge, in order to provide TAP functionality for
autotest users without relying on previous setup made by
the user.

This is a reimplementation of Amos's code, the differences
are:

 * Implemented as a setup class, taking advantage of object
internal state to use in different places of the code
 * Split up the operations to make it easier to understand
the steps and why we are doing them
 * Use of autotest API instead of commands

Changes from v2:
 * Changed firewall rules to match libvirt rules,
made the rules to be added with -I, which will place
them before exclusion rules
 * Safer cleanup, always try to cleanup when something
wrong happens during setup.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
Signed-off-by: Amos Kong <akong@redhat.com>
---
 client/virt/virt_test_setup.py |  172 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 172 insertions(+), 0 deletions(-)

diff --git a/client/virt/virt_test_setup.py b/client/virt/virt_test_setup.py
index f915c1b..8e69b01 100644
--- a/client/virt/virt_test_setup.py
+++ b/client/virt/virt_test_setup.py
@@ -105,3 +105,175 @@ class HugePageConfig(object):
             return
         utils.system("echo 0 > %s" % self.kernel_hp_file)
         logging.debug("Hugepage memory successfuly dealocated")
+
+
+class PrivateBridgeError(Exception):
+    def __init__(self, brname):
+        self.brname = brname
+
+    def __str__(self):
+        return "Bridge %s not available after setup" % self.brname
+
+
+class PrivateBridgeConfig(object):
+    __shared_state = {}
+    def __init__(self, params=None):
+        self.__dict__ = self.__shared_state
+        if params is not None:
+            self.brname = params.get("priv_brname", 'atbr0')
+            self.subnet = params.get("priv_subnet", '192.168.58')
+            self.ip_version = params.get("bridge_ip_version", "ipv4")
+            self.dhcp_server_pid = None
+            self.iptables_rules = [
+            "INPUT 1 -i %s -p udp -m udp --dport 53 -j ACCEPT" % self.brname,
+            "INPUT 2 -i %s -p tcp -m tcp --dport 53 -j ACCEPT" % self.brname,
+            "INPUT 3 -i %s -p udp -m udp --dport 67 -j ACCEPT" % self.brname,
+            "INPUT 4 -i %s -p tcp -m tcp --dport 67 -j ACCEPT" % self.brname,
+            "INPUT 5 -i %s -p tcp -m tcp --dport 12323 -j ACCEPT" % self.brname,
+            "FORWARD 1 -m physdev --physdev-is-bridged -j ACCEPT",
+            "FORWARD 2 -d %s.0/24 -o %s -m state --state RELATED,ESTABLISHED "
+            "-j ACCEPT" % (self.subnet, self.brname),
+            "FORWARD 3 -s %s.0/24 -i %s -j ACCEPT" % (self.subnet, self.brname),
+            "FORWARD 4 -i %s -o %s -j ACCEPT" % (self.brname, self.brname),
+            ("FORWARD 5 -o %s -j REJECT --reject-with icmp-port-unreachable" %
+             self.brname),
+            ("FORWARD 6 -i %s -j REJECT --reject-with icmp-port-unreachable" %
+             self.brname)]
+
+
+    def _add_bridge(self):
+        utils.system("brctl addbr %s" % self.brname)
+        ip_fwd_path = "/proc/sys/net/%s/ip_forward" % self.ip_version
+        ip_fwd = open(ip_fwd_path, "w")
+        ip_fwd.write("1\n")
+        utils.system("brctl stp %s on" % self.brname)
+        utils.system("brctl setfd %s 0" % self.brname)
+
+
+    def _bring_bridge_up(self):
+        utils.system("ifconfig %s %s.1 up" % (self.brname, self.subnet))
+
+
+    def _iptables_add(self, cmd):
+        return utils.system("iptables -I %s" % cmd)
+
+
+    def _iptables_del(self, cmd):
+        return utils.system("iptables -D %s" % cmd)
+
+
+    def _enable_nat(self):
+        for rule in self.iptables_rules:
+            self._iptables_add(rule)
+
+
+    def _start_dhcp_server(self):
+        utils.system("service dnsmasq stop")
+        utils.system("dnsmasq --strict-order --bind-interfaces "
+                     "--listen-address %s.1 --dhcp-range %s.2,%s.254 "
+                     "--dhcp-lease-max=253 "
+                     "--dhcp-no-override "
+                     "--pid-file=/tmp/dnsmasq.pid "
+                     "--log-facility=/tmp/dnsmasq.log" %
+                     (self.subnet, self.subnet, self.subnet))
+        self.dhcp_server_pid = None
+        try:
+            self.dhcp_server_pid = int(open('/tmp/dnsmasq.pid', 'r').read())
+        except ValueError:
+            raise PrivateBridgeError(self.brname)
+        logging.debug("Started internal DHCP server with PID %s",
+                      self.dhcp_server_pid)
+
+
+    def _verify_bridge(self):
+        brctl_output = utils.system_output("brctl show")
+        if self.brname not in brctl_output:
+            raise PrivateBridgeError(self.brname)
+
+
+    def setup(self):
+        brctl_output = utils.system_output("brctl show")
+        for line in brctl_output:
+            if line.startswith(self.brname):
+                logging.critical("%s", len(line.split()))
+        if self.brname not in brctl_output:
+            logging.info("Configuring KVM test private bridge %s", self.brname)
+            try:
+                self._add_bridge()
+            except:
+                self._remove_bridge()
+                raise
+            try:
+                self._bring_bridge_up()
+            except:
+                self._bring_bridge_down()
+                self._remove_bridge()
+                raise
+            try:
+                self._enable_nat()
+            except:
+                self._disable_nat()
+                self._bring_bridge_down()
+                self._remove_bridge()
+                raise
+            try:
+                self._start_dhcp_server()
+            except:
+                self._stop_dhcp_server()
+                self._disable_nat()
+                self._bring_bridge_down()
+                self._remove_bridge()
+                raise
+            self._verify_bridge()
+
+
+    def _stop_dhcp_server(self):
+        if self.dhcp_server_pid is not None:
+            try:
+                os.kill(self.dhcp_server_pid, 15)
+            except OSError:
+                pass
+        else:
+            try:
+                dhcp_server_pid = int(open('/tmp/dnsmasq.pid', 'r').read())
+            except ValueError:
+                return
+            try:
+                os.kill(dhcp_server_pid, 15)
+            except OSError:
+                pass
+
+
+    def _bring_bridge_down(self):
+        utils.system("ifconfig %s down" % self.brname, ignore_status=True)
+
+
+    def _disable_nat(self):
+        for rule in self.iptables_rules:
+            split_list = rule.split(' ')
+            # We need to remove numbering here
+            split_list.pop(1)
+            rule = " ".join(split_list)
+            self._iptables_del(rule)
+
+
+    def _remove_bridge(self):
+        utils.system("brctl delbr %s" % self.brname, ignore_status=True)
+
+
+    def cleanup(self):
+        brctl_output = utils.system_output("brctl show")
+        cleanup = False
+        for line in brctl_output.split("\n"):
+            if line.startswith(self.brname):
+                # len == 4 means there is a TAP using the bridge
+                # so don't try to clean it up
+                if len(line.split()) < 4:
+                    cleanup = True
+                    break
+        if cleanup:
+            logging.debug("Cleaning up KVM test private bridge %s", self.brname)
+            self._stop_dhcp_server()
+            self._disable_nat()
+            self._bring_bridge_down()
+            self._remove_bridge()
-- 
1.7.5.2

[PATCH 1/5] KVM test: Adding framework code to control bridge creation

[Lucas Meneghel Rodrigues]

Provide in framework utility code to control the creation
of a bridge, in order to provide TAP functionality for
autotest users without relying on previous setup made by
the user.

This is a reimplementation of Amos's code, the differences
are:

 * Implemented as a setup class, taking advantage of object
internal state to use in different places of the code
 * Split up the operations to make it easier to understand
the steps and why we are doing them
 * Use of autotest API instead of commands

Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
Signed-off-by: Amos Kong <akong@redhat.com>
---
 client/virt/virt_test_setup.py |  103 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 103 insertions(+), 0 deletions(-)

diff --git a/client/virt/virt_test_setup.py b/client/virt/virt_test_setup.py
index f915c1b..cdc0e23 100644
--- a/client/virt/virt_test_setup.py
+++ b/client/virt/virt_test_setup.py
@@ -105,3 +105,106 @@ class HugePageConfig(object):
             return
         utils.system("echo 0 > %s" % self.kernel_hp_file)
         logging.debug("Hugepage memory successfuly dealocated")
+
+
+class PrivateBridgeError(Exception):
+    def __init__(self, brname):
+        self.brname = brname
+
+    def __str__(self):
+        return "Bridge %s not available after setup" % self.brname
+
+
+class PrivateBridgeConfig(object):
+    __shared_state = {}
+    def __init__(self, params=None):
+        self.__dict__ = self.__shared_state
+        if params is not None:
+            self.brname = params.get("priv_brname", 'atbr0')
+            self.subnet = params.get("priv_subnet", '192.168.58')
+            self.ip_version = params.get("bridge_ip_version", "ipv4")
+            self.dhcp_server_pid = None
+
+
+    def _add_bridge(self):
+        utils.system("brctl addbr %s" % self.brname)
+        ip_fwd_path = "/proc/sys/net/%s/ip_forward" % self.ip_version
+        ip_fwd = open(ip_fwd_path, "w")
+        ip_fwd.write("1\n")
+        utils.system("brctl stp %s on" % self.brname)
+        utils.system("brctl setfd %s 0" % self.brname)
+
+
+    def _bring_bridge_up(self):
+        utils.system("ifconfig %s %s.1 up" % (self.brname, self.subnet))
+
+
+    def _enable_nat(self):
+        utils.system("iptables -t nat -A POSTROUTING -s %s.254/24 ! "
+                     "-d %s.254/24 -j MASQUERADE" % (self.subnet, self.subnet))
+
+
+    def _start_dhcp_server(self):
+        utils.system("service dnsmasq stop")
+        utils.system("dnsmasq --strict-order --bind-interfaces "
+                     "--listen-address %s.1 --dhcp-range %s.1,%s.254 "
+                     "--pid-file=/tmp/dnsmasq.pid "
+                     "--log-facility=/tmp/dnsmasq.log" %
+                     (self.subnet, self.subnet, self.subnet))
+        try:
+            self.dhcp_server_pid = int(open('/tmp/dnsmasq.pid', 'r').read())
+        except ValueError:
+            raise PrivateBridgeError(self.brname)
+        logging.debug("Started internal DHCP server with PID %s",
+                      self.dhcp_server_pid)
+
+
+    def _verify_bridge(self):
+        brctl_output = utils.system_output("brctl show")
+        if self.brname not in brctl_output:
+            raise PrivateBridgeError(self.brname)
+
+
+    def setup(self):
+        brctl_output = utils.system_output("brctl show")
+        if self.brname not in brctl_output:
+            logging.info("Configuring KVM test private bridge %s", self.brname)
+            self._add_bridge()
+            self._bring_bridge_up()
+            self._enable_nat()
+            self._start_dhcp_server()
+            self._verify_bridge()
+
+
+    def _stop_dhcp_server(self):
+        os.kill(self.dhcp_server_pid, 15)
+
+
+    def _bring_bridge_down(self):
+        utils.system("ifconfig %s down" % self.brname)
+
+
+    def _disable_nat(self):
+        utils.system("iptables -t nat -D POSTROUTING -s %s.254/24 !"
+                     " -d %s.254/24 -j MASQUERADE" % (self.subnet, self.subnet))
+
+
+    def _remove_bridge(self):
+        utils.system("brctl delbr %s" % self.brname)
+
+
+    def cleanup(self):
+        brctl_output = utils.system_output("brctl show")
+        cleanup = False
+        for line in brctl_output:
+            if line.startswith(self.brname):
+                # len == 4 means there is a TAP using the bridge
+                # so don't try to clean up
+                if len(line.split()) < 4:
+                    cleanup = True
+        if cleanup:
+            logging.debug("Cleaning up KVM test private bridge %s", self.brname)
+            self._stop_dhcp_server()
+            self._bring_bridge_down()
+            self._disable_nat()
+            self._remove_bridge()
-- 
1.7.5.1