xl: bug-fix patches implementing libxl allocation policy
PATCH 1/1 xl: bug-fix patches implementing libxl allocation policy
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/1 xl: Return void from libxl_free() and libxl_free_all()
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).
tools/libxl/libxl_internal.c +5 -6
@@ -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)
tools/libxl/libxl_internal.h +2 -2
@@ -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 1/1 xl: allocate version info strings with strdup()
In reply to Gianni Tedescotools/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
tools/libxl/libxl.c +23 -8
@@ -70,11 +70,13 @@ int libxl_ctx_init(libxl_ctx *ctx, int vreturn 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, lreturn 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 1/1 xl: don't use libxl allocator for vcpu_list
In reply to Gianni Tedescotools/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.
tools/libxl/libxl.c +14 -3
@@ -2557,6 +2557,8 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctxc_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_ctreturn 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_ctreturn 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) {
tools/libxl/libxl.h +1 -0
@@ -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);
tools/libxl/xl_cmdimpl.c +5 -3
@@ -3243,7 +3243,7 @@ static void print_vcpuinfo(uint32_t tdomvoid 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 1/1 xl: don't use libxl allocator for nic_list
In reply to Gianni Tedescotools/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.
tools/libxl/libxl.c +20 -13
@@ -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_ctxif (!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; }
tools/libxl/libxl.h +1 -0
@@ -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);
tools/libxl/libxl_utils.c +9 -6
@@ -365,14 +365,14 @@ int libxl_pipe(libxl_ctx *ctx, int pipesint 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);
tools/libxl/xl_cmdimpl.c +5 -4
@@ -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 **arfprintf (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 1/1 xl: Fix invalid return of libxl-internal pointers via libxl_domid_to_name()
In reply to Gianni Tedescotools/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.
tools/libxl/libxl.c +6 -6
@@ -1221,7 +1221,7 @@ static int libxl_create_stubdom(libxl_ctmemset (&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(luint32_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" );
tools/libxl/libxl_internal.h +1 -2
@@ -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
tools/libxl/libxl_pci.c +1 -1
@@ -224,7 +224,7 @@ static int libxl_create_pci_backend(libxflexarray_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));
tools/libxl/libxl_utils.c +9 -2
@@ -51,7 +51,14 @@ char *libxl_domid_to_name(libxl_ctx *ctxsnprintf (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 ) {
tools/libxl/xl_cmdimpl.c +33 -16
150 lines not on this page · raw message
PATCH 1/1 xl: Fix invalid return of libxl-internal pointers via libxl_poolid_to_name()
In reply to Gianni Tedescotools/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.
tools/libxl/libxl_internal.h +1 -0
@@ -227,6 +227,7 @@ char *libxl_abs_path(libxl_ctx *ctx, cha#define XL_LOG_ERROR XTL_ERRORchar *_libxl_domid_to_name (libxl_ctx *ctx,uint32_t domid); +char *_libxl_poolid_to_name (libxl_ctx *ctx,uint32_t poolid);#endif
tools/libxl/libxl_utils.c +9 -2
@@ -98,7 +98,14 @@ char *libxl_poolid_to_name(libxl_ctx *ctreturn "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 1/1 xl: Use gcc hidden visibility attribute
In reply to Gianni Tedescotools/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.
tools/libxl/flexarray.c +1 -2
@@ -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) {
tools/libxl/flexarray.h +6 -6
@@ -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
tools/libxl/libxl_internal.h +55 -47
184 lines not on this page · raw message
Re: [PATCH 7 of 7] xl: Use gcc hidden visibility attribute
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 Trueif __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, 1Re: [PATCH 3 of 7] xl: don't use libxl allocator for vcpu_list
In reply to Gianni TedescoQuoted 10 lines · 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 <[email protected]>
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
Quoted 7 lines · Stefano Stabellini
Quoted 6 lines · 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(-)
Quoted 7 lines
Quoted 4 lines
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 <[email protected]>I applied all the patches of this series apart from this one that doesn't apply correctly. Please update and resend.
tools/libxl/libxl.c +13 -3
@@ -2722,6 +2722,8 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctxc_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_ctreturn 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_ctreturn 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) {
tools/libxl/libxl.h +1 -0
@@ -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);
tools/libxl/xl_cmdimpl.c +5 -3
@@ -3268,7 +3268,7 @@ static void print_vcpuinfo(uint32_t tdomvoid 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:
Stefano Stabellini
AppliedRe: [PATCH 3 of 7] xl: don't use libxl allocator for vcpu_list
applied, thanks
Quoted 119 lines · On Wed, 11 Aug 2010, Gianni Tedesco (3P) wrote:
Quoted 7 lines · Stefano Stabellini
Quoted 6 lines · 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(-)Quoted 7 lines
Quoted 4 lines
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 <[email protected]>I applied all the patches of this series apart from this one that doesn't apply correctly. Please update and resend.
diff -r
fffedd3d70e1tools/libxl/libxl.ctools/libxl/libxl.c +13
-3@@ -2722,6 +2722,8 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctxc_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_ctreturn 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_ctreturn 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
fffedd3d70e1tools/libxl/libxl.htools/libxl/libxl.h +1
-0@@ -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
fffedd3d70e1tools/libxl/xl_cmdimpl.ctools/libxl/xl_cmdimpl.c +5
-3@@ -3268,7 +3268,7 @@ static void print_vcpuinfo(uint32_t tdomvoid 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: