[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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




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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.