From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [RFC Patch v2 03/17] don't zero out ioreq page
Date: Fri, 8 Aug 2014 10:08:51 +0100
Message-ID: <53E493A3.6000209@citrix.com>
References: <1407486004-23487-1-git-send-email-wency@cn.fujitsu.com>
	<1407486004-23487-4-git-send-email-wency@cn.fujitsu.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1407486004-23487-4-git-send-email-wency@cn.fujitsu.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Wen Congyang <wency@cn.fujitsu.com>, xen devel <xen-devel@lists.xen.org>
Cc: Ian Campbell <Ian.Campbell@citrix.com>, Ian Jackson <Ian.Jackson@eu.citrix.com>, Jiang Yunhong <yunhong.jiang@intel.com>, Dong Eddie <eddie.dong@intel.com>, Paul Durrant <paul.durrant@citrix.com>, Yang Hongyang <yanghy@cn.fujitsu.com>, Lai Jiangshan <laijs@cn.fujitsu.com>
List-Id: xen-devel@lists.xenproject.org

On 08/08/14 09:19, Wen Congyang wrote:
> ioreq page may contain some pending I/O requests, and we need to
> handle the pending I/O req after migration.
>
> TODO:
> 1. update qemu to handle the pending I/O req

Your very reasoning for introducing this patch proves you have a race
condition between sending this page and sending the qemu state where an
ioreq might get dropped or processed twice.

As I said before, you are now doing things with the live migration code
which it was never intended/designed for.  I have learn't the hard way
that there are many subtle corner cases (most of which lead to VM
corruption), and you need to be able to prove that your new changes are
safe.

~Andrew

>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Cc: Paul Durrant <paul.durrant@citrix.com>
> ---
>  tools/libxc/xc_domain_restore.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c
> index 42abb22..2d6139c 100644
> --- a/tools/libxc/xc_domain_restore.c
> +++ b/tools/libxc/xc_domain_restore.c
> @@ -2301,9 +2301,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
>      }
>  
>      /* These comms pages need to be zeroed at the start of day */
> -    if ( xc_clear_domain_page(xch, dom, tailbuf.u.hvm.magicpfns[0]) ||
> -         xc_clear_domain_page(xch, dom, tailbuf.u.hvm.magicpfns[1]) ||
> -         xc_clear_domain_page(xch, dom, tailbuf.u.hvm.magicpfns[2]) )
> +    if ( xc_clear_domain_page(xch, dom, tailbuf.u.hvm.magicpfns[2]) )
>      {
>          PERROR("error zeroing magic pages");
>          goto out;