[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] net: Add persistent flag to -net tap option
From: |
Jason Wang |
Subject: |
Re: [Qemu-devel] [PATCH] net: Add persistent flag to -net tap option |
Date: |
Mon, 22 Dec 2014 14:33:25 +0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 |
On 12/21/2014 03:48 PM, 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
I'm interested of the user cases in the case. Usually, persistent flag
was used to let privileged application to create/configure the device
and then it could be used by non-privileged users. If qemu has already
had privilege, why need set persistent in this case?
> 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
But your patch will break non-linux builds, no?
>
> Signed-off-by: Roy Vardi <address@hidden>
> ---
> net/tap-linux.c | 14 +++++++++++++-
> net/tap.c | 10 ++++++----
> net/tap_int.h | 2 +-
> qapi-schema.json | 5 ++++-
> qemu-options.hx | 3 ++-
> 5 files changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/net/tap-linux.c b/net/tap-linux.c
> index 812bf2d..7a98390 100644
> --- a/net/tap-linux.c
> +++ b/net/tap-linux.c
> @@ -29,6 +29,7 @@
>
> #include <net/if.h>
> #include <sys/ioctl.h>
> +#include <linux/if_tun.h>
To reduce headers dependency, we put tun flags in net/tap-linux.h.
>
> #include "sysemu/sysemu.h"
> #include "qemu-common.h"
> @@ -37,7 +38,7 @@
> #define PATH_NET_TUN "/dev/net/tun"
>
> int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
> - int vnet_hdr_required, int mq_required)
> + int vnet_hdr_required, int mq_required, int persistent_required)
> {
> struct ifreq ifr;
> int fd, ret;
> @@ -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) {
> + 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..055863a 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_required)
> {
> int fd, vnet_hdr_required;
>
> @@ -569,7 +570,7 @@ static int net_tap_init(const NetdevTapOptions *tap, int
> *vnet_hdr,
> }
>
> TFR(fd = tap_open(ifname, ifname_sz, vnet_hdr, vnet_hdr_required,
> - mq_required));
> + mq_required, persistent_required));
> if (fd < 0) {
> return -1;
> }
> @@ -688,7 +689,7 @@ int net_init_tap(const NetClientOptions *opts, const char
> *name,
> NetClientState *peer)
> {
> const NetdevTapOptions *tap;
> - int fd, vnet_hdr = 0, i = 0, queues;
> + int fd, vnet_hdr = 0, i = 0, queues, persistent;
> /* for the no-fd, no-helper case */
> const char *script = NULL; /* suppress wrong "uninit'd use" gcc warning
> */
> const char *downscript = NULL;
> @@ -699,6 +700,7 @@ int net_init_tap(const NetClientOptions *opts, const char
> *name,
> tap = opts->tap;
> queues = tap->has_queues ? tap->queues : 1;
> vhostfdname = tap->has_vhostfd ? tap->vhostfd : NULL;
> + persistent = tap->has_persistent ? tap->persistent : 1;
>
> /* QEMU vlans does not support multiqueue tap, in this case peer is set.
> * For -netdev, peer is always NULL. */
> @@ -816,7 +818,7 @@ int net_init_tap(const NetClientOptions *opts, const char
> *name,
>
> for (i = 0; i < queues; i++) {
> fd = net_tap_init(tap, &vnet_hdr, i >= 1 ? "no" : script,
> - ifname, sizeof ifname, queues > 1);
> + ifname, sizeof ifname, queues > 1, persistent);
> if (fd == -1) {
> return -1;
> }
> diff --git a/net/tap_int.h b/net/tap_int.h
> index 79afdf2..0eb2168 100644
> --- a/net/tap_int.h
> +++ b/net/tap_int.h
> @@ -30,7 +30,7 @@
> #include "qapi-types.h"
>
> int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
> - int vnet_hdr_required, int mq_required);
> + int vnet_hdr_required, int mq_required, int
> persistent_required);
>
> ssize_t tap_read_packet(int tapfd, uint8_t *buf, int maxlen);
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 563b4ad..99e6482 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2007,6 +2007,8 @@
> #
> # @queues: #optional number of queues to be created for multiqueue capable
> tap
> #
> +# @persistent: #optional for opening tap in persistent mode (default: on)
> (Since: 2.3)
> +#
> # Since 1.2
> ##
> { 'type': 'NetdevTapOptions',
> @@ -2023,7 +2025,8 @@
> '*vhostfd': 'str',
> '*vhostfds': 'str',
> '*vhostforce': 'bool',
> - '*queues': 'uint32'} }
> + '*queues': 'uint32',
> + '*persistent': 'bool'} }
>
> ##
> # @NetdevSocketOptions
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 10b9568..d8c033d 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"
> " 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"
As Stefan mentioned, we don't set persistent by default in the past. So
please don't change this behaviour.
> "-net bridge[,vlan=n][,name=str][,br=bridge][,helper=helper]\n"
> " connects a host TAP network interface to a host bridge
> device 'br'\n"
> " (default=" DEFAULT_BRIDGE_INTERFACE ") using the
> program 'helper'\n"