WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH] Consistent handling of libxc errors in libxl (take 2

To: Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH] Consistent handling of libxc errors in libxl (take 2)
From: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>
Date: Thu, 22 Jul 2010 18:13:21 +0100
Cc: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Delivery-date: Thu, 22 Jul 2010 10:15:57 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
 tools/libxl/libxl.c     |  202 ++++++++++++++++++++++++++++++++---------------
 tools/libxl/libxl_dom.c |    8 +-
 2 files changed, 142 insertions(+), 68 deletions(-)

Removed mapping from libxc error codes to libxl - this is redundant because
libxl ought to return -1 with errno set to a meaningful value. Now simply log
the contents of errno before returning ERROR_FAIL if an xc call fails. Every
caller in libxl should now be covered this way except for pci device removals
and domain destroys where release of resources ought to continue even if an
error occurs in an early step. In this case the error is still logged at least

Signed-off-by: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>

diff -r b0b1a6163203 -r 8eaf5d79718f tools/libxl/libxl.c
--- a/tools/libxl/libxl.c       Thu Jul 22 15:24:49 2010 +0100
+++ b/tools/libxl/libxl.c       Thu Jul 22 18:11:52 2010 +0100
@@ -38,28 +38,6 @@
 
 #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)
 {
     if (version != LIBXL_VERSION)
@@ -456,10 +434,16 @@ struct libxl_dominfo * libxl_list_domain
     int size = 1024;
 
     ptr = calloc(size, sizeof(struct libxl_dominfo));
-    if (!ptr) return NULL;
+    if (!ptr) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "allocating domain info");
+        return NULL;
+    }
 
     ret = xc_domain_getinfolist(ctx->xch, 0, 1024, info);
-    if (ret<0) return NULL;
+    if (ret<0) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "geting domain info list");
+        return NULL;
+    }
 
     for (i = 0; i < ret; i++) {
         xcinfo2xlinfo(&info[i], &ptr[i]);
@@ -474,7 +458,10 @@ int libxl_domain_info(struct libxl_ctx *
     int ret;
 
     ret = xc_domain_getinfolist(ctx->xch, domid, 1, &xcinfo);
-    if (ret<0) return ERROR_FAIL;
+    if (ret<0) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "geting domain info list");
+        return ERROR_FAIL;
+    }
     if (ret==0 || xcinfo.domain != domid) return ERROR_INVAL;
 
     xcinfo2xlinfo(&xcinfo, info_r);
@@ -489,10 +476,16 @@ struct libxl_poolinfo * libxl_list_pool(
     int size = 256;
 
     ptr = calloc(size, sizeof(struct libxl_poolinfo));
-    if (!ptr) return NULL;
+    if (!ptr) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "allocating cpupool info");
+        return NULL;
+    }
 
     ret = xc_cpupool_getinfo(ctx->xch, 0, 256, info);
-    if (ret<0) return NULL;
+    if (ret<0) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "getting cpupool info");
+        return NULL;
+    }
 
     for (i = 0; i < ret; i++) {
         ptr[i].poolid = info[i].cpupool_id;
@@ -514,6 +507,10 @@ struct libxl_vminfo * libxl_list_vm(stru
         return NULL;
 
     ret = xc_domain_getinfolist(ctx->xch, 1, 1024, info);
+    if (ret<0) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "geting domain info list");
+        return NULL;
+    }
     for (index = i = 0; i < ret; i++) {
         if (libxl_is_stubdom(ctx, info[i].domain, NULL))
             continue;
@@ -541,23 +538,33 @@ 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);
+    int ret;
+    ret = xc_domain_pause(ctx->xch, domid);
+    if (ret<0) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "pausing domain %d", domid);
+        return ERROR_FAIL;
+    }
+    return 0;
 }
 
-int libxl_domain_core_dump(struct libxl_ctx *ctx, uint32_t domid, const char 
*filename)
+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);
+    int ret;
+    ret = xc_domain_dumpcore(ctx->xch, domid, filename);
+    if (ret<0) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "core dumping domain %d to %s",
+                     domid, filename);
+        return ERROR_FAIL;
+    }
+    return 0;
 }
 
 int libxl_domain_unpause(struct libxl_ctx *ctx, uint32_t domid)
 {
     char *path;
     char *state;
-    int rc;
+    int ret;
 
     if (is_hvm(ctx, domid)) {
         path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/state", 
domid);
@@ -567,8 +574,12 @@ 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);
+    ret = xc_domain_unpause(ctx->xch, domid);
+    if (ret<0) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "unpausing domain %d", domid);
+        return ERROR_FAIL;
+    }
+    return 0;
 }
 
 static char *req_table[] = {
@@ -583,7 +594,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;
@@ -598,12 +608,26 @@ int libxl_domain_shutdown(struct libxl_c
     if (is_hvm(ctx,domid)) {
         unsigned long acpi_s_state = 0;
         unsigned long pvdriver = 0;
-        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);
+        int ret;
+        ret = xc_get_hvm_param(ctx->xch, domid, HVM_PARAM_ACPI_S_STATE, 
&acpi_s_state);
+        if (ret<0) {
+            XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "getting ACPI S-state");
+            return ERROR_FAIL;
+        }
+        ret = xc_get_hvm_param(ctx->xch, domid, HVM_PARAM_CALLBACK_IRQ, 
&pvdriver);
+        if (ret<0) {
+            XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "getting HVM callback IRQ");
+            return ERROR_FAIL;
+        }
+        if (!pvdriver || acpi_s_state != 0) {
+            ret = xc_domain_shutdown(ctx->xch, domid, req);
+            if (ret<0) {
+                XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "unpausing domain");
+                return ERROR_FAIL;
+            }
+       }
     }
-    return libxl_xc_error(rc);
+    return 0;
 }
 
 int libxl_get_wait_fd(struct libxl_ctx *ctx, int *fd)
@@ -790,7 +814,6 @@ 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);
     }
     if (dm_present) {
         if (libxl_destroy_device_model(ctx, domid) < 0)
@@ -821,7 +844,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 ERROR_FAIL;
     }
     return 0;
 }
@@ -1187,7 +1210,11 @@ static int libxl_create_stubdom(struct l
     libxl_xs_write(ctx, XBT_NULL,
                    libxl_sprintf(ctx, "%s/target", libxl_xs_get_dompath(ctx, 
domid)),
                    "%d", info->domid);
-    xc_domain_set_target(ctx->xch, domid, info->domid);
+    ret = xc_domain_set_target(ctx->xch, domid, info->domid);
+    if (ret<0) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "setting target domain %d -> %d", 
domid, info->domid);
+        return ERROR_FAIL;
+    }
     xs_set_target(ctx->xsh, domid, info->domid);
 
     perm[0].id = domid;
@@ -2599,13 +2626,19 @@ int libxl_device_pci_add(struct libxl_ct
             if (start) {
                 if (flags & PCI_BAR_IO) {
                     rc = xc_domain_ioport_permission(ctx->xch, domid, start, 
size, 1);
-                    if (rc < 0)
+                    if (rc < 0) {
                         XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "Error: 
xc_domain_ioport_permission error 0x%llx/0x%llx", start, size);
+                        fclose(f);
+                        return ERROR_FAIL;
+                    }
                 } else {
                     rc = xc_domain_iomem_permission(ctx->xch, domid, 
start>>XC_PAGE_SHIFT,
                                                     
(size+(XC_PAGE_SIZE-1))>>XC_PAGE_SHIFT, 1);
-                    if (rc < 0)
+                    if (rc < 0) {
                         XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "Error: 
xc_domain_iomem_permission error 0x%llx/0x%llx", start, size);
+                        fclose(f);
+                        return ERROR_FAIL;
+                    }
                 }
             }
         }
@@ -2621,10 +2654,14 @@ int libxl_device_pci_add(struct libxl_ct
             rc = xc_physdev_map_pirq(ctx->xch, domid, irq, &irq);
             if (rc < 0) {
                 XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "Error: 
xc_physdev_map_pirq irq=%d", irq);
+                fclose(f);
+                return ERROR_FAIL;
             }
             rc = xc_domain_irq_permission(ctx->xch, domid, irq, 1);
             if (rc < 0) {
                 XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "Error: 
xc_domain_irq_permission irq=%d", irq);
+                fclose(f);
+                return ERROR_FAIL;
             }
         }
         fclose(f);
@@ -2632,8 +2669,10 @@ int libxl_device_pci_add(struct libxl_ct
 out:
     if (!libxl_is_stubdom(ctx, domid, NULL)) {
         rc = xc_assign_device(ctx->xch, domid, pcidev->value);
-        if (rc < 0)
+        if (rc < 0) {
             XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "xc_assign_device failed");
+            return ERROR_FAIL;
+        }
     }
 
     libxl_device_pci_add_xenstore(ctx, domid, pcidev);
@@ -2899,7 +2938,8 @@ int libxl_get_physinfo(struct libxl_ctx 
 
     rc = xc_physinfo(ctx->xch, &xcphysinfo);
     if (rc != 0) {
-        return rc;
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "getting physinfo");
+        return ERROR_FAIL;
     }
     physinfo->threads_per_core = xcphysinfo.threads_per_core;
     physinfo->cores_per_socket = xcphysinfo.cores_per_socket;
@@ -2970,9 +3010,11 @@ struct libxl_vcpuinfo *libxl_list_vcpu(s
     xc_physinfo_t physinfo = { 0 };
 
     if (xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo) != 1) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "getting infolist");
         return NULL;
     }
     if (xc_physinfo(ctx->xch, &physinfo) == -1) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "getting physinfo");
         return NULL;
     }
     *cpusize = physinfo.max_cpu_id + 1;
@@ -2988,9 +3030,11 @@ struct libxl_vcpuinfo *libxl_list_vcpu(s
             return NULL;
         }
         if (xc_vcpu_getinfo(ctx->xch, domid, *nb_vcpu, &vcpuinfo) == -1) {
+            XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "getting vcpu info");
             return NULL;
         }
         if (xc_vcpu_getaffinity(ctx->xch, domid, *nb_vcpu, ptr->cpumap, 
*cpusize) == -1) {
+            XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "getting vcpu affinity");
             return NULL;
         }
         ptr->vcpuid = *nb_vcpu;
@@ -3006,7 +3050,11 @@ struct libxl_vcpuinfo *libxl_list_vcpu(s
 int libxl_set_vcpuaffinity(struct libxl_ctx *ctx, uint32_t domid, uint32_t 
vcpuid,
                            uint64_t *cpumap, int cpusize)
 {
-    return (xc_vcpu_setaffinity(ctx->xch, domid, vcpuid, cpumap, cpusize));
+    if (xc_vcpu_setaffinity(ctx->xch, domid, vcpuid, cpumap, cpusize)) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "setting vcpu affinity");
+        return ERROR_FAIL;
+    }
+    return 0;
 }
 
 int libxl_set_vcpucount(struct libxl_ctx *ctx, uint32_t domid, uint32_t count)
@@ -3016,6 +3064,7 @@ int libxl_set_vcpucount(struct libxl_ctx
     int i;
 
     if (xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo) != 1) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "getting domain info list");
         return ERROR_FAIL;
     }
     if (!count || ((domaininfo.max_vcpu_id + 1) < count)) {
@@ -3034,14 +3083,15 @@ int libxl_set_vcpucount(struct libxl_ctx
 
 /*
  * returns one of the XEN_SCHEDULER_* constants from public/domctl.h
- * or -1 if an error occured.
  */
 int libxl_get_sched_id(struct libxl_ctx *ctx)
 {
     int sched, ret;
 
-    if ((ret = xc_sched_id(ctx->xch, &sched)) != 0)
-        return ret;
+    if ((ret = xc_sched_id(ctx->xch, &sched)) != 0) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "getting domain info list");
+        return ERROR_FAIL;
+    }
     return sched;
 }
 
@@ -3051,8 +3101,10 @@ int libxl_sched_credit_domain_get(struct
     int rc;
 
     rc = xc_sched_credit_domain_get(ctx->xch, domid, &sdom);
-    if (rc != 0)
-        return rc;
+    if (rc != 0) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "setting domain sched credit");
+        return ERROR_FAIL;
+    }
 
     scinfo->weight = sdom.weight;
     scinfo->cap = sdom.cap;
@@ -3067,8 +3119,12 @@ int libxl_sched_credit_domain_set(struct
     int rc;
 
     rc = xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo);
+    if (rc < 0) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "getting domain info list");
+        return ERROR_FAIL;
+    }
     if (rc != 1 || domaininfo.domain != domid)
-        return rc;
+        return ERROR_INVAL;
 
 
     if (scinfo->weight < 1 || scinfo->weight > 65535) {
@@ -3088,8 +3144,10 @@ int libxl_sched_credit_domain_set(struct
     sdom.cap = scinfo->cap;
 
     rc = xc_sched_credit_domain_set(ctx->xch, domid, &sdom);
-    if (rc != 0)
-        return libxl_xc_error(rc);
+    if ( rc < 0 ) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "setting domain sched credit");
+        return ERROR_FAIL;
+    }
 
     return 0;
 }
@@ -3122,11 +3180,13 @@ int libxl_send_trigger(struct libxl_ctx 
     }
 
     rc = xc_domain_send_trigger(ctx->xch, domid, trigger_type, vcpuid);
-    if (rc != 0)
+    if (rc != 0) {
         XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc,
             "Send trigger '%s' failed", trigger_name);
-
-    return libxl_xc_error(rc);
+        return ERROR_FAIL;
+    }
+
+    return 0;
 }
 
 int libxl_send_sysrq(struct libxl_ctx *ctx, uint32_t domid, char sysrq)
@@ -3140,7 +3200,13 @@ int libxl_send_sysrq(struct libxl_ctx *c
 
 int libxl_send_debug_keys(struct libxl_ctx *ctx, char *keys)
 {
-    return xc_send_debug_keys(ctx->xch, keys);
+    int ret;
+    ret = xc_send_debug_keys(ctx->xch, keys);
+    if ( ret < 0 ) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "sending debug keys");
+        return ERROR_FAIL;
+    }
+    return 0;
 }
 
 struct libxl_xen_console_reader *
@@ -3189,6 +3255,10 @@ int libxl_xen_console_read_line(struct l
     memset(cr->buffer, 0, cr->size);
     ret = xc_readconsolering(ctx->xch, &cr->buffer, &cr->count,
                              cr->clear, cr->incremental, &cr->index);
+    if (ret < 0) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "reading console ring buffer");
+        return ERROR_FAIL;
+    }
     if (!ret) {
         if (cr->count) {
             *line_r = cr->buffer;
@@ -3314,7 +3384,7 @@ int libxl_tmem_set(struct libxl_ctx *ctx
     if (rc < 0) {
         XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc,
             "Can not set tmem %s", name);
-        return libxl_xc_error(rc);
+        return ERROR_FAIL;
     }
 
     return rc;
@@ -3329,7 +3399,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 ERROR_FAIL;
     }
 
     return rc;
@@ -3343,7 +3413,7 @@ int libxl_tmem_freeable(struct libxl_ctx
     if (rc < 0) {
         XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc,
             "Can not get tmem freeable memory");
-        return -1;
+        return ERROR_FAIL;
     }
 
     return rc;
diff -r b0b1a6163203 -r 8eaf5d79718f tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c   Thu Jul 22 15:24:49 2010 +0100
+++ b/tools/libxl/libxl_dom.c   Thu Jul 22 18:11:52 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 ERROR_FAIL;
 }
 
 int build_hvm(struct libxl_ctx *ctx, uint32_t domid,
@@ -255,7 +255,11 @@ int restore_common(struct libxl_ctx *ctx
                              state->store_port, &state->store_mfn,
                              state->console_port, &state->console_mfn,
                              info->hvm, info->u.hvm.pae, 0);
-    return libxl_xc_error(rc);
+    if ( rc < 0 ) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "restoring domain");
+        return ERROR_FAIL;
+    }
+    return 0;
 }
 
 struct suspendinfo {




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

<Prev in Thread] Current Thread [Next in Thread>