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

[Roy Vardi]

Hi  Stefan,

 

When using "-net tap" flag today, the tap interface is not released on shutdown. This is the default behavior and I'm not breaking it.

Running with "-net tap persistent=off " will cause the interface to be released on shutdown, and running with "-net tap persistent=on " (or without the persistent flag at all), the behavior remains the same (i.e - interface won't be released. This is the status in qemu today, I didn't change any legacy command-lines)

Example for a command line:

 

. /qemu-system -net tap,ifname=tap0,script=no,downscript=no,persistent=off

. /qemu-system -net tap,ifname=tap0,script=no,downscript=no,persistent=on

. /qemu-system -net tap,ifname=tap0,script=no,downscript=no

 

The last two lines are identical as far as releasing the tap is concerned.

 

This is relevant for users who want their tap interfaces to be released on qemu shutdown. The rest of the users won't be influenced by this change.

 

I'll fix the spelling and format bugs and will send a new patch soon.

 

Thanks for your review and comments,

Roy Vardi

 

-----Original Message-----
From: Stefan Hajnoczi [mailto:address@hidden
Sent: Friday, December 19, 2014 3:14 PM
To: Roy Vardi
Cc: address@hidden; address@hidden; address@hidden; address@hidden; address@hidden
Subject: Re: [Qemu-devel] [PATCH] net: Add persistent flag to -net tap option

 

On Mon, Dec 15, 2014 at 02:05:23PM +0200, Roy Vardi wrote:

> From: Roy Vardi <address@hidden>

>

>     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 ...