From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <yuanhan.liu@linux.intel.com>
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
by dpdk.org (Postfix) with ESMTP id EB45D8D9A
for <dev@dpdk.org>; Tue, 27 Oct 2015 10:34:04 +0100 (CET)
Received: from fmsmga002.fm.intel.com ([10.253.24.26])
by orsmga101.jf.intel.com with ESMTP; 27 Oct 2015 02:33:51 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.20,204,1444719600"; d="scan'208";a="836405702"
Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.66.49])
by fmsmga002.fm.intel.com with ESMTP; 27 Oct 2015 02:29:21 -0700
Date: Tue, 27 Oct 2015 17:30:41 +0800
From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Message-ID: <20151027093041.GI3115@yliu-dev.sh.intel.com>
References: <1445399294-18826-1-git-send-email-yuanhan.liu@linux.intel.com>
<1445517356-19780-1-git-send-email-yuanhan.liu@linux.intel.com>
<1445517356-19780-4-git-send-email-yuanhan.liu@linux.intel.com>
<562DB8F8.4050707@igel.co.jp>
<20151026054215.GY3115@yliu-dev.sh.intel.com>
<562F17B8.5020107@igel.co.jp>
<20151027111444-mutt-send-email-mst@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20151027111444-mutt-send-email-mst@redhat.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: dev@dpdk.org, marcel@redhat.com
Subject: Re: [dpdk-dev] [PATCH v8 3/8] vhost: vring queue setup for multiple
queue support
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 27 Oct 2015 09:34:05 -0000
On Tue, Oct 27, 2015 at 11:17:16AM +0200, Michael S. Tsirkin wrote:
> 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.
Yes, that's only a workaround. I guess it has been there for quite a
while, maybe at the time qemu doesn't send RESET_OWNER message.
> There are ENABLE/DISABLE messages for that.
That's something new, though I have plan to use them instead, we still
need to make sure our code work with old qemu, without ENABLE/DISABLE
messages.
And I will think more while enabling live migration: I should have
more time to address issues like this at that time.
> 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!
Indeed. And we will try to address them bit by bit in future.
--yliu