| Message ID | 200912091549.04872.arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
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 <arnd@arndb.de> > Acked-by: Mark McLoughlin <markmc@redhat.com> > Cc: "Michael S. Tsirkin" <mst@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"
On Wednesday 09 December 2009, Michael S. Tsirkin wrote: > On Wed, Dec 09, 2009 at 03:49:04PM +0100, Arnd Bergmann wrote: > > > > - 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? D'oh. will fix. > 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. Yep. Thanks, Arnd
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"; + + 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" + " [,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"