Re: [RFC v3 8/8] vhost-vdpa: introduce vhost-vdpa net client

[Cindy Lu <lulu@redhat.com>]

On Fri, May 29, 2020 at 10:23 PM Eric Blake <eblake@redhat.com> wrote:
>
> On 5/29/20 9:06 AM, Cindy Lu wrote:
> > From: Tiwei Bie <tiwei.bie@intel.com>
> >
> > This patch set introduces a new net client type: vhost-vdpa.
> > vhost-vdpa net client will set up a vDPA device which is specified
> > by a "vhostdev" parameter.
> >
> > Co-authored-by: Lingshan Zhu <lingshan.zhu@intel.com>
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
>
> > +static int net_vhost_vdpa_init(NetClientState *peer, const char *device,
> > +                               const char *name, const char *vhostdev,
> > +                               bool has_fd, char *fd)
> > +{
>
> fd is usually an int, not a string.
>
will fix this
> > +    NetClientState *nc = NULL;
> > +    VhostVDPAState *s;
> > +    int vdpa_device_fd = -1;
> > +    Error *err = NULL;
> > +    int ret = 0;
> > +    assert(name);
> > +
> > +    nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, device, name);
> > +    snprintf(nc->info_str, sizeof(nc->info_str), "vhost-vdpa");
> > +    nc->queue_index = 0;
> > +
> > +    s = DO_UPCAST(VhostVDPAState, nc, nc);
> > +
> > +    if (has_fd) {
> > +        vdpa_device_fd = monitor_fd_param(cur_mon, fd, &err);
> > +    } else{
> > +        vdpa_device_fd = open(vhostdev, O_RDWR);
> > +    }
>
> Oh, you're trying to use the old way for passing in fds.  The preferred
> way is to use qemu_open(), at which point you can pass in fds via the
> add-fd QMP command, and then pass the string "/dev/fdset/NNN" as
> vhostdev.  Then you don't need a special fd parameter here.
>
Thanks Eric, I will try this.

> > +++ b/qapi/net.json
> > @@ -428,6 +428,27 @@
> >       '*vhostforce':    'bool',
> >       '*queues':        'int' } }
> >
> > +##
> > +# @NetdevVhostVDPAOptions:
> > +#
> > +# Vhost-vdpa network backend
> > +#
> > +# @vhostdev: name of a vdpa dev path in sysfs
> > +#            (default path:/dev/vhost-vdpa-$ID)
> > +#
> > +# @fd: file descriptor of an already opened vdpa device
> > +#
> > +# @queues: number of queues to be created for multiqueue vhost-vdpa
> > +#          (default: 1)
> > +#
> > +# Since: 5.1
> > +##
> > +{ 'struct': 'NetdevVhostVDPAOptions',
> > +  'data': {
> > +    '*vhostdev':     'str',
> > +    '*fd':           'str',
> > +    '*queues':       'int' } }
>
> Instead of having vhostdev and fd both be optional (but where the user
> has to specify exactly one of them), you should only have vhostdev be
> mandatory, and rely on the /dev/fdset/NNN string as a way to get
> vhostdev to point to a previously-passed fd.
>
will fix this
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>