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.mota...@virtualopensystems.com> > Signed-off-by: Nikolay Nikolaev <n.nikol...@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?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
