Re: [Xen-devel] [PATCH v1 1/5] libxl/save: Refactor libxl__domain_suspend_state

[Yang Hongyang <yanghy@xxxxxxxxxxxxxx>]

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel