[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] xl: avoid creating domains with duplicate names
Stefano Stabellini writes ("[Xen-devel] [PATCH] xl: avoid creating domains with duplicate names"): > Do not create the domain if another domain with the same name is already > running. Thanks. I approve of the principle of this patch, but: > + e = libxl_name_to_domid(&ctx, c_info->name, &domid_e); > + if (!e) { You should explicitly check the actual error return value of libxl_name_to_domid and check that it is the expected error code, and not some other error code meaning "general failure" or something. I went to look at the code for libxl_name_to_domid and it returns, entirely ad-hoc, -1 (which is now ERROR_VERSION), for "no such domain". IMO it should return ERROR_INVAL. I grepped the libxl source for "-1" and found that this practice is widespread. At this stage of the release I don't want to risk breaking everything by changing them all (since something may compare with -1, or something). So I suggest the attached fixup patch, and then a revised version of your patch. What do you think? Ian. libxl: band-aid for functions with return literal "-1" Many libxl functions erroneously return "-1" on error, rather than some ERROR_* value. To deal with this, invent a new ERROR_NONSPECIFIC "-1" which indicates that "the function which generated this error code is broken". Fix up the one we care about for forthcoming duplicate domain detection (libxl_name_to_domid) and the others following the same pattern nearby; leave the rest for post-4.1. Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx> diff -r 1d1eec7e1fb4 tools/libxl/libxl.h --- a/tools/libxl/libxl.h Tue Jan 25 18:09:49 2011 +0000 +++ b/tools/libxl/libxl.h Tue Jan 25 18:18:53 2011 +0000 @@ -232,12 +232,13 @@ typedef struct { } libxl_domain_suspend_info; enum { - ERROR_VERSION = -1, - ERROR_FAIL = -2, - ERROR_NI = -3, - ERROR_NOMEM = -4, - ERROR_INVAL = -5, - ERROR_BADFAIL = -6, + ERROR_NONSPECIFIC = -1, + ERROR_VERSION = -2, + ERROR_FAIL = -3, + ERROR_NI = -4, + ERROR_NOMEM = -5, + ERROR_INVAL = -6, + ERROR_BADFAIL = -7, }; #define LIBXL_VERSION 0 diff -r 1d1eec7e1fb4 tools/libxl/libxl_utils.c --- a/tools/libxl/libxl_utils.c Tue Jan 25 18:09:49 2011 +0000 +++ b/tools/libxl/libxl_utils.c Tue Jan 25 18:18:53 2011 +0000 @@ -93,7 +93,7 @@ int libxl_name_to_domid(libxl_ctx *ctx, int i, nb_domains; char *domname; libxl_dominfo *dominfo; - int ret = -1; + int ret = ERROR_INVAL; dominfo = libxl_list_domain(ctx, &nb_domains); if (!dominfo) @@ -142,7 +142,7 @@ int libxl_name_to_cpupoolid(libxl_ctx *c int i, nb_pools; char *poolname; libxl_cpupoolinfo *poolinfo; - int ret = -1; + int ret = ERROR_INVAL; poolinfo = libxl_list_cpupool(ctx, &nb_pools); if (!poolinfo) @@ -171,7 +171,7 @@ int libxl_name_to_schedid(libxl_ctx *ctx if (strcmp(name, schedid_name[i].name) == 0) return schedid_name[i].id; - return -1; + return ERROR_INVAL; } char *libxl_schedid_to_name(libxl_ctx *ctx, int schedid) @@ -287,7 +287,7 @@ int libxl_string_to_phystype(libxl_ctx * } else if (!strcmp(s, "tap")) { p = strchr(s, ':'); if (!p) { - rc = -1; + rc = ERROR_INVAL; goto out; } p++; _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |