diff mbox

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

Message ID 20140527120638.15172.80806.stgit@3820
State New
Headers show

Commit Message

Nikolay Nikolaev May 27, 2014, 12:06 p.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          |   25 ++++++-----
 net/vhost-user.c   |  114 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 qapi-schema.json   |   19 ++++++++-
 qemu-options.hx    |   18 ++++++++
 7 files changed, 169 insertions(+), 16 deletions(-)

Comments

Luiz Capitulino June 5, 2014, 2:37 p.m. UTC | #1 
On Tue, 27 May 2014 15:06:43 +0300
Nikolay Nikolaev <n.nikolaev@virtualopensystems.com> 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>

I gave a quick review and apart from some minor comments below it seems
good to me, but I think it would be good to have Eric's review too:

Acked-by: Luiz Capitulino <lcapitulino@redhat.com>

> ---
>  hmp-commands.hx    |    4 +-
>  hw/net/vhost_net.c |    4 ++
>  net/hub.c          |    1 
>  net/net.c          |   25 ++++++-----
>  net/vhost-user.c   |  114 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  qapi-schema.json   |   19 ++++++++-
>  qemu-options.hx    |   18 ++++++++
>  7 files changed, 169 insertions(+), 16 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 8971f1b..ef3782c 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1205,7 +1205,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,
>      },
> @@ -1233,7 +1233,7 @@ ETEXI
>      {
>          .name       = "netdev_add",
>          .args_type  = "netdev:O",
> -        .params     = "[user|tap|socket|hubport|netmap],id=str[,prop=value][,...]",
> +        .params     = "[user|tap|socket|hubport|netmap|vhost-user],id=str[,prop=value][,...]",
>          .help       = "add host network device",
>          .mhandler.cmd = hmp_netdev_add,
>      },
> 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 9db4dba..907f679 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -769,23 +769,24 @@ static int (* const net_client_init_fun[NET_CLIENT_OPTIONS_KIND_MAX])(
>      const NetClientOptions *opts,
>      const char *name,
>      NetClientState *peer) = {
> -        [NET_CLIENT_OPTIONS_KIND_NIC]       = net_init_nic,
> +        [NET_CLIENT_OPTIONS_KIND_NIC]           = net_init_nic,
>  #ifdef CONFIG_SLIRP
> -        [NET_CLIENT_OPTIONS_KIND_USER]      = net_init_slirp,
> +        [NET_CLIENT_OPTIONS_KIND_USER]          = net_init_slirp,
>  #endif
> -        [NET_CLIENT_OPTIONS_KIND_TAP]       = net_init_tap,
> -        [NET_CLIENT_OPTIONS_KIND_SOCKET]    = net_init_socket,
> +        [NET_CLIENT_OPTIONS_KIND_TAP]           = net_init_tap,
> +        [NET_CLIENT_OPTIONS_KIND_SOCKET]        = net_init_socket,
>  #ifdef CONFIG_VDE
> -        [NET_CLIENT_OPTIONS_KIND_VDE]       = net_init_vde,
> +        [NET_CLIENT_OPTIONS_KIND_VDE]           = net_init_vde,
>  #endif
>  #ifdef CONFIG_NETMAP
> -        [NET_CLIENT_OPTIONS_KIND_NETMAP]    = net_init_netmap,
> +        [NET_CLIENT_OPTIONS_KIND_NETMAP]        = net_init_netmap,
>  #endif
> -        [NET_CLIENT_OPTIONS_KIND_DUMP]      = net_init_dump,
> +        [NET_CLIENT_OPTIONS_KIND_DUMP]          = net_init_dump,
>  #ifdef CONFIG_NET_BRIDGE
> -        [NET_CLIENT_OPTIONS_KIND_BRIDGE]    = net_init_bridge,
> +        [NET_CLIENT_OPTIONS_KIND_BRIDGE]        = net_init_bridge,

These changes are unrelated.

>  #endif
> -        [NET_CLIENT_OPTIONS_KIND_HUBPORT]   = net_init_hubport,
> +        [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:
> @@ -902,11 +904,12 @@ static int net_host_check_device(const char *device)
>                                         , "bridge"
>  #endif
>  #ifdef CONFIG_SLIRP
> -                                       ,"user"
> +                                       , "user"
>  #endif
>  #ifdef CONFIG_VDE
> -                                       ,"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..69a5eb4 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,109 @@ 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 = 1;
> +    } else if (strcmp(name, "path") == 0) {
> +        props->is_unix = 1;
> +    } else if (strcmp(name, "server") == 0) {
> +        props->is_server = 1;
> +    } else {
> +        error_report("vhost-user does not support a chardev"
> +                     " with the following option:\n %s = %s",
> +                     name, value);
> +        props->has_unsupported = 1;
> +        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\n", opts->chardev);
> +        return 0;
> +    }
> +
> +    /* 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\n",
> +                     opts->chardev);
> +        return 0;
> +    }
> +
> +    if (props.has_unsupported) {
> +        error_report("chardev \"%s\" has an unsupported option\n",
> +                opts->chardev);
> +        return 0;
> +    }
> +
> +    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)
>  {
> -    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\n");
> +        return -1;
> +    }
> +
> +    /* verify net frontend */
> +    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
> +                          (void *)name, true) == -1) {
> +        return -1;
> +    }
> +
> +    /* vhostforce for non-MSIX */
> +    if (vhost_user_opts->has_vhostforce) {
> +        vhostforce = vhost_user_opts->vhostforce;
> +    } 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 1f28177..f458dd8 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3264,6 +3264,22 @@
>      '*devname':    'str' } }
>  
>  ##
> +# @NetdevVhostUserOptions
> +#
> +# Vhost-user network backend
> +#
> +# @path: control socket path
> +#
> +# @vhostforce: #optional vhost on for non-MSIX virtio guests (default: false).
> +#
> +# Since 2.1
> +##
> +{ 'type': 'NetdevVhostUserOptions',
> +  'data': {
> +    'chardev':        'str',

chardev or path?

> +    '*vhostforce':    'bool' } }
> +
> +##
>  # @NetClientOptions
>  #
>  # A discriminated record of network device traits.
> @@ -3281,7 +3297,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 7f4ab83..2514264 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1459,6 +1459,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
> @@ -1790,6 +1791,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
> 
>
Eric Blake June 5, 2014, 4:08 p.m. UTC | #2 
On 05/27/2014 06:06 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.
> 
> 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          |   25 ++++++-----
>  net/vhost-user.c   |  114 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  qapi-schema.json   |   19 ++++++++-
>  qemu-options.hx    |   18 ++++++++
>  7 files changed, 169 insertions(+), 16 deletions(-)
> 


>  
> +static int net_vhost_chardev_opts(const char *name, const char *value,
> +        void *opaque)

Indentation is off (second line should be aligned to the ( of the first).

> +{
> +    VhostUserChardevProps *props = opaque;
> +
> +    if (strcmp(name, "backend") == 0 && strcmp(value, "socket") == 0) {
> +        props->is_socket = 1;

s/1/true/ - when a field is a boolean, assign it boolean constants.

> +    } else if (strcmp(name, "path") == 0) {
> +        props->is_unix = 1;
> +    } else if (strcmp(name, "server") == 0) {
> +        props->is_server = 1;
> +    } else {
> +        error_report("vhost-user does not support a chardev"
> +                     " with the following option:\n %s = %s",
> +                     name, value);
> +        props->has_unsupported = 1;

and 3 more times.

> +        return -1;
> +    }
> +    return 0;
> +}
> +
> +static CharDriverState *net_vhost_parse_chardev(
> +        const NetdevVhostUserOptions *opts)

Unusual indentation, but I see your dilemma of fitting 80 columns.

> +{
> +    CharDriverState *chr = qemu_chr_find(opts->chardev);
> +    VhostUserChardevProps props;
> +
> +    if (chr == NULL) {
> +        error_report("chardev \"%s\" not found\n", opts->chardev);
> +        return 0;

s/0/NULL/ - much nicer to use the named constant when referring to a
null pointer.

> +    }
> +
> +    /* 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\n",
> +                     opts->chardev);
> +        return 0;
> +    }
> +
> +    if (props.has_unsupported) {
> +        error_report("chardev \"%s\" has an unsupported option\n",
> +                opts->chardev);
> +        return 0;

2 more instances

> +    }
> +
> +    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)) {

Unusual indentation and spurious ():

if (strcmp(netdev, name) == 0 &&
    strncmp(driver, virtio_name, strlen(virtio_name)) != 0) {

> +        error_report("vhost-user requires frontend driver virtio-net-*");

How many of these error_report() should be using Error **errp logistics
instead?

> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
>  int net_init_vhost_user(const NetClientOptions *opts, const char *name,
>                     NetClientState *peer)

Pre-existing indentation problem, but you could fix it here (or if you
introduced it earlier in the series, fix it there)

>  {
> -    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\n");

No \n in error_report() messages.

> +        return -1;
> +    }
> +
> +    /* verify net frontend */
> +    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
> +                          (void *)name, true) == -1) {

This is C; why do you need a cast to (void*)?

> +        return -1;
> +    }
> +
> +    /* vhostforce for non-MSIX */
> +    if (vhost_user_opts->has_vhostforce) {
> +        vhostforce = vhost_user_opts->vhostforce;
> +    } 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 1f28177..f458dd8 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3264,6 +3264,22 @@
>      '*devname':    'str' } }
>  
>  ##
> +# @NetdevVhostUserOptions
> +#
> +# Vhost-user network backend
> +#
> +# @path: control socket path
> +#
> +# @vhostforce: #optional vhost on for non-MSIX virtio guests (default: false).
> +#
> +# Since 2.1
> +##
> +{ 'type': 'NetdevVhostUserOptions',
> +  'data': {
> +    'chardev':        'str',
> +    '*vhostforce':    'bool' } }

Bikeshedding - is 'vhost-force' any more legible?
Nikolay Nikolaev June 9, 2014, 1:28 p.m. UTC | #3 
Hello,


On Thu, Jun 5, 2014 at 5:37 PM, Luiz Capitulino <lcapitulino@redhat.com>
wrote:

> On Tue, 27 May 2014 15:06:43 +0300
> Nikolay Nikolaev <n.nikolaev@virtualopensystems.com> 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>
>
> I gave a quick review and apart from some minor comments below it seems
> good to me, but I think it would be good to have Eric's review too:
>
> Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
>
Thanks!

>
> > ---
> >  hmp-commands.hx    |    4 +-
> >  hw/net/vhost_net.c |    4 ++
> >  net/hub.c          |    1
> >  net/net.c          |   25 ++++++-----
> >  net/vhost-user.c   |  114
> +++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  qapi-schema.json   |   19 ++++++++-
> >  qemu-options.hx    |   18 ++++++++
> >  7 files changed, 169 insertions(+), 16 deletions(-)
> >
> > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > index 8971f1b..ef3782c 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -1205,7 +1205,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,
> >      },
> > @@ -1233,7 +1233,7 @@ ETEXI
> >      {
> >          .name       = "netdev_add",
> >          .args_type  = "netdev:O",
> > -        .params     =
> "[user|tap|socket|hubport|netmap],id=str[,prop=value][,...]",
> > +        .params     =
> "[user|tap|socket|hubport|netmap|vhost-user],id=str[,prop=value][,...]",
> >          .help       = "add host network device",
> >          .mhandler.cmd = hmp_netdev_add,
> >      },
> > 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 9db4dba..907f679 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -769,23 +769,24 @@ static int (* const
> net_client_init_fun[NET_CLIENT_OPTIONS_KIND_MAX])(
> >      const NetClientOptions *opts,
> >      const char *name,
> >      NetClientState *peer) = {
> > -        [NET_CLIENT_OPTIONS_KIND_NIC]       = net_init_nic,
> > +        [NET_CLIENT_OPTIONS_KIND_NIC]           = net_init_nic,
> >  #ifdef CONFIG_SLIRP
> > -        [NET_CLIENT_OPTIONS_KIND_USER]      = net_init_slirp,
> > +        [NET_CLIENT_OPTIONS_KIND_USER]          = net_init_slirp,
> >  #endif
> > -        [NET_CLIENT_OPTIONS_KIND_TAP]       = net_init_tap,
> > -        [NET_CLIENT_OPTIONS_KIND_SOCKET]    = net_init_socket,
> > +        [NET_CLIENT_OPTIONS_KIND_TAP]           = net_init_tap,
> > +        [NET_CLIENT_OPTIONS_KIND_SOCKET]        = net_init_socket,
> >  #ifdef CONFIG_VDE
> > -        [NET_CLIENT_OPTIONS_KIND_VDE]       = net_init_vde,
> > +        [NET_CLIENT_OPTIONS_KIND_VDE]           = net_init_vde,
> >  #endif
> >  #ifdef CONFIG_NETMAP
> > -        [NET_CLIENT_OPTIONS_KIND_NETMAP]    = net_init_netmap,
> > +        [NET_CLIENT_OPTIONS_KIND_NETMAP]        = net_init_netmap,
> >  #endif
> > -        [NET_CLIENT_OPTIONS_KIND_DUMP]      = net_init_dump,
> > +        [NET_CLIENT_OPTIONS_KIND_DUMP]          = net_init_dump,
> >  #ifdef CONFIG_NET_BRIDGE
> > -        [NET_CLIENT_OPTIONS_KIND_BRIDGE]    = net_init_bridge,
> > +        [NET_CLIENT_OPTIONS_KIND_BRIDGE]        = net_init_bridge,
>
> These changes are unrelated.
>
OK. Removing them.

>
> >  #endif
> > -        [NET_CLIENT_OPTIONS_KIND_HUBPORT]   = net_init_hubport,
> > +        [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:
> > @@ -902,11 +904,12 @@ static int net_host_check_device(const char
> *device)
> >                                         , "bridge"
> >  #endif
> >  #ifdef CONFIG_SLIRP
> > -                                       ,"user"
> > +                                       , "user"
> >  #endif
> >  #ifdef CONFIG_VDE
> > -                                       ,"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..69a5eb4 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,109 @@ 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 = 1;
> > +    } else if (strcmp(name, "path") == 0) {
> > +        props->is_unix = 1;
> > +    } else if (strcmp(name, "server") == 0) {
> > +        props->is_server = 1;
> > +    } else {
> > +        error_report("vhost-user does not support a chardev"
> > +                     " with the following option:\n %s = %s",
> > +                     name, value);
> > +        props->has_unsupported = 1;
> > +        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\n", opts->chardev);
> > +        return 0;
> > +    }
> > +
> > +    /* 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\n",
> > +                     opts->chardev);
> > +        return 0;
> > +    }
> > +
> > +    if (props.has_unsupported) {
> > +        error_report("chardev \"%s\" has an unsupported option\n",
> > +                opts->chardev);
> > +        return 0;
> > +    }
> > +
> > +    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)
> >  {
> > -    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\n");
> > +        return -1;
> > +    }
> > +
> > +    /* verify net frontend */
> > +    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
> > +                          (void *)name, true) == -1) {
> > +        return -1;
> > +    }
> > +
> > +    /* vhostforce for non-MSIX */
> > +    if (vhost_user_opts->has_vhostforce) {
> > +        vhostforce = vhost_user_opts->vhostforce;
> > +    } 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 1f28177..f458dd8 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -3264,6 +3264,22 @@
> >      '*devname':    'str' } }
> >
> >  ##
> > +# @NetdevVhostUserOptions
> > +#
> > +# Vhost-user network backend
> > +#
> > +# @path: control socket path
> > +#
> > +# @vhostforce: #optional vhost on for non-MSIX virtio guests (default:
> false).
> > +#
> > +# Since 2.1
> > +##
> > +{ 'type': 'NetdevVhostUserOptions',
> > +  'data': {
> > +    'chardev':        'str',
>
> chardev or path?
>
Right, it's chardev.

>
> > +    '*vhostforce':    'bool' } }
> > +
> > +##
> >  # @NetClientOptions
> >  #
> >  # A discriminated record of network device traits.
> > @@ -3281,7 +3297,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 7f4ab83..2514264 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -1459,6 +1459,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
> > @@ -1790,6 +1791,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
> >
> >
>
> --
> 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.
>

regards,
Nikolay Nikolaev
Michael S. Tsirkin June 9, 2014, 1:31 p.m. UTC | #4 
On Mon, Jun 09, 2014 at 04:28:23PM +0300, Nikolay Nikolaev wrote:
> Hello,
> 
> 
> On Thu, Jun 5, 2014 at 5:37 PM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> 
>     On Tue, 27 May 2014 15:06:43 +0300
>     Nikolay Nikolaev <n.nikolaev@virtualopensystems.com> 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>
> 
>     I gave a quick review and apart from some minor comments below it seems
>     good to me, but I think it would be good to have Eric's review too:
> 
>     Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
> 
> Thanks!
> 
> 
>     > ---
>     >  hmp-commands.hx    |    4 +-
>     >  hw/net/vhost_net.c |    4 ++
>     >  net/hub.c          |    1
>     >  net/net.c          |   25 ++++++-----
>     >  net/vhost-user.c   |  114
>     +++++++++++++++++++++++++++++++++++++++++++++++++++-
>     >  qapi-schema.json   |   19 ++++++++-
>     >  qemu-options.hx    |   18 ++++++++
>     >  7 files changed, 169 insertions(+), 16 deletions(-)
>     >
>     > diff --git a/hmp-commands.hx b/hmp-commands.hx
>     > index 8971f1b..ef3782c 100644
>     > --- a/hmp-commands.hx
>     > +++ b/hmp-commands.hx
>     > @@ -1205,7 +1205,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,
>     >      },
>     > @@ -1233,7 +1233,7 @@ ETEXI
>     >      {
>     >          .name       = "netdev_add",
>     >          .args_type  = "netdev:O",
>     > -        .params     = "[user|tap|socket|hubport|netmap],id=str[,prop=
>     value][,...]",
>     > +        .params     = "[user|tap|socket|hubport|netmap|vhost-user],id=
>     str[,prop=value][,...]",
>     >          .help       = "add host network device",
>     >          .mhandler.cmd = hmp_netdev_add,
>     >      },
>     > 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 9db4dba..907f679 100644
>     > --- a/net/net.c
>     > +++ b/net/net.c
>     > @@ -769,23 +769,24 @@ static int (* const net_client_init_fun
>     [NET_CLIENT_OPTIONS_KIND_MAX])(
>     >      const NetClientOptions *opts,
>     >      const char *name,
>     >      NetClientState *peer) = {
>     > -        [NET_CLIENT_OPTIONS_KIND_NIC]       = net_init_nic,
>     > +        [NET_CLIENT_OPTIONS_KIND_NIC]           = net_init_nic,
>     >  #ifdef CONFIG_SLIRP
>     > -        [NET_CLIENT_OPTIONS_KIND_USER]      = net_init_slirp,
>     > +        [NET_CLIENT_OPTIONS_KIND_USER]          = net_init_slirp,
>     >  #endif
>     > -        [NET_CLIENT_OPTIONS_KIND_TAP]       = net_init_tap,
>     > -        [NET_CLIENT_OPTIONS_KIND_SOCKET]    = net_init_socket,
>     > +        [NET_CLIENT_OPTIONS_KIND_TAP]           = net_init_tap,
>     > +        [NET_CLIENT_OPTIONS_KIND_SOCKET]        = net_init_socket,
>     >  #ifdef CONFIG_VDE
>     > -        [NET_CLIENT_OPTIONS_KIND_VDE]       = net_init_vde,
>     > +        [NET_CLIENT_OPTIONS_KIND_VDE]           = net_init_vde,
>     >  #endif
>     >  #ifdef CONFIG_NETMAP
>     > -        [NET_CLIENT_OPTIONS_KIND_NETMAP]    = net_init_netmap,
>     > +        [NET_CLIENT_OPTIONS_KIND_NETMAP]        = net_init_netmap,
>     >  #endif
>     > -        [NET_CLIENT_OPTIONS_KIND_DUMP]      = net_init_dump,
>     > +        [NET_CLIENT_OPTIONS_KIND_DUMP]          = net_init_dump,
>     >  #ifdef CONFIG_NET_BRIDGE
>     > -        [NET_CLIENT_OPTIONS_KIND_BRIDGE]    = net_init_bridge,
>     > +        [NET_CLIENT_OPTIONS_KIND_BRIDGE]        = net_init_bridge,
> 
>     These changes are unrelated.
> 
> OK. Removing them.
> 
> 
>     >  #endif
>     > -        [NET_CLIENT_OPTIONS_KIND_HUBPORT]   = net_init_hubport,
>     > +        [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:
>     > @@ -902,11 +904,12 @@ static int net_host_check_device(const char
>     *device)
>     >                                         , "bridge"
>     >  #endif
>     >  #ifdef CONFIG_SLIRP
>     > -                                       ,"user"
>     > +                                       , "user"
>     >  #endif
>     >  #ifdef CONFIG_VDE
>     > -                                       ,"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..69a5eb4 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,109 @@ 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 = 1;
>     > +    } else if (strcmp(name, "path") == 0) {
>     > +        props->is_unix = 1;
>     > +    } else if (strcmp(name, "server") == 0) {
>     > +        props->is_server = 1;
>     > +    } else {
>     > +        error_report("vhost-user does not support a chardev"
>     > +                     " with the following option:\n %s = %s",
>     > +                     name, value);
>     > +        props->has_unsupported = 1;
>     > +        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\n", opts->chardev);
>     > +        return 0;
>     > +    }
>     > +
>     > +    /* 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\n",
>     > +                     opts->chardev);
>     > +        return 0;
>     > +    }
>     > +
>     > +    if (props.has_unsupported) {
>     > +        error_report("chardev \"%s\" has an unsupported option\n",
>     > +                opts->chardev);
>     > +        return 0;
>     > +    }
>     > +
>     > +    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)
>     >  {
>     > -    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\n");
>     > +        return -1;
>     > +    }
>     > +
>     > +    /* verify net frontend */
>     > +    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
>     > +                          (void *)name, true) == -1) {
>     > +        return -1;
>     > +    }
>     > +
>     > +    /* vhostforce for non-MSIX */
>     > +    if (vhost_user_opts->has_vhostforce) {
>     > +        vhostforce = vhost_user_opts->vhostforce;
>     > +    } 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 1f28177..f458dd8 100644
>     > --- a/qapi-schema.json
>     > +++ b/qapi-schema.json
>     > @@ -3264,6 +3264,22 @@
>     >      '*devname':    'str' } }
>     >
>     >  ##
>     > +# @NetdevVhostUserOptions
>     > +#
>     > +# Vhost-user network backend
>     > +#
>     > +# @path: control socket path
>     > +#
>     > +# @vhostforce: #optional vhost on for non-MSIX virtio guests (default:
>     false).
>     > +#
>     > +# Since 2.1
>     > +##
>     > +{ 'type': 'NetdevVhostUserOptions',
>     > +  'data': {
>     > +    'chardev':        'str',
> 
>     chardev or path?
> 
> Right, it's chardev.
> 
> 
>     > +    '*vhostforce':    'bool' } }
>     > +
>     > +##
>     >  # @NetClientOptions
>     >  #
>     >  # A discriminated record of network device traits.
>     > @@ -3281,7 +3297,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 7f4ab83..2514264 100644
>     > --- a/qemu-options.hx
>     > +++ b/qemu-options.hx
>     > @@ -1459,6 +1459,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
>     > @@ -1790,6 +1791,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
>     >
>     >
> 
>     --
>     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.
> 
> 
> regards,
> Nikolay Nikolaev


Pls remember to base on top of vhost branch in my tree,
this way you will not need to re-post merged patches.

If you want me to drop some merged patch from my tree,
include a revert and I'll figure it out.
Nikolay Nikolaev June 9, 2014, 1:43 p.m. UTC | #5 
On Mon, Jun 9, 2014 at 4:31 PM, Michael S. Tsirkin <mst@redhat.com> wrote:

> On Mon, Jun 09, 2014 at 04:28:23PM +0300, Nikolay Nikolaev wrote:
> > Hello,
> >
> >
> > On Thu, Jun 5, 2014 at 5:37 PM, Luiz Capitulino <lcapitulino@redhat.com>
> wrote:
> >
> >     On Tue, 27 May 2014 15:06:43 +0300
> >     Nikolay Nikolaev <n.nikolaev@virtualopensystems.com> 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
> >
> >
> >     I gave a quick review and apart from some minor comments below it
> seems
> >     good to me, but I think it would be good to have Eric's review too:
> >
> >     Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
> >
> > Thanks!
> >
> >
> >     > ---
> >     >  hmp-commands.hx    |    4 +-
> >     >  hw/net/vhost_net.c |    4 ++
> >     >  net/hub.c          |    1
> >     >  net/net.c          |   25 ++++++-----
> >     >  net/vhost-user.c   |  114
> >     +++++++++++++++++++++++++++++++++++++++++++++++++++-
> >     >  qapi-schema.json   |   19 ++++++++-
> >     >  qemu-options.hx    |   18 ++++++++
> >     >  7 files changed, 169 insertions(+), 16 deletions(-)
> >     >
> >     > diff --git a/hmp-commands.hx b/hmp-commands.hx
> >     > index 8971f1b..ef3782c 100644
> >     > --- a/hmp-commands.hx
> >     > +++ b/hmp-commands.hx
> >     > @@ -1205,7 +1205,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,
> >     >      },
> >     > @@ -1233,7 +1233,7 @@ ETEXI
> >     >      {
> >     >          .name       = "netdev_add",
> >     >          .args_type  = "netdev:O",
> >     > -        .params     =
> "[user|tap|socket|hubport|netmap],id=str[,prop=
> >     value][,...]",
> >     > +        .params     =
> "[user|tap|socket|hubport|netmap|vhost-user],id=
> >     str[,prop=value][,...]",
> >     >          .help       = "add host network device",
> >     >          .mhandler.cmd = hmp_netdev_add,
> >     >      },
> >     > 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 9db4dba..907f679 100644
> >     > --- a/net/net.c
> >     > +++ b/net/net.c
> >     > @@ -769,23 +769,24 @@ static int (* const net_client_init_fun
> >     [NET_CLIENT_OPTIONS_KIND_MAX])(
> >     >      const NetClientOptions *opts,
> >     >      const char *name,
> >     >      NetClientState *peer) = {
> >     > -        [NET_CLIENT_OPTIONS_KIND_NIC]       = net_init_nic,
> >     > +        [NET_CLIENT_OPTIONS_KIND_NIC]           = net_init_nic,
> >     >  #ifdef CONFIG_SLIRP
> >     > -        [NET_CLIENT_OPTIONS_KIND_USER]      = net_init_slirp,
> >     > +        [NET_CLIENT_OPTIONS_KIND_USER]          = net_init_slirp,
> >     >  #endif
> >     > -        [NET_CLIENT_OPTIONS_KIND_TAP]       = net_init_tap,
> >     > -        [NET_CLIENT_OPTIONS_KIND_SOCKET]    = net_init_socket,
> >     > +        [NET_CLIENT_OPTIONS_KIND_TAP]           = net_init_tap,
> >     > +        [NET_CLIENT_OPTIONS_KIND_SOCKET]        = net_init_socket,
> >     >  #ifdef CONFIG_VDE
> >     > -        [NET_CLIENT_OPTIONS_KIND_VDE]       = net_init_vde,
> >     > +        [NET_CLIENT_OPTIONS_KIND_VDE]           = net_init_vde,
> >     >  #endif
> >     >  #ifdef CONFIG_NETMAP
> >     > -        [NET_CLIENT_OPTIONS_KIND_NETMAP]    = net_init_netmap,
> >     > +        [NET_CLIENT_OPTIONS_KIND_NETMAP]        = net_init_netmap,
> >     >  #endif
> >     > -        [NET_CLIENT_OPTIONS_KIND_DUMP]      = net_init_dump,
> >     > +        [NET_CLIENT_OPTIONS_KIND_DUMP]          = net_init_dump,
> >     >  #ifdef CONFIG_NET_BRIDGE
> >     > -        [NET_CLIENT_OPTIONS_KIND_BRIDGE]    = net_init_bridge,
> >     > +        [NET_CLIENT_OPTIONS_KIND_BRIDGE]        = net_init_bridge,
> >
> >     These changes are unrelated.
> >
> > OK. Removing them.
> >
> >
> >     >  #endif
> >     > -        [NET_CLIENT_OPTIONS_KIND_HUBPORT]   = net_init_hubport,
> >     > +        [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:
> >     > @@ -902,11 +904,12 @@ static int net_host_check_device(const char
> >     *device)
> >     >                                         , "bridge"
> >     >  #endif
> >     >  #ifdef CONFIG_SLIRP
> >     > -                                       ,"user"
> >     > +                                       , "user"
> >     >  #endif
> >     >  #ifdef CONFIG_VDE
> >     > -                                       ,"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..69a5eb4 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,109 @@ 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 = 1;
> >     > +    } else if (strcmp(name, "path") == 0) {
> >     > +        props->is_unix = 1;
> >     > +    } else if (strcmp(name, "server") == 0) {
> >     > +        props->is_server = 1;
> >     > +    } else {
> >     > +        error_report("vhost-user does not support a chardev"
> >     > +                     " with the following option:\n %s = %s",
> >     > +                     name, value);
> >     > +        props->has_unsupported = 1;
> >     > +        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\n", opts->chardev);
> >     > +        return 0;
> >     > +    }
> >     > +
> >     > +    /* 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\n",
> >     > +                     opts->chardev);
> >     > +        return 0;
> >     > +    }
> >     > +
> >     > +    if (props.has_unsupported) {
> >     > +        error_report("chardev \"%s\" has an unsupported option\n",
> >     > +                opts->chardev);
> >     > +        return 0;
> >     > +    }
> >     > +
> >     > +    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)
> >     >  {
> >     > -    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\n");
> >     > +        return -1;
> >     > +    }
> >     > +
> >     > +    /* verify net frontend */
> >     > +    if (qemu_opts_foreach(qemu_find_opts("device"),
> net_vhost_check_net,
> >     > +                          (void *)name, true) == -1) {
> >     > +        return -1;
> >     > +    }
> >     > +
> >     > +    /* vhostforce for non-MSIX */
> >     > +    if (vhost_user_opts->has_vhostforce) {
> >     > +        vhostforce = vhost_user_opts->vhostforce;
> >     > +    } 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 1f28177..f458dd8 100644
> >     > --- a/qapi-schema.json
> >     > +++ b/qapi-schema.json
> >     > @@ -3264,6 +3264,22 @@
> >     >      '*devname':    'str' } }
> >     >
> >     >  ##
> >     > +# @NetdevVhostUserOptions
> >     > +#
> >     > +# Vhost-user network backend
> >     > +#
> >     > +# @path: control socket path
> >     > +#
> >     > +# @vhostforce: #optional vhost on for non-MSIX virtio guests
> (default:
> >     false).
> >     > +#
> >     > +# Since 2.1
> >     > +##
> >     > +{ 'type': 'NetdevVhostUserOptions',
> >     > +  'data': {
> >     > +    'chardev':        'str',
> >
> >     chardev or path?
> >
> > Right, it's chardev.
> >
> >
> >     > +    '*vhostforce':    'bool' } }
> >     > +
> >     > +##
> >     >  # @NetClientOptions
> >     >  #
> >     >  # A discriminated record of network device traits.
> >     > @@ -3281,7 +3297,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 7f4ab83..2514264 100644
> >     > --- a/qemu-options.hx
> >     > +++ b/qemu-options.hx
> >     > @@ -1459,6 +1459,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
> >     > @@ -1790,6 +1791,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
> >     >
> >     >
> >
> >     --
> >     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.
> >
> >
> > regards,
> > Nikolay Nikolaev
>
>
> Pls remember to base on top of vhost branch in my tree,
> this way you will not need to re-post merged patches.
>

OK - that's what we're doing. There are two questions though:

 - should we post with separate numbering - 15,16,18 will become 1/3, 2/3,
3/3? Or do you prefer to preserve the original numbering?

 - We can not test it. Probably we get something wrong, but vhost-user-v10
was rebased on Hu Tao's NUMA v3.2 series. And it uses
this on the command line to enalbe mmaped memory share flag (on HUGETLBFS):

     -object memory-file,id=mem,size=512M,mem-path=/hugetlbfs,share=on \
      -numa node,memdev=mem \

Now, this does not work, which will break the qtest (patch 18). Maybe we
misled you by saying it is rebased on memdev instead of naming it NUMA
patchseries.


>
> If you want me to drop some merged patch from my tree,
> include a revert and I'll figure it out.
>
> Will have this in mind - thanks.

>  --
> MST
>
> --
> 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.
>

regards,
Nikolay Nikolaev
Nikolay Nikolaev June 9, 2014, 9:19 p.m. UTC | #6 
On Thu, Jun 5, 2014 at 7:08 PM, Eric Blake <eblake@redhat.com> wrote:

> On 05/27/2014 06:06 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.
> >
> > 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          |   25 ++++++-----
> >  net/vhost-user.c   |  114
> +++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  qapi-schema.json   |   19 ++++++++-
> >  qemu-options.hx    |   18 ++++++++
> >  7 files changed, 169 insertions(+), 16 deletions(-)
> >
>
>
> >
> > +static int net_vhost_chardev_opts(const char *name, const char *value,
> > +        void *opaque)
>
> Indentation is off (second line should be aligned to the ( of the first).
>
OK, will fix it.

>
> > +{
> > +    VhostUserChardevProps *props = opaque;
> > +
> > +    if (strcmp(name, "backend") == 0 && strcmp(value, "socket") == 0) {
> > +        props->is_socket = 1;
>
> s/1/true/ - when a field is a boolean, assign it boolean constants.
>
OK, will fix all of them.

>
> > +    } else if (strcmp(name, "path") == 0) {
> > +        props->is_unix = 1;
> > +    } else if (strcmp(name, "server") == 0) {
> > +        props->is_server = 1;
> > +    } else {
> > +        error_report("vhost-user does not support a chardev"
> > +                     " with the following option:\n %s = %s",
> > +                     name, value);
> > +        props->has_unsupported = 1;
>
> and 3 more times.
>
> > +        return -1;
> > +    }
> > +    return 0;
> > +}
> > +
> > +static CharDriverState *net_vhost_parse_chardev(
> > +        const NetdevVhostUserOptions *opts)
>
> Unusual indentation, but I see your dilemma of fitting 80 columns.
>
What woudl be the right approach here, is leaving it 84 colums acceptable:

static CharDriverState *net_vhost_parse_chardev(const
NetdevVhostUserOptions *opts)

Or split it like this:
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\n", opts->chardev);
> > +        return 0;
>
> s/0/NULL/ - much nicer to use the named constant when referring to a
> null pointer.
>
OK - fixed on all places.


> > +    }
> > +
> > +    /* 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\n",
> > +                     opts->chardev);
> > +        return 0;
> > +    }
> > +
> > +    if (props.has_unsupported) {
> > +        error_report("chardev \"%s\" has an unsupported option\n",
> > +                opts->chardev);
> > +        return 0;
>
> 2 more instances
>
> > +    }
> > +
> > +    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)) {
>
> Unusual indentation and spurious ():
>
> if (strcmp(netdev, name) == 0 &&
>     strncmp(driver, virtio_name, strlen(virtio_name)) != 0) {
>
> OK - thanks for the providing a solution.

> > +        error_report("vhost-user requires frontend driver
> virtio-net-*");
>
> How many of these error_report() should be using Error **errp logistics
> instead?
>
> I don't see this 'errp' logic applied here. These are static functions
called in the init function to check for compatible command line parameters.
This init fucntion is part of 'net_client_init_fun[]" which does not
provide 'Error **errp' argument. The way I see it, there is no way to
propagate the 'errp' up. Or am I getting this wrong?


>  > +        return -1;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> >  int net_init_vhost_user(const NetClientOptions *opts, const char *name,
> >                     NetClientState *peer)
>
> Pre-existing indentation problem, but you could fix it here (or if you
> introduced it earlier in the series, fix it there)
>
Will fix it.

>
> >  {
> > -    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\n");
>
> No \n in error_report() messages.
>
OK.

>
> > +        return -1;
> > +    }
> > +
> > +    /* verify net frontend */
> > +    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
> > +                          (void *)name, true) == -1) {
>
> This is C; why do you need a cast to (void*)?
>
Because of the constness in 'const char *name'. I am casting to 'char *
now', is it better?

>
> > +        return -1;
> > +    }
> > +
> > +    /* vhostforce for non-MSIX */
> > +    if (vhost_user_opts->has_vhostforce) {
> > +        vhostforce = vhost_user_opts->vhostforce;
> > +    } 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 1f28177..f458dd8 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -3264,6 +3264,22 @@
> >      '*devname':    'str' } }
> >
> >  ##
> > +# @NetdevVhostUserOptions
> > +#
> > +# Vhost-user network backend
> > +#
> > +# @path: control socket path
> > +#
> > +# @vhostforce: #optional vhost on for non-MSIX virtio guests (default:
> false).
> > +#
> > +# Since 2.1
> > +##
> > +{ 'type': 'NetdevVhostUserOptions',
> > +  'data': {
> > +    'chardev':        'str',
> > +    '*vhostforce':    'bool' } }
>
> Bikeshedding - is 'vhost-force' any more legible?
>
'vhost-force' it is.

>
> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
> Thanks!

regards,
Nikolay Nikolaev
Eric Blake June 9, 2014, 10:22 p.m. UTC | #7 
On 06/09/2014 03:19 PM, Nikolay Nikolaev wrote:

>>> +static CharDriverState *net_vhost_parse_chardev(
>>> +        const NetdevVhostUserOptions *opts)
>>
>> Unusual indentation, but I see your dilemma of fitting 80 columns.
>>
> What woudl be the right approach here, is leaving it 84 colums acceptable:

checkpatch.pl is for guidance; it is okay to have a patch that doesn't
pass. At any rate, I won't reject a patch for long lines, or for unusual
style, where it is obvious that there is no way to fit things into 80
columns while still complying with all other style issues.


>>> +        error_report("vhost-user requires frontend driver
>> virtio-net-*");
>>
>> How many of these error_report() should be using Error **errp logistics
>> instead?
>>
>> I don't see this 'errp' logic applied here. These are static functions
> called in the init function to check for compatible command line parameters.
> This init fucntion is part of 'net_client_init_fun[]" which does not
> provide 'Error **errp' argument. The way I see it, there is no way to
> propagate the 'errp' up. Or am I getting this wrong?

It was more of a 'food for thought' question - we may eventually want to
enhance the code base to be able to propagate errp up; but since that is
a more invasive patch, and your series isn't making the situation worse,
it does not necessarily mean you have to do that additional work.  On
the other hand, if the conversion is worth doing, then getting it done
sooner makes it easier to take advantage of the improved error handling
for other new code in the same area.


>>> +
>>> +    /* verify net frontend */
>>> +    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
>>> +                          (void *)name, true) == -1) {
>>
>> This is C; why do you need a cast to (void*)?
>>
> Because of the constness in 'const char *name'. I am casting to 'char *
> now', is it better?

Okay, I see.  Sometimes, I like to put in a comment /* cast away const
*/ to make it obvious why I'm doing things like that.  It also serves as
a spur to investigate a couple of things: 1) should the callback
signature should have allowed a const in the first place, rather than
making callers have to cast away const, 2) if the callback can modify
the pointer but I'm casting away const, am I going to cause data
corruption and/or a crash
diff mbox

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8971f1b..ef3782c 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1205,7 +1205,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,
     },
@@ -1233,7 +1233,7 @@  ETEXI
     {
         .name       = "netdev_add",
         .args_type  = "netdev:O",
-        .params     = "[user|tap|socket|hubport|netmap],id=str[,prop=value][,...]",
+        .params     = "[user|tap|socket|hubport|netmap|vhost-user],id=str[,prop=value][,...]",
         .help       = "add host network device",
         .mhandler.cmd = hmp_netdev_add,
     },
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 9db4dba..907f679 100644
--- a/net/net.c
+++ b/net/net.c
@@ -769,23 +769,24 @@  static int (* const net_client_init_fun[NET_CLIENT_OPTIONS_KIND_MAX])(
     const NetClientOptions *opts,
     const char *name,
     NetClientState *peer) = {
-        [NET_CLIENT_OPTIONS_KIND_NIC]       = net_init_nic,
+        [NET_CLIENT_OPTIONS_KIND_NIC]           = net_init_nic,
 #ifdef CONFIG_SLIRP
-        [NET_CLIENT_OPTIONS_KIND_USER]      = net_init_slirp,
+        [NET_CLIENT_OPTIONS_KIND_USER]          = net_init_slirp,
 #endif
-        [NET_CLIENT_OPTIONS_KIND_TAP]       = net_init_tap,
-        [NET_CLIENT_OPTIONS_KIND_SOCKET]    = net_init_socket,
+        [NET_CLIENT_OPTIONS_KIND_TAP]           = net_init_tap,
+        [NET_CLIENT_OPTIONS_KIND_SOCKET]        = net_init_socket,
 #ifdef CONFIG_VDE
-        [NET_CLIENT_OPTIONS_KIND_VDE]       = net_init_vde,
+        [NET_CLIENT_OPTIONS_KIND_VDE]           = net_init_vde,
 #endif
 #ifdef CONFIG_NETMAP
-        [NET_CLIENT_OPTIONS_KIND_NETMAP]    = net_init_netmap,
+        [NET_CLIENT_OPTIONS_KIND_NETMAP]        = net_init_netmap,
 #endif
-        [NET_CLIENT_OPTIONS_KIND_DUMP]      = net_init_dump,
+        [NET_CLIENT_OPTIONS_KIND_DUMP]          = net_init_dump,
 #ifdef CONFIG_NET_BRIDGE
-        [NET_CLIENT_OPTIONS_KIND_BRIDGE]    = net_init_bridge,
+        [NET_CLIENT_OPTIONS_KIND_BRIDGE]        = net_init_bridge,
 #endif
-        [NET_CLIENT_OPTIONS_KIND_HUBPORT]   = net_init_hubport,
+        [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:
@@ -902,11 +904,12 @@  static int net_host_check_device(const char *device)
                                        , "bridge"
 #endif
 #ifdef CONFIG_SLIRP
-                                       ,"user"
+                                       , "user"
 #endif
 #ifdef CONFIG_VDE
-                                       ,"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..69a5eb4 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,109 @@  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 = 1;
+    } else if (strcmp(name, "path") == 0) {
+        props->is_unix = 1;
+    } else if (strcmp(name, "server") == 0) {
+        props->is_server = 1;
+    } else {
+        error_report("vhost-user does not support a chardev"
+                     " with the following option:\n %s = %s",
+                     name, value);
+        props->has_unsupported = 1;
+        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\n", opts->chardev);
+        return 0;
+    }
+
+    /* 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\n",
+                     opts->chardev);
+        return 0;
+    }
+
+    if (props.has_unsupported) {
+        error_report("chardev \"%s\" has an unsupported option\n",
+                opts->chardev);
+        return 0;
+    }
+
+    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)
 {
-    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\n");
+        return -1;
+    }
+
+    /* verify net frontend */
+    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
+                          (void *)name, true) == -1) {
+        return -1;
+    }
+
+    /* vhostforce for non-MSIX */
+    if (vhost_user_opts->has_vhostforce) {
+        vhostforce = vhost_user_opts->vhostforce;
+    } 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 1f28177..f458dd8 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3264,6 +3264,22 @@ 
     '*devname':    'str' } }
 
 ##
+# @NetdevVhostUserOptions
+#
+# Vhost-user network backend
+#
+# @path: control socket path
+#
+# @vhostforce: #optional vhost on for non-MSIX virtio guests (default: false).
+#
+# Since 2.1
+##
+{ 'type': 'NetdevVhostUserOptions',
+  'data': {
+    'chardev':        'str',
+    '*vhostforce':    'bool' } }
+
+##
 # @NetClientOptions
 #
 # A discriminated record of network device traits.
@@ -3281,7 +3297,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 7f4ab83..2514264 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1459,6 +1459,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
@@ -1790,6 +1791,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