Re: [Qemu-devel] [PATCH v2 5/5] vhost: send VHOST_USER_SET_VRING_ENABLE at start/stop

[Yuanhan Liu]

On Wed, Oct 21, 2015 at 01:39:11PM +0300, Michael S. Tsirkin wrote:
> On Wed, Oct 21, 2015 at 05:07:18PM +0800, Yuanhan Liu wrote:
> > Send VHOST_USER_SET_VRING_ENABLE at start/stop when multiple queue
> > is negotiated, to inform the backend that we are ready or not.
> 
> OK but that's only if MQ is set.

Maybe we could just call vhost_backend_set_vring_enable() unconditionally?
It's nil operation when MQ is not set.

> If now, we need to do
> RESET_OWNER followed by SET_OWNER.

Could you be more specific? Why sending RESET_OWNER followed by
SET_OWNER?

TBH, I'm a bit confused with RESET_OWNER now: what it does, and when is
supposed to send it :(

And, sending RESET_OWNER inside virtio_net_reset() also looks weird.
I made a quick try before sending this patchset, and the vhost-user
request dump doesn't look right to me: the message is sent after
vhost dev init (GET_FEATURES, GET_PROTOCOL_FEATURE, SET_OWNER, ...,
SET_VRING_CALL),  and before peer attach (SET_VRING_ENABLE) and
vhost_dev_start (SET_MEM_TABLE, ... SET_VRING_KICK ...):


# start of a VM

VHOST_CONFIG: new virtio connection is 28
VHOST_CONFIG: new device, handle is 0
VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
VHOST_CONFIG: read message VHOST_USER_GET_PROTOCOL_FEATURES
VHOST_CONFIG: read message VHOST_USER_SET_PROTOCOL_FEATURES
VHOST_CONFIG: read message VHOST_USER_GET_QUEUE_NUM
VHOST_CONFIG: read message VHOST_USER_SET_OWNER
VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
VHOST_CONFIG: vring call idx:0 file:29
VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
VHOST_CONFIG: vring call idx:1 file:30
VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
VHOST_CONFIG: read message VHOST_USER_GET_PROTOCOL_FEATURES
VHOST_CONFIG: read message VHOST_USER_SET_PROTOCOL_FEATURES
VHOST_CONFIG: read message VHOST_USER_GET_QUEUE_NUM
VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
...
...
VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
VHOST_CONFIG: vring call idx:6 file:35
VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
VHOST_CONFIG: vring call idx:7 file:36

==> VHOST_CONFIG: read message VHOST_USER_RESET_OWNER
VHOST_CONFIG: read message VHOST_USER_RESET_OWNER

VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
VHOST_CONFIG: set queue enable: 1 to qp idx: 0
VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
VHOST_CONFIG: set queue enable: 0 to qp idx: 2
VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
VHOST_CONFIG: set queue enable: 0 to qp idx: 4
VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
VHOST_CONFIG: set queue enable: 0 to qp idx: 6
VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
VHOST_CONFIG: vring call idx:0 file:29
VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
VHOST_CONFIG: vring call idx:1 file:30
VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
VHOST_CONFIG: vring call idx:2 file:31
VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
VHOST_CONFIG: vring call idx:3 file:32
VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
VHOST_CONFIG: vring call idx:4 file:33
VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
VHOST_CONFIG: vring call idx:5 file:34
VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
VHOST_CONFIG: vring call idx:6 file:35
VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
VHOST_CONFIG: vring call idx:7 file:36
VHOST_CONFIG: read message VHOST_USER_SET_FEATURES
VHOST_CONFIG: read message VHOST_USER_SET_MEM_TABLE
VHOST_CONFIG: mapped region 0 fd:37 to 0x2aaac000 sz:0xa off:0x0
VHOST_CONFIG: mapped region 1 fd:38 to 0x2aab sz:0x8000 
off:0xc
VHOST_CONFIG: read message VHOST_USER_SET_VRING_NUM
VHOST_CONFIG: read message VHOST_USER_SET_VRING_BASE
VHOST_CONFIG: read message VHOST_USER_SET_VRING_ADDR
VHOST_CONFIG: read message VHOST_USER_SET_VRING_KICK
VHOST_CONFIG: vring kick idx:0 file:39
VHOST_CONFIG: virtio is not ready for processing.
VHOST_CONFIG: read message VHOST_USER_SET_VRING_NUM
VHOST_CONFIG: read message VHOST_USER_SET_VRING_BASE
VHOST_CONFIG: read message VHOST_USER_SET_VRING_ADDR
VHOST_CONFIG: read message VHOST_USER_SET_VRING_KICK
VHOST_CONFIG: vring kick idx:1 file:40
VHOST_CONFIG: virtio is not ready for processing.
VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
VHOST_CONFIG: set queue enable: 1 to qp idx: 0
VHOST_CONFIG: read message VHOST_USER_SET_FEATURES
VHOST_CONFIG: read message VHOST_USER_SET_VRING_NUM
VHOST_CONFIG: read message VHOST_USER_SET_VRING_BASE
VHOST_CONFIG: read message VHOST_USER_SET_VRING_ADDR
VHOST_CONFIG: read message VHOST_USER_SET_VRING_KICK
VHOST_CONFIG: vring kick idx:2 file:41
VHOST_CONFIG: 

Re: [Qemu-devel] [PATCH v2 5/5] vhost: send VHOST_USER_SET_VRING_ENABLE at start/stop

[Michael S. Tsirkin]

On Wed, Oct 21, 2015 at 10:55:39PM +0800, Yuanhan Liu wrote:
> On Wed, Oct 21, 2015 at 05:11:44PM +0300, Michael S. Tsirkin wrote:
> > On Wed, Oct 21, 2015 at 09:43:16PM +0800, Yuanhan Liu wrote:
> > > On Wed, Oct 21, 2015 at 01:39:11PM +0300, Michael S. Tsirkin wrote:
> > > > On Wed, Oct 21, 2015 at 05:07:18PM +0800, Yuanhan Liu wrote:
> > > > > Send VHOST_USER_SET_VRING_ENABLE at start/stop when multiple queue
> > > > > is negotiated, to inform the backend that we are ready or not.
> > > > 
> > > > OK but that's only if MQ is set.
> > > 
> > > Maybe we could just call vhost_backend_set_vring_enable() unconditionally?
> > > It's nil operation when MQ is not set.
> > > 
> > > > If now, we need to do
> > > > RESET_OWNER followed by SET_OWNER.
> > > 
> > > Could you be more specific? Why sending RESET_OWNER followed by
> > > SET_OWNER?
> > > 
> > > TBH, I'm a bit confused with RESET_OWNER now: what it does, and when is
> > > supposed to send it :(
> > 
> > It's not well specified, but it does say it's analogous to RESET_OWNER
> > in kernel. That one is well documented:
> > 
> > /* Set current process as the (exclusive) owner of this file descriptor.
> >  * This must be called before any other vhost command.  Further calls to
> >  * VHOST_OWNER_SET fail until VHOST_OWNER_RESET is called. */
> > #define VHOST_SET_OWNER _IO(VHOST_VIRTIO, 0x01)
> > /* Give up ownership, and reset the device to default values.
> >  * Allows subsequent call to VHOST_OWNER_SET to succeed. */
> > #define VHOST_RESET_OWNER _IO(VHOST_VIRTIO, 0x02)
> 
> Thanks, that helps (I think).
> 
> I recalled my old question, and rechecked your answer again:
> 
> Because we need to get the state from remote after stop.
> RESET_OWNER discards that, so we can't resume the VM.
> 
> So, if I understand it correctly this time, you want to keep the
> VM state at the backend side even after the VM is shut down, and
> then we can resume it with those saved state

Not shut down. That makes no sense. When VM is stopped.

> And why don't do that when MQ is enabled? I don't see it has anyting
> to do with MQ.

With MQ we have enable/disable vq so we can just stop them
cleanly.

> > 
> > So if we want just the reset part, we need to do VHOST_RESET_OWNER
> > then redo everything that we did previously: VHOST_SET_OWNER
> > SET_VRING_CALL etc etc.
> > 
> > > And, sending RESET_OWNER inside virtio_net_reset() also looks weird.
> > > I made a quick try before sending this patchset, and the vhost-user
> > > request dump doesn't look right to me: the message is sent after
> > > vhost dev init (GET_FEATURES, GET_PROTOCOL_FEATURE, SET_OWNER, ...,
> > > SET_VRING_CALL),  and before peer attach (SET_VRING_ENABLE) and
> > > vhost_dev_start (SET_MEM_TABLE, ... SET_VRING_KICK ...):
> > 
> > Food for thought.
> 
> Aha...
> 
> > 
> > > 
> > > # start of a VM
> > > 
> > > VHOST_CONFIG: new virtio connection is 28
> > > VHOST_CONFIG: new device, handle is 0
> > > VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
> > > VHOST_CONFIG: read message VHOST_USER_GET_PROTOCOL_FEATURES
> > > VHOST_CONFIG: read message VHOST_USER_SET_PROTOCOL_FEATURES
> > > VHOST_CONFIG: read message VHOST_USER_GET_QUEUE_NUM
> > > VHOST_CONFIG: read message VHOST_USER_SET_OWNER
> > > VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
> > > VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> > > VHOST_CONFIG: vring call idx:0 file:29
> > > VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> > > VHOST_CONFIG: vring call idx:1 file:30
> > > VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
> > > VHOST_CONFIG: read message VHOST_USER_GET_PROTOCOL_FEATURES
> > > VHOST_CONFIG: read message VHOST_USER_SET_PROTOCOL_FEATURES
> > > VHOST_CONFIG: read message VHOST_USER_GET_QUEUE_NUM
> > > VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
> > > VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> > > ...
> > > ...
> > > VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> > > VHOST_CONFIG: vring call idx:6 file:35
> > > VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> > > VHOST_CONFIG: vring call idx:7 file:36
> > > 
> > > ==> VHOST_CONFIG: read message VHOST_USER_RESET_OWNER
> > > VHOST_CONFIG: read message VHOST_USER_RESET_OWNER
> > > 
> > > VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
> > > VHOST_CONFIG: set queue enable: 1 to qp idx: 0
> > > VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
> > > VHOST_CONFIG: set queue enable: 0 to qp idx: 2
> > > VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
> > > VHOST_CONFIG: set queue enable: 0 to qp idx: 4
> > > VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
> > > VHOST_CONFIG: set queue enable: 0 to qp idx: 6
> > > VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> > > VHOST_CONFIG: vring call idx:0 file:29
> > > VHOST_CONFIG: read message 

Re: [Qemu-devel] [PATCH v2 5/5] vhost: send VHOST_USER_SET_VRING_ENABLE at start/stop

[Michael S. Tsirkin]

On Wed, Oct 21, 2015 at 09:43:16PM +0800, Yuanhan Liu wrote:
> On Wed, Oct 21, 2015 at 01:39:11PM +0300, Michael S. Tsirkin wrote:
> > On Wed, Oct 21, 2015 at 05:07:18PM +0800, Yuanhan Liu wrote:
> > > Send VHOST_USER_SET_VRING_ENABLE at start/stop when multiple queue
> > > is negotiated, to inform the backend that we are ready or not.
> > 
> > OK but that's only if MQ is set.
> 
> Maybe we could just call vhost_backend_set_vring_enable() unconditionally?
> It's nil operation when MQ is not set.
> 
> > If now, we need to do
> > RESET_OWNER followed by SET_OWNER.
> 
> Could you be more specific? Why sending RESET_OWNER followed by
> SET_OWNER?
> 
> TBH, I'm a bit confused with RESET_OWNER now: what it does, and when is
> supposed to send it :(

It's not well specified, but it does say it's analogous to RESET_OWNER
in kernel. That one is well documented:

/* Set current process as the (exclusive) owner of this file descriptor.
 * This must be called before any other vhost command.  Further calls to
 * VHOST_OWNER_SET fail until VHOST_OWNER_RESET is called. */
#define VHOST_SET_OWNER _IO(VHOST_VIRTIO, 0x01)
/* Give up ownership, and reset the device to default values.
 * Allows subsequent call to VHOST_OWNER_SET to succeed. */
#define VHOST_RESET_OWNER _IO(VHOST_VIRTIO, 0x02)


So if we want just the reset part, we need to do VHOST_RESET_OWNER
then redo everything that we did previously: VHOST_SET_OWNER
SET_VRING_CALL etc etc.

> And, sending RESET_OWNER inside virtio_net_reset() also looks weird.
> I made a quick try before sending this patchset, and the vhost-user
> request dump doesn't look right to me: the message is sent after
> vhost dev init (GET_FEATURES, GET_PROTOCOL_FEATURE, SET_OWNER, ...,
> SET_VRING_CALL),  and before peer attach (SET_VRING_ENABLE) and
> vhost_dev_start (SET_MEM_TABLE, ... SET_VRING_KICK ...):

Food for thought.


> 
> # start of a VM
> 
> VHOST_CONFIG: new virtio connection is 28
> VHOST_CONFIG: new device, handle is 0
> VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
> VHOST_CONFIG: read message VHOST_USER_GET_PROTOCOL_FEATURES
> VHOST_CONFIG: read message VHOST_USER_SET_PROTOCOL_FEATURES
> VHOST_CONFIG: read message VHOST_USER_GET_QUEUE_NUM
> VHOST_CONFIG: read message VHOST_USER_SET_OWNER
> VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> VHOST_CONFIG: vring call idx:0 file:29
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> VHOST_CONFIG: vring call idx:1 file:30
> VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
> VHOST_CONFIG: read message VHOST_USER_GET_PROTOCOL_FEATURES
> VHOST_CONFIG: read message VHOST_USER_SET_PROTOCOL_FEATURES
> VHOST_CONFIG: read message VHOST_USER_GET_QUEUE_NUM
> VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> ...
> ...
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> VHOST_CONFIG: vring call idx:6 file:35
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> VHOST_CONFIG: vring call idx:7 file:36
> 
> ==> VHOST_CONFIG: read message VHOST_USER_RESET_OWNER
> VHOST_CONFIG: read message VHOST_USER_RESET_OWNER
> 
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
> VHOST_CONFIG: set queue enable: 1 to qp idx: 0
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
> VHOST_CONFIG: set queue enable: 0 to qp idx: 2
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
> VHOST_CONFIG: set queue enable: 0 to qp idx: 4
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
> VHOST_CONFIG: set queue enable: 0 to qp idx: 6
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> VHOST_CONFIG: vring call idx:0 file:29
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> VHOST_CONFIG: vring call idx:1 file:30
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> VHOST_CONFIG: vring call idx:2 file:31
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> VHOST_CONFIG: vring call idx:3 file:32
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> VHOST_CONFIG: vring call idx:4 file:33
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> VHOST_CONFIG: vring call idx:5 file:34
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> VHOST_CONFIG: vring call idx:6 file:35
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> VHOST_CONFIG: vring call idx:7 file:36
> VHOST_CONFIG: read message VHOST_USER_SET_FEATURES
> VHOST_CONFIG: read message VHOST_USER_SET_MEM_TABLE
> VHOST_CONFIG: mapped region 0 fd:37 to 0x2aaac000 sz:0xa off:0x0
> VHOST_CONFIG: mapped region 1 fd:38 to 0x2aab sz:0x8000 
> off:0xc
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_NUM
> VHOST_CONFIG: read message VHOST_USER_SET_VRING_BASE
> VHOST_CONFIG: read message 

Re: [Qemu-devel] [PATCH v2 5/5] vhost: send VHOST_USER_SET_VRING_ENABLE at start/stop

[Yuanhan Liu]

On Wed, Oct 21, 2015 at 05:11:44PM +0300, Michael S. Tsirkin wrote:
> On Wed, Oct 21, 2015 at 09:43:16PM +0800, Yuanhan Liu wrote:
> > On Wed, Oct 21, 2015 at 01:39:11PM +0300, Michael S. Tsirkin wrote:
> > > On Wed, Oct 21, 2015 at 05:07:18PM +0800, Yuanhan Liu wrote:
> > > > Send VHOST_USER_SET_VRING_ENABLE at start/stop when multiple queue
> > > > is negotiated, to inform the backend that we are ready or not.
> > > 
> > > OK but that's only if MQ is set.
> > 
> > Maybe we could just call vhost_backend_set_vring_enable() unconditionally?
> > It's nil operation when MQ is not set.
> > 
> > > If now, we need to do
> > > RESET_OWNER followed by SET_OWNER.
> > 
> > Could you be more specific? Why sending RESET_OWNER followed by
> > SET_OWNER?
> > 
> > TBH, I'm a bit confused with RESET_OWNER now: what it does, and when is
> > supposed to send it :(
> 
> It's not well specified, but it does say it's analogous to RESET_OWNER
> in kernel. That one is well documented:
> 
> /* Set current process as the (exclusive) owner of this file descriptor.
>  * This must be called before any other vhost command.  Further calls to
>  * VHOST_OWNER_SET fail until VHOST_OWNER_RESET is called. */
> #define VHOST_SET_OWNER _IO(VHOST_VIRTIO, 0x01)
> /* Give up ownership, and reset the device to default values.
>  * Allows subsequent call to VHOST_OWNER_SET to succeed. */
> #define VHOST_RESET_OWNER _IO(VHOST_VIRTIO, 0x02)

Thanks, that helps (I think).

I recalled my old question, and rechecked your answer again:

Because we need to get the state from remote after stop.
RESET_OWNER discards that, so we can't resume the VM.

So, if I understand it correctly this time, you want to keep the
VM state at the backend side even after the VM is shut down, and
then we can resume it with those saved state

And why don't do that when MQ is enabled? I don't see it has anyting
to do with MQ.

> 
> So if we want just the reset part, we need to do VHOST_RESET_OWNER
> then redo everything that we did previously: VHOST_SET_OWNER
> SET_VRING_CALL etc etc.
> 
> > And, sending RESET_OWNER inside virtio_net_reset() also looks weird.
> > I made a quick try before sending this patchset, and the vhost-user
> > request dump doesn't look right to me: the message is sent after
> > vhost dev init (GET_FEATURES, GET_PROTOCOL_FEATURE, SET_OWNER, ...,
> > SET_VRING_CALL),  and before peer attach (SET_VRING_ENABLE) and
> > vhost_dev_start (SET_MEM_TABLE, ... SET_VRING_KICK ...):
> 
> Food for thought.

Aha...

> 
> > 
> > # start of a VM
> > 
> > VHOST_CONFIG: new virtio connection is 28
> > VHOST_CONFIG: new device, handle is 0
> > VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
> > VHOST_CONFIG: read message VHOST_USER_GET_PROTOCOL_FEATURES
> > VHOST_CONFIG: read message VHOST_USER_SET_PROTOCOL_FEATURES
> > VHOST_CONFIG: read message VHOST_USER_GET_QUEUE_NUM
> > VHOST_CONFIG: read message VHOST_USER_SET_OWNER
> > VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
> > VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> > VHOST_CONFIG: vring call idx:0 file:29
> > VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> > VHOST_CONFIG: vring call idx:1 file:30
> > VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
> > VHOST_CONFIG: read message VHOST_USER_GET_PROTOCOL_FEATURES
> > VHOST_CONFIG: read message VHOST_USER_SET_PROTOCOL_FEATURES
> > VHOST_CONFIG: read message VHOST_USER_GET_QUEUE_NUM
> > VHOST_CONFIG: read message VHOST_USER_GET_FEATURES
> > VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> > ...
> > ...
> > VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> > VHOST_CONFIG: vring call idx:6 file:35
> > VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> > VHOST_CONFIG: vring call idx:7 file:36
> > 
> > ==> VHOST_CONFIG: read message VHOST_USER_RESET_OWNER
> > VHOST_CONFIG: read message VHOST_USER_RESET_OWNER
> > 
> > VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
> > VHOST_CONFIG: set queue enable: 1 to qp idx: 0
> > VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
> > VHOST_CONFIG: set queue enable: 0 to qp idx: 2
> > VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
> > VHOST_CONFIG: set queue enable: 0 to qp idx: 4
> > VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
> > VHOST_CONFIG: set queue enable: 0 to qp idx: 6
> > VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> > VHOST_CONFIG: vring call idx:0 file:29
> > VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> > VHOST_CONFIG: vring call idx:1 file:30
> > VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> > VHOST_CONFIG: vring call idx:2 file:31
> > VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> > VHOST_CONFIG: vring call idx:3 file:32
> > VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
> > VHOST_CONFIG: vring call idx:4 file:33
> > 

Re: [Qemu-devel] [PATCH v2 5/5] vhost: send VHOST_USER_SET_VRING_ENABLE at start/stop

[Michael S. Tsirkin]

On Wed, Oct 21, 2015 at 05:07:18PM +0800, Yuanhan Liu wrote:
> Send VHOST_USER_SET_VRING_ENABLE at start/stop when multiple queue
> is negotiated, to inform the backend that we are ready or not.

OK but that's only if MQ is set. If now, we need to do
RESET_OWNER followed by SET_OWNER.

> And exclude VHOST_USER_GET_QUEUE_NUM as one time request, as we need
> to get max_queues for each vhost_dev.

Pls split this part out.

> Suggested-by: Michael S. Tsirkin 
> Signed-off-by: Yuanhan Liu 
> ---
>  hw/virtio/vhost-user.c |  1 -
>  hw/virtio/vhost.c  | 18 ++
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 12a9104..6532a73 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -194,7 +194,6 @@ static bool vhost_user_one_time_request(VhostUserRequest 
> request)
>  case VHOST_USER_SET_OWNER:
>  case VHOST_USER_RESET_OWNER:
>  case VHOST_USER_SET_MEM_TABLE:
> -case VHOST_USER_GET_QUEUE_NUM:
>  return true;
>  default:
>  return false;
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index c0ed5b2..54a4633 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1146,6 +1146,19 @@ int vhost_dev_start(struct vhost_dev *hdev, 
> VirtIODevice *vdev)
>  }
>  }
>  
> +/*
> + * Send VHOST_USER_SET_VRING_ENABLE message when multiple queue
> + * is negotiated to inform the back end that we are ready.
> + *
> + * Only set enable to 1 for first queue pair, as we enable one
> + * queue pair by default.
> + */
> +if (hdev->max_queues > 1 &&

this makes no sense in the generic code.
This is a work around for a protocol bug - belongs in
vhost user internally.
And that's not the way to test this. MQ could be negotiated
even if there is a single pair of queues.

> +hdev->vhost_ops->vhost_backend_set_vring_enable) {
> +hdev->vhost_ops->vhost_backend_set_vring_enable(hdev,
> +hdev->vq_index == 0);
> +}
> +
>  return 0;
>  fail_log:
>  vhost_log_put(hdev, false);
> @@ -1180,5 +1193,10 @@ void vhost_dev_stop(struct vhost_dev *hdev, 
> VirtIODevice *vdev)
>  hdev->started = false;
>  hdev->log = NULL;
>  hdev->log_size = 0;
> +
> +if (hdev->max_queues > 1 &&
> +hdev->vhost_ops->vhost_backend_set_vring_enable) {
> +hdev->vhost_ops->vhost_backend_set_vring_enable(hdev, 0);
> +}
>  }
>  
> -- 
> 1.9.0
>