Re: [Qemu-devel] [PATCH] net: Add persistent flag to -net tap option

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3892 bytes --]

On Mon, Dec 15, 2014 at 02:05:23PM +0200, Roy Vardi wrote:
> From: Roy Vardi <royv@ezchip.com>
> 
>     Add 'persistent' boolean flag to -net tap option.
>     When set to off - tap interface will be released on shutdown
>     When set to on\not specified - tap interface will remain
> 
>     Running with -net tap,persistent=off will force the tap interface
>     down when qemu goes down, thus ensuring that there're no zombie tap
>     interfaces left
> 
>     This is achieved using another ioctl
> 
>     Note: This commit includes the above support only for linux systems

I don't understand the point of this patch.  The following doesn't
persist the tun interface:

qemu-system-i386 -net tap,script=myscript.sh,downscript=no -net nic

You are changing the default to persist the interface, won't this cause
problems for existing users who don't expect persistent interfaces?

> @@ -109,6 +110,17 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
>          close(fd);
>          return -1;
>      }
> +
> +    if (!persistent_required) {
> +        ret = ioctl(fd, TUNSETPERSIST, 0);
> +            if (ret != 0) {

Indentation is off here.  QEMU uses 4-space indentation and this if
statement should not be indented.

> +                error_report("could not configure non-persistent %s (%s): %m",
> +                             PATH_NET_TUN, ifr.ifr_name);
> +                close(fd);
> +                return -1;
> +        }
> +    }
> +
>      pstrcpy(ifname, ifname_size, ifr.ifr_name);
>      fcntl(fd, F_SETFL, O_NONBLOCK);
>      return fd;
> diff --git a/net/tap.c b/net/tap.c
> index bde6b58..43267bb 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -556,7 +556,8 @@ int net_init_bridge(const NetClientOptions *opts, const char *name,
>  
>  static int net_tap_init(const NetdevTapOptions *tap, int *vnet_hdr,
>                          const char *setup_script, char *ifname,
> -                        size_t ifname_sz, int mq_required)
> +                        size_t ifname_sz, int mq_required,
> +                        int persistent_reuired)

Typo s/reuired/required/

> diff --git a/qemu-options.hx b/qemu-options.hx
> index 10b9568..d26215a 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1417,7 +1417,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
>      "-net tap[,vlan=n][,name=str],ifname=name\n"
>      "                connect the host TAP network interface to VLAN 'n'\n"
>  #else
> -    "-net tap[,vlan=n][,name=str][,fd=h][,fds=x:y:...:z][,ifname=name][,script=file][,downscript=dfile][,helper=helper][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off][,vhostfd=h][,vhostfds=x:y:...:z][,vhostforce=on|off][,queues=n]\n"
> +    "-net tap[,vlan=n][,name=str][,fd=h][,fds=x:y:...:z][,ifname=name][,script=file][,downscript=dfile][,helper=helper][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off][,vhostfd=h][,vhostfds=x:y:...:z][,vhostforce=on|off][,queues=n][persistent=on|off]\n"

Missing comma:
[,persistent=on|off]

>      "                connect the host TAP network interface to VLAN 'n'\n"
>      "                use network scripts 'file' (default=" DEFAULT_NETWORK_SCRIPT ")\n"
>      "                to configure it and 'dfile' (default=" DEFAULT_NETWORK_DOWN_SCRIPT ")\n"
> @@ -1437,6 +1437,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
>      "                use 'vhostfd=h' to connect to an already opened vhost net device\n"
>      "                use 'vhostfds=x:y:...:z to connect to multiple already opened vhost net devices\n"
>      "                use 'queues=n' to specify the number of queues to be created for multiqueue TAP\n"
> +    "                use persistent=off to release the TAP interface on shutdown (default=on)\n"

Please use the same formatting as for the other options:

use 'persistent=off' to release ...

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]