RE: [Qemu-devel] [PATCH 4/4] Add support for -net bridge

[Krumme, Chris]

Hello Anthony,

Now that I have read the whole series I say again great patch. 

> -----Original Message-----
> From: 
> address@hidden 
> [mailto:address@hidden
> rg] On Behalf Of Anthony Liguori
> Sent: Tuesday, November 03, 2009 6:28 PM
> To: address@hidden
> Cc: Mark McLoughlin; Anthony Liguori; Arnd Bergmann; Dustin 
> Kirkland; Michael Tsirkin; Juan Quintela
> Subject: [Qemu-devel] [PATCH 4/4] Add support for -net bridge
> 
> The most common use of -net tap is to connect a tap device to 
> a bridge.  This
> requires the use of a script and running qemu as root in 
> order to allocate a
> tap device to pass to the script.
> 
> This model is great for portability and flexibility but it's 
> incredibly
> difficult to eliminate the need to run qemu as root.  The 
> only really viable
> mechanism is to use tunctl to create a tap device, attach it 
> to a bridge as
> root, and then hand that tap device to qemu.  The problem 
> with this mechanism
> is that it requires administrator intervention whenever a 
> user wants to create
> a guest.
> 
> By essentially writing a helper that implements the most 
> common qemu-ifup
> script that can be safely given cap_net_admin, we can 
> dramatically simplify
> things for non-privileged users.  We still support -net tap 
> as a mechanism
> for advanced users and backwards compatibility.
> 
> Currently, this is very Linux centric but there's really no 
> reason why it
> couldn't be extended for other Unixes.
> 
> A typical invocation of -net bridge would be:
> 
>   qemu -net bridge -net nic,model=virtio
> 
> The default bridge that we attach to is qemubr0.  The 
> thinking is that a distro
> could preconfigure such an interface to allow out-of-the-box 
> bridged networking.
> 
> Alternatively, if a user wants to use a different bridge, 
> they can say:
> 
>   qemu -net bridge,br=br0 -net nic,model=virtio
> 
> Signed-off-by: Anthony Liguori <address@hidden>
> ---
>  configure       |    1 +
>  net.c           |   20 +++++++-
>  net/tap.c       |  142 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  net/tap.h       |    2 +
>  qemu-options.hx |    3 +
>  5 files changed, 167 insertions(+), 1 deletions(-)
> 
> diff --git a/configure b/configure
> index 7c9d3a2..55a1a4f 100755
> --- a/configure
> +++ b/configure
> @@ -1896,6 +1896,7 @@ echo >> $config_host_mak
>  
>  echo "CONFIG_QEMU_SHAREDIR=\"$prefix$datasuffix\"" >> 
> $config_host_mak
>  echo "CONFIG_QEMU_CONFDIR=\"/etc/qemu\"" >> $config_host_mak
> +echo "CONFIG_QEMU_HELPERDIR=\"$prefix/libexec\"" >> $config_host_mak
>  
>  case "$cpu" in
>    
> i386|x86_64|alpha|cris|hppa|ia64|m68k|microblaze|mips|mips64|p
> pc|ppc64|s390|sparc|sparc64)
> diff --git a/net.c b/net.c
> index 37662c6..54a7a5b 100644
> --- a/net.c
> +++ b/net.c
> @@ -2541,6 +2541,22 @@ static struct {
>              },
>              { /* end of list */ }
>          },
> +    }, {
> +        .type = "bridge",
> +        .init = net_init_bridge,
> +        .desc = {
> +            NET_COMMON_PARAMS_DESC,
> +            {
> +                .name = "br",
> +                .type = QEMU_OPT_STRING,
> +                .help = "bridge name",
> +            }, {
> +                .name = "helper",
> +                .type = QEMU_OPT_STRING,
> +                .help = "command to execute to configure bridge",
> +            },
> +            { /* end of list */ }
> +        },
>      },
>      { /* end of list */ }
>  };
> @@ -2565,7 +2581,8 @@ int net_client_init(Monitor *mon, 
> QemuOpts *opts, int is_netdev)
>  #ifdef CONFIG_VDE
>              strcmp(type, "vde") != 0 &&
>  #endif
> -            strcmp(type, "socket") != 0) {
> +            strcmp(type, "socket") != 0 &&
> +            strcmp(type, "bridge") != 0) {
>              qemu_error("The '%s' network backend type is not 
> valid with -netdev\n",
>                         type);
>              return -1;
> @@ -2641,6 +2658,7 @@ static int net_host_check_device(const 
> char *device)
>  #ifdef CONFIG_VDE
>                                         ,"vde"
>  #endif
> +                                       , "bridge"
>      };
>      for (i = 0; i < sizeof(valid_param_list) / sizeof(char *); i++) {
>          if (!strncmp(valid_param_list[i], device,
> diff --git a/net/tap.c b/net/tap.c
> index bdb4a15..f5abed6 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -436,3 +436,145 @@ int net_init_tap(QemuOpts *opts, 
> Monitor *mon, const char *name, VLANState *vlan
>  
>      return 0;
>  }
> +
> +#define DEFAULT_BRIDGE_INTERFACE "qemubr0"
> +#define DEFAULT_BRIDGE_HELPER CONFIG_QEMU_HELPERDIR 
> "/qemu-bridge-helper"
> +
> +static int recv_fd(int c)
> +{
> +    int fd;
> +    uint8_t msgbuf[CMSG_SPACE(sizeof(fd))];
> +    struct msghdr msg = {
> +        .msg_control = msgbuf,
> +        .msg_controllen = sizeof(msgbuf),
> +    };
> +    struct cmsghdr *cmsg;
> +    struct iovec iov;
> +    uint8_t req[1];
> +    ssize_t len;
> +
> +    cmsg = CMSG_FIRSTHDR(&msg);
> +    cmsg->cmsg_level = SOL_SOCKET;
> +    cmsg->cmsg_type = SCM_RIGHTS;
> +    cmsg->cmsg_len = CMSG_LEN(sizeof(fd));
> +    msg.msg_controllen = cmsg->cmsg_len;
> +
> +    iov.iov_base = req;
> +    iov.iov_len = sizeof(req);
> +
> +    msg.msg_iov = &iov;
> +    msg.msg_iovlen = 1;
> +
> +    len = recvmsg(c, &msg, 0);
> +    if (len > 0) {
> +        memcpy(&fd, CMSG_DATA(cmsg), sizeof(fd));
> +
> +        return fd;
> +    }
> +
> +    return len;
> +}
> +
> +static int net_bridge_run_helper(const char *helper, const 
> char *bridge)
> +{
> +    sigset_t oldmask, mask;
> +    int pid, status;
> +    char *args[5];
> +    char **parg;
> +    int sv[2];
> +
> +    sigemptyset(&mask);
> +    sigaddset(&mask, SIGCHLD);
> +    sigprocmask(SIG_BLOCK, &mask, &oldmask);
> +
> +    if (socketpair(PF_UNIX, SOCK_STREAM, 0, sv) == -1) {
> +        return -1;
> +    }
> +
> +    /* try to launch network script */
> +    pid = fork();
> +    if (pid == 0) {
> +        int open_max = sysconf(_SC_OPEN_MAX), i;
> +        char buf[32];
> +
> +        snprintf(buf, sizeof(buf), "%d", sv[1]);
> +
> +        for (i = 0; i < open_max; i++) {
> +            if (i != STDIN_FILENO &&
> +                i != STDOUT_FILENO &&
> +                i != STDERR_FILENO &&
> +                i != sv[1]) {
> +                close(i);
> +            }
> +        }
> +        parg = args;
> +        *parg++ = (char *)helper;
> +        *parg++ = (char *)"--use-vnet";
> +        *parg++ = (char *)bridge;
> +        *parg++ = buf;
> +        *parg++ = NULL;
> +        execv(helper, args);
> +        _exit(1);
> +    } else if (pid > 0) {
> +        int fd;
> +
> +        close(sv[1]);
> +
> +        do {
> +            fd = recv_fd(sv[0]);
> +        } while (fd == -1 && errno == EINTR);
> +
> +        close(sv[0]);
> +
> +        while (waitpid(pid, &status, 0) != pid) {
> +            /* loop */
> +        }
> +        sigprocmask(SIG_SETMASK, &oldmask, NULL);
> +
> +        if (fd < 0) {
> +            fprintf(stderr, "failed to recv file descriptor\n");
> +            return -1;
> +        }
> +
> +        if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
> +            return fd;
> +        }
> +    }
> +    fprintf(stderr, "failed to launch bridge helper\n");
> +    return -1;
> +}
> +
> +int net_init_bridge(QemuOpts *opts, Monitor *mon, const char 
> *name, VLANState *vlan)
> +{
> +    TAPState *s;
> +    int fd, vnet_hdr;
> +
> +    if (!qemu_opt_get(opts, "br")) {
> +        qemu_opt_set(opts, "br", DEFAULT_BRIDGE_INTERFACE);
> +    }
> +    if (!qemu_opt_get(opts, "helper")) {
> +        qemu_opt_set(opts, "helper", DEFAULT_BRIDGE_HELPER);
> +    }
> +
> +    fd = net_bridge_run_helper(qemu_opt_get(opts, "helper"),
> +                               qemu_opt_get(opts, "br"));
> +
> +    fcntl(fd, F_SETFL, O_NONBLOCK);
> +
> +    vnet_hdr = tap_probe_vnet_hdr(fd);
> +
> +    s = net_tap_fd_init(vlan, "bridge", name, fd, vnet_hdr);
> +    if (!s) {
> +        close(fd);
> +        return -1;
> +    }
> +
> +    snprintf(s->vc->info_str, sizeof(s->vc->info_str),
> +             "br=%s", qemu_opt_get(opts, "br"));
> +
> +    if (vlan) {
> +        vlan->nb_host_devs++;
> +    }
> +
> +    return 0;
> +}
> diff --git a/net/tap.h b/net/tap.h
> index 538a562..9527db4 100644
> --- a/net/tap.h
> +++ b/net/tap.h
> @@ -48,4 +48,6 @@ int tap_probe_vnet_hdr(int fd);
>  int tap_probe_has_ufo(int fd);
>  void tap_fd_set_offload(int fd, int csum, int tso4, int 
> tso6, int ecn, int ufo);
>  
> +int net_init_bridge(QemuOpts *opts, Monitor *mon, const char 
> *name, VLANState *vlan);
> +
>  #endif /* QEMU_NET_TAP_H */
> diff --git a/qemu-options.hx b/qemu-options.hx
> index d78b738..8f20aa6 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -820,6 +820,9 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
>      "                default of 'sndbuf=1048576' can be 
> disabled using 'sndbuf=0'\n"
>      "                use vnet_hdr=off to avoid enabling the 
> IFF_VNET_HDR tap flag; use\n"
>      "                vnet_hdr=on to make the lack of 
> IFF_VNET_HDR support an error condition\n"
> +    "-net bridge[,vlan=n][,name=str][,br=bridge][,helper=helper]\n"
> +    "                connects to a host bridge device 'br' 
> using the program 'helper'\n"

Do you need to mention the default name qemubr0 here?

Thanks

Chris


> +    "                to create a tap device and attach it to 
> the bridge\n"
>  #endif
>      "-net 
> socket[,vlan=n][,name=str][,fd=h][,listen=[host]:port][,connec
> t=host:port]\n"
>      "                connect the vlan 'n' to another VLAN 
> using a socket connection\n"
> -- 
> 1.6.2.5
> 
> 
> 
>