Re: [PATCH v8 3/8] vhost: vring queue setup for multiple queue support

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Oct 27, 2015 at 03:20:40PM +0900, Tetsuya Mukawa wrote:
> On 2015/10/26 14:42, Yuanhan Liu wrote:
> > On Mon, Oct 26, 2015 at 02:24:08PM +0900, Tetsuya Mukawa wrote:
> >> On 2015/10/22 21:35, Yuanhan Liu wrote:
> > ...
> >>> @@ -292,13 +300,13 @@ user_get_vring_base(struct vhost_device_ctx ctx,
> >>>  	 * sent and only sent in vhost_vring_stop.
> >>>  	 * TODO: cleanup the vring, it isn't usable since here.
> >>>  	 */
> >>> -	if ((dev->virtqueue[VIRTIO_RXQ]->kickfd) >= 0) {
> >>> -		close(dev->virtqueue[VIRTIO_RXQ]->kickfd);
> >>> -		dev->virtqueue[VIRTIO_RXQ]->kickfd = -1;
> >>> +	if ((dev->virtqueue[state->index]->kickfd + VIRTIO_RXQ) >= 0) {
> >>> +		close(dev->virtqueue[state->index + VIRTIO_RXQ]->kickfd);
> >>> +		dev->virtqueue[state->index + VIRTIO_RXQ]->kickfd = -1;
> >>>  	}
> >> Hi Yuanhan,
> >>
> >> Please let me make sure whether below is correct.
> >>     if ((dev->virtqueue[state->index]->kickfd + VIRTIO_RXQ) >= 0) {
> >>
> >>> -	if ((dev->virtqueue[VIRTIO_TXQ]->kickfd) >= 0) {
> >>> -		close(dev->virtqueue[VIRTIO_TXQ]->kickfd);
> >>> -		dev->virtqueue[VIRTIO_TXQ]->kickfd = -1;
> >>> +	if ((dev->virtqueue[state->index]->kickfd + VIRTIO_TXQ) >= 0) {
> >>> +		close(dev->virtqueue[state->index + VIRTIO_TXQ]->kickfd);
> >>> +		dev->virtqueue[state->index + VIRTIO_TXQ]->kickfd = -1;
> >> Also, same question here.
> > Oops, silly typos... Thanks for catching it out!
> >
> > Here is an update patch (Thomas, please let me know if you prefer me
> > to send the whole patchset for you to apply):
> 
> Hi Yuanhan,
> 
> I may miss one more issue here.
> Could you please see below patch I've submitted today?
> (I may find a similar issue, so I've fixed it also in below patch.)
>  
> - http://dpdk.org/dev/patchwork/patch/8038/
>  
> Thanks,
> Tetsuya

Looking at that, at least when MQ is enabled, please don't key
stopping queues off GET_VRING_BASE.

There are ENABLE/DISABLE messages for that.

Generally guys, don't take whatever QEMU happens to do for
granted! Look at the protocol spec under doc/specs directory,
if you are making more assumptions you must document them!

-- 
MST