[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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |