From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yang Hongyang Subject: Re: [PATCH v6 COLO 06/15] libxc/save: support COLO save Date: Tue, 9 Jun 2015 17:09:11 +0800 Message-ID: <5576AD37.1000603@cn.fujitsu.com> References: <1433735159-26739-1-git-send-email-yanghy@cn.fujitsu.com> <1433735159-26739-7-git-send-email-yanghy@cn.fujitsu.com> <557592C0.6020709@citrix.com> <55765A49.9010101@cn.fujitsu.com> <557693CA.9090700@citrix.com> <5576A7A5.9020703@cn.fujitsu.com> <5576A8F7.70302@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5576A8F7.70302@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper , xen-devel@lists.xen.org Cc: wei.liu2@citrix.com, ian.campbell@citrix.com, wency@cn.fujitsu.com, guijianfeng@cn.fujitsu.com, yunhong.jiang@intel.com, eddie.dong@intel.com, rshriram@cs.ubc.ca, ian.jackson@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On 06/09/2015 04:51 PM, Andrew Cooper wrote: > On 09/06/15 09:45, Yang Hongyang wrote: >> >>>> Even if there are no dirty pages on secondary, pfn_list shouldn't be >>>> NULL, it's just that pfn_list[0] will be 0. if pfn_list is NULL, >>>> there might be unexpected error happened. >>> >>> get_dirty_pfn() should be declared alongside a >>> >>> struct pfn_data >>> { >>> uint64_t count; >>> uint64_t *pfns; >>> }; >>> >>> and this function here should create one of these on the stack and pass >>> it by pointer to get_dirty_pfn(). I might also be tempted to rename >>> this to get_remote_logdirty() or similar, to indicate that it is a >>> source of logdirty data from something other than the current >>> hypervisor. >> >> This is a callback, I can't find a way to pass pointer from libxc to >> libxl, >> libxl can not access the pointer data...The struct can be used for >> represent >> the data however. > > Right - my point is that it should be the implementation of > get_remote_logdirty() (i.e. in libxl_save_helper) which is responsible > for unpackaging the data from whatever RPC method is used, rather than > the caller. Now I know what you mean, I will fix it in the next version, thanks! > >> >>>>> Shouldn't get_dirty_pfn be mandatory for COLO streams (even if it is a >>>>> noop to start with) ? >>>> >>>> It should be mandatory, it shouldn't be noop under COLO. perhaps we >>>> should >>>> add sanity check at the beginning. But problem is save side do not >>>> have a param >>>> passed from libxl to indicate the stream type(like >>>> checkpointed_stream in >>>> restore side). So we may need to add another XCFLAGS? Currently >>>> there is >>>> XCFLAGS_CHECKPOINTED which represents Remus, we might need to change >>>> this to >>>> XCFLAGS_STREAM_REMUS >>>> XCFLAGS_STREAM_COLO >>>> so that we can know what kind of stream we are handling? >>> >>> checkpointed_stream started out as a bugfix for a legacy stream >>> migration breakage. Really, this information should have been passed >>> right from the start. >> >> Did I miss the bugfix? is it not in upstream? > > c/s 7051d5c Ah, you are talking about the restore side, I'm talking about the save side checkpointed_stream, so I should also post a prereq patch to add checkpointed_stream to the save side? or there's already the fix out there? > > ~Andrew > . > -- Thanks, Yang.