Re: [PATCH v7 12/18] tools/libx{l, c}: add back channel to libxc

From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Wen Congyang <wency@cn.fujitsu.com>
Cc: Lars Kurth <lars.kurth@citrix.com>,
	Changlong Xie <xiecl.fnst@cn.fujitsu.com>,
	Wei Liu <wei.liu2@citrix.com>,
	Ian Campbell <ian.campbell@citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Jiang Yunhong <yunhong.jiang@intel.com>,
	Dong Eddie <eddie.dong@intel.com>,
	xen devel <xen-devel@lists.xen.org>,
	Gui Jianfeng <guijianfeng@cn.fujitsu.com>,
	Shriram Rajagopalan <rshriram@cs.ubc.ca>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	Yang Hongyang <hongyang.yang@easystack.cn>
Subject: Re: [PATCH v7 12/18] tools/libx{l, c}: add back channel to libxc
Date: Fri, 29 Jan 2016 11:38:39 -0500	[thread overview]
Message-ID: <20160129163838.GM22787@char.us.oracle.com> (raw)
In-Reply-To: <1454045254-3711-13-git-send-email-wency@cn.fujitsu.com>

On Fri, Jan 29, 2016 at 01:27:28PM +0800, Wen Congyang wrote:
> In COLO mode, both VMs are running, and are considered in sync if the
> visible network traffic is identical.  After some time, they fall out of
> sync.
> 
> At this point, the two VMs have definitely diverged.  Lets call the
> primary dirty bitmap set A, while the secondary dirty bitmap set B.
> 
> Sets A and B are different.
> 
> Under normal migration, the page data for set A will be sent from the
> primary to the secondary.
> 
> However, the set difference B - A (the one in B but not in A, lets
> call this C) is out-of-date on the secondary (with respect to the
> primary) and will not be sent by the primary (to secondary), as it
> was not memory dirtied by the primary. The secondary needs C page data
> to reconstruct an exact copy of the primary at the checkpoint.
> 
> The secondary cannot calculate C as it doesn't know A.  Instead, the
> secondary must send B to the primary, at which point the primary
> calculates the union of A and B (lets call this D) which is all the
> pages dirtied by both the primary and the secondary, and sends all page
> data covered by D.
> 
> In the general case, D is a superset of both A and B.  Without the
> backchannel dirty bitmap, a COLO checkpoint can't reconstruct a valid
> copy of the primary.
> 
> We transfer the dirty bitmap on libxc side, so we need to introduce back
> channel to libxc.
> 
> Note: it is different from the paper. We change the original design to
> the current one, according to our following concerns:
> 1. The original design needs extra memory on Secondary host. When there's
>    multiple backups on one host, the memory cost is high.
> 2. The memory cache code will be another 1k+, it will make the review
>    more time consuming.
> 
> Note: the back channel will be used in the patch
>  libxc/restore: send dirty pfn list to primary when checkpoint under COLO
> to send dirty pfn list from secondary to primary. The patch is posted in
> another series.
> 
> Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>

It is a bit confusing to have 'back_fd' and then 'send_fd'. 

Could you change the 'send_fd' (in this patch) to be called 
'send_back_fd' so that the connection between:
 tools/libxl: Add back channel to allow migration target send data back
and this patch is clear?

Or perhaps also add it in the commit description that you are using
the 'send_fd' provided by ' tools/libxl: Add back channel to allow migration target send data back'

Otherwise: Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>