From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50276) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1L3u-0005O1-Ur for qemu-devel@nongnu.org; Tue, 24 Nov 2015 16:23:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a1L3q-0001fo-Vi for qemu-devel@nongnu.org; Tue, 24 Nov 2015 16:23:43 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48791) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1L3q-0001f1-Qm for qemu-devel@nongnu.org; Tue, 24 Nov 2015 16:23:38 -0500 Date: Tue, 24 Nov 2015 23:23:34 +0200 From: "Michael S. Tsirkin" Message-ID: <20151124232037-mutt-send-email-mst@redhat.com> References: <1448381436-32106-1-git-send-email-thibaut.collet@6wind.com> <1448381436-32106-2-git-send-email-thibaut.collet@6wind.com> <20151124224712-mutt-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH for-2.5 1/1] vhost-user: do not send SET_VRING_ENABLE at start List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thibaut Collet Cc: yuanhan.liu@linux.intel.com, Jason Wang , Victor Kaplansky , =?iso-8859-1?Q?Marc-Andr=E9?= Lureau , qemu-devel On Tue, Nov 24, 2015 at 10:05:27PM +0100, Thibaut Collet wrote: > On Tue, Nov 24, 2015 at 9:52 PM, Michael S. Tsirkin wrote: > > On Tue, Nov 24, 2015 at 05:10:36PM +0100, Thibaut Collet wrote: > >> This patch reverts partially commit 3a12f32229a. > >> > >> In case of live migration several queues can be enabled and not only the first > >> one. So inform backend that only the first queue is enabled is wrong. > >> > >> Since commit 7263a0ad7899 backend is already notified of the state of the vring > >> through the vring attach operation. This function, called during the startup > >> sequence, provides the correct state of the vring, even in case of live > >> migration. > >> > >> So nothing has to be added to give the vring state to the backend at the startup. > >> > >> Signed-off-by: Thibaut Collet > >> --- > >> hw/virtio/vhost.c | 5 ----- > >> 1 file changed, 5 deletions(-) > >> > >> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > >> index 1794f0d..870cd12 100644 > >> --- a/hw/virtio/vhost.c > >> +++ b/hw/virtio/vhost.c > >> @@ -1226,11 +1226,6 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev) > >> } > >> } > >> > >> - if (hdev->vhost_ops->vhost_set_vring_enable) { > >> - /* only enable first vq pair by default */ > >> - hdev->vhost_ops->vhost_set_vring_enable(hdev, hdev->vq_index == 0); > >> - } > >> - > >> return 0; > >> fail_log: > >> vhost_log_put(hdev, false); > >> -- > >> 2.1.4 > > > > Yes - and I'm beginning to think that maybe we should revert > > all of 3a12f32229a then, for symmetry. > > > > Keep the disable vring on the stop can be useful. For example if the > VM is rebooted all the vring will be disabled and backend will avoid > to send packet to the VM in this case (I am not sure the virtio ring > address is always valid during a reboot and writingg data in this > memory can cause unexpected behaviour in this case). I think there's still some confusion: writing memory can still happen even if you disable the ring since the TX ring is still processed so we write into the used ring. We call GET_VRING_BASE on stop and that ensures rings are stopped. > > Yunnan, Victor - what do you think? > > > > -- > > MST