Re: [Qemu-devel] [PATCH v6 1/2] vhost-user: add multi queue support

[Michael S. Tsirkin]

On Mon, Sep 07, 2015 at 02:07:38PM +0300, Marcel Apfelbaum wrote:
> On 08/13/2015 12:18 PM, Michael S. Tsirkin wrote:
> >On Wed, Aug 12, 2015 at 02:25:41PM +0800, Ouyang Changchun wrote:
> >>Based on patch by Nikolay Nikolaev:
> >>Vhost-user will implement the multi queue support in a similar way
> >>to what vhost already has - a separate thread for each queue.
> >>To enable the multi queue functionality - a new command line parameter
> >>"queues" is introduced for the vhost-user netdev.
> >>
> >>The RESET_OWNER change is based on commit:
> >>    294ce717e0f212ed0763307f3eab72b4a1bdf4d0
> >>If it is reverted, the patch need update for it accordingly.
> >>
> >>Signed-off-by: Nikolay Nikolaev <address@hidden>
> >>Signed-off-by: Changchun Ouyang <address@hidden>
> >>---
> >>Changes since v5:
> >>  - fix the message descption for VHOST_RESET_OWNER in vhost-user txt
> >>
> >>Changes since v4:
> >>  - remove the unnecessary trailing '\n'
> >>
> >>Changes since v3:
> >>  - fix one typo and wrap one long line
> >>
> >>Changes since v2:
> >>  - fix vq index issue for set_vring_call
> >>    When it is the case of VHOST_SET_VRING_CALL, The vq_index is not 
> >> initialized before it is used,
> >>    thus it could be a random value. The random value leads to crash in 
> >> vhost after passing down
> >>    to vhost, as vhost use this random value to index an array index.
> >>  - fix the typo in the doc and description
> >>  - address vq index for reset_owner
> >>
> >>Changes since v1:
> >>  - use s->nc.info_str when bringing up/down the backend
> >>
> >>  docs/specs/vhost-user.txt |  7 ++++++-
> >>  hw/net/vhost_net.c        |  3 ++-
> >>  hw/virtio/vhost-user.c    | 11 ++++++++++-
> >>  net/vhost-user.c          | 37 ++++++++++++++++++++++++-------------
> >>  qapi-schema.json          |  6 +++++-
> >>  qemu-options.hx           |  5 +++--
> >>  6 files changed, 50 insertions(+), 19 deletions(-)
> >>
> 
> [...]
> 
> >
> >
> >There are two problems here:
> >
> >1. we don't really know that the backend
> >    is able to support the requested number of queues.
> >    If not, everything will fail, silently.
> >    A new message to query the # of queues could help, though
> >    I'm not sure what can be done on failure. Fail connection?
> >
> >2. each message (e.g. set memory table) is sent multiple times,
> >    on the same socket.
> 
> Hi,
> 
> Actually each queue has its own vhost_dev device which in turn has his own
> memory mappings. Because of this VHOST_SET_MEM_TABLE should be sent for each 
> queue.
> 
> Should we change it to VHOST_SET_VRING_MEM_TABLE? Or maybe I got this wrong...
> 
> Thanks,
> Marcel
> 

You got it wrong, the table is the same for all rings.

> 
> 
> >
> >
> >
> 
> [...]