libxl: generate a _dispose function for all Aggregate types

Don't special case types which we happen to know do not contain allocated data
such that in the future if this changes we do not need to add an API. Although
there is likely to be latent bugs in callers due to this having the API in old
libraries mean that when callers are fixed they will not need to make special
arrangements to handle old and new versions of the library.

Adds dispose functions for:
 - libxl_dominfo
 - libxl_cpupoolinfo
 - libxl_physinfo
 - libxl_sched_credit
 - libxl_sched_credit2

I have attempted to fix any latent bugs in xl by inspection but have not
bothered with libxl (on the basis that internally the library is allowed to
make these sorts of assumptions and because it was looking like a very invasive
job and that more would only creep in anyway).

Several callsites use libxl_domain_info to check for the presence of a domain
and throw away the actual info. As a convenience accept a NULL info pointer and
just return the status.

Also fix a memory leak in libxl_domain_list.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 8c9259ffc8b3cb12c0f044cc7e73d95aab3cf70f..53037885a4b67f808baafaf867c75ccd27dd4ce4 100644 (file)
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -442,6 +442,7 @@ libxl_dominfo * libxl_list_domain(libxl_ctx *ctx, int *nb_domain)
     ret = xc_domain_getinfolist(ctx->xch, 0, 1024, info);
     if (ret<0) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "geting domain info list");
+        free(ptr);
         return NULL;
     }
 
@@ -464,7 +465,8 @@ int libxl_domain_info(libxl_ctx *ctx, libxl_dominfo *info_r,
     }
     if (ret==0 || xcinfo.domain != domid) return ERROR_INVAL;
 
-    xcinfo2xlinfo(&xcinfo, info_r);
+    if (info_r)
+        xcinfo2xlinfo(&xcinfo, info_r);
     return 0;
 }
 
@@ -986,13 +988,12 @@ void libxl_evdisable_disk_eject(libxl_ctx *ctx, libxl_evgen_disk_eject *evg) {
 int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid)
 {
     GC_INIT(ctx);
-    libxl_dominfo dominfo;
     char *dom_path;
     char *vm_path;
     char *pid;
     int rc, dm_present;
 
-    rc = libxl_domain_info(ctx, &dominfo, domid);
+    rc = libxl_domain_info(ctx, NULL, domid);
     switch(rc) {
     case 0:
         break;

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index f44f1de9b41c34d9f281398eb0ec4c8eb5fcfdfc..9eb8347bbb1f2234d99b5823c5ec78512759e889 100644 (file)
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -387,11 +387,14 @@ int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, libxl_conso
  * guests using pygrub. */
 int libxl_primary_console_exec(libxl_ctx *ctx, uint32_t domid_vm);
 
+/* May be called with info_r == NULL to check for domain's existance */
 int libxl_domain_info(libxl_ctx*, libxl_dominfo *info_r,
                       uint32_t domid);
 libxl_dominfo * libxl_list_domain(libxl_ctx*, int *nb_domain);
+void libxl_dominfo_list_free(libxl_dominfo *list, int nr);
 libxl_cpupoolinfo * libxl_list_cpupool(libxl_ctx*, int *nb_pool);
 libxl_vminfo * libxl_list_vm(libxl_ctx *ctx, int *nb_vm);
+void libxl_vminfo_list_free(libxl_vminfo *list, int nr);
 
 /*
  * Devices

diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 7f656ab926a379f65ea903af6bc8421c376fcfa6..d4d052e8fb4d55bcbddc3ee9e09ccbff061d770f 100644 (file)
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -177,7 +177,7 @@ libxl_dominfo = Struct("dominfo",[
     ("vcpu_max_id", uint32),
     ("vcpu_online", uint32),
     ("cpupool",     uint32),
-    ], dispose_fn=None)
+    ])
 
 libxl_cpupoolinfo = Struct("cpupoolinfo", [
     ("poolid",      uint32),
@@ -189,7 +189,7 @@ libxl_cpupoolinfo = Struct("cpupoolinfo", [
 libxl_vminfo = Struct("vminfo", [
     ("uuid", libxl_uuid),
     ("domid", libxl_domid),
-    ], dispose_fn=None)
+    ])
 
 libxl_version_info = Struct("version_info", [
     ("xen_version_major", integer),
@@ -408,7 +408,7 @@ libxl_physinfo = Struct("physinfo", [
 
     ("cap_hvm", bool),
     ("cap_hvm_directio", bool),
-    ], dispose_fn=None, dir=DIR_OUT)
+    ], dir=DIR_OUT)
 
 libxl_cputopology = Struct("cputopology", [
     ("core", uint32),
@@ -419,11 +419,11 @@ libxl_cputopology = Struct("cputopology", [
 libxl_sched_credit_domain = Struct("sched_credit_domain", [
     ("weight", integer),
     ("cap", integer),
-    ], dispose_fn=None)
+    ])
 
 libxl_sched_credit2_domain = Struct("sched_credit2_domain", [
     ("weight", integer),
-    ], dispose_fn=None)
+    ])
 
 libxl_sched_sedf_domain = Struct("sched_sedf_domain", [
     ("period", integer),
@@ -431,7 +431,7 @@ libxl_sched_sedf_domain = Struct("sched_sedf_domain", [
     ("latency", integer),
     ("extratime", integer),
     ("weight", integer),
-    ], dispose_fn=None)
+    ])
 
 libxl_event_type = Enumeration("event_type", [
     (1, "DOMAIN_SHUTDOWN"),

diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index a5b3c17f5507e0de201dcfcab2bd3aedf7d2528b..d6cd78d8f27973d2f307f9eb24af37003261c5cc 100644 (file)
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -507,6 +507,22 @@ void libxl_cputopology_list_free(libxl_cputopology *list, int nr)
     free(list);
 }
 
+void libxl_dominfo_list_free(libxl_dominfo *list, int nr)
+{
+    int i;
+    for (i = 0; i < nr; i++)
+        libxl_dominfo_dispose(&list[i]);
+    free(list);
+}
+
+void libxl_vminfo_list_free(libxl_vminfo *list, int nr)
+{
+    int i;
+    for (i = 0; i < nr; i++)
+        libxl_vminfo_dispose(&list[i]);
+    free(list);
+}
+
 int libxl_domid_valid_guest(uint32_t domid)
 {
     /* returns 1 if the value _could_ be a valid guest domid, 0 otherwise

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 9bcc7fe24f3de22d3dc004fd05cf51241620f049..39f5d83db0010e343ee330aff9469b674561717b 100644 (file)
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -144,7 +144,6 @@ static int qualifier_to_id(const char *p, uint32_t *id_r)
 static int domain_qualifier_to_domid(const char *p, uint32_t *domid_r,
                                      int *was_name_r)
 {
-    libxl_dominfo dominfo;
     int was_name, rc;
 
     was_name = qualifier_to_id(p, domid_r);
@@ -156,7 +155,7 @@ static int domain_qualifier_to_domid(const char *p, uint32_t *domid_r,
         if (rc)
             return rc;
     } else {
-        rc = libxl_domain_info(ctx, &dominfo, *domid_r);
+        rc = libxl_domain_info(ctx, NULL, *domid_r);
         /* error only if domain does not exist */
         if (rc == ERROR_INVAL)
             return rc;
@@ -2505,7 +2504,7 @@ static void list_vm(void)
             info[i].domid, domname);
         free(domname);
     }
-    free(info);
+    libxl_vminfo_list_free(info, nb_vm);
 }
 
 static void save_domain_core_begin(const char *domain_spec,
@@ -3302,7 +3301,10 @@ int main_list(int argc, char **argv)
     else
         list_domains(verbose, context, info, nb_domain);
 
-    free(info_free);
+    if (info_free)
+        libxl_dominfo_list_free(info, nb_domain);
+    else
+        libxl_dominfo_dispose(info);
 
     return 0;
 }
@@ -3565,8 +3567,7 @@ static void vcpulist(int argc, char **argv)
         for (i = 0; i<nb_domain; i++)
             print_domain_vcpuinfo(dominfo[i].domid, physinfo.nr_cpus);
 
-        free(dominfo);
-
+        libxl_dominfo_list_free(dominfo, nb_domain);
     } else {
         for (; argc > 0; ++argv, --argc) {
             if (domain_qualifier_to_domid(*argv, &domid, 0) < 0) {
@@ -3578,7 +3579,7 @@ static void vcpulist(int argc, char **argv)
         }
     }
   vcpulist_out:
-    ;
+    libxl_physinfo_dispose(&physinfo);
 }
 
 int main_vcpulist(int argc, char **argv)
@@ -3778,6 +3779,7 @@ static void output_physinfo(void)
         free(cpumap.map);
     }
 
+    libxl_physinfo_dispose(&info);
     return;
 }
 
@@ -3912,7 +3914,9 @@ int main_sharing(int argc, char **argv)
     sharing(info, nb_domain);
 
     if (info_free)
-        free(info_free);
+        libxl_dominfo_list_free(info_free, nb_domain);
+    else
+        libxl_dominfo_dispose(info);
 
     return 0;
 }
@@ -4968,6 +4972,7 @@ static void print_uptime(int short_mode, uint32_t doms[], int nb_doms)
         info = libxl_list_vm(ctx, &nb_vm);
         for (i = 0; i < nb_vm; i++)
             print_domU_uptime(info[i].domid, short_mode, now);
+        libxl_vminfo_list_free(info, nb_vm);
     } else {
         for (i = 0; i < nb_doms; i++) {
             if (doms[i] == 0)