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
|
Home