From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yang Hongyang Subject: Re: [PATCH v1 1/5] libxl/save: Refactor libxl__domain_suspend_state Date: Wed, 3 Jun 2015 00:10:03 +0800 Message-ID: <556DD55B.1070606@cn.fujitsu.com> References: <1432116090-19486-1-git-send-email-yanghy@cn.fujitsu.com> <1432116090-19486-2-git-send-email-yanghy@cn.fujitsu.com> <1433256008.15036.290.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1433256008.15036.290.camel@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: Ian Campbell Cc: wei.liu2@citrix.com, wency@cn.fujitsu.com, andrew.cooper3@citrix.com, yunhong.jiang@intel.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, rshriram@cs.ubc.ca List-Id: xen-devel@lists.xenproject.org On 06/02/2015 10:40 PM, Ian Campbell wrote: > On Wed, 2015-05-20 at 18:01 +0800, Yang Hongyang wrote: >> @@ -1762,16 +1762,18 @@ static void libxl__domain_suspend_callback(void *data) >> { >> libxl__save_helper_state *shs = data; >> libxl__egc *egc = shs->egc; >> - libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs); >> + libxl__domain_save_state *dss = CONTAINER_OF(shs, *dss, shs); >> + libxl__domain_suspend_state *dss2 = &dss->dss; > > With dss now being ambiguously save vs suspend I don't think adding a 2 > suffix to one of the usages is the right answer. > > I think in contexts where you are dealing with both that *save_state and > *suspend_state are the way to go for the local variables. I'm afraid > this will make the change noisier, sorry. > > I'm afraid I think that the dss member of struct > libxl__domain_save_state will therefore also need to be called > suspend_state too. > > I think we can tolerate using dss in contexts where there is only one of > the two structs in active use, just to avoid even more noise. > > Alternatively if there is another name for either "save" or "suspend" > which doesn't start with an s (or conflict with some other type) perhaps > we could go with that. I can't think of one off hand. > > Another name might also help because the semantic difference between > suspend and save is something I have to think about every time. Is there > a split along live/dead lines which we could use here perhaps? > > >> static void domain_suspend_callback_common_done(libxl__egc *egc, >> libxl__domain_suspend_state *dss, int ok) >> { >> - libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, ok); >> + libxl__domain_save_state *dsvs = CONTAINER_OF(dss, *dsvs, dss); > > I suppose dsvs is a bit better then dss2. Maybe that's the answer, if > used consistently. If use dsvs to represent save_state consistently, the modification of the code will be too much. I'm thinking of using "dsps" stands for domain_suspend_state, is it ok? > > Ian. > > . > -- Thanks, Yang.