diff mbox

[v10-fix,15/18] Add the vhost-user netdev backend to the command line

Message ID 20140610100157.11064.61717.stgit@3820
State New
Headers show

Commit Message

Nikolay Nikolaev June 10, 2014, 10:02 a.m. UTC
The supplied chardev id will be inspected for supported options. Only
a socket backend, with a set path (i.e. a Unix socket) and optionally
the server parameter set, will be allowed. Other options (nowait, telnet)
will make the chardev unusable and the netdev will not be initialised.

Additional checks for validity:
  - requires `-numa node,memdev=..`
  - requires `-device virtio-net-*`

The `vhostforce` option is used to force vhost-net when we deal with
non-MSIX guests.

Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
---
 hmp-commands.hx    |    4 +-
 hw/net/vhost_net.c |    4 ++
 net/hub.c          |    1 
 net/net.c          |    3 +
 net/vhost-user.c   |  115 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 qapi-schema.json   |   19 ++++++++-
 qemu-options.hx    |   18 ++++++++
 7 files changed, 158 insertions(+), 6 deletions(-)

Comments

Michael S. Tsirkin June 10, 2014, 10:34 a.m. UTC | #1
On Tue, Jun 10, 2014 at 01:02:16PM +0300, Nikolay Nikolaev wrote:
> The supplied chardev id will be inspected for supported options. Only
> a socket backend, with a set path (i.e. a Unix socket) and optionally
> the server parameter set, will be allowed. Other options (nowait, telnet)
> will make the chardev unusable and the netdev will not be initialised.
> 
> Additional checks for validity:
>   - requires `-numa node,memdev=..`
>   - requires `-device virtio-net-*`
> 
> The `vhostforce` option is used to force vhost-net when we deal with
> non-MSIX guests.
> 
> Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
> Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
> ---
>
Applied, thanks!
In the future pls just number them 0-18.
Also in the future, pls include the changelog here after ---

>  hmp-commands.hx    |    4 +-
>  hw/net/vhost_net.c |    4 ++
>  net/hub.c          |    1 
>  net/net.c          |    3 +
>  net/vhost-user.c   |  115 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  qapi-schema.json   |   19 ++++++++-
>  qemu-options.hx    |   18 ++++++++
>  7 files changed, 158 insertions(+), 6 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 2e462c0..bc9c032 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1206,7 +1206,7 @@ ETEXI
>      {
>          .name       = "host_net_add",
>          .args_type  = "device:s,opts:s?",
> -        .params     = "tap|user|socket|vde|netmap|dump [options]",
> +        .params     = "tap|user|socket|vde|netmap|vhost-user|dump [options]",
>          .help       = "add host VLAN client",
>          .mhandler.cmd = net_host_device_add,
>      },
> @@ -1234,7 +1234,7 @@ ETEXI
>      {
>          .name       = "netdev_add",
>          .args_type  = "netdev:O",
> -        .params     = "[user|tap|socket|vde|bridge|hubport|netmap],id=str[,prop=value][,...]",
> +        .params     = "[user|tap|socket|vde|bridge|hubport|netmap|vhost-user],id=str[,prop=value][,...]",
>          .help       = "add host network device",
>          .mhandler.cmd = hmp_netdev_add,
>          .command_completion = netdev_add_completion,
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 5f06736..7ac7c21 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -15,6 +15,7 @@
>  
>  #include "net/net.h"
>  #include "net/tap.h"
> +#include "net/vhost-user.h"
>  
>  #include "hw/virtio/virtio-net.h"
>  #include "net/vhost_net.h"
> @@ -360,6 +361,9 @@ VHostNetState *get_vhost_net(NetClientState *nc)
>      case NET_CLIENT_OPTIONS_KIND_TAP:
>          vhost_net = tap_get_vhost_net(nc);
>          break;
> +    case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
> +        vhost_net = vhost_user_get_vhost_net(nc);
> +        break;
>      default:
>          break;
>      }
> diff --git a/net/hub.c b/net/hub.c
> index 33a99c9..7e0f2d6 100644
> --- a/net/hub.c
> +++ b/net/hub.c
> @@ -322,6 +322,7 @@ void net_hub_check_clients(void)
>              case NET_CLIENT_OPTIONS_KIND_TAP:
>              case NET_CLIENT_OPTIONS_KIND_SOCKET:
>              case NET_CLIENT_OPTIONS_KIND_VDE:
> +            case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
>                  has_host_dev = 1;
>                  break;
>              default:
> diff --git a/net/net.c b/net/net.c
> index 0ff2e40..c91b67b 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -786,6 +786,7 @@ static int (* const net_client_init_fun[NET_CLIENT_OPTIONS_KIND_MAX])(
>          [NET_CLIENT_OPTIONS_KIND_BRIDGE]    = net_init_bridge,
>  #endif
>          [NET_CLIENT_OPTIONS_KIND_HUBPORT]   = net_init_hubport,
> +        [NET_CLIENT_OPTIONS_KIND_VHOST_USER] = net_init_vhost_user,
>  };
>  
>  
> @@ -819,6 +820,7 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
>          case NET_CLIENT_OPTIONS_KIND_BRIDGE:
>  #endif
>          case NET_CLIENT_OPTIONS_KIND_HUBPORT:
> +        case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
>              break;
>  
>          default:
> @@ -907,6 +909,7 @@ static int net_host_check_device(const char *device)
>  #ifdef CONFIG_VDE
>                                         ,"vde"
>  #endif
> +                                       ,"vhost-user"
>      };
>      for (i = 0; i < ARRAY_SIZE(valid_param_list); i++) {
>          if (!strncmp(valid_param_list[i], device,
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index 4bdd19d..32b78fb 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -12,6 +12,7 @@
>  #include "net/vhost_net.h"
>  #include "net/vhost-user.h"
>  #include "sysemu/char.h"
> +#include "qemu/config-file.h"
>  #include "qemu/error-report.h"
>  
>  typedef struct VhostUserState {
> @@ -21,9 +22,17 @@ typedef struct VhostUserState {
>      VHostNetState *vhost_net;
>  } VhostUserState;
>  
> +typedef struct VhostUserChardevProps {
> +    bool is_socket;
> +    bool is_unix;
> +    bool is_server;
> +    bool has_unsupported;
> +} VhostUserChardevProps;
> +
>  VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
>  {
>      VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
> +    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
>      return s->vhost_net;
>  }
>  
> @@ -82,7 +91,7 @@ static bool vhost_user_has_ufo(NetClientState *nc)
>  }
>  
>  static NetClientInfo net_vhost_user_info = {
> -        .type = 0,
> +        .type = NET_CLIENT_OPTIONS_KIND_VHOST_USER,
>          .size = sizeof(VhostUserState),
>          .cleanup = vhost_user_cleanup,
>          .has_vnet_hdr = vhost_user_has_vnet_hdr,
> @@ -148,8 +157,108 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
>      return 0;
>  }
>  
> +static int net_vhost_chardev_opts(const char *name, const char *value,
> +                                  void *opaque)
> +{
> +    VhostUserChardevProps *props = opaque;
> +
> +    if (strcmp(name, "backend") == 0 && strcmp(value, "socket") == 0) {
> +        props->is_socket = true;
> +    } else if (strcmp(name, "path") == 0) {
> +        props->is_unix = true;
> +    } else if (strcmp(name, "server") == 0) {
> +        props->is_server = true;
> +    } else {
> +        error_report("vhost-user does not support a chardev"
> +                     " with the following option:\n %s = %s",
> +                     name, value);
> +        props->has_unsupported = true;
> +        return -1;
> +    }
> +    return 0;
> +}
> +
> +static CharDriverState *net_vhost_parse_chardev(const NetdevVhostUserOptions *opts)
> +{
> +    CharDriverState *chr = qemu_chr_find(opts->chardev);
> +    VhostUserChardevProps props;
> +
> +    if (chr == NULL) {
> +        error_report("chardev \"%s\" not found", opts->chardev);
> +        return NULL;
> +    }
> +
> +    /* inspect chardev opts */
> +    memset(&props, 0, sizeof(props));
> +    qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, false);
> +
> +    if (!props.is_socket || !props.is_unix) {
> +        error_report("chardev \"%s\" is not a unix socket",
> +                     opts->chardev);
> +        return NULL;
> +    }
> +
> +    if (props.has_unsupported) {
> +        error_report("chardev \"%s\" has an unsupported option",
> +                opts->chardev);
> +        return NULL;
> +    }
> +
> +    qemu_chr_fe_claim_no_fail(chr);
> +
> +    return chr;
> +}
> +
> +static int net_vhost_check_net(QemuOpts *opts, void *opaque)
> +{
> +    const char *name = opaque;
> +    const char *driver, *netdev;
> +    const char virtio_name[] = "virtio-net-";
> +
> +    driver = qemu_opt_get(opts, "driver");
> +    netdev = qemu_opt_get(opts, "netdev");
> +
> +    if (!driver || !netdev) {
> +        return 0;
> +    }
> +
> +    if (strcmp(netdev, name) == 0 &&
> +        strncmp(driver, virtio_name, strlen(virtio_name)) != 0) {
> +        error_report("vhost-user requires frontend driver virtio-net-*");
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
>  int net_init_vhost_user(const NetClientOptions *opts, const char *name,
> -                   NetClientState *peer)
> +                        NetClientState *peer)
>  {
> -    return net_vhost_user_init(peer, "vhost_user", 0, 0, 0);
> +    const NetdevVhostUserOptions *vhost_user_opts;
> +    CharDriverState *chr;
> +    bool vhostforce;
> +
> +    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
> +    vhost_user_opts = opts->vhost_user;
> +
> +    chr = net_vhost_parse_chardev(vhost_user_opts);
> +    if (!chr) {
> +        error_report("No suitable chardev found");
> +        return -1;
> +    }
> +
> +    /* verify net frontend */
> +    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
> +                          (char *)name, true) == -1) {
> +        return -1;
> +    }
> +
> +    /* vhost-force for non-MSIX */
> +    if (vhost_user_opts->has_vhost_force) {
> +        vhostforce = vhost_user_opts->vhost_force;
> +    } else {
> +        vhostforce = false;
> +    }
> +
> +    return net_vhost_user_init(peer, "vhost_user", name, chr, vhostforce);
>  }
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 7bc33ea..f062ce9 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3267,6 +3267,22 @@
>      '*devname':    'str' } }
>  
>  ##
> +# @NetdevVhostUserOptions
> +#
> +# Vhost-user network backend
> +#
> +# @chardev: name of a unix socket chardev
> +#
> +# @vhost-force: #optional vhost on for non-MSIX virtio guests (default: false).
> +#
> +# Since 2.1
> +##
> +{ 'type': 'NetdevVhostUserOptions',
> +  'data': {
> +    'chardev':        'str',
> +    '*vhost-force':    'bool' } }
> +
> +##
>  # @NetClientOptions
>  #
>  # A discriminated record of network device traits.
> @@ -3284,7 +3300,8 @@
>      'dump':     'NetdevDumpOptions',
>      'bridge':   'NetdevBridgeOptions',
>      'hubport':  'NetdevHubPortOptions',
> -    'netmap':   'NetdevNetmapOptions' } }
> +    'netmap':   'NetdevNetmapOptions',
> +    'vhost-user': 'NetdevVhostUserOptions' } }
>  
>  ##
>  # @NetLegacy
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 5a4eff9..1ad2528 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1460,6 +1460,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>  #ifdef CONFIG_NETMAP
>      "netmap|"
>  #endif
> +    "vhost-user|"
>      "socket|"
>      "hubport],id=str[,option][,option][,...]\n", QEMU_ARCH_ALL)
>  STEXI
> @@ -1791,6 +1792,23 @@ The hubport netdev lets you connect a NIC to a QEMU "vlan" instead of a single
>  netdev.  @code{-net} and @code{-device} with parameter @option{vlan} create the
>  required hub automatically.
>  
> +@item -netdev vhost-user,chardev=@var{id}[,vhostforce=on|off]
> +
> +Establish a vhost-user netdev, backed by a chardev @var{id}. The chardev should
> +be a unix domain socket backed one. The vhost-user uses a specifically defined
> +protocol to pass vhost ioctl replacement messages to an application on the other
> +end of the socket. On non-MSIX guests, the feature can be forced with
> +@var{vhostforce}.
> +
> +Example:
> +@example
> +qemu -m 512 -object memory-file,id=mem,size=512M,mem-path=/hugetlbfs,share=on \
> +     -numa node,memdev=mem \
> +     -chardev socket,path=/path/to/socket \
> +     -netdev type=vhost-user,id=net0,chardev=chr0 \
> +     -device virtio-net-pci,netdev=net0
> +@end example
> +
>  @item -net dump[,vlan=@var{n}][,file=@var{file}][,len=@var{len}]
>  Dump network traffic on VLAN @var{n} to file @var{file} (@file{qemu-vlan0.pcap} by default).
>  At most @var{len} bytes (64k by default) per packet are stored. The file format is
Michael S. Tsirkin June 10, 2014, 10:50 a.m. UTC | #2
On Tue, Jun 10, 2014 at 01:34:13PM +0300, Michael S. Tsirkin wrote:
> On Tue, Jun 10, 2014 at 01:02:16PM +0300, Nikolay Nikolaev wrote:
> > The supplied chardev id will be inspected for supported options. Only
> > a socket backend, with a set path (i.e. a Unix socket) and optionally
> > the server parameter set, will be allowed. Other options (nowait, telnet)
> > will make the chardev unusable and the netdev will not be initialised.
> > 
> > Additional checks for validity:
> >   - requires `-numa node,memdev=..`
> >   - requires `-device virtio-net-*`
> > 
> > The `vhostforce` option is used to force vhost-net when we deal with
> > non-MSIX guests.
> > 
> > Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
> > Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
> > ---
> >
> Applied, thanks!
> In the future pls just number them 0-18.
> Also in the future, pls include the changelog here after ---

Please check out and test the vhost branch, if everything is
in order let me know and I'll merge this upstream.

> >  hw/net/vhost_net.c |    4 ++
> >  net/hub.c          |    1 
> >  net/net.c          |    3 +
> >  net/vhost-user.c   |  115 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  qapi-schema.json   |   19 ++++++++-
> >  qemu-options.hx    |   18 ++++++++
> >  7 files changed, 158 insertions(+), 6 deletions(-)
> > 
> > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > index 2e462c0..bc9c032 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -1206,7 +1206,7 @@ ETEXI
> >      {
> >          .name       = "host_net_add",
> >          .args_type  = "device:s,opts:s?",
> > -        .params     = "tap|user|socket|vde|netmap|dump [options]",
> > +        .params     = "tap|user|socket|vde|netmap|vhost-user|dump [options]",
> >          .help       = "add host VLAN client",
> >          .mhandler.cmd = net_host_device_add,
> >      },
> > @@ -1234,7 +1234,7 @@ ETEXI
> >      {
> >          .name       = "netdev_add",
> >          .args_type  = "netdev:O",
> > -        .params     = "[user|tap|socket|vde|bridge|hubport|netmap],id=str[,prop=value][,...]",
> > +        .params     = "[user|tap|socket|vde|bridge|hubport|netmap|vhost-user],id=str[,prop=value][,...]",
> >          .help       = "add host network device",
> >          .mhandler.cmd = hmp_netdev_add,
> >          .command_completion = netdev_add_completion,
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index 5f06736..7ac7c21 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -15,6 +15,7 @@
> >  
> >  #include "net/net.h"
> >  #include "net/tap.h"
> > +#include "net/vhost-user.h"
> >  
> >  #include "hw/virtio/virtio-net.h"
> >  #include "net/vhost_net.h"
> > @@ -360,6 +361,9 @@ VHostNetState *get_vhost_net(NetClientState *nc)
> >      case NET_CLIENT_OPTIONS_KIND_TAP:
> >          vhost_net = tap_get_vhost_net(nc);
> >          break;
> > +    case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
> > +        vhost_net = vhost_user_get_vhost_net(nc);
> > +        break;
> >      default:
> >          break;
> >      }
> > diff --git a/net/hub.c b/net/hub.c
> > index 33a99c9..7e0f2d6 100644
> > --- a/net/hub.c
> > +++ b/net/hub.c
> > @@ -322,6 +322,7 @@ void net_hub_check_clients(void)
> >              case NET_CLIENT_OPTIONS_KIND_TAP:
> >              case NET_CLIENT_OPTIONS_KIND_SOCKET:
> >              case NET_CLIENT_OPTIONS_KIND_VDE:
> > +            case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
> >                  has_host_dev = 1;
> >                  break;
> >              default:
> > diff --git a/net/net.c b/net/net.c
> > index 0ff2e40..c91b67b 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -786,6 +786,7 @@ static int (* const net_client_init_fun[NET_CLIENT_OPTIONS_KIND_MAX])(
> >          [NET_CLIENT_OPTIONS_KIND_BRIDGE]    = net_init_bridge,
> >  #endif
> >          [NET_CLIENT_OPTIONS_KIND_HUBPORT]   = net_init_hubport,
> > +        [NET_CLIENT_OPTIONS_KIND_VHOST_USER] = net_init_vhost_user,
> >  };
> >  
> >  
> > @@ -819,6 +820,7 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
> >          case NET_CLIENT_OPTIONS_KIND_BRIDGE:
> >  #endif
> >          case NET_CLIENT_OPTIONS_KIND_HUBPORT:
> > +        case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
> >              break;
> >  
> >          default:
> > @@ -907,6 +909,7 @@ static int net_host_check_device(const char *device)
> >  #ifdef CONFIG_VDE
> >                                         ,"vde"
> >  #endif
> > +                                       ,"vhost-user"
> >      };
> >      for (i = 0; i < ARRAY_SIZE(valid_param_list); i++) {
> >          if (!strncmp(valid_param_list[i], device,
> > diff --git a/net/vhost-user.c b/net/vhost-user.c
> > index 4bdd19d..32b78fb 100644
> > --- a/net/vhost-user.c
> > +++ b/net/vhost-user.c
> > @@ -12,6 +12,7 @@
> >  #include "net/vhost_net.h"
> >  #include "net/vhost-user.h"
> >  #include "sysemu/char.h"
> > +#include "qemu/config-file.h"
> >  #include "qemu/error-report.h"
> >  
> >  typedef struct VhostUserState {
> > @@ -21,9 +22,17 @@ typedef struct VhostUserState {
> >      VHostNetState *vhost_net;
> >  } VhostUserState;
> >  
> > +typedef struct VhostUserChardevProps {
> > +    bool is_socket;
> > +    bool is_unix;
> > +    bool is_server;
> > +    bool has_unsupported;
> > +} VhostUserChardevProps;
> > +
> >  VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
> >  {
> >      VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
> > +    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
> >      return s->vhost_net;
> >  }
> >  
> > @@ -82,7 +91,7 @@ static bool vhost_user_has_ufo(NetClientState *nc)
> >  }
> >  
> >  static NetClientInfo net_vhost_user_info = {
> > -        .type = 0,
> > +        .type = NET_CLIENT_OPTIONS_KIND_VHOST_USER,
> >          .size = sizeof(VhostUserState),
> >          .cleanup = vhost_user_cleanup,
> >          .has_vnet_hdr = vhost_user_has_vnet_hdr,
> > @@ -148,8 +157,108 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
> >      return 0;
> >  }
> >  
> > +static int net_vhost_chardev_opts(const char *name, const char *value,
> > +                                  void *opaque)
> > +{
> > +    VhostUserChardevProps *props = opaque;
> > +
> > +    if (strcmp(name, "backend") == 0 && strcmp(value, "socket") == 0) {
> > +        props->is_socket = true;
> > +    } else if (strcmp(name, "path") == 0) {
> > +        props->is_unix = true;
> > +    } else if (strcmp(name, "server") == 0) {
> > +        props->is_server = true;
> > +    } else {
> > +        error_report("vhost-user does not support a chardev"
> > +                     " with the following option:\n %s = %s",
> > +                     name, value);
> > +        props->has_unsupported = true;
> > +        return -1;
> > +    }
> > +    return 0;
> > +}
> > +
> > +static CharDriverState *net_vhost_parse_chardev(const NetdevVhostUserOptions *opts)
> > +{
> > +    CharDriverState *chr = qemu_chr_find(opts->chardev);
> > +    VhostUserChardevProps props;
> > +
> > +    if (chr == NULL) {
> > +        error_report("chardev \"%s\" not found", opts->chardev);
> > +        return NULL;
> > +    }
> > +
> > +    /* inspect chardev opts */
> > +    memset(&props, 0, sizeof(props));
> > +    qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, false);
> > +
> > +    if (!props.is_socket || !props.is_unix) {
> > +        error_report("chardev \"%s\" is not a unix socket",
> > +                     opts->chardev);
> > +        return NULL;
> > +    }
> > +
> > +    if (props.has_unsupported) {
> > +        error_report("chardev \"%s\" has an unsupported option",
> > +                opts->chardev);
> > +        return NULL;
> > +    }
> > +
> > +    qemu_chr_fe_claim_no_fail(chr);
> > +
> > +    return chr;
> > +}
> > +
> > +static int net_vhost_check_net(QemuOpts *opts, void *opaque)
> > +{
> > +    const char *name = opaque;
> > +    const char *driver, *netdev;
> > +    const char virtio_name[] = "virtio-net-";
> > +
> > +    driver = qemu_opt_get(opts, "driver");
> > +    netdev = qemu_opt_get(opts, "netdev");
> > +
> > +    if (!driver || !netdev) {
> > +        return 0;
> > +    }
> > +
> > +    if (strcmp(netdev, name) == 0 &&
> > +        strncmp(driver, virtio_name, strlen(virtio_name)) != 0) {
> > +        error_report("vhost-user requires frontend driver virtio-net-*");
> > +        return -1;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> >  int net_init_vhost_user(const NetClientOptions *opts, const char *name,
> > -                   NetClientState *peer)
> > +                        NetClientState *peer)
> >  {
> > -    return net_vhost_user_init(peer, "vhost_user", 0, 0, 0);
> > +    const NetdevVhostUserOptions *vhost_user_opts;
> > +    CharDriverState *chr;
> > +    bool vhostforce;
> > +
> > +    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
> > +    vhost_user_opts = opts->vhost_user;
> > +
> > +    chr = net_vhost_parse_chardev(vhost_user_opts);
> > +    if (!chr) {
> > +        error_report("No suitable chardev found");
> > +        return -1;
> > +    }
> > +
> > +    /* verify net frontend */
> > +    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
> > +                          (char *)name, true) == -1) {
> > +        return -1;
> > +    }
> > +
> > +    /* vhost-force for non-MSIX */
> > +    if (vhost_user_opts->has_vhost_force) {
> > +        vhostforce = vhost_user_opts->vhost_force;
> > +    } else {
> > +        vhostforce = false;
> > +    }
> > +
> > +    return net_vhost_user_init(peer, "vhost_user", name, chr, vhostforce);
> >  }
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 7bc33ea..f062ce9 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -3267,6 +3267,22 @@
> >      '*devname':    'str' } }
> >  
> >  ##
> > +# @NetdevVhostUserOptions
> > +#
> > +# Vhost-user network backend
> > +#
> > +# @chardev: name of a unix socket chardev
> > +#
> > +# @vhost-force: #optional vhost on for non-MSIX virtio guests (default: false).
> > +#
> > +# Since 2.1
> > +##
> > +{ 'type': 'NetdevVhostUserOptions',
> > +  'data': {
> > +    'chardev':        'str',
> > +    '*vhost-force':    'bool' } }
> > +
> > +##
> >  # @NetClientOptions
> >  #
> >  # A discriminated record of network device traits.
> > @@ -3284,7 +3300,8 @@
> >      'dump':     'NetdevDumpOptions',
> >      'bridge':   'NetdevBridgeOptions',
> >      'hubport':  'NetdevHubPortOptions',
> > -    'netmap':   'NetdevNetmapOptions' } }
> > +    'netmap':   'NetdevNetmapOptions',
> > +    'vhost-user': 'NetdevVhostUserOptions' } }
> >  
> >  ##
> >  # @NetLegacy
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 5a4eff9..1ad2528 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -1460,6 +1460,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
> >  #ifdef CONFIG_NETMAP
> >      "netmap|"
> >  #endif
> > +    "vhost-user|"
> >      "socket|"
> >      "hubport],id=str[,option][,option][,...]\n", QEMU_ARCH_ALL)
> >  STEXI
> > @@ -1791,6 +1792,23 @@ The hubport netdev lets you connect a NIC to a QEMU "vlan" instead of a single
> >  netdev.  @code{-net} and @code{-device} with parameter @option{vlan} create the
> >  required hub automatically.
> >  
> > +@item -netdev vhost-user,chardev=@var{id}[,vhostforce=on|off]
> > +
> > +Establish a vhost-user netdev, backed by a chardev @var{id}. The chardev should
> > +be a unix domain socket backed one. The vhost-user uses a specifically defined
> > +protocol to pass vhost ioctl replacement messages to an application on the other
> > +end of the socket. On non-MSIX guests, the feature can be forced with
> > +@var{vhostforce}.
> > +
> > +Example:
> > +@example
> > +qemu -m 512 -object memory-file,id=mem,size=512M,mem-path=/hugetlbfs,share=on \
> > +     -numa node,memdev=mem \
> > +     -chardev socket,path=/path/to/socket \
> > +     -netdev type=vhost-user,id=net0,chardev=chr0 \
> > +     -device virtio-net-pci,netdev=net0
> > +@end example
> > +
> >  @item -net dump[,vlan=@var{n}][,file=@var{file}][,len=@var{len}]
> >  Dump network traffic on VLAN @var{n} to file @var{file} (@file{qemu-vlan0.pcap} by default).
> >  At most @var{len} bytes (64k by default) per packet are stored. The file format is
Nikolay Nikolaev June 10, 2014, 11:04 a.m. UTC | #3
Hello,


On Tue, Jun 10, 2014 at 1:50 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Jun 10, 2014 at 01:34:13PM +0300, Michael S. Tsirkin wrote:
> > On Tue, Jun 10, 2014 at 01:02:16PM +0300, Nikolay Nikolaev wrote:
> > > The supplied chardev id will be inspected for supported options. Only
> > > a socket backend, with a set path (i.e. a Unix socket) and optionally
> > > the server parameter set, will be allowed. Other options (nowait, telnet)
> > > will make the chardev unusable and the netdev will not be initialised.
> > >
> > > Additional checks for validity:
> > >   - requires `-numa node,memdev=..`
> > >   - requires `-device virtio-net-*`
> > >
> > > The `vhostforce` option is used to force vhost-net when we deal with
> > > non-MSIX guests.
> > >
> > > Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
> > > Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
> > > ---
> > >
> > Applied, thanks!
> > In the future pls just number them 0-18.
> > Also in the future, pls include the changelog here after ---
>
> Please check out and test the vhost branch, if everything is
> in order let me know and I'll merge this upstream.
>
This patchseries requires the NUMA patches (I see v4 is underway). It
is not working from your tree.
I wrote yesterday in reply to your question - memdev is not enough. We
want to enable memory sharing on HUGETLBFS and this is not available
on this branch. In effect vhost-user is compiled but it can not be
used. Probably you were misled by mentioning 'memdev' in the
changelog, while it shoudl have been NUMA.


>
> > >  hw/net/vhost_net.c |    4 ++
> > >  net/hub.c          |    1
> > >  net/net.c          |    3 +
> > >  net/vhost-user.c   |  115 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> > >  qapi-schema.json   |   19 ++++++++-
> > >  qemu-options.hx    |   18 ++++++++
> > >  7 files changed, 158 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > > index 2e462c0..bc9c032 100644
> > > --- a/hmp-commands.hx
> > > +++ b/hmp-commands.hx
> > > @@ -1206,7 +1206,7 @@ ETEXI
> > >      {
> > >          .name       = "host_net_add",
> > >          .args_type  = "device:s,opts:s?",
> > > -        .params     = "tap|user|socket|vde|netmap|dump [options]",
> > > +        .params     = "tap|user|socket|vde|netmap|vhost-user|dump [options]",
> > >          .help       = "add host VLAN client",
> > >          .mhandler.cmd = net_host_device_add,
> > >      },
> > > @@ -1234,7 +1234,7 @@ ETEXI
> > >      {
> > >          .name       = "netdev_add",
> > >          .args_type  = "netdev:O",
> > > -        .params     = "[user|tap|socket|vde|bridge|hubport|netmap],id=str[,prop=value][,...]",
> > > +        .params     = "[user|tap|socket|vde|bridge|hubport|netmap|vhost-user],id=str[,prop=value][,...]",
> > >          .help       = "add host network device",
> > >          .mhandler.cmd = hmp_netdev_add,
> > >          .command_completion = netdev_add_completion,
> > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > index 5f06736..7ac7c21 100644
> > > --- a/hw/net/vhost_net.c
> > > +++ b/hw/net/vhost_net.c
> > > @@ -15,6 +15,7 @@
> > >
> > >  #include "net/net.h"
> > >  #include "net/tap.h"
> > > +#include "net/vhost-user.h"
> > >
> > >  #include "hw/virtio/virtio-net.h"
> > >  #include "net/vhost_net.h"
> > > @@ -360,6 +361,9 @@ VHostNetState *get_vhost_net(NetClientState *nc)
> > >      case NET_CLIENT_OPTIONS_KIND_TAP:
> > >          vhost_net = tap_get_vhost_net(nc);
> > >          break;
> > > +    case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
> > > +        vhost_net = vhost_user_get_vhost_net(nc);
> > > +        break;
> > >      default:
> > >          break;
> > >      }
> > > diff --git a/net/hub.c b/net/hub.c
> > > index 33a99c9..7e0f2d6 100644
> > > --- a/net/hub.c
> > > +++ b/net/hub.c
> > > @@ -322,6 +322,7 @@ void net_hub_check_clients(void)
> > >              case NET_CLIENT_OPTIONS_KIND_TAP:
> > >              case NET_CLIENT_OPTIONS_KIND_SOCKET:
> > >              case NET_CLIENT_OPTIONS_KIND_VDE:
> > > +            case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
> > >                  has_host_dev = 1;
> > >                  break;
> > >              default:
> > > diff --git a/net/net.c b/net/net.c
> > > index 0ff2e40..c91b67b 100644
> > > --- a/net/net.c
> > > +++ b/net/net.c
> > > @@ -786,6 +786,7 @@ static int (* const net_client_init_fun[NET_CLIENT_OPTIONS_KIND_MAX])(
> > >          [NET_CLIENT_OPTIONS_KIND_BRIDGE]    = net_init_bridge,
> > >  #endif
> > >          [NET_CLIENT_OPTIONS_KIND_HUBPORT]   = net_init_hubport,
> > > +        [NET_CLIENT_OPTIONS_KIND_VHOST_USER] = net_init_vhost_user,
> > >  };
> > >
> > >
> > > @@ -819,6 +820,7 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
> > >          case NET_CLIENT_OPTIONS_KIND_BRIDGE:
> > >  #endif
> > >          case NET_CLIENT_OPTIONS_KIND_HUBPORT:
> > > +        case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
> > >              break;
> > >
> > >          default:
> > > @@ -907,6 +909,7 @@ static int net_host_check_device(const char *device)
> > >  #ifdef CONFIG_VDE
> > >                                         ,"vde"
> > >  #endif
> > > +                                       ,"vhost-user"
> > >      };
> > >      for (i = 0; i < ARRAY_SIZE(valid_param_list); i++) {
> > >          if (!strncmp(valid_param_list[i], device,
> > > diff --git a/net/vhost-user.c b/net/vhost-user.c
> > > index 4bdd19d..32b78fb 100644
> > > --- a/net/vhost-user.c
> > > +++ b/net/vhost-user.c
> > > @@ -12,6 +12,7 @@
> > >  #include "net/vhost_net.h"
> > >  #include "net/vhost-user.h"
> > >  #include "sysemu/char.h"
> > > +#include "qemu/config-file.h"
> > >  #include "qemu/error-report.h"
> > >
> > >  typedef struct VhostUserState {
> > > @@ -21,9 +22,17 @@ typedef struct VhostUserState {
> > >      VHostNetState *vhost_net;
> > >  } VhostUserState;
> > >
> > > +typedef struct VhostUserChardevProps {
> > > +    bool is_socket;
> > > +    bool is_unix;
> > > +    bool is_server;
> > > +    bool has_unsupported;
> > > +} VhostUserChardevProps;
> > > +
> > >  VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
> > >  {
> > >      VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
> > > +    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
> > >      return s->vhost_net;
> > >  }
> > >
> > > @@ -82,7 +91,7 @@ static bool vhost_user_has_ufo(NetClientState *nc)
> > >  }
> > >
> > >  static NetClientInfo net_vhost_user_info = {
> > > -        .type = 0,
> > > +        .type = NET_CLIENT_OPTIONS_KIND_VHOST_USER,
> > >          .size = sizeof(VhostUserState),
> > >          .cleanup = vhost_user_cleanup,
> > >          .has_vnet_hdr = vhost_user_has_vnet_hdr,
> > > @@ -148,8 +157,108 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
> > >      return 0;
> > >  }
> > >
> > > +static int net_vhost_chardev_opts(const char *name, const char *value,
> > > +                                  void *opaque)
> > > +{
> > > +    VhostUserChardevProps *props = opaque;
> > > +
> > > +    if (strcmp(name, "backend") == 0 && strcmp(value, "socket") == 0) {
> > > +        props->is_socket = true;
> > > +    } else if (strcmp(name, "path") == 0) {
> > > +        props->is_unix = true;
> > > +    } else if (strcmp(name, "server") == 0) {
> > > +        props->is_server = true;
> > > +    } else {
> > > +        error_report("vhost-user does not support a chardev"
> > > +                     " with the following option:\n %s = %s",
> > > +                     name, value);
> > > +        props->has_unsupported = true;
> > > +        return -1;
> > > +    }
> > > +    return 0;
> > > +}
> > > +
> > > +static CharDriverState *net_vhost_parse_chardev(const NetdevVhostUserOptions *opts)
> > > +{
> > > +    CharDriverState *chr = qemu_chr_find(opts->chardev);
> > > +    VhostUserChardevProps props;
> > > +
> > > +    if (chr == NULL) {
> > > +        error_report("chardev \"%s\" not found", opts->chardev);
> > > +        return NULL;
> > > +    }
> > > +
> > > +    /* inspect chardev opts */
> > > +    memset(&props, 0, sizeof(props));
> > > +    qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, false);
> > > +
> > > +    if (!props.is_socket || !props.is_unix) {
> > > +        error_report("chardev \"%s\" is not a unix socket",
> > > +                     opts->chardev);
> > > +        return NULL;
> > > +    }
> > > +
> > > +    if (props.has_unsupported) {
> > > +        error_report("chardev \"%s\" has an unsupported option",
> > > +                opts->chardev);
> > > +        return NULL;
> > > +    }
> > > +
> > > +    qemu_chr_fe_claim_no_fail(chr);
> > > +
> > > +    return chr;
> > > +}
> > > +
> > > +static int net_vhost_check_net(QemuOpts *opts, void *opaque)
> > > +{
> > > +    const char *name = opaque;
> > > +    const char *driver, *netdev;
> > > +    const char virtio_name[] = "virtio-net-";
> > > +
> > > +    driver = qemu_opt_get(opts, "driver");
> > > +    netdev = qemu_opt_get(opts, "netdev");
> > > +
> > > +    if (!driver || !netdev) {
> > > +        return 0;
> > > +    }
> > > +
> > > +    if (strcmp(netdev, name) == 0 &&
> > > +        strncmp(driver, virtio_name, strlen(virtio_name)) != 0) {
> > > +        error_report("vhost-user requires frontend driver virtio-net-*");
> > > +        return -1;
> > > +    }
> > > +
> > > +    return 0;
> > > +}
> > > +
> > >  int net_init_vhost_user(const NetClientOptions *opts, const char *name,
> > > -                   NetClientState *peer)
> > > +                        NetClientState *peer)
> > >  {
> > > -    return net_vhost_user_init(peer, "vhost_user", 0, 0, 0);
> > > +    const NetdevVhostUserOptions *vhost_user_opts;
> > > +    CharDriverState *chr;
> > > +    bool vhostforce;
> > > +
> > > +    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
> > > +    vhost_user_opts = opts->vhost_user;
> > > +
> > > +    chr = net_vhost_parse_chardev(vhost_user_opts);
> > > +    if (!chr) {
> > > +        error_report("No suitable chardev found");
> > > +        return -1;
> > > +    }
> > > +
> > > +    /* verify net frontend */
> > > +    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
> > > +                          (char *)name, true) == -1) {
> > > +        return -1;
> > > +    }
> > > +
> > > +    /* vhost-force for non-MSIX */
> > > +    if (vhost_user_opts->has_vhost_force) {
> > > +        vhostforce = vhost_user_opts->vhost_force;
> > > +    } else {
> > > +        vhostforce = false;
> > > +    }
> > > +
> > > +    return net_vhost_user_init(peer, "vhost_user", name, chr, vhostforce);
> > >  }
> > > diff --git a/qapi-schema.json b/qapi-schema.json
> > > index 7bc33ea..f062ce9 100644
> > > --- a/qapi-schema.json
> > > +++ b/qapi-schema.json
> > > @@ -3267,6 +3267,22 @@
> > >      '*devname':    'str' } }
> > >
> > >  ##
> > > +# @NetdevVhostUserOptions
> > > +#
> > > +# Vhost-user network backend
> > > +#
> > > +# @chardev: name of a unix socket chardev
> > > +#
> > > +# @vhost-force: #optional vhost on for non-MSIX virtio guests (default: false).
> > > +#
> > > +# Since 2.1
> > > +##
> > > +{ 'type': 'NetdevVhostUserOptions',
> > > +  'data': {
> > > +    'chardev':        'str',
> > > +    '*vhost-force':    'bool' } }
> > > +
> > > +##
> > >  # @NetClientOptions
> > >  #
> > >  # A discriminated record of network device traits.
> > > @@ -3284,7 +3300,8 @@
> > >      'dump':     'NetdevDumpOptions',
> > >      'bridge':   'NetdevBridgeOptions',
> > >      'hubport':  'NetdevHubPortOptions',
> > > -    'netmap':   'NetdevNetmapOptions' } }
> > > +    'netmap':   'NetdevNetmapOptions',
> > > +    'vhost-user': 'NetdevVhostUserOptions' } }
> > >
> > >  ##
> > >  # @NetLegacy
> > > diff --git a/qemu-options.hx b/qemu-options.hx
> > > index 5a4eff9..1ad2528 100644
> > > --- a/qemu-options.hx
> > > +++ b/qemu-options.hx
> > > @@ -1460,6 +1460,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
> > >  #ifdef CONFIG_NETMAP
> > >      "netmap|"
> > >  #endif
> > > +    "vhost-user|"
> > >      "socket|"
> > >      "hubport],id=str[,option][,option][,...]\n", QEMU_ARCH_ALL)
> > >  STEXI
> > > @@ -1791,6 +1792,23 @@ The hubport netdev lets you connect a NIC to a QEMU "vlan" instead of a single
> > >  netdev.  @code{-net} and @code{-device} with parameter @option{vlan} create the
> > >  required hub automatically.
> > >
> > > +@item -netdev vhost-user,chardev=@var{id}[,vhostforce=on|off]
> > > +
> > > +Establish a vhost-user netdev, backed by a chardev @var{id}. The chardev should
> > > +be a unix domain socket backed one. The vhost-user uses a specifically defined
> > > +protocol to pass vhost ioctl replacement messages to an application on the other
> > > +end of the socket. On non-MSIX guests, the feature can be forced with
> > > +@var{vhostforce}.
> > > +
> > > +Example:
> > > +@example
> > > +qemu -m 512 -object memory-file,id=mem,size=512M,mem-path=/hugetlbfs,share=on \
> > > +     -numa node,memdev=mem \
Here we give and example and rely on NUMA,memdev, memory-file and share=on.

> > > +     -chardev socket,path=/path/to/socket \
> > > +     -netdev type=vhost-user,id=net0,chardev=chr0 \
> > > +     -device virtio-net-pci,netdev=net0
> > > +@end example
> > > +
> > >  @item -net dump[,vlan=@var{n}][,file=@var{file}][,len=@var{len}]
> > >  Dump network traffic on VLAN @var{n} to file @var{file} (@file{qemu-vlan0.pcap} by default).
> > >  At most @var{len} bytes (64k by default) per packet are stored. The file format is
>
> --
> You received this message because you are subscribed to the Google Groups "Snabb Switch development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to snabb-devel+unsubscribe@googlegroups.com.
> To post to this group, send an email to snabb-devel@googlegroups.com.
> Visit this group at http://groups.google.com/group/snabb-devel.

Once again, I'm sorry if you were misled by seeing 'memdev' in the
changelog, it's a confusion on our side.
Please advice how to proceed.

regards,
Nikolay Nikolaev
Eric Blake June 10, 2014, 12:11 p.m. UTC | #4
On 06/10/2014 04:02 AM, Nikolay Nikolaev wrote:
> The supplied chardev id will be inspected for supported options. Only
> a socket backend, with a set path (i.e. a Unix socket) and optionally
> the server parameter set, will be allowed. Other options (nowait, telnet)
> will make the chardev unusable and the netdev will not be initialised.
> 
> Additional checks for validity:
>   - requires `-numa node,memdev=..`
>   - requires `-device virtio-net-*`
> 
> The `vhostforce` option is used to force vhost-net when we deal with
> non-MSIX guests.

Here you call it vhostforce...[1]

>  
> +static int net_vhost_chardev_opts(const char *name, const char *value,
> +                                  void *opaque)
> +{
> +    VhostUserChardevProps *props = opaque;
> +
> +    if (strcmp(name, "backend") == 0 && strcmp(value, "socket") == 0) {
> +        props->is_socket = true;
> +    } else if (strcmp(name, "path") == 0) {
> +        props->is_unix = true;
> +    } else if (strcmp(name, "server") == 0) {
> +        props->is_server = true;
> +    } else {
> +        error_report("vhost-user does not support a chardev"
> +                     " with the following option:\n %s = %s",
> +                     name, value);
> +        props->has_unsupported = true;
> +        return -1;

Here you reported an error...[2]

> +    }
> +    return 0;
> +}
> +
> +static CharDriverState *net_vhost_parse_chardev(const NetdevVhostUserOptions *opts)
> +{
> +    CharDriverState *chr = qemu_chr_find(opts->chardev);
> +    VhostUserChardevProps props;
> +
> +    if (chr == NULL) {
> +        error_report("chardev \"%s\" not found", opts->chardev);
> +        return NULL;
> +    }
> +
> +    /* inspect chardev opts */
> +    memset(&props, 0, sizeof(props));
> +    qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, false);
> +
> +    if (!props.is_socket || !props.is_unix) {
> +        error_report("chardev \"%s\" is not a unix socket",
> +                     opts->chardev);
> +        return NULL;
> +    }
> +
> +    if (props.has_unsupported) {
> +        error_report("chardev \"%s\" has an unsupported option",
> +                opts->chardev);
> +        return NULL;

[2]...and again another error.  One report is sufficient.  For that
matter, I highly doubt you even need a has_unsupported member - since
you always error out early before allowing the device to be created, it
is just redundant information and will always be false for any device
that gets past creation without reporting an error.


> +##
> +{ 'type': 'NetdevVhostUserOptions',
> +  'data': {
> +    'chardev':        'str',
> +    '*vhost-force':    'bool' } }

[1]...and here you call it vhost-force...


> +Establish a vhost-user netdev, backed by a chardev @var{id}. The chardev should
> +be a unix domain socket backed one. The vhost-user uses a specifically defined
> +protocol to pass vhost ioctl replacement messages to an application on the other
> +end of the socket. On non-MSIX guests, the feature can be forced with
> +@var{vhostforce}.

[1]...yet document it as vhostforce.

If this has already been applied, then you need to submit a followup patch.
Michael S. Tsirkin June 10, 2014, 1:03 p.m. UTC | #5
On Tue, Jun 10, 2014 at 06:11:16AM -0600, Eric Blake wrote:
> On 06/10/2014 04:02 AM, Nikolay Nikolaev wrote:
> > The supplied chardev id will be inspected for supported options. Only
> > a socket backend, with a set path (i.e. a Unix socket) and optionally
> > the server parameter set, will be allowed. Other options (nowait, telnet)
> > will make the chardev unusable and the netdev will not be initialised.
> > 
> > Additional checks for validity:
> >   - requires `-numa node,memdev=..`
> >   - requires `-device virtio-net-*`
> > 
> > The `vhostforce` option is used to force vhost-net when we deal with
> > non-MSIX guests.
> 
> Here you call it vhostforce...[1]
> 
> >  
> > +static int net_vhost_chardev_opts(const char *name, const char *value,
> > +                                  void *opaque)
> > +{
> > +    VhostUserChardevProps *props = opaque;
> > +
> > +    if (strcmp(name, "backend") == 0 && strcmp(value, "socket") == 0) {
> > +        props->is_socket = true;
> > +    } else if (strcmp(name, "path") == 0) {
> > +        props->is_unix = true;
> > +    } else if (strcmp(name, "server") == 0) {
> > +        props->is_server = true;
> > +    } else {
> > +        error_report("vhost-user does not support a chardev"
> > +                     " with the following option:\n %s = %s",
> > +                     name, value);
> > +        props->has_unsupported = true;
> > +        return -1;
> 
> Here you reported an error...[2]
> 
> > +    }
> > +    return 0;
> > +}
> > +
> > +static CharDriverState *net_vhost_parse_chardev(const NetdevVhostUserOptions *opts)
> > +{
> > +    CharDriverState *chr = qemu_chr_find(opts->chardev);
> > +    VhostUserChardevProps props;
> > +
> > +    if (chr == NULL) {
> > +        error_report("chardev \"%s\" not found", opts->chardev);
> > +        return NULL;
> > +    }
> > +
> > +    /* inspect chardev opts */
> > +    memset(&props, 0, sizeof(props));
> > +    qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, false);
> > +
> > +    if (!props.is_socket || !props.is_unix) {
> > +        error_report("chardev \"%s\" is not a unix socket",
> > +                     opts->chardev);
> > +        return NULL;
> > +    }
> > +
> > +    if (props.has_unsupported) {
> > +        error_report("chardev \"%s\" has an unsupported option",
> > +                opts->chardev);
> > +        return NULL;
> 
> [2]...and again another error.  One report is sufficient.  For that
> matter, I highly doubt you even need a has_unsupported member - since
> you always error out early before allowing the device to be created, it
> is just redundant information and will always be false for any device
> that gets past creation without reporting an error.
> 
> 
> > +##
> > +{ 'type': 'NetdevVhostUserOptions',
> > +  'data': {
> > +    'chardev':        'str',
> > +    '*vhost-force':    'bool' } }
> 
> [1]...and here you call it vhost-force...
> 

Should be vhostforce, consistent with tap.

> > +Establish a vhost-user netdev, backed by a chardev @var{id}. The chardev should
> > +be a unix domain socket backed one. The vhost-user uses a specifically defined
> > +protocol to pass vhost ioctl replacement messages to an application on the other
> > +end of the socket. On non-MSIX guests, the feature can be forced with
> > +@var{vhostforce}.
> 
> [1]...yet document it as vhostforce.
> 
> If this has already been applied, then you need to submit a followup patch.


Pls do, use fixup! "original subject" for clarity.
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
Eric Blake June 10, 2014, 1:10 p.m. UTC | #6
On 06/10/2014 07:03 AM, Michael S. Tsirkin wrote:
> On Tue, Jun 10, 2014 at 06:11:16AM -0600, Eric Blake wrote:
>> On 06/10/2014 04:02 AM, Nikolay Nikolaev wrote:
>>> The supplied chardev id will be inspected for supported options. Only
>>> a socket backend, with a set path (i.e. a Unix socket) and optionally
>>> the server parameter set, will be allowed. Other options (nowait, telnet)
>>> will make the chardev unusable and the netdev will not be initialised.
>>>

>>> +##
>>> +{ 'type': 'NetdevVhostUserOptions',
>>> +  'data': {
>>> +    'chardev':        'str',
>>> +    '*vhost-force':    'bool' } }
>>
>> [1]...and here you call it vhost-force...
>>
> 
> Should be vhostforce, consistent with tap.

Oh, good point - no need to make the user have to guess which of two
spellings to use.  Sorry for not noticing that earlier when I was
bikeshedding in my request to add a '-'.
diff mbox

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 2e462c0..bc9c032 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1206,7 +1206,7 @@  ETEXI
     {
         .name       = "host_net_add",
         .args_type  = "device:s,opts:s?",
-        .params     = "tap|user|socket|vde|netmap|dump [options]",
+        .params     = "tap|user|socket|vde|netmap|vhost-user|dump [options]",
         .help       = "add host VLAN client",
         .mhandler.cmd = net_host_device_add,
     },
@@ -1234,7 +1234,7 @@  ETEXI
     {
         .name       = "netdev_add",
         .args_type  = "netdev:O",
-        .params     = "[user|tap|socket|vde|bridge|hubport|netmap],id=str[,prop=value][,...]",
+        .params     = "[user|tap|socket|vde|bridge|hubport|netmap|vhost-user],id=str[,prop=value][,...]",
         .help       = "add host network device",
         .mhandler.cmd = hmp_netdev_add,
         .command_completion = netdev_add_completion,
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 5f06736..7ac7c21 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -15,6 +15,7 @@ 
 
 #include "net/net.h"
 #include "net/tap.h"
+#include "net/vhost-user.h"
 
 #include "hw/virtio/virtio-net.h"
 #include "net/vhost_net.h"
@@ -360,6 +361,9 @@  VHostNetState *get_vhost_net(NetClientState *nc)
     case NET_CLIENT_OPTIONS_KIND_TAP:
         vhost_net = tap_get_vhost_net(nc);
         break;
+    case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
+        vhost_net = vhost_user_get_vhost_net(nc);
+        break;
     default:
         break;
     }
diff --git a/net/hub.c b/net/hub.c
index 33a99c9..7e0f2d6 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -322,6 +322,7 @@  void net_hub_check_clients(void)
             case NET_CLIENT_OPTIONS_KIND_TAP:
             case NET_CLIENT_OPTIONS_KIND_SOCKET:
             case NET_CLIENT_OPTIONS_KIND_VDE:
+            case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
                 has_host_dev = 1;
                 break;
             default:
diff --git a/net/net.c b/net/net.c
index 0ff2e40..c91b67b 100644
--- a/net/net.c
+++ b/net/net.c
@@ -786,6 +786,7 @@  static int (* const net_client_init_fun[NET_CLIENT_OPTIONS_KIND_MAX])(
         [NET_CLIENT_OPTIONS_KIND_BRIDGE]    = net_init_bridge,
 #endif
         [NET_CLIENT_OPTIONS_KIND_HUBPORT]   = net_init_hubport,
+        [NET_CLIENT_OPTIONS_KIND_VHOST_USER] = net_init_vhost_user,
 };
 
 
@@ -819,6 +820,7 @@  static int net_client_init1(const void *object, int is_netdev, Error **errp)
         case NET_CLIENT_OPTIONS_KIND_BRIDGE:
 #endif
         case NET_CLIENT_OPTIONS_KIND_HUBPORT:
+        case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
             break;
 
         default:
@@ -907,6 +909,7 @@  static int net_host_check_device(const char *device)
 #ifdef CONFIG_VDE
                                        ,"vde"
 #endif
+                                       ,"vhost-user"
     };
     for (i = 0; i < ARRAY_SIZE(valid_param_list); i++) {
         if (!strncmp(valid_param_list[i], device,
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 4bdd19d..32b78fb 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -12,6 +12,7 @@ 
 #include "net/vhost_net.h"
 #include "net/vhost-user.h"
 #include "sysemu/char.h"
+#include "qemu/config-file.h"
 #include "qemu/error-report.h"
 
 typedef struct VhostUserState {
@@ -21,9 +22,17 @@  typedef struct VhostUserState {
     VHostNetState *vhost_net;
 } VhostUserState;
 
+typedef struct VhostUserChardevProps {
+    bool is_socket;
+    bool is_unix;
+    bool is_server;
+    bool has_unsupported;
+} VhostUserChardevProps;
+
 VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
 {
     VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
+    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
     return s->vhost_net;
 }
 
@@ -82,7 +91,7 @@  static bool vhost_user_has_ufo(NetClientState *nc)
 }
 
 static NetClientInfo net_vhost_user_info = {
-        .type = 0,
+        .type = NET_CLIENT_OPTIONS_KIND_VHOST_USER,
         .size = sizeof(VhostUserState),
         .cleanup = vhost_user_cleanup,
         .has_vnet_hdr = vhost_user_has_vnet_hdr,
@@ -148,8 +157,108 @@  static int net_vhost_user_init(NetClientState *peer, const char *device,
     return 0;
 }
 
+static int net_vhost_chardev_opts(const char *name, const char *value,
+                                  void *opaque)
+{
+    VhostUserChardevProps *props = opaque;
+
+    if (strcmp(name, "backend") == 0 && strcmp(value, "socket") == 0) {
+        props->is_socket = true;
+    } else if (strcmp(name, "path") == 0) {
+        props->is_unix = true;
+    } else if (strcmp(name, "server") == 0) {
+        props->is_server = true;
+    } else {
+        error_report("vhost-user does not support a chardev"
+                     " with the following option:\n %s = %s",
+                     name, value);
+        props->has_unsupported = true;
+        return -1;
+    }
+    return 0;
+}
+
+static CharDriverState *net_vhost_parse_chardev(const NetdevVhostUserOptions *opts)
+{
+    CharDriverState *chr = qemu_chr_find(opts->chardev);
+    VhostUserChardevProps props;
+
+    if (chr == NULL) {
+        error_report("chardev \"%s\" not found", opts->chardev);
+        return NULL;
+    }
+
+    /* inspect chardev opts */
+    memset(&props, 0, sizeof(props));
+    qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, false);
+
+    if (!props.is_socket || !props.is_unix) {
+        error_report("chardev \"%s\" is not a unix socket",
+                     opts->chardev);
+        return NULL;
+    }
+
+    if (props.has_unsupported) {
+        error_report("chardev \"%s\" has an unsupported option",
+                opts->chardev);
+        return NULL;
+    }
+
+    qemu_chr_fe_claim_no_fail(chr);
+
+    return chr;
+}
+
+static int net_vhost_check_net(QemuOpts *opts, void *opaque)
+{
+    const char *name = opaque;
+    const char *driver, *netdev;
+    const char virtio_name[] = "virtio-net-";
+
+    driver = qemu_opt_get(opts, "driver");
+    netdev = qemu_opt_get(opts, "netdev");
+
+    if (!driver || !netdev) {
+        return 0;
+    }
+
+    if (strcmp(netdev, name) == 0 &&
+        strncmp(driver, virtio_name, strlen(virtio_name)) != 0) {
+        error_report("vhost-user requires frontend driver virtio-net-*");
+        return -1;
+    }
+
+    return 0;
+}
+
 int net_init_vhost_user(const NetClientOptions *opts, const char *name,
-                   NetClientState *peer)
+                        NetClientState *peer)
 {
-    return net_vhost_user_init(peer, "vhost_user", 0, 0, 0);
+    const NetdevVhostUserOptions *vhost_user_opts;
+    CharDriverState *chr;
+    bool vhostforce;
+
+    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
+    vhost_user_opts = opts->vhost_user;
+
+    chr = net_vhost_parse_chardev(vhost_user_opts);
+    if (!chr) {
+        error_report("No suitable chardev found");
+        return -1;
+    }
+
+    /* verify net frontend */
+    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
+                          (char *)name, true) == -1) {
+        return -1;
+    }
+
+    /* vhost-force for non-MSIX */
+    if (vhost_user_opts->has_vhost_force) {
+        vhostforce = vhost_user_opts->vhost_force;
+    } else {
+        vhostforce = false;
+    }
+
+    return net_vhost_user_init(peer, "vhost_user", name, chr, vhostforce);
 }
diff --git a/qapi-schema.json b/qapi-schema.json
index 7bc33ea..f062ce9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3267,6 +3267,22 @@ 
     '*devname':    'str' } }
 
 ##
+# @NetdevVhostUserOptions
+#
+# Vhost-user network backend
+#
+# @chardev: name of a unix socket chardev
+#
+# @vhost-force: #optional vhost on for non-MSIX virtio guests (default: false).
+#
+# Since 2.1
+##
+{ 'type': 'NetdevVhostUserOptions',
+  'data': {
+    'chardev':        'str',
+    '*vhost-force':    'bool' } }
+
+##
 # @NetClientOptions
 #
 # A discriminated record of network device traits.
@@ -3284,7 +3300,8 @@ 
     'dump':     'NetdevDumpOptions',
     'bridge':   'NetdevBridgeOptions',
     'hubport':  'NetdevHubPortOptions',
-    'netmap':   'NetdevNetmapOptions' } }
+    'netmap':   'NetdevNetmapOptions',
+    'vhost-user': 'NetdevVhostUserOptions' } }
 
 ##
 # @NetLegacy
diff --git a/qemu-options.hx b/qemu-options.hx
index 5a4eff9..1ad2528 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1460,6 +1460,7 @@  DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
 #ifdef CONFIG_NETMAP
     "netmap|"
 #endif
+    "vhost-user|"
     "socket|"
     "hubport],id=str[,option][,option][,...]\n", QEMU_ARCH_ALL)
 STEXI
@@ -1791,6 +1792,23 @@  The hubport netdev lets you connect a NIC to a QEMU "vlan" instead of a single
 netdev.  @code{-net} and @code{-device} with parameter @option{vlan} create the
 required hub automatically.
 
+@item -netdev vhost-user,chardev=@var{id}[,vhostforce=on|off]
+
+Establish a vhost-user netdev, backed by a chardev @var{id}. The chardev should
+be a unix domain socket backed one. The vhost-user uses a specifically defined
+protocol to pass vhost ioctl replacement messages to an application on the other
+end of the socket. On non-MSIX guests, the feature can be forced with
+@var{vhostforce}.
+
+Example:
+@example
+qemu -m 512 -object memory-file,id=mem,size=512M,mem-path=/hugetlbfs,share=on \
+     -numa node,memdev=mem \
+     -chardev socket,path=/path/to/socket \
+     -netdev type=vhost-user,id=net0,chardev=chr0 \
+     -device virtio-net-pci,netdev=net0
+@end example
+
 @item -net dump[,vlan=@var{n}][,file=@var{file}][,len=@var{len}]
 Dump network traffic on VLAN @var{n} to file @var{file} (@file{qemu-vlan0.pcap} by default).
 At most @var{len} bytes (64k by default) per packet are stored. The file format is