On Thu, Aug 13, 2015 at 11:18 AM, Michael S. Tsirkin <m...@redhat.com> 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 <n.nikol...@virtualopensystems.com> >> Signed-off-by: Changchun Ouyang <changchun.ouy...@intel.com> >> --- >> 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(-) >> >> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt >> index 70da3b1..9390f89 100644 >> --- a/docs/specs/vhost-user.txt >> +++ b/docs/specs/vhost-user.txt >> @@ -135,6 +135,11 @@ As older slaves don't support negotiating protocol >> features, >> a feature bit was dedicated for this purpose: >> #define VHOST_USER_F_PROTOCOL_FEATURES 30 >> >> +Multi queue support >> +------------------- >> +The protocol supports multiple queues by setting all index fields in the >> sent >> +messages to a properly calculated value. >> + >> Message types >> ------------- >> >> @@ -198,7 +203,7 @@ Message types >> >> Id: 4 >> Equivalent ioctl: VHOST_RESET_OWNER >> - Master payload: N/A >> + Master payload: vring state description >> >> Issued when a new connection is about to be closed. The Master will no >> longer own this connection (and will usually close it). > > This is an interface change, isn't it? > We can't make it unconditionally, need to make it dependent > on a protocol flag.
Agree. It can potential break vhost-user driver implementation checking the size of the message. We should not change the vhost-user protocol without a new protocol flag. I think the first issue here that VHOST_RESET_OWNER should happen on vhost_dev_cleanup and not in vhost_net_stop_one. VHOST_RESET_OWNER should be the counter part of VHOST_SET_OWNER. So it don't need to have a payload like VHOST_SET_OWNER. Thus I agree with this email (http://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg05971.html) Maybe should we use an other message to tell to the backend that the vring is not anymore available in vhost_net_stop_one ? Maxime