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

[Xen-changelog] [xen-unstable] Revert 91c486918e02 "More consistent error handling in libxl"



# HG changeset patch
# User Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
# Date 1279727415 -3600
# Node ID fcbdfb368e20e23bbdeb84f2d876a5c974c271cf
# Parent  8f5c50120366884d40a81f82c477bffea82f5c88
Revert 91c486918e02 "More consistent error handling in libxl"

I'm afraid this pattern is wrong for two reasons.  Firstly,
xc_domain_pause and functions like it do not return XC_ERROR_* values.
They typically return -1 on error, and set errno.

Secondly, the error codes from libxc are not really all that useful.
They mostly serve to identify where the error originated.

See the comment in xenctrl.h near line 70.
---
 tools/libxl/libxl.c          |  108 ++++++++++++++++---------------------------
 tools/libxl/libxl_dom.c      |   21 +++-----
 tools/libxl/libxl_internal.h |    3 -
 3 files changed, 51 insertions(+), 81 deletions(-)

diff -r 8f5c50120366 -r fcbdfb368e20 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c       Wed Jul 21 16:36:19 2010 +0100
+++ b/tools/libxl/libxl.c       Wed Jul 21 16:50:15 2010 +0100
@@ -37,28 +37,6 @@
 #include "tap-ctl.h"
 
 #define PAGE_TO_MEMKB(pages) ((pages) * 4)
-
-int libxl_xc_error(int xc_err)
-{
-    int ret = ERROR_FAIL;
-
-    switch(xc_err) {
-    case XC_ERROR_NONE:
-        return 0;
-    case XC_INVALID_PARAM:
-        ret = ERROR_INVAL;
-        break;
-    case XC_OUT_OF_MEMORY:
-        ret = ERROR_NOMEM;
-        break;
-    case XC_INTERNAL_ERROR:
-    case XC_INVALID_KERNEL:
-    default:
-        break;
-    }
-
-    return ret;
-}
 
 int libxl_ctx_init(struct libxl_ctx *ctx, int version, xentoollog_logger *lg)
 {
@@ -541,23 +519,21 @@ int libxl_domain_suspend(struct libxl_ct
 
 int libxl_domain_pause(struct libxl_ctx *ctx, uint32_t domid)
 {
-    int rc;
-    rc = xc_domain_pause(ctx->xch, domid);
-    return libxl_xc_error(rc);
+    xc_domain_pause(ctx->xch, domid);
+    return 0;
 }
 
 int libxl_domain_core_dump(struct libxl_ctx *ctx, uint32_t domid, const char 
*filename)
 {
-    int rc;
-    rc = xc_domain_dumpcore(ctx->xch, domid, filename);
-    return libxl_xc_error(rc);
+    if ( xc_domain_dumpcore(ctx->xch, domid, filename) )
+        return ERROR_FAIL;
+    return 0;
 }
 
 int libxl_domain_unpause(struct libxl_ctx *ctx, uint32_t domid)
 {
     char *path;
     char *state;
-    int rc;
 
     if (is_hvm(ctx, domid)) {
         path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/state", 
domid);
@@ -567,8 +543,9 @@ int libxl_domain_unpause(struct libxl_ct
             libxl_wait_for_device_model(ctx, domid, "running", NULL, NULL);
         }
     }
-    rc = xc_domain_unpause(ctx->xch, domid);
-    return libxl_xc_error(rc);
+    xc_domain_unpause(ctx->xch, domid);
+
+    return 0;
 }
 
 static char *req_table[] = {
@@ -583,7 +560,6 @@ int libxl_domain_shutdown(struct libxl_c
 {
     char *shutdown_path;
     char *dom_path;
-    int rc = 0;
 
     if (req > ARRAY_SIZE(req_table))
         return ERROR_INVAL;
@@ -601,9 +577,9 @@ int libxl_domain_shutdown(struct libxl_c
         xc_get_hvm_param(ctx->xch, domid, HVM_PARAM_ACPI_S_STATE, 
&acpi_s_state);
         xc_get_hvm_param(ctx->xch, domid, HVM_PARAM_CALLBACK_IRQ, &pvdriver);
         if (!pvdriver || acpi_s_state != 0)
-            rc = xc_domain_shutdown(ctx->xch, domid, req);
-    }
-    return libxl_xc_error(rc);
+            xc_domain_shutdown(ctx->xch, domid, req);
+    }
+    return 0;
 }
 
 int libxl_get_wait_fd(struct libxl_ctx *ctx, int *fd)
@@ -648,7 +624,7 @@ int libxl_get_event(struct libxl_ctx *ct
     char **events = xs_read_watch(ctx->xsh, &num);
     if (num != 2) {
         free(events);
-        return ERROR_FAIL;
+        return -1;
     }
     event->path = strdup(events[XS_WATCH_PATH]);
     event->token = strdup(events[XS_WATCH_TOKEN]);
@@ -660,7 +636,7 @@ int libxl_stop_waiting(struct libxl_ctx 
 int libxl_stop_waiting(struct libxl_ctx *ctx, libxl_waiter *waiter)
 {
     if (!xs_unwatch(ctx->xsh, waiter->path, waiter->token))
-        return ERROR_FAIL;
+        return -1;
     else
         return 0;
 }
@@ -740,7 +716,7 @@ static int libxl_destroy_device_model(st
         int stubdomid = libxl_get_stubdom_id(ctx, domid);
         if (!stubdomid) {
             XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Couldn't find device model's 
pid");
-            return ERROR_INVAL;
+            return -1;
         }
         XL_LOG(ctx, XL_LOG_ERROR, "Device model is a stubdom, domid=%d\n", 
stubdomid);
         return libxl_domain_destroy(ctx, stubdomid, 0);
@@ -778,7 +754,7 @@ int libxl_domain_destroy(struct libxl_ct
 
     dom_path = libxl_xs_get_dompath(ctx, domid);
     if (!dom_path)
-        return ERROR_FAIL;
+        return -1;
 
     if (libxl_device_pci_shutdown(ctx, domid) < 0)
         XL_LOG(ctx, XL_LOG_ERROR, "pci shutdown failed for domid %d", domid);
@@ -790,7 +766,7 @@ int libxl_domain_destroy(struct libxl_ct
     rc = xc_domain_pause(ctx->xch, domid);
     if (rc < 0) {
         XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "xc_domain_pause failed for 
%d", domid);
-        return libxl_xc_error(rc);
+        return -1;
     }
     if (dm_present) {
         if (libxl_destroy_device_model(ctx, domid) < 0)
@@ -821,7 +797,7 @@ int libxl_domain_destroy(struct libxl_ct
     rc = xc_domain_destroy(ctx->xch, domid);
     if (rc < 0) {
         XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "xc_domain_destroy failed for 
%d", domid);
-        return libxl_xc_error(rc);
+        return -1;
     }
     return 0;
 }
@@ -1239,11 +1215,11 @@ retry_transaction:
     }
     if (libxl_create_xenpv_qemu(ctx, vfb, num_console, console, &dm_starting) 
< 0) {
         free(args);
-        return ERROR_FAIL;
+        return -1;
     }
     if (libxl_confirm_device_model_startup(ctx, dm_starting) < 0) {
         free(args);
-        return ERROR_FAIL;
+        return -1;
     }
 
     libxl_domain_unpause(ctx, domid);
@@ -1447,7 +1423,7 @@ int libxl_device_disk_add(struct libxl_c
                 if (!dev)
                     dev = make_blktap2_device(ctx, disk->physpath, type);
                 if (!dev)
-                    return ERROR_FAIL;
+                    return -1;
                 flexarray_set(back, boffset++, "tapdisk-params");
                 flexarray_set(back, boffset++, libxl_sprintf(ctx, "%s:%s", 
device_disk_string_of_phystype(disk->phystype), disk->physpath));
                 flexarray_set(back, boffset++, "params");
@@ -2147,7 +2123,7 @@ int libxl_cdrom_insert(struct libxl_ctx 
     }
     if (i == num) {
         XL_LOG(ctx, XL_LOG_ERROR, "Virtual device not found");
-        return ERROR_FAIL;
+        return -1;
     }
     libxl_device_disk_del(ctx, disks + i, 1);
     libxl_device_disk_add(ctx, domid, disk);
@@ -2397,7 +2373,7 @@ static int libxl_device_pci_add_xenstore
 
     if (!is_hvm(ctx, domid)) {
         if (libxl_wait_for_backend(ctx, be_path, "4") < 0)
-            return ERROR_FAIL;
+            return -1;
     }
 
     back = flexarray_make(16, 1);
@@ -2446,13 +2422,13 @@ static int libxl_device_pci_remove_xenst
     num_devs_path = libxl_sprintf(ctx, "%s/num_devs", be_path);
     num_devs = libxl_xs_read(ctx, XBT_NULL, num_devs_path);
     if (!num_devs)
-        return ERROR_INVAL;
+        return -1;
     num = atoi(num_devs);
 
     if (!is_hvm(ctx, domid)) {
         if (libxl_wait_for_backend(ctx, be_path, "4") < 0) {
             XL_LOG(ctx, XL_LOG_DEBUG, "pci backend at %s is not ready", 
be_path);
-            return ERROR_FAIL;
+            return -1;
         }
     }
 
@@ -2466,7 +2442,7 @@ static int libxl_device_pci_remove_xenst
     }
     if (i == num) {
         XL_LOG(ctx, XL_LOG_ERROR, "Couldn't find the device on xenstore");
-        return ERROR_INVAL;
+        return -1;
     }
 
 retry_transaction:
@@ -2480,7 +2456,7 @@ retry_transaction:
     if (!is_hvm(ctx, domid)) {
         if (libxl_wait_for_backend(ctx, be_path, "4") < 0) {
             XL_LOG(ctx, XL_LOG_DEBUG, "pci backend at %s is not ready", 
be_path);
-            return ERROR_FAIL;
+            return -1;
         }
     }
 
@@ -2560,7 +2536,7 @@ int libxl_device_pci_add(struct libxl_ct
     hvm = is_hvm(ctx, domid);
     if (hvm) {
         if (libxl_wait_for_device_model(ctx, domid, "running", NULL, NULL) < 
0) {
-            return ERROR_FAIL;
+            return -1;
         }
         path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/state", 
domid);
         state = libxl_xs_read(ctx, XBT_NULL, path);
@@ -2590,7 +2566,7 @@ int libxl_device_pci_add(struct libxl_ct
 
         if (f == NULL) {
             XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Couldn't open %s", sysfs_path);
-            return ERROR_FAIL;
+            return -1;
         }
         for (i = 0; i < PROC_PCI_NUM_RESOURCES; i++) {
             if (fscanf(f, "0x%llx 0x%llx 0x%llx\n", &start, &end, &flags) != 3)
@@ -2653,7 +2629,7 @@ int libxl_device_pci_remove(struct libxl
     hvm = is_hvm(ctx, domid);
     if (hvm) {
         if (libxl_wait_for_device_model(ctx, domid, "running", NULL, NULL) < 
0) {
-            return ERROR_FAIL;
+            return -1;
         }
         path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/state", 
domid);
         state = libxl_xs_read(ctx, XBT_NULL, path);
@@ -2664,7 +2640,7 @@ int libxl_device_pci_remove(struct libxl
         xs_write(ctx->xsh, XBT_NULL, path, "pci-rem", strlen("pci-rem"));
         if (libxl_wait_for_device_model(ctx, domid, "pci-removed", NULL, NULL) 
< 0) {
             XL_LOG(ctx, XL_LOG_ERROR, "Device Model didn't respond in time");
-            return ERROR_FAIL;
+            return -1;
         }
         path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/state", 
domid);
         xs_write(ctx->xsh, XBT_NULL, path, state, strlen(state));
@@ -2788,7 +2764,7 @@ int libxl_device_pci_shutdown(struct lib
     pcidevs = libxl_device_pci_list(ctx, domid, &num);
     for (i = 0; i < num; i++) {
         if (libxl_device_pci_remove(ctx, domid, pcidevs + i) < 0)
-            return ERROR_FAIL;
+            return -1;
     }
     free(pcidevs);
     return 0;
@@ -3074,14 +3050,14 @@ int libxl_sched_credit_domain_set(struct
     if (scinfo->weight < 1 || scinfo->weight > 65535) {
         XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc,
             "Cpu weight out of range, valid values are within range from 1 to 
65535");
-        return ERROR_INVAL;
+        return -1;
     }
 
     if (scinfo->cap < 0 || scinfo->cap > (domaininfo.max_vcpu_id + 1) * 100) {
         XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc,
             "Cpu cap out of range, valid range is from 0 to %d for specified 
number of vcpus",
             ((domaininfo.max_vcpu_id + 1) * 100));
-        return ERROR_INVAL;
+        return -1;
     }
 
     sdom.weight = scinfo->weight;
@@ -3089,7 +3065,7 @@ int libxl_sched_credit_domain_set(struct
 
     rc = xc_sched_credit_domain_set(ctx->xch, domid, &sdom);
     if (rc != 0)
-        return libxl_xc_error(rc);
+        return rc;
 
     return 0;
 }
@@ -3118,7 +3094,7 @@ int libxl_send_trigger(struct libxl_ctx 
     if (trigger_type == -1) {
         XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, -1,
             "Invalid trigger, valid triggers are 
<nmi|reset|init|power|sleep>");
-        return ERROR_INVAL;
+        return -1;
     }
 
     rc = xc_domain_send_trigger(ctx->xch, domid, trigger_type, vcpuid);
@@ -3126,7 +3102,7 @@ int libxl_send_trigger(struct libxl_ctx 
         XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc,
             "Send trigger '%s' failed", trigger_name);
 
-    return libxl_xc_error(rc);
+    return rc;
 }
 
 int libxl_send_sysrq(struct libxl_ctx *ctx, uint32_t domid, char sysrq)
@@ -3252,7 +3228,7 @@ int libxl_tmem_freeze(struct libxl_ctx *
     if (rc < 0) {
         XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc,
             "Can not freeze tmem pools");
-        return ERROR_FAIL;
+        return -1;
     }
 
     return rc;
@@ -3267,7 +3243,7 @@ int libxl_tmem_destroy(struct libxl_ctx 
     if (rc < 0) {
         XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc,
             "Can not destroy tmem pools");
-        return ERROR_FAIL;
+        return -1;
     }
 
     return rc;
@@ -3282,7 +3258,7 @@ int libxl_tmem_thaw(struct libxl_ctx *ct
     if (rc < 0) {
         XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc,
             "Can not thaw tmem pools");
-        return ERROR_FAIL;
+        return -1;
     }
 
     return rc;
@@ -3308,13 +3284,13 @@ int libxl_tmem_set(struct libxl_ctx *ctx
     if (subop == -1) {
         XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, -1,
             "Invalid set, valid sets are <weight|cap|compress>");
-        return ERROR_INVAL;
+        return -1;
     }
     rc = xc_tmem_control(ctx->xch, -1, subop, domid, set, 0, 0, NULL);
     if (rc < 0) {
         XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc,
             "Can not set tmem %s", name);
-        return libxl_xc_error(rc);
+        return -1;
     }
 
     return rc;
@@ -3329,7 +3305,7 @@ int libxl_tmem_shared_auth(struct libxl_
     if (rc < 0) {
         XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc,
             "Can not set tmem shared auth");
-        return libxl_xc_error(rc);
+        return -1;
     }
 
     return rc;
diff -r 8f5c50120366 -r fcbdfb368e20 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c   Wed Jul 21 16:36:19 2010 +0100
+++ b/tools/libxl/libxl_dom.c   Wed Jul 21 16:50:15 2010 +0100
@@ -212,7 +212,7 @@ int build_pv(struct libxl_ctx *ctx, uint
     ret = 0;
 out:
     xc_dom_release(dom);
-    return libxl_xc_error(ret);
+    return ret == 0 ? 0 : ERROR_FAIL;
 }
 
 int build_hvm(struct libxl_ctx *ctx, uint32_t domid,
@@ -250,12 +250,10 @@ int restore_common(struct libxl_ctx *ctx
                    int fd)
 {
     /* read signature */
-    int rc;
-    rc = xc_domain_restore(ctx->xch, fd, domid,
+    return xc_domain_restore(ctx->xch, fd, domid,
                              state->store_port, &state->store_mfn,
                              state->console_port, &state->console_mfn,
                              info->hvm, info->u.hvm.pae, 0);
-    return libxl_xc_error(rc);
 }
 
 struct suspendinfo {
@@ -361,7 +359,7 @@ int core_suspend(struct libxl_ctx *ctx, 
 
     si.xce = xc_evtchn_open();
     if (si.xce < 0)
-        return ERROR_FAIL;
+        return -1;
 
     if (si.xce > 0) {
         port = xs_suspend_evtchn_port(si.domid);
@@ -440,7 +438,7 @@ static const char *userdata_path(struct 
     if (rc) {
         XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "unable to find domain info"
                      " for domain %"PRIu32, domid);
-        return NULL;
+        return 0;
     }
     uuid_string = string_of_uuid(ctx, info.uuid);
 
@@ -495,13 +493,13 @@ int libxl_userdata_store(struct libxl_ct
     size_t rs;
 
     filename = userdata_path(ctx, domid, userdata_userid, "d");
-    if (!filename) return ERROR_NOMEM;
+    if (!filename) return ENOMEM;
 
     if (!datalen)
         return userdata_delete(ctx, filename);
 
     newfilename = userdata_path(ctx, domid, userdata_userid, "n");
-    if (!newfilename) return ERROR_NOMEM;
+    if (!newfilename) return ENOMEM;
 
     fd= open(newfilename, O_RDWR|O_CREAT|O_TRUNC, 0600);
     if (fd<0) goto xe;
@@ -525,10 +523,9 @@ int libxl_userdata_store(struct libxl_ct
     if (f) fclose(f);
     if (fd>=0) close(fd);
 
-    errno = e;
     XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "cannot write %s for %s",
                  newfilename, filename);
-    return ERROR_FAIL;
+    return e;
 }
 
 int libxl_userdata_retrieve(struct libxl_ctx *ctx, uint32_t domid,
@@ -540,14 +537,14 @@ int libxl_userdata_retrieve(struct libxl
     void *data = 0;
 
     filename = userdata_path(ctx, domid, userdata_userid, "d");
-    if (!filename) return ERROR_NOMEM;
+    if (!filename) return ENOMEM;
 
     e = libxl_read_file_contents(ctx, filename, data_r ? &data : 0, &datalen);
 
     if (!e && !datalen) {
         XL_LOG(ctx, XL_LOG_ERROR, "userdata file %s is empty", filename);
         if (data_r) assert(!*data_r);
-        return ERROR_FAIL;
+        return EPROTO;
     }
 
     if (data_r) *data_r = data;
diff -r 8f5c50120366 -r fcbdfb368e20 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h      Wed Jul 21 16:36:19 2010 +0100
+++ b/tools/libxl/libxl_internal.h      Wed Jul 21 16:50:15 2010 +0100
@@ -222,8 +222,5 @@ char *libxl_abs_path(struct libxl_ctx *c
 #define XL_LOG_WARNING XTL_WARN
 #define XL_LOG_ERROR   XTL_ERROR
 
-/* Error handling */
-int libxl_xc_error(int xc_err);
-
 #endif
 

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog


 


Rackspace

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