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

[Ian Campbell <ian.campbell@citrix.com>]

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.

Ian.