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

Re: [Xen-devel] [PATCH v11 03/27] tools/libxl: Add back channel to allow migration target send data back

On 03/05/2016 12:38 AM, Ian Jackson wrote:
Changlong Xie writes ("[PATCH v11 03/27] tools/libxl: Add back channel to allow 
migration target send data back"):
From: Wen Congyang <wency@xxxxxxxxxxxxxx>

In COLO mode, secondary needs to send the following data to primary:
1. In libxl
    Secondary sends the following CHECKPOINT_CONTEXT to primary:
2. In libxc
    Secondary sends the dirty pfn list to primary

The overall API approach here seems good to me.

But, my reading of the code is that this new fd is currently ignored.
This is, AFAICT, intentional in the non-colo case, and we have no colo
case yet.

So I think that this new parameter needs to be slightly better
documented.  I suggest:

  * In this patch, add a comment next to it saying "always pass -1".
  * In the patch were the fd actually starts to do something, change
     this comment to something more meaningful.


+ *
+ * If this is defined, libxl_domain_create_restore()'s API has changed to
+ * include a send_back_fd param which used for libxl migration back channel
+ * during COLO.
+ */

I have a minor grammar quibble with this.  I would write:

   "If this is defined, libxl_domain_create_restore()'s API
    includes the send_back_fd param.  This is used only with
    COLO, for the libxl migration back channel; other callers
    should pass -1."

And, with this definition of the API, I think that the code should
actually check that -1 is passed.  Personally I would be happy with
the error case either failing assert() or returning ERROR_INVAL, but
maybe other maintainers have a specific view.

I prefer assert(), will fix in next version




Xen-devel mailing list



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