On Wed, Dec 09, 2009 at 03:49:04PM +0100, Arnd Bergmann wrote: > In order to support macvtap, we need a way to select the actual > tap endpoint. While it would be nice to pass it by network interface > name, passing the character device is more flexible, and we can > easily do both in the long run. > > This version makes it possible to use macvtap without introducing > any macvtap specific code in qemu, only a natural extension to > the existing interface. > > Signed-off-by: Arnd Bergmann <a...@arndb.de> > Acked-by: Mark McLoughlin <mar...@redhat.com> > Cc: "Michael S. Tsirkin" <m...@redhat.com> > --- > Hopefully addressed all comments from Michael. I did compile-test > the non-linux files I touched (the solaris file failed to build for another > reason), and this now prevents passing dev= on non-linux machines.
Found a couple more minor nits in the new versions, otherwise looks good. > net.c | 5 +++++ > net/tap-aix.c | 3 ++- > net/tap-bsd.c | 9 ++++++++- > net/tap-linux.c | 12 ++++++++---- > net/tap-solaris.c | 7 ++++++- > net/tap.c | 17 ++++++++++++++--- > net/tap.h | 3 ++- > qemu-options.hx | 10 +++++++++- > 8 files changed, 54 insertions(+), 12 deletions(-) > > diff --git a/net.c b/net.c > index 13bdbb2..deb12fd 100644 > --- a/net.c > +++ b/net.c > @@ -955,6 +955,11 @@ static struct { > }, > #ifndef _WIN32 > { > + .name = "dev", > + .type = QEMU_OPT_STRING, > + .help = "character device pathname", > + }, > + { > .name = "fd", > .type = QEMU_OPT_STRING, > .help = "file descriptor of an already opened tap", > diff --git a/net/tap-aix.c b/net/tap-aix.c > index 4bc9f2f..238c190 100644 > --- a/net/tap-aix.c > +++ b/net/tap-aix.c > @@ -25,7 +25,8 @@ > #include "net/tap.h" > #include <stdio.h> > > -int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int > vnet_hdr_required) > +int tap_open(char *ifname, int ifname_size, const char *dev, > + int *vnet_hdr, int vnet_hdr_required) > { > fprintf(stderr, "no tap on AIX\n"); > return -1; > diff --git a/net/tap-bsd.c b/net/tap-bsd.c > index 815997d..8a7334c 100644 > --- a/net/tap-bsd.c > +++ b/net/tap-bsd.c > @@ -40,7 +40,8 @@ > #include <util.h> > #endif > > -int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int > vnet_hdr_required) > +int tap_open(char *ifname, int ifname_size, const char *devarg, > + int *vnet_hdr, int vnet_hdr_required) > { > int fd; > char *dev; > @@ -80,6 +81,12 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, > int vnet_hdr_required > } > #endif > > + if (devarg) { > + qemu_error("dev= not supported\n"); > + close(fd); > + return -1; > + } > + > fstat(fd, &s); > dev = devname(s.st_rdev, S_IFCHR); > pstrcpy(ifname, ifname_size, dev); > diff --git a/net/tap-linux.c b/net/tap-linux.c > index 6af9e82..d6831c0 100644 > --- a/net/tap-linux.c > +++ b/net/tap-linux.c > @@ -32,14 +32,18 @@ > #include "sysemu.h" > #include "qemu-common.h" > > -int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int > vnet_hdr_required) > +int tap_open(char *ifname, int ifname_size, const char *dev, > + int *vnet_hdr, int vnet_hdr_required) > { > struct ifreq ifr; > int fd, ret; > > - TFR(fd = open("/dev/net/tun", O_RDWR)); > + if (!*dev) > + dev = "/dev/net/tun"; > + Did you test without dev parameter? I think dev will be NULL so this will deference a nullpointer ... probably if (!dev) is what you mean? > + TFR(fd = open(dev, O_RDWR)); > if (fd < 0) { > - fprintf(stderr, "warning: could not open /dev/net/tun: no virtual > network emulation\n"); > + fprintf(stderr, "warning: could not open %s: no virtual network > emulation\n", dev); > return -1; > } > memset(&ifr, 0, sizeof(ifr)); > @@ -70,7 +74,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, > int vnet_hdr_required > pstrcpy(ifr.ifr_name, IFNAMSIZ, "tap%d"); > ret = ioctl(fd, TUNSETIFF, (void *) &ifr); > if (ret != 0) { > - fprintf(stderr, "warning: could not configure /dev/net/tun: no > virtual network emulation\n"); > + fprintf(stderr, "warning: could not configure %s: no virtual network > emulation\n", dev); > close(fd); > return -1; > } > diff --git a/net/tap-solaris.c b/net/tap-solaris.c > index e14fe36..3d10984 100644 > --- a/net/tap-solaris.c > +++ b/net/tap-solaris.c > @@ -171,10 +171,15 @@ static int tap_alloc(char *dev, size_t dev_size) > return tap_fd; > } > > -int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int > vnet_hdr_required) > +int tap_open(char *ifname, int ifname_size, const char *devarg, > + int *vnet_hdr, int vnet_hdr_required) > { > char dev[10]=""; > int fd; > + if (devarg) { > + fprintf(stderr, "dev= not supported\n"); > + return -1; > + } > if( (fd = tap_alloc(dev, sizeof(dev))) < 0 ){ > fprintf(stderr, "Cannot allocate TAP device\n"); > return -1; > diff --git a/net/tap.c b/net/tap.c > index 0d8b424..8635ae2 100644 > --- a/net/tap.c > +++ b/net/tap.c > @@ -343,12 +343,15 @@ static int net_tap_init(QemuOpts *opts, int *vnet_hdr) > { > int fd, vnet_hdr_required; > char ifname[128] = {0,}; > + const char *dev; > const char *setup_script; > > if (qemu_opt_get(opts, "ifname")) { > pstrcpy(ifname, sizeof(ifname), qemu_opt_get(opts, "ifname")); > } > > + dev = qemu_opt_get(opts, "dev"); > + > *vnet_hdr = qemu_opt_get_bool(opts, "vnet_hdr", 1); > if (qemu_opt_get(opts, "vnet_hdr")) { > vnet_hdr_required = *vnet_hdr; > @@ -356,7 +359,8 @@ static int net_tap_init(QemuOpts *opts, int *vnet_hdr) > vnet_hdr_required = 0; > } > > - TFR(fd = tap_open(ifname, sizeof(ifname), vnet_hdr, vnet_hdr_required)); > + TFR(fd = tap_open(ifname, sizeof(ifname), dev, vnet_hdr, > + vnet_hdr_required)); > if (fd < 0) { > return -1; > } > @@ -382,10 +386,12 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const > char *name, VLANState *vlan > > if (qemu_opt_get(opts, "fd")) { > if (qemu_opt_get(opts, "ifname") || > + qemu_opt_get(opts, "dev") || > qemu_opt_get(opts, "script") || > qemu_opt_get(opts, "downscript") || > qemu_opt_get(opts, "vnet_hdr")) { > - qemu_error("ifname=, script=, downscript= and vnet_hdr= is > invalid with fd=\n"); > + qemu_error("ifname=, dev=, script=, downscript= and vnet_hdr= is > " > + "invalid with fd=\n"); > return -1; > } > > @@ -425,15 +431,20 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const > char *name, VLANState *vlan > if (qemu_opt_get(opts, "fd")) { > snprintf(s->nc.info_str, sizeof(s->nc.info_str), "fd=%d", fd); > } else { > - const char *ifname, *script, *downscript; > + const char *ifname, *dev, *script, *downscript; > > ifname = qemu_opt_get(opts, "ifname"); > + dev = qemu_opt_get(opts, "dev"); > script = qemu_opt_get(opts, "script"); > downscript = qemu_opt_get(opts, "downscript"); > > snprintf(s->nc.info_str, sizeof(s->nc.info_str), > "ifname=%s,script=%s,downscript=%s", > ifname, script, downscript); > + if (dev) { > + strncat(s->nc.info_str, ",dev=", sizeof(s->nc.info_str)); > + strncat(s->nc.info_str, dev, sizeof(s->nc.info_str)); > + } > > if (strcmp(downscript, "no") != 0) { > snprintf(s->down_script, sizeof(s->down_script), "%s", > downscript); > diff --git a/net/tap.h b/net/tap.h > index 538a562..6e76903 100644 > --- a/net/tap.h > +++ b/net/tap.h > @@ -34,7 +34,8 @@ > > int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState > *vlan); > > -int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int > vnet_hdr_required); > +int tap_open(char *ifname, int ifname_size, const char *devarg, > + int *vnet_hdr, int vnet_hdr_required); > > ssize_t tap_read_packet(int tapfd, uint8_t *buf, int maxlen); > > diff --git a/qemu-options.hx b/qemu-options.hx > index 1b5781a..586aec3 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -810,12 +810,20 @@ 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][,ifname=name][,script=file][,downscript=dfile][,sndbuf=nbytes][,vnet_hdr=on|off]\n" > + "-net tap[,vlan=n][,name=str][,fd=h][,ifname=name]" > +#ifdef __linux__ > + "[,dev=str]" > +#endif > + > "[,script=file]\n" will be neater if you put [,dev=str] after [,script=file] Also - it does need a string, but only insofar as all options are strings. Maybe dev=devfile or dev=file would be clearer. > + " [,downscript=dfile][,sndbuf=nbytes][,vnet_hdr=on|off]\n" > " connect the host TAP network interface to VLAN 'n' and > use the\n" > " network scripts 'file' (default=%s)\n" > " and 'dfile' (default=%s);\n" > " use '[down]script=no' to disable script execution;\n" > " use 'fd=h' to connect to an already opened TAP > interface\n" > +#ifdef __linux__ > + " use 'dev=str' to open a specific tap device > (default=/dev/net/tun)\n" > +#endif > " use 'sndbuf=nbytes' to limit the size of the send > buffer; the\n" > " 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"