From patchwork Wed Dec 1 08:03:43 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Yoshiaki Tamura X-Patchwork-Id: 73826 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id A1591B70AF for ; Thu, 2 Dec 2010 01:52:52 +1100 (EST) Received: from localhost ([127.0.0.1]:43586 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PNo37-0001hL-EF for incoming@patchwork.ozlabs.org; Wed, 01 Dec 2010 09:52:49 -0500 Received: from [140.186.70.92] (port=39022 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PNhfH-0003kF-V8 for qemu-devel@nongnu.org; Wed, 01 Dec 2010 03:03:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PNhfF-0002XQ-78 for qemu-devel@nongnu.org; Wed, 01 Dec 2010 03:03:47 -0500 Received: from mail-ww0-f67.google.com ([74.125.82.67]:59505) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PNhfE-0002Wv-PY for qemu-devel@nongnu.org; Wed, 01 Dec 2010 03:03:45 -0500 Received: by wwb28 with SMTP id 28so182822wwb.10 for ; Wed, 01 Dec 2010 00:03:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:sender:received :in-reply-to:references:date:x-google-sender-auth:message-id:subject :from:to:cc:content-type:content-transfer-encoding; bh=dqVJCZ5IscYxCz29Yt7dq/JY8eRI1LDdJfvXILaCSnk=; b=fPaZ1a1myFcCCuiOv6DH2HfumgSM/HWxJq8sYT72x5Io/DM/9EWSY0yaUBETUqCdpn I0JZt2n91AqojOSGpFoIKLDyWFFKZ3psoRYNLV7JVYhvXNbMnU21eOo/EP0gHyulw1IJ GQs4/ovt7/QmN+NlgfgM5fonQuCuSCLHhZ+bI= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=bP4KRqjCSeTBZfFJGJ6wft6VenInp8Dx/TDqqOY3nXlMpnQ8hh8gFWxHwwzt24Zytz fY7d3GqtO+BceJ7WYAOVnEUghvEKOKC+CBjRPAjtos8NPOpf1Q+iPzu1jDyNz3KGjKNK axlXokqKE1qSJgYXMNtpMT2fu32ItjxHvNYX4= MIME-Version: 1.0 Received: by 10.216.186.141 with SMTP id w13mr1524724wem.91.1291190623118; Wed, 01 Dec 2010 00:03:43 -0800 (PST) Received: by 10.216.10.3 with HTTP; Wed, 1 Dec 2010 00:03:43 -0800 (PST) In-Reply-To: <20101128114627.GC4499@redhat.com> References: <1290665220-26478-1-git-send-email-tamura.yoshiaki@lab.ntt.co.jp> <1290665220-26478-6-git-send-email-tamura.yoshiaki@lab.ntt.co.jp> <20101128092857.GA3342@redhat.com> <20101128114627.GC4499@redhat.com> Date: Wed, 1 Dec 2010 17:03:43 +0900 X-Google-Sender-Auth: NcLmTfVVxMQbqODSdZx2DxAXV6E Message-ID: From: Yoshiaki Tamura To: "Michael S. Tsirkin" , Marcelo Tosatti X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) Cc: aliguori@us.ibm.com, ananth@in.ibm.com, kvm@vger.kernel.org, ohmura.kei@lab.ntt.co.jp, dlaor@redhat.com, qemu-devel@nongnu.org, vatsa@linux.vnet.ibm.com, avi@redhat.com, psuriset@linux.vnet.ibm.com, stefanha@linux.vnet.ibm.com Subject: [Qemu-devel] Re: [PATCH 05/21] virtio: modify save/load handler to handle inuse varialble. X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 2010/11/28 Michael S. Tsirkin : > On Sun, Nov 28, 2010 at 08:27:58PM +0900, Yoshiaki Tamura wrote: >> 2010/11/28 Michael S. Tsirkin : >> > On Thu, Nov 25, 2010 at 03:06:44PM +0900, Yoshiaki Tamura wrote: >> >> Modify inuse type to uint16_t, let save/load to handle, and revert >> >> last_avail_idx with inuse if there are outstanding emulation. >> >> >> >> Signed-off-by: Yoshiaki Tamura >> > >> > This changes migration format, so it will break compatibility with >> > existing drivers. More generally, I think migrating internal >> > state that is not guest visible is always a mistake >> > as it ties migration format to an internal implementation >> > (yes, I know we do this sometimes, but we should at least >> > try not to add such cases).  I think the right thing to do in this case >> > is to flush outstanding >> > work when vm is stopped.  Then, we are guaranteed that inuse is 0. >> > I sent patches that do this for virtio net and block. >> >> Could you give me the link of your patches?  I'd like to test >> whether they work with Kemari upon failover.  If they do, I'm >> happy to drop this patch. >> >> Yoshi > > Look for this: > stable migration image on a stopped vm > sent on: > Wed, 24 Nov 2010 17:52:49 +0200 Thanks for the info. However, The patch series above didn't solve the issue. In case of Kemari, inuse is mostly > 0 because it queues the output, and while last_avail_idx gets incremented immediately, not sending inuse makes the state inconsistent between Primary and Secondary. I'm wondering why last_avail_idx is OK to send but not inuse. The following patch does the same thing as original, yet keeps the format of the virtio. It shouldn't break live migration either because inuse should be 0. Yoshi > >> > >> >> --- >> >>  hw/virtio.c |    8 +++++++- >> >>  1 files changed, 7 insertions(+), 1 deletions(-) >> >> >> >> diff --git a/hw/virtio.c b/hw/virtio.c >> >> index 849a60f..5509644 100644 >> >> --- a/hw/virtio.c >> >> +++ b/hw/virtio.c >> >> @@ -72,7 +72,7 @@ struct VirtQueue >> >>      VRing vring; >> >>      target_phys_addr_t pa; >> >>      uint16_t last_avail_idx; >> >> -    int inuse; >> >> +    uint16_t inuse; >> >>      uint16_t vector; >> >>      void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq); >> >>      VirtIODevice *vdev; >> >> @@ -671,6 +671,7 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) >> >>          qemu_put_be32(f, vdev->vq[i].vring.num); >> >>          qemu_put_be64(f, vdev->vq[i].pa); >> >>          qemu_put_be16s(f, &vdev->vq[i].last_avail_idx); >> >> +        qemu_put_be16s(f, &vdev->vq[i].inuse); >> >>          if (vdev->binding->save_queue) >> >>              vdev->binding->save_queue(vdev->binding_opaque, i, f); >> >>      } >> >> @@ -711,6 +712,11 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f) >> >>          vdev->vq[i].vring.num = qemu_get_be32(f); >> >>          vdev->vq[i].pa = qemu_get_be64(f); >> >>          qemu_get_be16s(f, &vdev->vq[i].last_avail_idx); >> >> +        qemu_get_be16s(f, &vdev->vq[i].inuse); >> >> + >> >> +        /* revert last_avail_idx if there are outstanding emulation. */ >> >> +        vdev->vq[i].last_avail_idx -= vdev->vq[i].inuse; >> >> +        vdev->vq[i].inuse = 0; >> >> >> >>          if (vdev->vq[i].pa) { >> >>              virtqueue_init(&vdev->vq[i]); >> >> -- >> >> 1.7.1.2 >> >> >> >> -- >> >> To unsubscribe from this list: send the line "unsubscribe kvm" in >> >> the body of a message to majordomo@vger.kernel.org >> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html >> > -- >> > To unsubscribe from this list: send the line "unsubscribe kvm" in >> > the body of a message to majordomo@vger.kernel.org >> > More majordomo info at  http://vger.kernel.org/majordomo-info.html >> > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at  http://vger.kernel.org/majordomo-info.html > diff --git a/hw/virtio.c b/hw/virtio.c index c8a0fc6..875c7ca 100644 --- a/hw/virtio.c +++ b/hw/virtio.c @@ -664,12 +664,16 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) qemu_put_be32(f, i); for (i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) { + uint16_t last_avail_idx; + if (vdev->vq[i].vring.num == 0) break; + last_avail_idx = vdev->vq[i].last_avail_idx - vdev->vq[i].inuse; + qemu_put_be32(f, vdev->vq[i].vring.num); qemu_put_be64(f, vdev->vq[i].pa); - qemu_put_be16s(f, &vdev->vq[i].last_avail_idx); + qemu_put_be16s(f, &last_avail_idx); if (vdev->binding->save_queue) vdev->binding->save_queue(vdev->binding_opaque, i, f); }