[PATCH 0 of 7] xl: bug-fix patches implementing libxl allocation policy

[PATCH 0 of 7] xl: bug-fix patches implementing libxl allocation policy

[Gianni Tedesco]

This patch series implements several bug-fixes and implements "the memory
allocation policy" of libxl. All touched functions have been tested with
valgrind and are free of leaks, double-free and free-non-malloce'd pointer
errors. This patch-set prepares for another patch to actually implement
per-function-call lifetime for libxl garbage collection which is coming
RealSoonNow(tm) - but touches almost every code path so requires more
verification. This is all a step towards allowing demons to use libxl.

[PATCH 1 of 7] xl: Return void from libxl_free() and libxl_free_all()

[Gianni Tedesco]

 tools/libxl/libxl_internal.c |  11 +++++------
 tools/libxl/libxl_internal.h |   4 ++--
 2 files changed, 7 insertions(+), 8 deletions(-)


Also abort() if a pointer passed in to libxl_free() cannot be found in the
mini-gc structures. This exposes several real bugs (ie. not just the
libxl_free() something that was allocated via malloc variety).

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

diff -r 8992134dcfd0 -r 1b538b12751c tools/libxl/libxl_internal.c
--- a/tools/libxl/libxl_internal.c	Wed Aug 04 19:24:17 2010 +0100
+++ b/tools/libxl/libxl_internal.c	Tue Aug 10 15:44:05 2010 +0100
@@ -59,26 +59,26 @@ int libxl_ptr_add(libxl_ctx *ctx, void *
     return 0;
 }
 
-int libxl_free(libxl_ctx *ctx, void *ptr)
+void libxl_free(libxl_ctx *ctx, void *ptr)
 {
     int i;
 
     if (!ptr)
-        return 0;
+        return;
 
     /* remove the pointer from the tracked ptrs */
     for (i = 0; i < ctx->alloc_maxsize; i++) {
         if (ctx->alloc_ptrs[i] == ptr) {
             ctx->alloc_ptrs[i] = NULL;
             free(ptr);
-            return 0;
+            return;
         }
     }
     /* haven't find the pointer, really bad */
-    return -1;
+    abort();
 }
 
-int libxl_free_all(libxl_ctx *ctx)
+void libxl_free_all(libxl_ctx *ctx)
 {
     void *ptr;
     int i;
@@ -88,7 +88,6 @@ int libxl_free_all(libxl_ctx *ctx)
         ctx->alloc_ptrs[i] = NULL;
         free(ptr);
     }
-    return 0;
 }
 
 void *libxl_zalloc(libxl_ctx *ctx, int bytes)
diff -r 8992134dcfd0 -r 1b538b12751c tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Wed Aug 04 19:24:17 2010 +0100
+++ b/tools/libxl/libxl_internal.h	Tue Aug 10 15:44:05 2010 +0100
@@ -108,8 +108,8 @@ int xs_writev(struct xs_handle *xsh, xs_
 
 /* memory allocation tracking/helpers */
 int libxl_ptr_add(libxl_ctx *ctx, void *ptr);
-int libxl_free(libxl_ctx *ctx, void *ptr);
-int libxl_free_all(libxl_ctx *ctx);
+void libxl_free(libxl_ctx *ctx, void *ptr);
+void libxl_free_all(libxl_ctx *ctx);
 void *libxl_zalloc(libxl_ctx *ctx, int bytes);
 void *libxl_calloc(libxl_ctx *ctx, size_t nmemb, size_t size);
 char *libxl_sprintf(libxl_ctx *ctx, const char *fmt, ...) PRINTF_ATTRIBUTE(2, 3);

[PATCH 2 of 7] xl: allocate version info strings with strdup()

[Gianni Tedesco]

 tools/libxl/libxl.c |  31 +++++++++++++++++++++++--------
 1 files changed, 23 insertions(+), 8 deletions(-)


libxl_strdup() will soon have a per-call rather than per-context lifetime so
use strdup() to allocate version info and manually free in context destructor

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

diff -r 1b538b12751c -r 7ffe8e01c721 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Tue Aug 10 15:44:05 2010 +0100
+++ b/tools/libxl/libxl.c	Tue Aug 10 15:45:42 2010 +0100
@@ -70,11 +70,13 @@ int libxl_ctx_init(libxl_ctx *ctx, int v
     return 0;
 }
 
+static void do_free_version_info(libxl_version_info *info);
 int libxl_ctx_free(libxl_ctx *ctx)
 {
     libxl_free_all(ctx);
     free(ctx->alloc_ptrs);
     xc_interface_close(ctx->xch);
+    do_free_version_info(&ctx->version_info);
     if (ctx->xsh) xs_daemon_close(ctx->xsh); 
     return 0;
 }
@@ -2490,6 +2492,18 @@ int libxl_get_physinfo(libxl_ctx *ctx, l
     return 0;
 }
 
+static void do_free_version_info(libxl_version_info *info)
+{
+    free(info->xen_version_extra);
+    free(info->compiler);
+    free(info->compile_by);
+    free(info->compile_domain);
+    free(info->compile_date);
+    free(info->capabilities);
+    free(info->changeset);
+    free(info->commandline);
+}
+
 const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
 {
     union {
@@ -2509,20 +2523,21 @@ const libxl_version_info* libxl_get_vers
     xen_version = xc_version(ctx->xch, XENVER_version, NULL);
     info->xen_version_major = xen_version >> 16;
     info->xen_version_minor = xen_version & 0xFF;
+
     xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra);
-    info->xen_version_extra = libxl_strdup(ctx, u.xen_extra);
+    info->xen_version_extra = strdup(u.xen_extra);
 
     xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc);
-    info->compiler = libxl_strdup(ctx, u.xen_cc.compiler);
-    info->compile_by = libxl_strdup(ctx, u.xen_cc.compile_by);
-    info->compile_domain = libxl_strdup(ctx, u.xen_cc.compile_domain);
-    info->compile_date = libxl_strdup(ctx, u.xen_cc.compile_date);
+    info->compiler = strdup(u.xen_cc.compiler);
+    info->compile_by = strdup(u.xen_cc.compile_by);
+    info->compile_domain = strdup(u.xen_cc.compile_domain);
+    info->compile_date = strdup(u.xen_cc.compile_date);
 
     xc_version(ctx->xch, XENVER_capabilities, &u.xen_caps);
-    info->capabilities = libxl_strdup(ctx, u.xen_caps);
+    info->capabilities = strdup(u.xen_caps);
 
     xc_version(ctx->xch, XENVER_changeset, &u.xen_chgset);
-    info->changeset = libxl_strdup(ctx, u.xen_chgset);
+    info->changeset = strdup(u.xen_chgset);
 
     xc_version(ctx->xch, XENVER_platform_parameters, &u.p_parms);
     info->virt_start = u.p_parms.virt_start;
@@ -2530,7 +2545,7 @@ const libxl_version_info* libxl_get_vers
     info->pagesize = xc_version(ctx->xch, XENVER_pagesize, NULL);
 
     xc_version(ctx->xch, XENVER_commandline, &u.xen_commandline);
-    info->commandline = libxl_strdup(ctx, u.xen_commandline);
+    info->commandline = strdup(u.xen_commandline);
 
     return info;
 }

[PATCH 3 of 7] xl: don't use libxl allocator for vcpu_list

[Gianni Tedesco]

 tools/libxl/libxl.c      |  17 ++++++++++++++---
 tools/libxl/libxl.h      |   1 +
 tools/libxl/xl_cmdimpl.c |   8 +++++---
 3 files changed, 20 insertions(+), 6 deletions(-)


This also fixes a bug with an erroneous call to libxl_free(). A destructor for
the vpcu list is also implemented which is called from xl.

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

diff -r 7ffe8e01c721 -r 4a877c3d722f tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Tue Aug 10 15:45:42 2010 +0100
+++ b/tools/libxl/libxl.c	Tue Aug 10 15:47:00 2010 +0100
@@ -2557,6 +2557,8 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ct
     xc_domaininfo_t domaininfo;
     xc_vcpuinfo_t vcpuinfo;
     xc_physinfo_t physinfo = { 0 };
+    uint64_t *cpumaps;
+    unsigned num_cpuwords;
 
     if (xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo) != 1) {
         XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "getting infolist");
@@ -2567,14 +2569,16 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ct
         return NULL;
     }
     *cpusize = physinfo.max_cpu_id + 1;
-    ptr = libxl_calloc(ctx, domaininfo.max_vcpu_id + 1, sizeof (libxl_vcpuinfo));
+    ret = ptr = calloc(domaininfo.max_vcpu_id + 1, sizeof (libxl_vcpuinfo));
     if (!ptr) {
         return NULL;
     }
 
-    ret = ptr;
+    num_cpuwords = ((physinfo.max_cpu_id + 64) / 64);
+    cpumaps = calloc(num_cpuwords * sizeof(*cpumaps), domaininfo.max_vcpu_id + 1);
+
     for (*nb_vcpu = 0; *nb_vcpu <= domaininfo.max_vcpu_id; ++*nb_vcpu, ++ptr) {
-        ptr->cpumap = libxl_calloc(ctx, (*cpusize + 63) / 64, sizeof (uint64_t));
+        ptr->cpumap = cpumaps + (num_cpuwords * *nb_vcpu);
         if (!ptr->cpumap) {
             return NULL;
         }
@@ -2596,6 +2600,13 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ct
     return ret;
 }
 
+void libxl_free_vcpu_list(libxl_vcpuinfo *vcpu)
+{
+    if ( vcpu )
+        free(vcpu[0].cpumap);
+    free(vcpu);
+}
+
 int libxl_set_vcpuaffinity(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid,
                            uint64_t *cpumap, int cpusize)
 {
diff -r 7ffe8e01c721 -r 4a877c3d722f tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Tue Aug 10 15:45:42 2010 +0100
+++ b/tools/libxl/libxl.h	Tue Aug 10 15:47:00 2010 +0100
@@ -595,6 +595,7 @@ typedef struct {
 int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo);
 libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
                                        int *nb_vcpu, int *cpusize);
+void libxl_free_vcpu_list(libxl_vcpuinfo *vcpu);
 int libxl_set_vcpuaffinity(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid,
                            uint64_t *cpumap, int cpusize);
 int libxl_set_vcpucount(libxl_ctx *ctx, uint32_t domid, uint32_t count);
diff -r 7ffe8e01c721 -r 4a877c3d722f tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Tue Aug 10 15:45:42 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Tue Aug 10 15:47:00 2010 +0100
@@ -3243,7 +3243,7 @@ static void print_vcpuinfo(uint32_t tdom
 void vcpulist(int argc, char **argv)
 {
     libxl_dominfo *dominfo;
-    libxl_vcpuinfo *vcpuinfo;
+    libxl_vcpuinfo *vcpuinfo, *list = NULL;
     libxl_physinfo physinfo;
     int nb_vcpu, nb_domain, cpusize;
 
@@ -3259,26 +3259,28 @@ void vcpulist(int argc, char **argv)
             goto vcpulist_out;
         }
         for (; nb_domain > 0; --nb_domain, ++dominfo) {
-            if (!(vcpuinfo = libxl_list_vcpu(&ctx, dominfo->domid, &nb_vcpu, &cpusize))) {
+            if (!(list = vcpuinfo = libxl_list_vcpu(&ctx, dominfo->domid, &nb_vcpu, &cpusize))) {
                 fprintf(stderr, "libxl_list_vcpu failed.\n");
                 goto vcpulist_out;
             }
             for (; nb_vcpu > 0; --nb_vcpu, ++vcpuinfo) {
                 print_vcpuinfo(dominfo->domid, vcpuinfo, physinfo.nr_cpus);
             }
+            libxl_free_vcpu_list(list);
         }
     } else {
         for (; argc > 0; ++argv, --argc) {
             if (domain_qualifier_to_domid(*argv, &domid, 0) < 0) {
                 fprintf(stderr, "%s is an invalid domain identifier\n", *argv);
             }
-            if (!(vcpuinfo = libxl_list_vcpu(&ctx, domid, &nb_vcpu, &cpusize))) {
+            if (!(vcpuinfo = list = libxl_list_vcpu(&ctx, domid, &nb_vcpu, &cpusize))) {
                 fprintf(stderr, "libxl_list_vcpu failed.\n");
                 goto vcpulist_out;
             }
             for (; nb_vcpu > 0; --nb_vcpu, ++vcpuinfo) {
                 print_vcpuinfo(domid, vcpuinfo, physinfo.nr_cpus);
             }
+            libxl_free_vcpu_list(list);
         }
     }
   vcpulist_out:

[PATCH 4 of 7] xl: don't use libxl allocator for nic_list

[Gianni Tedesco]

 tools/libxl/libxl.c       |  33 ++++++++++++++++++++-------------
 tools/libxl/libxl.h       |   1 +
 tools/libxl/libxl_utils.c |  15 +++++++++------
 tools/libxl/xl_cmdimpl.c  |   9 +++++----
 4 files changed, 35 insertions(+), 23 deletions(-)


This also fixes a bug with an erroneous call to libxl_free(). A destructor for
the nic list is also implemented which is called from xl.

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

diff -r 4a877c3d722f -r 0c8c78fea130 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Tue Aug 10 15:47:00 2010 +0100
+++ b/tools/libxl/libxl.c	Tue Aug 10 15:47:56 2010 +0100
@@ -1729,10 +1729,21 @@ int libxl_device_nic_del(libxl_ctx *ctx,
     return libxl_device_del(ctx, &device, wait);
 }
 
+void libxl_free_nics_list(libxl_nicinfo *nics, unsigned int nb)
+{
+    unsigned int i;
+    for(i = 0; i < nb; i++) {
+        free(nics[i].backend);
+        free(nics[i].frontend);
+        free(nics[i].script);
+    }
+    free(nics);
+}
+
 libxl_nicinfo *libxl_list_nics(libxl_ctx *ctx, uint32_t domid, unsigned int *nb)
 {
     char *dompath, *nic_path_fe;
-    char **l;
+    char **l, **list;
     char *val, *tok;
     unsigned int nb_nics, i;
     libxl_nicinfo *res, *nics;
@@ -1741,22 +1752,21 @@ libxl_nicinfo *libxl_list_nics(libxl_ctx
     if (!dompath) {
         return NULL;
     }
-    l = libxl_xs_directory(ctx, XBT_NULL,
+    list = l = libxl_xs_directory(ctx, XBT_NULL,
                            libxl_sprintf(ctx, "%s/device/vif", dompath), &nb_nics);
     if (!l) {
         return NULL;
     }
-    res = libxl_calloc(ctx, nb_nics, sizeof (libxl_device_nic));
+    nics = res = calloc(nb_nics, sizeof (libxl_device_nic));
     if (!res) {
         libxl_free(ctx, l);
         return NULL;
     }
-    nics = res;
     for (*nb = nb_nics; nb_nics > 0; --nb_nics, ++l, ++nics) {
         nic_path_fe = libxl_sprintf(ctx, "%s/device/vif/%s", dompath, *l);
 
-        nics->backend = libxl_xs_read(ctx, XBT_NULL,
-                                      libxl_sprintf(ctx, "%s/backend", nic_path_fe));
+        nics->backend = xs_read(ctx->xsh, XBT_NULL,
+                                libxl_sprintf(ctx, "%s/backend", nic_path_fe), NULL);
         val = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/backend-id", nic_path_fe));
         nics->backend_id = val ? strtoul(val, NULL, 10) : -1;
 
@@ -1774,17 +1784,14 @@ libxl_nicinfo *libxl_list_nics(libxl_ctx
         nics->rref_tx = val ? strtol(val, NULL, 10) : -1;
         val = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/rx-ring-ref", nic_path_fe));
         nics->rref_rx = val ? strtol(val, NULL, 10) : -1;
-        nics->frontend = libxl_xs_read(ctx, XBT_NULL,
-                                       libxl_sprintf(ctx, "%s/frontend", nics->backend));
+        nics->frontend = xs_read(ctx->xsh, XBT_NULL,
+                                 libxl_sprintf(ctx, "%s/frontend", nics->backend), NULL);
         val = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/frontend-id", nics->backend));
         nics->frontend_id = val ? strtoul(val, NULL, 10) : -1;
-        nics->script = libxl_xs_read(ctx, XBT_NULL,
-                                     libxl_sprintf(ctx, "%s/script", nics->backend));
-
-        libxl_free(ctx, nic_path_fe);
+        nics->script = xs_read(ctx->xsh, XBT_NULL,
+                               libxl_sprintf(ctx, "%s/script", nics->backend), NULL);
     }
 
-    libxl_free(ctx, l);
     return res;
 }
 
diff -r 4a877c3d722f -r 0c8c78fea130 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Tue Aug 10 15:47:00 2010 +0100
+++ b/tools/libxl/libxl.h	Tue Aug 10 15:47:56 2010 +0100
@@ -509,6 +509,7 @@ typedef struct {
 int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic);
 int libxl_device_nic_del(libxl_ctx *ctx, libxl_device_nic *nic, int wait);
 libxl_nicinfo *libxl_list_nics(libxl_ctx *ctx, uint32_t domid, unsigned int *nb);
+void libxl_free_nics_list(libxl_nicinfo *nics, unsigned int nb);
 
 int libxl_device_console_add(libxl_ctx *ctx, uint32_t domid, libxl_device_console *console);
 
diff -r 4a877c3d722f -r 0c8c78fea130 tools/libxl/libxl_utils.c
--- a/tools/libxl/libxl_utils.c	Tue Aug 10 15:47:00 2010 +0100
+++ b/tools/libxl/libxl_utils.c	Tue Aug 10 15:47:56 2010 +0100
@@ -365,14 +365,14 @@ int libxl_pipe(libxl_ctx *ctx, int pipes
 int libxl_mac_to_device_nic(libxl_ctx *ctx, uint32_t domid,
                             const char *mac, libxl_device_nic *nic)
 {
-    libxl_nicinfo *nics;
-    unsigned int nb, i;
+    libxl_nicinfo *nics, *list;
+    unsigned int nb, i, j;
     uint8_t mac_n[6];
     uint8_t *a, *b;
     const char *tok;
     char *endptr;
 
-    nics = libxl_list_nics(ctx, domid, &nb);
+    list = nics = libxl_list_nics(ctx, domid, &nb);
     if (!nics) {
         return ERROR_FAIL;
     }
@@ -384,7 +384,7 @@ int libxl_mac_to_device_nic(libxl_ctx *c
         }
     }
     memset(nic, 0, sizeof (libxl_device_nic));
-    for (; nb; --nb, ++nics) {
+    for (j = 0; j < nb; ++j, ++nics) {
         for (i = 0, a = nics->mac, b = mac_n;
              (b < mac_n + 6) && (*a == *b); ++a, ++b)
             ;
@@ -394,12 +394,12 @@ int libxl_mac_to_device_nic(libxl_ctx *c
             nic->devid = nics->devid;
             memcpy(nic->mac, nics->mac, sizeof (nic->mac));
             nic->script = nics->script;
-            libxl_free(ctx, nics);
+            libxl_free_nics_list(list, nb);
             return 0;
         }
     }
 
-    libxl_free(ctx, nics);
+    libxl_free_nics_list(list, nb);
     return 0;
 }
 
@@ -419,6 +419,9 @@ int libxl_devid_to_device_nic(libxl_ctx 
     nic_path_be = libxl_xs_read(ctx, XBT_NULL,
                                 libxl_sprintf(ctx, "%s/backend", nic_path_fe));
     val = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/backend-id", nic_path_fe));
+    if ( NULL == val ) {
+        return ERROR_FAIL;
+    }
     nic->backend_domid = strtoul(val, NULL, 10);
     nic->devid = strtoul(devid, NULL, 10);
     libxl_free(ctx, val);
diff -r 4a877c3d722f -r 0c8c78fea130 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Tue Aug 10 15:47:00 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Tue Aug 10 15:47:56 2010 +0100
@@ -4031,8 +4031,8 @@ int main_networkattach(int argc, char **
 int main_networklist(int argc, char **argv)
 {
     int opt;
-    libxl_nicinfo *nics;
-    unsigned int nb;
+    libxl_nicinfo *nics, *list;
+    unsigned int nb, i;
 
     if (argc < 3) {
         help("network-list");
@@ -4057,10 +4057,10 @@ int main_networklist(int argc, char **ar
             fprintf(stderr, "%s is an invalid domain identifier\n", *argv);
             continue;
         }
-        if (!(nics = libxl_list_nics(&ctx, domid, &nb))) {
+        if (!(list = nics = libxl_list_nics(&ctx, domid, &nb))) {
             continue;
         }
-        for (; nb > 0; --nb, ++nics) {
+        for (i = 0; i < nb; ++i, ++nics) {
             /* Idx BE */
             printf("%-3d %-2d ", nics->devid, nics->backend_id);
             /* MAC */
@@ -4072,6 +4072,7 @@ int main_networklist(int argc, char **ar
                    nics->devid, nics->state, nics->evtch,
                    nics->rref_tx, nics->rref_rx, nics->backend);
         }
+        libxl_free_nics_list(list, nb);
     }
     return 0;
 }

[PATCH 5 of 7] xl: Fix invalid return of libxl-internal pointers via libxl_domid_to_name()

[Gianni Tedesco]

 tools/libxl/libxl.c          |  12 +++++-----
 tools/libxl/libxl_internal.h |   3 +-
 tools/libxl/libxl_pci.c      |   2 +-
 tools/libxl/libxl_utils.c    |  11 ++++++++-
 tools/libxl/xl_cmdimpl.c     |  49 +++++++++++++++++++++++++++++--------------
 5 files changed, 50 insertions(+), 27 deletions(-)


This function has numerous in-and-out-of-library callers. In library callers
now use _libxl_domid_to_name() which participates in garbage collection and
out-of-library callers are fixed up to free() the domain name.

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

diff -r 0c8c78fea130 -r b4d2cf5a2a60 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Tue Aug 10 15:47:56 2010 +0100
+++ b/tools/libxl/libxl.c	Tue Aug 10 15:50:03 2010 +0100
@@ -1221,7 +1221,7 @@ static int libxl_create_stubdom(libxl_ct
 
     memset(&c_info, 0x00, sizeof(libxl_domain_create_info));
     c_info.hvm = 0;
-    c_info.name = libxl_sprintf(ctx, "%s-dm", libxl_domid_to_name(ctx, info->domid));
+    c_info.name = libxl_sprintf(ctx, "%s-dm", _libxl_domid_to_name(ctx, info->domid));
     for (i = 0; i < 16; i++)
         c_info.uuid[i] = info->uuid[i];
 
@@ -2028,7 +2028,7 @@ retry_transaction:
     flexarray_set(back, boffset++, "state");
     flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", 1));
     flexarray_set(back, boffset++, "domain");
-    flexarray_set(back, boffset++, libxl_domid_to_name(ctx, domid));
+    flexarray_set(back, boffset++, _libxl_domid_to_name(ctx, domid));
     flexarray_set(back, boffset++, "protocol");
     flexarray_set(back, boffset++, LIBXL_XENCONSOLE_PROTOCOL);
 
@@ -2085,7 +2085,7 @@ int libxl_device_vkb_add(libxl_ctx *ctx,
     flexarray_set(back, boffset++, "state");
     flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", 1));
     flexarray_set(back, boffset++, "domain");
-    flexarray_set(back, boffset++, libxl_domid_to_name(ctx, domid));
+    flexarray_set(back, boffset++, _libxl_domid_to_name(ctx, domid));
 
     flexarray_set(front, foffset++, "backend-id");
     flexarray_set(front, foffset++, libxl_sprintf(ctx, "%d", vkb->backend_domid));
@@ -2262,7 +2262,7 @@ static int libxl_build_xenpv_qemu_args(l
         uint32_t guest_domid;
         if (libxl_is_stubdom(ctx, vfb->domid, &guest_domid)) {
             char *filename;
-            char *name = libxl_sprintf(ctx, "qemu-dm-%s", libxl_domid_to_name(ctx, guest_domid));
+            char *name = libxl_sprintf(ctx, "qemu-dm-%s", _libxl_domid_to_name(ctx, guest_domid));
             libxl_create_logfile(ctx, name, &filename);
             info->serial = libxl_sprintf(ctx, "file:%s", filename);
             free(filename);
@@ -2280,7 +2280,7 @@ static int libxl_build_xenpv_qemu_args(l
         info->extra[j] = NULL;
     }
     info->domid = vfb->domid;
-    info->dom_name = libxl_domid_to_name(ctx, vfb->domid);
+    info->dom_name = _libxl_domid_to_name(ctx, vfb->domid);
     info->device_model = libxl_abs_path(ctx, "qemu-dm", libxl_libexec_path());
     info->type = XENPV;
     return 0;
@@ -2326,7 +2326,7 @@ int libxl_device_vfb_add(libxl_ctx *ctx,
     flexarray_set(back, boffset++, "state");
     flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", 1));
     flexarray_set(back, boffset++, "domain");
-    flexarray_set(back, boffset++, libxl_domid_to_name(ctx, domid));
+    flexarray_set(back, boffset++, _libxl_domid_to_name(ctx, domid));
     flexarray_set(back, boffset++, "vnc");
     flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", vfb->vnc));
     flexarray_set(back, boffset++, "vnclisten");
diff -r 0c8c78fea130 -r b4d2cf5a2a60 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Tue Aug 10 15:47:56 2010 +0100
+++ b/tools/libxl/libxl_internal.h	Tue Aug 10 15:50:03 2010 +0100
@@ -226,8 +226,7 @@ char *libxl_abs_path(libxl_ctx *ctx, cha
 #define XL_LOG_WARNING XTL_WARN
 #define XL_LOG_ERROR   XTL_ERROR
 
-/* Error handling */
-int libxl_xc_error(int xc_err);
+char *_libxl_domid_to_name(libxl_ctx *ctx, uint32_t domid);
 
 #endif
 
diff -r 0c8c78fea130 -r b4d2cf5a2a60 tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c	Tue Aug 10 15:47:56 2010 +0100
+++ b/tools/libxl/libxl_pci.c	Tue Aug 10 15:50:03 2010 +0100
@@ -224,7 +224,7 @@ static int libxl_create_pci_backend(libx
     flexarray_set(back, boffset++, "state");
     flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", 1));
     flexarray_set(back, boffset++, "domain");
-    flexarray_set(back, boffset++, libxl_domid_to_name(ctx, domid));
+    flexarray_set(back, boffset++, _libxl_domid_to_name(ctx, domid));
     for (i = 0; i < num; i++) {
         flexarray_set(back, boffset++, libxl_sprintf(ctx, "key-%d", i));
         flexarray_set(back, boffset++, libxl_sprintf(ctx, PCI_BDF, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func));
diff -r 0c8c78fea130 -r b4d2cf5a2a60 tools/libxl/libxl_utils.c
--- a/tools/libxl/libxl_utils.c	Tue Aug 10 15:47:56 2010 +0100
+++ b/tools/libxl/libxl_utils.c	Tue Aug 10 15:50:03 2010 +0100
@@ -51,7 +51,14 @@ char *libxl_domid_to_name(libxl_ctx *ctx
 
     snprintf(path, sizeof(path), "/local/domain/%d/name", domid);
     s = xs_read(ctx->xsh, XBT_NULL, path, &len);
-    libxl_ptr_add(ctx, s);
+    return s;
+}
+
+char *_libxl_domid_to_name(libxl_ctx *ctx, uint32_t domid)
+{
+    char *s = libxl_domid_to_name(ctx, domid);
+    if ( s )
+        libxl_ptr_add(ctx, s);
     return s;
 }
 
@@ -68,7 +75,7 @@ int libxl_name_to_domid(libxl_ctx *ctx, 
         return ERROR_NOMEM;
 
     for (i = 0; i < nb_domains; i++) {
-        domname = libxl_domid_to_name(ctx, dominfo[i].domid);
+        domname = _libxl_domid_to_name(ctx, dominfo[i].domid);
         if (!domname)
             continue;
         if (strcmp(domname, name) == 0) {
diff -r 0c8c78fea130 -r b4d2cf5a2a60 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Tue Aug 10 15:47:56 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Tue Aug 10 15:50:03 2010 +0100
@@ -2143,8 +2143,10 @@ void list_domains(int verbose, const lib
 
     printf("Name                                        ID   Mem VCPUs\tState\tTime(s)\n");
     for (i = 0; i < nb_domain; i++) {
+        char *domname;
+        domname = libxl_domid_to_name(&ctx, info[i].domid);
         printf("%-40s %5d %5lu %5d     %c%c%c%c%c%c  %8.1f",
-                libxl_domid_to_name(&ctx, info[i].domid),
+                domname,
                 info[i].domid,
                 (unsigned long) (info[i].max_memkb / 1024),
                 info[i].vcpu_online,
@@ -2155,6 +2157,7 @@ void list_domains(int verbose, const lib
                 info[i].shutdown_reason == SHUTDOWN_crash ? 'c' : '-',
                 info[i].dying ? 'd' : '-',
                 ((float)info[i].cpu_time / 1e9));
+        free(domname);
         if (verbose) {
             char *uuid = libxl_uuid2string(&ctx, info[i].uuid);
             printf(" %s", uuid);
@@ -2166,6 +2169,7 @@ void list_domains(int verbose, const lib
 void list_vm(void)
 {
     libxl_vminfo *info;
+    char *domname;
     int nb_vm, i;
 
     info = libxl_list_vm(&ctx, &nb_vm);
@@ -2176,12 +2180,14 @@ void list_vm(void)
     }
     printf("UUID                                  ID    name\n");
     for (i = 0; i < nb_vm; i++) {
+        domname = libxl_domid_to_name(&ctx, info[i].domid);
         printf(UUID_FMT "  %d    %-30s\n",
             info[i].uuid[0], info[i].uuid[1], info[i].uuid[2], info[i].uuid[3],
             info[i].uuid[4], info[i].uuid[5], info[i].uuid[6], info[i].uuid[7],
             info[i].uuid[8], info[i].uuid[9], info[i].uuid[10], info[i].uuid[11],
             info[i].uuid[12], info[i].uuid[13], info[i].uuid[14], info[i].uuid[15],
-            info[i].domid, libxl_domid_to_name(&ctx, info[i].domid));
+            info[i].domid, domname);
+        free(domname);
     }
     free(info);
 }
@@ -3181,10 +3187,13 @@ static void print_vcpuinfo(uint32_t tdom
     int i, l;
     uint64_t *cpumap;
     uint64_t pcpumap;
+    char *domname;
 
     /*      NAME  ID  VCPU */
+    domname = libxl_domid_to_name(&ctx, tdomid);
     printf("%-32s %5u %5u",
-           libxl_domid_to_name(&ctx, tdomid), tdomid, vcpuinfo->vcpuid);
+           domname, tdomid, vcpuinfo->vcpuid);
+    free(domname);
     if (!vcpuinfo->online) {
         /*      CPU STA */
         printf("%5c %3c%cp ", '-', '-', '-');
@@ -3593,11 +3602,14 @@ static int sched_credit_domain_set(
 static void sched_credit_domain_output(
     int domid, libxl_sched_credit *scinfo)
 {
+    char *domname;
+    domname = libxl_domid_to_name(&ctx, domid);
     printf("%-33s %4d %6d %4d\n",
-        libxl_domid_to_name(&ctx, domid),
+        domname,
         domid,
         scinfo->weight,
         scinfo->cap);
+    free(domname);
 }
 
 int main_sched_credit(int argc, char **argv)
@@ -3743,6 +3755,7 @@ int main_domname(int argc, char **argv)
     }
 
     printf("%s\n", domname);
+    free(domname);
 
     return 0;
 }
@@ -4505,6 +4518,7 @@ static void print_dom0_uptime(int short_
     uint32_t uptime = 0;
     char *uptime_str = NULL;
     char *now_str = NULL;
+    char *domname;
 
     fd = open("/proc/uptime", O_RDONLY);
     if (fd == -1)
@@ -4519,24 +4533,25 @@ static void print_dom0_uptime(int short_
     strtok(buf, " ");
     uptime = strtoul(buf, NULL, 10);
 
+    domname = libxl_domid_to_name(&ctx, 0);
     if (short_mode)
     {
         now_str = current_time_to_string(now);
         uptime_str = uptime_to_string(uptime, 1);
         printf(" %s up %s, %s (%d)\n", now_str, uptime_str,
-               libxl_domid_to_name(&ctx, 0), 0);
+               domname, 0);
     }
     else
     {
+        now_str = NULL;
         uptime_str = uptime_to_string(uptime, 0);
-        printf("%-33s %4d %s\n", libxl_domid_to_name(&ctx, 0),
+        printf("%-33s %4d %s\n", domname,
                0, uptime_str);
     }
 
-    if (now_str)
-        free(now_str);
-    if (uptime_str)
-        free(uptime_str);
+    free(now_str);
+    free(uptime_str);
+    free(domname);
     return;
 err:
     fprintf(stderr, "Can not get Dom0 uptime.\n");
@@ -4549,29 +4564,31 @@ static void print_domU_uptime(uint32_t d
     uint32_t uptime = 0;
     char *uptime_str = NULL;
     char *now_str = NULL;
+    char *domname;
 
     s_time = libxl_vm_get_start_time(&ctx, domuid);
     if (s_time == -1)
         return;
     uptime = now - s_time;
+    domname = libxl_domid_to_name(&ctx, domuid);
     if (short_mode)
     {
         now_str = current_time_to_string(now);
         uptime_str = uptime_to_string(uptime, 1);
         printf(" %s up %s, %s (%d)\n", now_str, uptime_str,
-               libxl_domid_to_name(&ctx, domuid), domuid);
+               domname, domuid);
     }
     else
     {
+        now_str = NULL;
         uptime_str = uptime_to_string(uptime, 0);
-        printf("%-33s %4d %s\n", libxl_domid_to_name(&ctx, domuid),
+        printf("%-33s %4d %s\n", domname,
                domuid, uptime_str);
     }
 
-    if (now_str)
-        free(now_str);
-    if (uptime_str)
-        free(uptime_str);
+    free(domname);
+    free(now_str);
+    free(uptime_str);
     return;
 }

[PATCH 6 of 7] xl: Fix invalid return of libxl-internal pointers via libxl_poolid_to_name()

[Gianni Tedesco]

 tools/libxl/libxl_internal.h |   1 +
 tools/libxl/libxl_utils.c    |  11 +++++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)


This function has in-and-out-of-library callers. In library callers now use
_libxl_poolid_to_name() which participates in garbage collection and
out-of-library callers are fixed up to free() the domain name.

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

diff -r b4d2cf5a2a60 -r 4c643fd7aaa2 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Tue Aug 10 15:50:03 2010 +0100
+++ b/tools/libxl/libxl_internal.h	Tue Aug 10 15:50:42 2010 +0100
@@ -227,6 +227,7 @@ char *libxl_abs_path(libxl_ctx *ctx, cha
 #define XL_LOG_ERROR   XTL_ERROR
 
 char *_libxl_domid_to_name(libxl_ctx *ctx, uint32_t domid);
+char *_libxl_poolid_to_name(libxl_ctx *ctx, uint32_t poolid);
 
 #endif
 
diff -r b4d2cf5a2a60 -r 4c643fd7aaa2 tools/libxl/libxl_utils.c
--- a/tools/libxl/libxl_utils.c	Tue Aug 10 15:50:03 2010 +0100
+++ b/tools/libxl/libxl_utils.c	Tue Aug 10 15:50:42 2010 +0100
@@ -98,7 +98,14 @@ char *libxl_poolid_to_name(libxl_ctx *ct
         return "Pool-0";
     snprintf(path, sizeof(path), "/local/pool/%d/name", poolid);
     s = xs_read(ctx->xsh, XBT_NULL, path, &len);
-    libxl_ptr_add(ctx, s);
+    return s;
+}
+
+char *_libxl_poolid_to_name(libxl_ctx *ctx, uint32_t poolid)
+{
+    char *s = libxl_poolid_to_name(ctx, poolid);
+    if ( s )
+        libxl_ptr_add(ctx, s);
     return s;
 }
 
@@ -114,7 +121,7 @@ int libxl_name_to_poolid(libxl_ctx *ctx,
         return ERROR_NOMEM;
 
     for (i = 0; i < nb_pools; i++) {
-        poolname = libxl_poolid_to_name(ctx, poolinfo[i].poolid);
+        poolname = _libxl_poolid_to_name(ctx, poolinfo[i].poolid);
         if (!poolname)
             continue;
         if (strcmp(poolname, name) == 0) {

[PATCH 7 of 7] xl: Use gcc hidden visibility attribute

[Gianni Tedesco]

 tools/libxl/flexarray.c      |    3 +-
 tools/libxl/flexarray.h      |   12 ++--
 tools/libxl/libxl_internal.h |  102 +++++++++++++++++++++++-------------------
 3 files changed, 62 insertions(+), 55 deletions(-)


This kills off about 1,000 PLT entries speeding up link time and shaving about
1-2KB off of binary size. It also prevents users of the library erroneously
calling libxl internal functions.

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

diff -r 4c643fd7aaa2 -r e3677e1a76c9 tools/libxl/flexarray.c
--- a/tools/libxl/flexarray.c	Tue Aug 10 15:50:42 2010 +0100
+++ b/tools/libxl/flexarray.c	Tue Aug 10 15:52:11 2010 +0100
@@ -13,8 +13,7 @@
  * GNU Lesser General Public License for more details.
  */
 
-#include <stdlib.h>
-#include "flexarray.h"
+#include "libxl_internal.h"
 
 flexarray_t *flexarray_make(int size, int autogrow)
 {
diff -r 4c643fd7aaa2 -r e3677e1a76c9 tools/libxl/flexarray.h
--- a/tools/libxl/flexarray.h	Tue Aug 10 15:50:42 2010 +0100
+++ b/tools/libxl/flexarray.h	Tue Aug 10 15:52:11 2010 +0100
@@ -22,12 +22,12 @@ typedef struct flexarray {
     void **data; /* array of pointer */
 } flexarray_t;
 
-flexarray_t *flexarray_make(int size, int autogrow);
-void flexarray_free(flexarray_t *array);
-int flexarray_grow(flexarray_t *array, int extents);
-int flexarray_set(flexarray_t *array, unsigned int index, void *ptr);
-int flexarray_get(flexarray_t *array, int index, void **ptr);
+_hidden flexarray_t *flexarray_make(int size, int autogrow);
+_hidden void flexarray_free(flexarray_t *array);
+_hidden int flexarray_grow(flexarray_t *array, int extents);
+_hidden int flexarray_set(flexarray_t *array, unsigned int index, void *ptr);
+_hidden int flexarray_get(flexarray_t *array, int index, void **ptr);
 
-void **flexarray_contents(flexarray_t *array);
+_hidden void **flexarray_contents(flexarray_t *array);
 
 #endif
diff -r 4c643fd7aaa2 -r e3677e1a76c9 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Tue Aug 10 15:50:42 2010 +0100
+++ b/tools/libxl/libxl_internal.h	Tue Aug 10 15:52:11 2010 +0100
@@ -25,6 +25,14 @@
 #include <xenctrl.h>
 #include "xentoollog.h"
 
+#if __GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ >= 1)
+#define _hidden __attribute__((visibility("hidden")))
+#define _protected __attribute__((visibility("protected")))
+#else
+#define _hidden
+#define _protected
+#endif
+
 #include "flexarray.h"
 #include "libxl_utils.h"
 
@@ -51,13 +59,13 @@
   /* all of these macros preserve errno (saving and restoring) */
 
 /* logging */
-void xl_logv(libxl_ctx *ctx, xentoollog_level msglevel, int errnoval,
+_hidden void xl_logv(libxl_ctx *ctx, xentoollog_level msglevel, int errnoval,
              const char *file /* may be 0 */, int line /* ignored if !file */,
              const char *func /* may be 0 */,
              char *fmt, va_list al)
      __attribute__((format(printf,7,0)));
 
-void xl_log(libxl_ctx *ctx, xentoollog_level msglevel, int errnoval,
+_hidden void xl_log(libxl_ctx *ctx, xentoollog_level msglevel, int errnoval,
             const char *file /* may be 0 */, int line /* ignored if !file */,
             const char *func /* may be 0 */,
             char *fmt, ...)
@@ -104,74 +112,74 @@ typedef struct {
                 (u)[0], (u)[1], (u)[2], (u)[3], (u)[4], (u)[5], (u)[6], (u)[7], \
                 (u)[8], (u)[9], (u)[10], (u)[11], (u)[12], (u)[13], (u)[14], (u)[15])
 
-int xs_writev(struct xs_handle *xsh, xs_transaction_t t, char *dir, char *kvs[]);
+_hidden int xs_writev(struct xs_handle *xsh, xs_transaction_t t, char *dir, char *kvs[]);
 
 /* memory allocation tracking/helpers */
-int libxl_ptr_add(libxl_ctx *ctx, void *ptr);
-void libxl_free(libxl_ctx *ctx, void *ptr);
-void libxl_free_all(libxl_ctx *ctx);
-void *libxl_zalloc(libxl_ctx *ctx, int bytes);
-void *libxl_calloc(libxl_ctx *ctx, size_t nmemb, size_t size);
-char *libxl_sprintf(libxl_ctx *ctx, const char *fmt, ...) PRINTF_ATTRIBUTE(2, 3);
-char *libxl_strdup(libxl_ctx *ctx, const char *c);
-char *libxl_dirname(libxl_ctx *ctx, const char *s);
+_hidden int libxl_ptr_add(libxl_ctx *ctx, void *ptr);
+_hidden void libxl_free(libxl_ctx *ctx, void *ptr);
+_hidden void libxl_free_all(libxl_ctx *ctx);
+_hidden void *libxl_zalloc(libxl_ctx *ctx, int bytes);
+_hidden void *libxl_calloc(libxl_ctx *ctx, size_t nmemb, size_t size);
+_hidden char *libxl_sprintf(libxl_ctx *ctx, const char *fmt, ...) PRINTF_ATTRIBUTE(2, 3);
+_hidden char *libxl_strdup(libxl_ctx *ctx, const char *c);
+_hidden char *libxl_dirname(libxl_ctx *ctx, const char *s);
 
-char **libxl_xs_kvs_of_flexarray(libxl_ctx *ctx, flexarray_t *array, int length);
-int libxl_xs_writev(libxl_ctx *ctx, xs_transaction_t t,
+_hidden char **libxl_xs_kvs_of_flexarray(libxl_ctx *ctx, flexarray_t *array, int length);
+_hidden int libxl_xs_writev(libxl_ctx *ctx, xs_transaction_t t,
                     char *dir, char **kvs);
-int libxl_xs_write(libxl_ctx *ctx, xs_transaction_t t,
+_hidden int libxl_xs_write(libxl_ctx *ctx, xs_transaction_t t,
                    char *path, char *fmt, ...) PRINTF_ATTRIBUTE(4, 5);
-char *libxl_xs_get_dompath(libxl_ctx *ctx, uint32_t domid); // logs errs
-char *libxl_xs_read(libxl_ctx *ctx, xs_transaction_t t, char *path);
-char **libxl_xs_directory(libxl_ctx *ctx, xs_transaction_t t, char *path, unsigned int *nb);
+_hidden char *libxl_xs_get_dompath(libxl_ctx *ctx, uint32_t domid); // logs errs
+_hidden char *libxl_xs_read(libxl_ctx *ctx, xs_transaction_t t, char *path);
+_hidden char **libxl_xs_directory(libxl_ctx *ctx, xs_transaction_t t, char *path, unsigned int *nb);
 
 /* from xl_dom */
-int is_hvm(libxl_ctx *ctx, uint32_t domid);
-int get_shutdown_reason(libxl_ctx *ctx, uint32_t domid);
+_hidden int is_hvm(libxl_ctx *ctx, uint32_t domid);
+_hidden int get_shutdown_reason(libxl_ctx *ctx, uint32_t domid);
 #define dominfo_get_shutdown_reason(info) (((info)->flags >> XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask)
 
-int build_pre(libxl_ctx *ctx, uint32_t domid,
+_hidden int build_pre(libxl_ctx *ctx, uint32_t domid,
               libxl_domain_build_info *info, libxl_domain_build_state *state);
-int build_post(libxl_ctx *ctx, uint32_t domid,
+_hidden int build_post(libxl_ctx *ctx, uint32_t domid,
                libxl_domain_build_info *info, libxl_domain_build_state *state,
                char **vms_ents, char **local_ents);
 
-int build_pv(libxl_ctx *ctx, uint32_t domid,
+_hidden int build_pv(libxl_ctx *ctx, uint32_t domid,
              libxl_domain_build_info *info, libxl_domain_build_state *state);
-int build_hvm(libxl_ctx *ctx, uint32_t domid,
+_hidden int build_hvm(libxl_ctx *ctx, uint32_t domid,
               libxl_domain_build_info *info, libxl_domain_build_state *state);
 
-int restore_common(libxl_ctx *ctx, uint32_t domid,
+_hidden int restore_common(libxl_ctx *ctx, uint32_t domid,
                    libxl_domain_build_info *info, libxl_domain_build_state *state, int fd);
-int core_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int hvm, int live, int debug);
-int save_device_model(libxl_ctx *ctx, uint32_t domid, int fd);
-void libxl__userdata_destroyall(libxl_ctx *ctx, uint32_t domid);
+_hidden int core_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int hvm, int live, int debug);
+_hidden int save_device_model(libxl_ctx *ctx, uint32_t domid, int fd);
+_hidden void libxl__userdata_destroyall(libxl_ctx *ctx, uint32_t domid);
 
 /* from xl_device */
-char *device_disk_backend_type_of_phystype(libxl_disk_phystype phystype);
-char *device_disk_string_of_phystype(libxl_disk_phystype phystype);
+_hidden char *device_disk_backend_type_of_phystype(libxl_disk_phystype phystype);
+_hidden char *device_disk_string_of_phystype(libxl_disk_phystype phystype);
 
-int device_physdisk_major_minor(const char *physpath, int *major, int *minor);
-int device_disk_dev_number(char *virtpath);
+_hidden int device_physdisk_major_minor(const char *physpath, int *major, int *minor);
+_hidden int device_disk_dev_number(char *virtpath);
 
-int libxl_device_generic_add(libxl_ctx *ctx, libxl_device *device,
+_hidden int libxl_device_generic_add(libxl_ctx *ctx, libxl_device *device,
                              char **bents, char **fents);
-int libxl_device_del(libxl_ctx *ctx, libxl_device *dev, int wait);
-int libxl_device_destroy(libxl_ctx *ctx, char *be_path, int force);
-int libxl_devices_destroy(libxl_ctx *ctx, uint32_t domid, int force);
-int libxl_wait_for_device_model(libxl_ctx *ctx,
+_hidden int libxl_device_del(libxl_ctx *ctx, libxl_device *dev, int wait);
+_hidden int libxl_device_destroy(libxl_ctx *ctx, char *be_path, int force);
+_hidden int libxl_devices_destroy(libxl_ctx *ctx, uint32_t domid, int force);
+_hidden int libxl_wait_for_device_model(libxl_ctx *ctx,
                                 uint32_t domid, char *state,
                                 int (*check_callback)(libxl_ctx *ctx,
                                                       uint32_t domid,
                                                       const char *state,
                                                       void *userdata),
                                 void *check_callback_userdata);
-int libxl_wait_for_backend(libxl_ctx *ctx, char *be_path, char *state);
-int libxl_device_pci_reset(libxl_ctx *ctx, unsigned int domain, unsigned int bus,
+_hidden int libxl_wait_for_backend(libxl_ctx *ctx, char *be_path, char *state);
+_hidden int libxl_device_pci_reset(libxl_ctx *ctx, unsigned int domain, unsigned int bus,
                            unsigned int dev, unsigned int func);
 
 /* from xenguest (helper */
-int hvm_build_set_params(xc_interface *handle, uint32_t domid,
+_hidden int hvm_build_set_params(xc_interface *handle, uint32_t domid,
                          libxl_domain_build_info *info,
                          int store_evtchn, unsigned long *store_mfn);
 
@@ -192,7 +200,7 @@ struct libxl_device_model_starting {
     int domid;
 };
 
-int libxl_spawn_spawn(libxl_ctx *ctx,
+_hidden int libxl_spawn_spawn(libxl_ctx *ctx,
                       libxl_device_model_starting *starting,
                       const char *what,
                       void (*intermediate_hook)(void *for_spawn, pid_t innerchild));
@@ -202,11 +210,11 @@ int libxl_spawn_spawn(libxl_ctx *ctx,
    *    0   caller is now the inner child, should probably call libxl_exec
    * Caller, may pass 0 for for_spawn, in which case no need to detach.
    */
-int libxl_spawn_detach(libxl_ctx *ctx,
+_hidden int libxl_spawn_detach(libxl_ctx *ctx,
                        libxl_spawn_starting *for_spawn);
   /* Logs errors.  Idempotent, but only permitted after successful
    * call to libxl_spawn_spawn, and no point calling it again if it fails. */
-int libxl_spawn_check(libxl_ctx *ctx,
+_hidden int libxl_spawn_check(libxl_ctx *ctx,
                       void *for_spawn);
   /* Logs errors but also returns them.
    * for_spawn must actually be a  libxl_spawn_starting*  but
@@ -215,19 +223,19 @@ int libxl_spawn_check(libxl_ctx *ctx,
 
  /* low-level stuff, for synchronous subprocesses etc. */
 
-void libxl_exec(int stdinfd, int stdoutfd, int stderrfd, char *arg0, char **args); // logs errors, never returns
-void libxl_log_child_exitstatus(libxl_ctx *ctx,
+_hidden void libxl_exec(int stdinfd, int stdoutfd, int stderrfd, char *arg0, char **args); // logs errors, never returns
+_hidden void libxl_log_child_exitstatus(libxl_ctx *ctx,
                                 const char *what, pid_t pid, int status);
 
-char *libxl_abs_path(libxl_ctx *ctx, char *s, const char *path);
+_hidden char *libxl_abs_path(libxl_ctx *ctx, char *s, const char *path);
 
 #define XL_LOG_DEBUG   XTL_DEBUG
 #define XL_LOG_INFO    XTL_INFO
 #define XL_LOG_WARNING XTL_WARN
 #define XL_LOG_ERROR   XTL_ERROR
 
-char *_libxl_domid_to_name(libxl_ctx *ctx, uint32_t domid);
-char *_libxl_poolid_to_name(libxl_ctx *ctx, uint32_t poolid);
+_hidden char *_libxl_domid_to_name(libxl_ctx *ctx, uint32_t domid);
+_hidden char *_libxl_poolid_to_name(libxl_ctx *ctx, uint32_t poolid);
 
 #endif

Re: [PATCH 7 of 7] xl: Use gcc hidden visibility attribute

[Gianni Tedesco]

FYI, here is a hacky script to locate self-PLT entries in x86_64
binaries. It depends on binutils and elfutils and clobbers a file called
'./syms' in order to avoid having to think about deadlocks in rw popen.

8<-----------------------------------
#!/usr/bin/python

import os,sys

def check_file(fn):
    f_plt = os.popen("eu-readelf --relocs %s| grep 'X86_64_JUMP_SLOT 0x' | awk '{print $5}'"%fn)
    plt = {}.fromkeys(map(lambda x:x.rstrip("\r\n"), f_plt.readlines()))
    del f_plt

    f_ref = os.popen("objdump -d %s | egrep '\<callq\>.*@plt' | awk '{print $1 $9}'"%fn)

    tmp = map(lambda x:x.split(":"), f_ref.readlines())
    del f_ref

    refs = []
    for x in tmp:
        assert(len(x) == 2)
        (addr, sym) = x
        sym = sym.rstrip(">\r\n").lstrip("<")
        assert(len(sym) > 4 and sym[-4:] == '@plt')
        sym = sym[:-4]
        addr = int(addr, 16)
        refs.append((addr,sym))
    del tmp

    refs = filter(lambda x:plt.has_key(x[1]), refs)
    f_eu = os.popen("xargs eu-addr2line -e %s > syms"%fn, 'w')
    for (addr,sym) in refs:
        f_eu.write("0x%x\n"%addr)
        #print "0x%x %s"%(addr,sym)
    f_eu.close()

    f_eu = open("syms", 'r')
    os.unlink("syms")
    addrlines = map(lambda x:x.rstrip("\r\n"), f_eu.readlines())
    del f_eu

    assert(len(addrlines) == len(refs))

    for ((addr, sym), line) in zip(refs, addrlines):
        print "%s -> %s"%(line, sym)

    return True

if __name__ == '__main__':
    if len(sys.argv) != 2:
        print "Usage: %s <ELF-lib>"%sys.argv[0]
        raise SystemExit, 1
    if check_file(sys.argv[1]):
        raise SystemExit
    raise SystemExit, 1

Re: [PATCH 3 of 7] xl: don't use libxl allocator for vcpu_list

[Stefano Stabellini]

On Tue, 10 Aug 2010, Gianni Tedesco (3P) wrote:
>  tools/libxl/libxl.c      |  17 ++++++++++++++---
>  tools/libxl/libxl.h      |   1 +
>  tools/libxl/xl_cmdimpl.c |   8 +++++---
>  3 files changed, 20 insertions(+), 6 deletions(-)
> 
> 
> This also fixes a bug with an erroneous call to libxl_free(). A destructor for
> the vpcu list is also implemented which is called from xl.
> 
> Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>

I applied all the patches of this series apart from this one that
doesn't apply correctly. Please update and resend.

Thanks,

Stefano

Re: [PATCH 3 of 7] xl: don't use libxl allocator for vcpu_list

[Gianni Tedesco]

On Wed, 2010-08-11 at 13:11 +0100, Stefano Stabellini wrote:
> On Tue, 10 Aug 2010, Gianni Tedesco (3P) wrote:
> >  tools/libxl/libxl.c      |  17 ++++++++++++++---
> >  tools/libxl/libxl.h      |   1 +
> >  tools/libxl/xl_cmdimpl.c |   8 +++++---
> >  3 files changed, 20 insertions(+), 6 deletions(-)
> > 
> > 
> > This also fixes a bug with an erroneous call to libxl_free(). A destructor for
> > the vpcu list is also implemented which is called from xl.
> > 
> > Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
> 
> I applied all the patches of this series apart from this one that
> doesn't apply correctly. Please update and resend.

diff -r fffedd3d70e1 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Wed Aug 11 13:18:05 2010 +0100
+++ b/tools/libxl/libxl.c	Wed Aug 11 14:16:26 2010 +0100
@@ -2722,6 +2722,8 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ct
     xc_domaininfo_t domaininfo;
     xc_vcpuinfo_t vcpuinfo;
     xc_physinfo_t physinfo = { 0 };
+    uint64_t *cpumaps;
+    unsigned num_cpuwords;
 
     if (xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo) != 1) {
         XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "getting infolist");
@@ -2732,14 +2734,15 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ct
         return NULL;
     }
     *nrcpus = physinfo.max_cpu_id + 1;
-    ptr = libxl_calloc(ctx, domaininfo.max_vcpu_id + 1, sizeof (libxl_vcpuinfo));
+    ret = ptr = calloc(domaininfo.max_vcpu_id + 1, sizeof (libxl_vcpuinfo));
     if (!ptr) {
         return NULL;
     }
 
-    ret = ptr;
+    num_cpuwords = ((physinfo.max_cpu_id + 64) / 64);
+    cpumaps = calloc(num_cpuwords * sizeof(*cpumaps), domaininfo.max_vcpu_id + 1);
     for (*nb_vcpu = 0; *nb_vcpu <= domaininfo.max_vcpu_id; ++*nb_vcpu, ++ptr) {
-        ptr->cpumap = libxl_calloc(ctx, (*nrcpus + 63) / 64, sizeof (uint64_t));
+        ptr->cpumap = cpumaps + (num_cpuwords * *nb_vcpu);
         if (!ptr->cpumap) {
             return NULL;
         }
@@ -2762,6 +2765,13 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ct
     return ret;
 }
 
+void libxl_free_vcpu_list(libxl_vcpuinfo *vcpu)
+{
+    if ( vcpu )
+        free(vcpu[0].cpumap);
+    free(vcpu);
+}
+
 int libxl_set_vcpuaffinity(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid,
                            uint64_t *cpumap, int nrcpus)
 {
diff -r fffedd3d70e1 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Wed Aug 11 13:18:05 2010 +0100
+++ b/tools/libxl/libxl.h	Wed Aug 11 14:16:26 2010 +0100
@@ -598,6 +598,7 @@ typedef struct {
 int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo);
 libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
                                        int *nb_vcpu, int *nrcpus);
+void libxl_free_vcpu_list(libxl_vcpuinfo *vcpu);
 int libxl_set_vcpuaffinity(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid,
                            uint64_t *cpumap, int nrcpus);
 int libxl_set_vcpucount(libxl_ctx *ctx, uint32_t domid, uint32_t count);
diff -r fffedd3d70e1 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Wed Aug 11 13:18:05 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Wed Aug 11 14:16:26 2010 +0100
@@ -3268,7 +3268,7 @@ static void print_vcpuinfo(uint32_t tdom
 void vcpulist(int argc, char **argv)
 {
     libxl_dominfo *dominfo;
-    libxl_vcpuinfo *vcpuinfo;
+    libxl_vcpuinfo *vcpuinfo, *list = NULL;
     libxl_physinfo physinfo;
     int nb_vcpu, nb_domain, nrcpus;
 
@@ -3284,7 +3284,7 @@ void vcpulist(int argc, char **argv)
             goto vcpulist_out;
         }
         for (; nb_domain > 0; --nb_domain, ++dominfo) {
-            if (!(vcpuinfo = libxl_list_vcpu(&ctx, dominfo->domid, &nb_vcpu,
+            if (!(list = vcpuinfo = libxl_list_vcpu(&ctx, dominfo->domid, &nb_vcpu,
                 &nrcpus))) {
                 fprintf(stderr, "libxl_list_vcpu failed.\n");
                 goto vcpulist_out;
@@ -3292,19 +3292,21 @@ void vcpulist(int argc, char **argv)
             for (; nb_vcpu > 0; --nb_vcpu, ++vcpuinfo) {
                 print_vcpuinfo(dominfo->domid, vcpuinfo, physinfo.nr_cpus);
             }
+            libxl_free_vcpu_list(list);
         }
     } else {
         for (; argc > 0; ++argv, --argc) {
             if (domain_qualifier_to_domid(*argv, &domid, 0) < 0) {
                 fprintf(stderr, "%s is an invalid domain identifier\n", *argv);
             }
-            if (!(vcpuinfo = libxl_list_vcpu(&ctx, domid, &nb_vcpu, &nrcpus))) {
+            if (!(list = vcpuinfo = libxl_list_vcpu(&ctx, domid, &nb_vcpu, &nrcpus))) {
                 fprintf(stderr, "libxl_list_vcpu failed.\n");
                 goto vcpulist_out;
             }
             for (; nb_vcpu > 0; --nb_vcpu, ++vcpuinfo) {
                 print_vcpuinfo(domid, vcpuinfo, physinfo.nr_cpus);
             }
+            libxl_free_vcpu_list(list);
         }
     }
   vcpulist_out:

Re: [PATCH 3 of 7] xl: don't use libxl allocator for vcpu_list

[Stefano Stabellini]

applied, thanks

On Wed, 11 Aug 2010, Gianni Tedesco (3P) wrote:
> On Wed, 2010-08-11 at 13:11 +0100, Stefano Stabellini wrote:
> > On Tue, 10 Aug 2010, Gianni Tedesco (3P) wrote:
> > >  tools/libxl/libxl.c      |  17 ++++++++++++++---
> > >  tools/libxl/libxl.h      |   1 +
> > >  tools/libxl/xl_cmdimpl.c |   8 +++++---
> > >  3 files changed, 20 insertions(+), 6 deletions(-)
> > > 
> > > 
> > > This also fixes a bug with an erroneous call to libxl_free(). A destructor for
> > > the vpcu list is also implemented which is called from xl.
> > > 
> > > Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
> > 
> > I applied all the patches of this series apart from this one that
> > doesn't apply correctly. Please update and resend.
> 
> diff -r fffedd3d70e1 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c	Wed Aug 11 13:18:05 2010 +0100
> +++ b/tools/libxl/libxl.c	Wed Aug 11 14:16:26 2010 +0100
> @@ -2722,6 +2722,8 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ct
>      xc_domaininfo_t domaininfo;
>      xc_vcpuinfo_t vcpuinfo;
>      xc_physinfo_t physinfo = { 0 };
> +    uint64_t *cpumaps;
> +    unsigned num_cpuwords;
>  
>      if (xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo) != 1) {
>          XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "getting infolist");
> @@ -2732,14 +2734,15 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ct
>          return NULL;
>      }
>      *nrcpus = physinfo.max_cpu_id + 1;
> -    ptr = libxl_calloc(ctx, domaininfo.max_vcpu_id + 1, sizeof (libxl_vcpuinfo));
> +    ret = ptr = calloc(domaininfo.max_vcpu_id + 1, sizeof (libxl_vcpuinfo));
>      if (!ptr) {
>          return NULL;
>      }
>  
> -    ret = ptr;
> +    num_cpuwords = ((physinfo.max_cpu_id + 64) / 64);
> +    cpumaps = calloc(num_cpuwords * sizeof(*cpumaps), domaininfo.max_vcpu_id + 1);
>      for (*nb_vcpu = 0; *nb_vcpu <= domaininfo.max_vcpu_id; ++*nb_vcpu, ++ptr) {
> -        ptr->cpumap = libxl_calloc(ctx, (*nrcpus + 63) / 64, sizeof (uint64_t));
> +        ptr->cpumap = cpumaps + (num_cpuwords * *nb_vcpu);
>          if (!ptr->cpumap) {
>              return NULL;
>          }
> @@ -2762,6 +2765,13 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ct
>      return ret;
>  }
>  
> +void libxl_free_vcpu_list(libxl_vcpuinfo *vcpu)
> +{
> +    if ( vcpu )
> +        free(vcpu[0].cpumap);
> +    free(vcpu);
> +}
> +
>  int libxl_set_vcpuaffinity(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid,
>                             uint64_t *cpumap, int nrcpus)
>  {
> diff -r fffedd3d70e1 tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h	Wed Aug 11 13:18:05 2010 +0100
> +++ b/tools/libxl/libxl.h	Wed Aug 11 14:16:26 2010 +0100
> @@ -598,6 +598,7 @@ typedef struct {
>  int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo);
>  libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
>                                         int *nb_vcpu, int *nrcpus);
> +void libxl_free_vcpu_list(libxl_vcpuinfo *vcpu);
>  int libxl_set_vcpuaffinity(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid,
>                             uint64_t *cpumap, int nrcpus);
>  int libxl_set_vcpucount(libxl_ctx *ctx, uint32_t domid, uint32_t count);
> diff -r fffedd3d70e1 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c	Wed Aug 11 13:18:05 2010 +0100
> +++ b/tools/libxl/xl_cmdimpl.c	Wed Aug 11 14:16:26 2010 +0100
> @@ -3268,7 +3268,7 @@ static void print_vcpuinfo(uint32_t tdom
>  void vcpulist(int argc, char **argv)
>  {
>      libxl_dominfo *dominfo;
> -    libxl_vcpuinfo *vcpuinfo;
> +    libxl_vcpuinfo *vcpuinfo, *list = NULL;
>      libxl_physinfo physinfo;
>      int nb_vcpu, nb_domain, nrcpus;
>  
> @@ -3284,7 +3284,7 @@ void vcpulist(int argc, char **argv)
>              goto vcpulist_out;
>          }
>          for (; nb_domain > 0; --nb_domain, ++dominfo) {
> -            if (!(vcpuinfo = libxl_list_vcpu(&ctx, dominfo->domid, &nb_vcpu,
> +            if (!(list = vcpuinfo = libxl_list_vcpu(&ctx, dominfo->domid, &nb_vcpu,
>                  &nrcpus))) {
>                  fprintf(stderr, "libxl_list_vcpu failed.\n");
>                  goto vcpulist_out;
> @@ -3292,19 +3292,21 @@ void vcpulist(int argc, char **argv)
>              for (; nb_vcpu > 0; --nb_vcpu, ++vcpuinfo) {
>                  print_vcpuinfo(dominfo->domid, vcpuinfo, physinfo.nr_cpus);
>              }
> +            libxl_free_vcpu_list(list);
>          }
>      } else {
>          for (; argc > 0; ++argv, --argc) {
>              if (domain_qualifier_to_domid(*argv, &domid, 0) < 0) {
>                  fprintf(stderr, "%s is an invalid domain identifier\n", *argv);
>              }
> -            if (!(vcpuinfo = libxl_list_vcpu(&ctx, domid, &nb_vcpu, &nrcpus))) {
> +            if (!(list = vcpuinfo = libxl_list_vcpu(&ctx, domid, &nb_vcpu, &nrcpus))) {
>                  fprintf(stderr, "libxl_list_vcpu failed.\n");
>                  goto vcpulist_out;
>              }
>              for (; nb_vcpu > 0; --nb_vcpu, ++vcpuinfo) {
>                  print_vcpuinfo(domid, vcpuinfo, physinfo.nr_cpus);
>              }
> +            libxl_free_vcpu_list(list);
>          }
>      }
>    vcpulist_out:
> 
> 
>