On Mon, 2011-02-28 at 17:03 +0800, Jason Wang wrote:
> 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 old qemu-ifup style bridge detection is kept, when specify the bridge as
> "auto", it would be automatically detected.

By default I'd like to see 'auto' management. Also, if we are to keep
the qemu-ifup script for people wanting to do things 'the old way', why
did we delete all its contents?

Second comment, about the ipv6 setup contributed by IBM. Now that we are
going for better TAP management, we can setup ipv6 (which boils down to
write 0 or 1 to this file):

/proc/sys/net/ipv6/conf/${switch}/disable_ipv6

On the TAP management code. An extra param such as

disable_ipv6 = yes

To reflect the kernel interface, or a similar idea

enable_ipv6 = no

 would be all that the user needs to set in case he/she wants ipv6. As
the IBM folks need to do their ipv6 testing, it is only fair we keep the
same set of functionality available.

> 
> Signed-off-by: Jason Wang <jasow...@redhat.com>
> ---
>  client/tests/kvm/kvm_vm.py             |   78 
> +++++++++++++++++++++++++++++---
>  client/tests/kvm/scripts/qemu-ifup     |   11 -----
>  client/tests/kvm/tests_base.cfg.sample |    3 -
>  3 files changed, 72 insertions(+), 20 deletions(-)
>  delete mode 100755 client/tests/kvm/scripts/qemu-ifup
> 
> diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
> index 41f7491..6d30bca 100755
> --- a/client/tests/kvm/kvm_vm.py
> +++ b/client/tests/kvm/kvm_vm.py
> @@ -15,6 +15,10 @@ class VMError(Exception):
>      pass
>  
> 
> +class NetError(Exception):
> +    pass
> +
> +
>  class VMCreateError(VMError):
>      def __init__(self, cmd, status, output):
>          VMError.__init__(self, cmd, status, output)
> @@ -178,6 +182,37 @@ class VMRebootError(VMError):
>      pass
>  
> 
> +class TAPCreationError(NetError):
> +    def __init__(self, ifname):
> +        NetError.__init__(self, ifname)
> +        self.ifname = ifname
> +
> +    def __str__(self):
> +        return "Cannot create TAP device %s" % self.ifname
> +
> +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):
> +        NetError.__init__(self, ifname, brname)
> +        self.ifname = ifname
> +        self.brname = brname
> +
> +    def __str__(self):
> +        return "Can not add if %s to bridge %s" % (self.ifname, self.brname)
> +
> +
> +class BRAutoDetectError(NetError):
> +    def __str__(self):
> +        return "Can not detect the bridge automatically"
> +
> +
>  def get_image_filename(params, root_dir):
>      """
>      Generate an image path from params and root_dir.
> @@ -490,7 +525,7 @@ class VM:
>  
>          def add_net(help, vlan, mode, ifname=None, script=None,
>                      downscript=None, tftp=None, bootfile=None, hostfwd=[],
> -                    netdev_id=None, netdev_extra_params=None):
> +                    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:
> @@ -498,9 +533,7 @@ class VM:
>              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")
> +                cmd += ",fd=%d" % tapfd
>              elif mode == "user":
>                  if tftp and "[,tftp=" in help:
>                      cmd += ",tftp='%s'" % tftp
> @@ -658,11 +691,16 @@ class VM:
>                  downscript = kvm_utils.get_path(root_dir, downscript)
>              if tftp:
>                  tftp = kvm_utils.get_path(root_dir, tftp)
> +            if nic_params.get("nic_mode") == "tap":
> +                tapfd = vm.tapfds[vlan]
> +            else:
> +                tapfd = None
>              qemu_cmd += add_net(help, vlan, nic_params.get("nic_mode", 
> "user"),
>                                  vm.get_ifname(vlan),
>                                  script, downscript, 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
>  
> @@ -772,6 +810,10 @@ class VM:
>          @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 BRAutoDetectError: If can not detect the bridge automatically
>          """
>          error.context("creating '%s'" % self.name)
>          self.destroy(free_mac_addresses=False)
> @@ -834,12 +876,34 @@ class VM:
>                  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(kvm_utils.generate_random_id())
>                  self.device_id.append(kvm_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 == "auto":
> +                        # Provides the same function as the old qemu-ifup
> +                        try:
> +                            brctl_output = utils.system_output("brctl show",
> +                                                               
> retain_output=True)
> +                            brname = brctl_output.splitlines()[1].split()[0]
> +                        except:
> +                            raise BRAutoDetectError()
> +                    tapfd = kvm_utils.open_tap(ifname)
> +                    if tapfd == -1:
> +                        raise TAPCreationError(ifname)
> +                    if kvm_utils.add_to_bridge(ifname, brname) != 0:
> +                        raise BRAddIfError(ifname, brname)
> +                    if kvm_utils.bring_up_ifname(ifname) != 0:
> +                        raise TAPBringUpError(ifname)
> +                    self.tapfds.append(tapfd)
> +                vlan += 1
>  
>              # Find available VNC port, if needed
>              if params.get("display") == "vnc":
> 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/tests/kvm/tests_base.cfg.sample 
> b/client/tests/kvm/tests_base.cfg.sample
> index 661d6fe..8159594 100644
> --- a/client/tests/kvm/tests_base.cfg.sample
> +++ b/client/tests/kvm/tests_base.cfg.sample
> @@ -54,8 +54,7 @@ guest_port_remote_shell = 22
>  # NIC parameters
>  nic_mode = user
>  #nic_mode = tap
> -nic_script = scripts/qemu-ifup
> -#nic_script = scripts/qemu-ifup-ipv6
> +#bridge = auto
>  run_tcpdump = yes
>  
>  # Misc
> 


_______________________________________________
Autotest mailing list
Autotest@test.kernel.org
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest

Reply via email to