From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:48968) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QOSt4-0004gq-6Z for qemu-devel@nongnu.org; Mon, 23 May 2011 07:01:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QOSt0-0002ZK-AM for qemu-devel@nongnu.org; Mon, 23 May 2011 07:01:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:8812) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QOSt0-0002Z4-3E for qemu-devel@nongnu.org; Mon, 23 May 2011 07:01:22 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p4NB1KsI016592 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 23 May 2011 07:01:20 -0400 Date: Mon, 23 May 2011 14:01:31 +0300 From: "Michael S. Tsirkin" Message-ID: <20110523110003.GA25012@redhat.com> References: <20110518055737.10458.65866.stgit@dhcp-91-7.nay.redhat.com.englab.nay.redhat.com> <20110518085440.GI7589@redhat.com> <4DD9F765.1030104@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4DD9F765.1030104@redhat.com> Subject: Re: [Qemu-devel] [PATCH] virtio: correctly initialize vm_running List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang Cc: qemu-devel@nongnu.org On Mon, May 23, 2011 at 01:57:57PM +0800, Jason Wang wrote: > On 05/18/2011 04:54 PM, Michael S. Tsirkin wrote: > >On Wed, May 18, 2011 at 01:57:37PM +0800, Jason Wang wrote: > >>Current vm_running was not explicitly initialized and its value was changed by > >>vm state notifier, this may confuse the virtio device being hotplugged such as > >>virtio-net with vhost backend as it may think the vm was not running. Solve this > >>by initialize this value explicitly in virtio_common_init(). > >> > >>Signed-off-by: Jason Wang > >I think a slightly cleaner way would be to invoke > >virtio_vmstate_change after it is registered, > >have that update the vm_running field. > > > > Do you mean call it in virtio_common_init() directly? But the > bindings are not initialized then. Or is there anything I missed? My idea was that when we register a callback, it can immediately be invoked with the current vm state. But this has other implications so would have to be tested more. So I think your patch is a good start. Acked-by: Michael S. Tsirkin > >>--- > >> 0 files changed, 0 insertions(+), 0 deletions(-) > >> > >>diff --git a/hw/virtio.c b/hw/virtio.c > >>index 6e8814c..27d7e50 100644 > >>--- a/hw/virtio.c > >>+++ b/hw/virtio.c > >>@@ -789,6 +789,7 @@ VirtIODevice *virtio_common_init(const char *name, uint16_t device_id, > >> vdev->queue_sel = 0; > >> vdev->config_vector = VIRTIO_NO_VECTOR; > >> vdev->vq = qemu_mallocz(sizeof(VirtQueue) * VIRTIO_PCI_QUEUE_MAX); > >>+ vdev->vm_running = vm_running; > >> for(i = 0; i< VIRTIO_PCI_QUEUE_MAX; i++) { > >> vdev->vq[i].vector = VIRTIO_NO_VECTOR; > >> vdev->vq[i].vdev = vdev;