On Fri, May 29, 2020 at 10:23 PM Eric Blake <ebl...@redhat.com> wrote:
>
> On 5/29/20 9:06 AM, Cindy Lu wrote:
> > From: Tiwei Bie <tiwei....@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....@intel.com>
> > Signed-off-by: Cindy Lu <l...@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
>
