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

Re: [Xen-devel] [PATCH v2 COLOPre 11/13] tools/libxl: rename remus device to checkpoint device





On 06/25/2015 05:09 PM, Wei Liu wrote:
On Thu, Jun 25, 2015 at 01:00:14PM +0800, Yang Hongyang wrote:


On 06/16/2015 06:53 PM, Ian Campbell wrote:
On Mon, 2015-06-15 at 17:24 +0100, Wei Liu wrote:
On Mon, Jun 15, 2015 at 09:45:54AM +0800, Yang Hongyang wrote:


On 06/12/2015 10:57 PM, Ian Jackson wrote:
Wei Liu writes ("Re: [Xen-devel] [PATCH v2 COLOPre 11/13] tools/libxl: rename remus 
device to checkpoint device"):
On Fri, Jun 12, 2015 at 02:30:46PM +0100, Wei Liu wrote:
On Mon, Jun 08, 2015 at 11:43:15AM +0800, Yang Hongyang wrote:
-    (-18, "REMUS_DEVOPS_DOES_NOT_MATCH"),
-    (-19, "REMUS_DEVICE_NOT_SUPPORTED"),
+    (-18, "CHECKPOINT_DEVOPS_DOES_NOT_MATCH"),
+    (-19, "CHECKPOINT_DEVICE_NOT_SUPPORTED"),

You should add two new error numbers.


And in that case you might also need to go through all places to make
sure the correct error numbers are return. I.e. old remus code path
still returns REMUS error code and new CHECKPOINT code path returns new
error code.

I merely speak from API backward compatibility point of view. If you
think what I suggest doesn't make sense, please let me know.

To me this line of reasons prompts me to ask: what would be wrong with
leaving the word REMUS in the error names, and simply updating the
descriptions ?

After all AFIACT the circumstances are very similar.  I don't think it
makes sense to require libxl to do something like
    rc = were_we_doing_colo_not_remus ? CHECKPOINT_BLAH : REMUS_BLAH;

Please to contradict me if I have misunderstood...

COLO and REMUS both are checkpoint device. We use checkpoint device layer
as a more abstract layer for both COLO and REMUS, come to the error code,
these can be used by both COLO and REMUS. So we don't distinguish if we
are doing COLO or REMUS, uses are aware of what they're executing(colo
or remus).


Right. So continue using REMUS_ error code is fine.

Seems like it would also be OK to switch the name and then in libxl,h

#ifdef LIB_API_VERSION < 0xWHENEVER
#define REMUS_BLAH CHECKPOINT_BLAH
#define ...
#endif

_If_ we think the new names make more sense going fwd...

Well, I think the new names are better, I also think it is safe to just rename
them, I don't find any other users using these error codes except Remus/COLO,
it is only used by Remus/COLO internally.


The main point is, this is external visible interface. Some user might
have also developed their solution based on remus. In their code they
check for REMUS_$FOO error code.

I agree renaming to CHECKPOINT even for the sake of matching API names
is good. I think Ian's suggestion should be the simplest way of moving
forward.

Ok, will add an extra patch to deal with this back compatibility issue
in the next version.


Wei.



.


--
Thanks,
Yang.
.


--
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®.