On Mon, Dec 08, 2014 at 10:04:37AM +0100, Olivier MATZ wrote:
> Hi Nikolay,
>
> On 12/06/2014 05:52 PM, Nikolay Nikolaev wrote:
> > Vhost-user will implement the multiqueueu support in a similar way to what
> > vhost already has - a separate thread for each queue.
> >
> > To enable multiquue funcionality - a new command line parameter
> > "queues" is introduced for the vhost-user netdev.
> >
> > Signed-off-by: Nikolay Nikolaev <n.nikol...@virtualopensystems.com>
> > ---
> > docs/specs/vhost-user.txt | 7 +++++++
> > hw/virtio/vhost-user.c | 6 +++++-
> > net/vhost-user.c | 35 +++++++++++++++++++++++------------
> > qapi-schema.json | 5 ++++-
> > qemu-options.hx | 5 +++--
> > 5 files changed, 42 insertions(+), 16 deletions(-)
> > [...]
> > --- a/net/vhost-user.c
> > +++ b/net/vhost-user.c
> > @@ -134,25 +134,27 @@ static void net_vhost_user_event(void *opaque, int
> > event)
> >
> > static int net_vhost_user_init(NetClientState *peer, const char *device,
> > const char *name, CharDriverState *chr,
> > - bool vhostforce)
> > + bool vhostforce, uint32_t queues)
> > {
> > NetClientState *nc;
> > VhostUserState *s;
> > + int i;
> >
> > - nc = qemu_new_net_client(&net_vhost_user_info, peer, device, name);
> > + for (i = 0; i < queues; i++) {
> > + nc = qemu_new_net_client(&net_vhost_user_info, peer, device, name);
> >
> > - snprintf(nc->info_str, sizeof(nc->info_str), "vhost-user to %s",
> > - chr->label);
> > + snprintf(nc->info_str, sizeof(nc->info_str), "vhost-user%d to %s",
> > + i, chr->label);
> >
>
> Now that there several vhost-user are pointing to the same unix socket,
> it could make sense to display "nc->info_str" instead of "s->chr->label"
> in net_vhost_user_event(). Something like that:
>
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -122,36 +122,39 @@ static void net_vhost_user_event(void *opaque, int
> event)
> case CHR_EVENT_OPENED:
> vhost_user_start(s);
> net_vhost_link_down(s, false);
> - error_report("chardev \"%s\" went up\n", s->chr->label);
> + error_report("chardev \"%s\" went up\n", s->nc.info_str);
> break;
> case CHR_EVENT_CLOSED:
> net_vhost_link_down(s, true);
> vhost_user_stop(s);
> - error_report("chardev \"%s\" went down\n", s->chr->label);
> + error_report("chardev \"%s\" went down\n", s->nc.info_str);
> break;
> }
> }
>
>
> Also, another comment: if I understand well, the messages like
> VHOST_USER_SET_OWNER, VHOST_USER_SET_FEATURES, VHOST_SET_MEM_TABLE,
> (...) will be send once per queue pair and not once per device.
One wonders why that's necessary.
> I don't think it's a problem, but maybe it deserves a small comment
> in the protocol documentation.
>
>
> Apart from these 2 small comments, the approach looks correct, so
> Acked-by: Olivier Matz <olivier.m...@6wind.com>
>
>
> Regards,
> Olivier
