All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00 of 23] libxl: improved handling for default values in API
@ 2012-02-13 16:52 Ian Campbell
  2012-02-13 16:52 ` [PATCH 01 of 23] libxl: Document _init/_dispose/_setdefault functions Ian Campbell
                   ` (22 more replies)
  0 siblings, 23 replies; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:52 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

Introduce a mechanism for users of libxl to explicitly say "pick the
default for me".

To do this each field has a distinguished "init_val" and each struct
has a "_setdefault" method which sets (idempotently) the defaults for
fields which have not been set to an explicit value.

Previously I went through some contortions (with "timer_mode" in
particular but also with *_memkb fields) to try and arrange that this
distinguished value was the all zeroes bit pattern.

Instead of that pain this time I have arranged that the IDL supports
the specification the distinguished val for each type/field and we
autogenerate an _init function for each data type based on that.

Changes since last time:

* I have posted large parts of the previous series as
  * libxl: drop device_model_info
  * libxl: API updates + xl: JSON
  These have been committed thereby reducing the size of this series.
* _init function support as described above
* Dropped final patch to actually select stubdoms when possible --
  this needs more investigation/sanity checking.

This series has been lightly tested with PV domains (with and without
PVFB) and HVM domains (with and without stubdom).

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 01 of 23] libxl: Document _init/_dispose/_setdefault functions
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
@ 2012-02-13 16:52 ` Ian Campbell
  2012-02-20 19:08   ` Ian Jackson
  2012-02-13 16:52 ` [PATCH 02 of 23] libxl: provide _init and _setdefault for libxl_domain_create_info Ian Campbell
                   ` (21 subsequent siblings)
  22 siblings, 1 reply; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:52 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329147318 0
# Node ID d50c4dc752717e2742a7e9018fcbaffbfff9ea6c
# Parent  1d0bcc8cafd5abd34da13ce32758c505c73cfa48
libxl: Document _init/_dispose/_setdefault functions.

Subsequent patches will transition to them.

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

diff -r 1d0bcc8cafd5 -r d50c4dc75271 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Mon Feb 13 15:35:18 2012 +0000
+++ b/tools/libxl/libxl.h	Mon Feb 13 15:35:18 2012 +0000
@@ -124,6 +124,54 @@
  * Therefore public functions which initialize a libxl__gc MUST call
  * libxl__free_all() before returning.
  */
+/*
+ * libxl types
+ *
+ * Most libxl types are defined by the libxl IDL (see
+ * libxl_types.idl). The library provides a common set of methods for
+ * initialising and freeing these types.
+ *
+ * void libxl_<type>_init(<type> *p):
+ *
+ *    Initialises the members of "p" to all defaults. These may either
+ *    be special value which indicates to the library that it should
+ *    select an appropriate default when using this field or actual
+ *    default values.
+ *
+ *    Some fields within a data type (e.g. unions) cannot be sensibly
+ *    initialised without further information. In these cases a
+ *    separate subfield initialisation function is provided (see
+ *    below).
+ *
+ *    An instance which has been initialised using this method can
+ *    always be safely passed to the dispose function (see
+ *    below). This is true even if the data type contains fields which
+ *    require a separate call to a subfield initialisation function.
+ *
+ *    This method is provided for any aggregate type which is used as
+ *    an input parameter.
+ *
+ * void libxl_<type>_init_<subfield>(<type> *p, subfield):
+ *
+ *    Initialise those parts of "p" which are not initialised by the
+ *    main init function due to the unknown value of "subfield". Sets
+ *    p->subfield as well as initialising any fields to their default
+ *    values.
+ *
+ *    p->subfield must not have been previously initialised.
+ *
+ *    This method is provided for any aggregate type which is used as
+ *    an input parameter.
+ *
+ * void libxl_<type>_dispose(instance *p):
+ *
+ *    Frees any dynamically allocated memory used by the members of
+ *    "p" but not the storage used by "p" itself (this allows for the
+ *    allocation of arrays of types and for the composition of types).
+ *
+ *    In general this method is only provided for types where memory
+ *    can be dynamically allocated as part of that type.
+ */
 #ifndef LIBXL_H
 #define LIBXL_H
 
@@ -405,8 +453,9 @@ libxl_vminfo * libxl_list_vm(libxl_ctx *
  * additional data type libxl_device_<TYPE>_getinfo which contains
  * further runtime information about the device.
  *
- * A common set of methods are available for each device type. These
- * are described below.
+ * In addition to the general methods available for libxl types (see
+ * "libxl types" above) a common set of methods are available for each
+ * device type. These are described below.
  *
  * Querying
  * --------
@@ -424,10 +473,6 @@ libxl_vminfo * libxl_list_vm(libxl_ctx *
  * Creation / Control
  * ------------------
  *
- * libxl_device_<type>_init(ctx, device):
- *
- *    Initalises device to a default configuration.
- *
  * libxl_device_<type>_add(ctx, domid, device):
  *
  *   Adds the given device to the specified domain. This can be called
diff -r 1d0bcc8cafd5 -r d50c4dc75271 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Mon Feb 13 15:35:18 2012 +0000
+++ b/tools/libxl/libxl_internal.h	Mon Feb 13 15:35:18 2012 +0000
@@ -665,6 +665,19 @@ _hidden int libxl__device_destroy(libxl_
 _hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid);
 _hidden int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char *state);
 
+/*
+ * For each aggregate type which can be used as an input we provide:
+ *
+ * int libxl__<type>_setdefault(gc, <type> *p):
+ *
+ *     Idempotently sets any members of "p" which is currently set to
+ *     a special value indicating that the defaults should be used
+ *     (per libxl_<type>_init) to a specific value.
+ *
+ *     All libxl API functions are expected to have arranged for this
+ *     to be called before using any values within these structures.
+ */
+
 /* Arranges that dev will be removed from its guest.  When
  * this is done, the ao will be completed.  An error
  * return from libxl__initiate_device_remove means that the ao

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 02 of 23] libxl: provide _init and _setdefault for libxl_domain_create_info
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
  2012-02-13 16:52 ` [PATCH 01 of 23] libxl: Document _init/_dispose/_setdefault functions Ian Campbell
@ 2012-02-13 16:52 ` Ian Campbell
  2012-02-20 19:09   ` Ian Jackson
  2012-02-13 16:52 ` [PATCH 03 of 23] libxl: provide _init and _setdefault for libxl_domain_build_info Ian Campbell
                   ` (20 subsequent siblings)
  22 siblings, 1 reply; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:52 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329148119 0
# Node ID 53d4e198863a29e204186ab10c8950c3bd4235a8
# Parent  d50c4dc752717e2742a7e9018fcbaffbfff9ea6c
libxl: provide _init and _setdefault for libxl_domain_create_info.

Some fields require further scaffolding before they can use the
_init/_setdefault scheme and are handled in later patches.

It doesn't seem reasonable for the library to pick a default for the domain
type so reject as invalid.

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

diff -r d50c4dc75271 -r 53d4e198863a tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Mon Feb 13 15:35:18 2012 +0000
+++ b/tools/libxl/libxl.h	Mon Feb 13 15:48:39 2012 +0000
@@ -363,7 +363,7 @@ int libxl_ctx_free(libxl_ctx *ctx /* 0 i
 int libxl_ctx_postfork(libxl_ctx *ctx);
 
 /* domain related functions */
-int libxl_init_create_info(libxl_ctx *ctx, libxl_domain_create_info *c_info);
+void libxl_domain_create_info_init(libxl_domain_create_info *c_info);
 int libxl_init_build_info(libxl_ctx *ctx,
                           libxl_domain_build_info *b_info,
                           libxl_domain_create_info *c_info);
diff -r d50c4dc75271 -r 53d4e198863a tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Mon Feb 13 15:35:18 2012 +0000
+++ b/tools/libxl/libxl_create.c	Mon Feb 13 15:48:39 2012 +0000
@@ -50,16 +50,19 @@ void libxl_domain_config_dispose(libxl_d
     libxl_domain_build_info_dispose(&d_config->b_info);
 }
 
-int libxl_init_create_info(libxl_ctx *ctx, libxl_domain_create_info *c_info)
+void libxl_domain_create_info_init(libxl_domain_create_info *c_info)
 {
     memset(c_info, '\0', sizeof(*c_info));
-    c_info->xsdata = NULL;
-    c_info->platformdata = NULL;
     c_info->hap = 1;
-    c_info->type = LIBXL_DOMAIN_TYPE_HVM;
     c_info->oos = 1;
-    c_info->ssidref = 0;
-    c_info->poolid = 0;
+}
+
+int libxl__domain_create_info_setdefault(libxl__gc *gc,
+                                         libxl_domain_create_info *c_info)
+{
+    if (!c_info->type)
+        return ERROR_INVAL;
+
     return 0;
 }
 
@@ -469,6 +472,9 @@ static int do_domain_create(libxl__gc *g
 
     domid = 0;
 
+    ret = libxl__domain_create_info_setdefault(gc, &d_config->c_info);
+    if (ret) goto error_out;
+
     ret = libxl__domain_make(gc, &d_config->c_info, &domid);
     if (ret) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot make domain: %d", ret);
diff -r d50c4dc75271 -r 53d4e198863a tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Mon Feb 13 15:35:18 2012 +0000
+++ b/tools/libxl/libxl_dm.c	Mon Feb 13 15:48:39 2012 +0000
@@ -699,7 +699,7 @@ static int libxl__create_stubdom(libxl__
         goto out;
     }
 
-    memset(&dm_config.c_info, 0x00, sizeof(libxl_domain_create_info));
+    libxl_domain_create_info_init(&dm_config.c_info);
     dm_config.c_info.type = LIBXL_DOMAIN_TYPE_PV;
     dm_config.c_info.name = libxl__sprintf(gc, "%s-dm",
                                     libxl__domid_to_name(gc, guest_domid));
@@ -734,6 +734,9 @@ static int libxl__create_stubdom(libxl__
     dm_config.vifs = guest_config->vifs;
     dm_config.num_vifs = guest_config->num_vifs;
 
+    ret = libxl__domain_create_info_setdefault(gc, &dm_config.c_info);
+    if (ret) goto out;
+
     libxl__vfb_and_vkb_from_hvm_guest_config(gc, guest_config, &vfb, &vkb);
     dm_config.vfbs = &vfb;
     dm_config.num_vfbs = 1;
diff -r d50c4dc75271 -r 53d4e198863a tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Mon Feb 13 15:35:18 2012 +0000
+++ b/tools/libxl/libxl_internal.h	Mon Feb 13 15:48:39 2012 +0000
@@ -187,6 +187,8 @@ libxl__ev_xswatch *libxl__watch_slot_con
  * version of the _evdisable_FOO function; the internal one is
  * used during cleanup.
  */
+_hidden int libxl__domain_create_info_setdefault(libxl__gc *gc,
+                                        libxl_domain_create_info *c_info);
 
 struct libxl__evgen_domain_death {
     uint32_t domid;
diff -r d50c4dc75271 -r 53d4e198863a tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Mon Feb 13 15:35:18 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Mon Feb 13 15:48:39 2012 +0000
@@ -537,8 +537,7 @@ static void parse_config_data(const char
         exit(1);
     }
 
-    if (libxl_init_create_info(ctx, c_info))
-        exit(1);
+    libxl_domain_create_info_init(c_info);
 
     if (!xlu_cfg_get_string (config, "seclabel", &buf, 0)) {
         e = libxl_flask_context_to_sid(ctx, (char *)buf, strlen(buf),

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 03 of 23] libxl: provide _init and _setdefault for libxl_domain_build_info
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
  2012-02-13 16:52 ` [PATCH 01 of 23] libxl: Document _init/_dispose/_setdefault functions Ian Campbell
  2012-02-13 16:52 ` [PATCH 02 of 23] libxl: provide _init and _setdefault for libxl_domain_create_info Ian Campbell
@ 2012-02-13 16:52 ` Ian Campbell
  2012-02-13 16:52 ` [PATCH 04 of 23] libxl: use an explicit LIBXL_TIMER_MODE_DEFAULT value Ian Campbell
                   ` (19 subsequent siblings)
  22 siblings, 0 replies; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:52 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329148126 0
# Node ID af0d151cd2d524f13a13e2fb85e727e8733dcd81
# Parent  53d4e198863a29e204186ab10c8950c3bd4235a8
libxl: provide _init and _setdefault for libxl_domain_build_info.

Some fields require further scaffolding before they can use the
_init/_setdefault scheme and are handled in later patches.

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

diff -r 53d4e198863a -r af0d151cd2d5 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Feb 13 15:48:39 2012 +0000
+++ b/tools/libxl/libxl.c	Mon Feb 13 15:48:46 2012 +0000
@@ -2624,7 +2624,11 @@ int libxl_domain_need_memory(libxl_ctx *
                              uint32_t *need_memkb)
 {
     GC_INIT(ctx);
-    int rc = ERROR_INVAL;
+    int rc;
+
+    rc = libxl__domain_build_info_setdefault(gc, b_info);
+    if (rc) goto out;
+
     *need_memkb = b_info->target_memkb;
     switch (b_info->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
@@ -2636,6 +2640,7 @@ int libxl_domain_need_memory(libxl_ctx *
         *need_memkb += b_info->shadow_memkb + LIBXL_PV_EXTRA_MEMORY;
         break;
     default:
+        rc = ERROR_INVAL;
         goto out;
     }
     if (*need_memkb % (2 * 1024))
diff -r 53d4e198863a -r af0d151cd2d5 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Mon Feb 13 15:48:39 2012 +0000
+++ b/tools/libxl/libxl.h	Mon Feb 13 15:48:46 2012 +0000
@@ -364,9 +364,8 @@ int libxl_ctx_postfork(libxl_ctx *ctx);
 
 /* domain related functions */
 void libxl_domain_create_info_init(libxl_domain_create_info *c_info);
-int libxl_init_build_info(libxl_ctx *ctx,
-                          libxl_domain_build_info *b_info,
-                          libxl_domain_create_info *c_info);
+void libxl_domain_build_info_init(libxl_domain_build_info *b_info,
+                          const libxl_domain_create_info *c_info);
 typedef int (*libxl_console_ready)(libxl_ctx *ctx, uint32_t domid, void *priv);
 int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid);
 int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid, int restore_fd);
diff -r 53d4e198863a -r af0d151cd2d5 tools/libxl/libxl_bootloader.c
--- a/tools/libxl/libxl_bootloader.c	Mon Feb 13 15:48:39 2012 +0000
+++ b/tools/libxl/libxl_bootloader.c	Mon Feb 13 15:48:46 2012 +0000
@@ -346,6 +346,9 @@ int libxl_run_bootloader(libxl_ctx *ctx,
 
     struct stat st_buf;
 
+    rc = libxl__domain_build_info_setdefault(gc, info);
+    if (rc) goto out;
+
     if (info->type != LIBXL_DOMAIN_TYPE_PV || !info->u.pv.bootloader)
         goto out;
 
diff -r 53d4e198863a -r af0d151cd2d5 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Mon Feb 13 15:48:39 2012 +0000
+++ b/tools/libxl/libxl_create.c	Mon Feb 13 15:48:46 2012 +0000
@@ -66,16 +66,10 @@ int libxl__domain_create_info_setdefault
     return 0;
 }
 
-int libxl_init_build_info(libxl_ctx *ctx,
-                          libxl_domain_build_info *b_info,
-                          libxl_domain_create_info *c_info)
+void libxl_domain_build_info_init(libxl_domain_build_info *b_info,
+                                  const libxl_domain_create_info *c_info)
 {
     memset(b_info, '\0', sizeof(*b_info));
-    b_info->max_vcpus = 1;
-    b_info->cur_vcpus = 1;
-    if (libxl_cpumap_alloc(ctx, &b_info->cpumap))
-        return ERROR_NOMEM;
-    libxl_cpumap_set_any(&b_info->cpumap);
     b_info->max_memkb = 32 * 1024;
     b_info->target_memkb = b_info->max_memkb;
     b_info->disable_migrate = 0;
@@ -83,8 +77,6 @@ int libxl_init_build_info(libxl_ctx *ctx
     b_info->shadow_memkb = 0;
     b_info->type = c_info->type;
 
-    b_info->device_model_version =
-        LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
     b_info->device_model_stubdomain = false;
     b_info->device_model = NULL;
 
@@ -107,15 +99,9 @@ int libxl_init_build_info(libxl_ctx *ctx
 
         b_info->u.hvm.stdvga = 0;
         b_info->u.hvm.vnc.enable = 1;
-        b_info->u.hvm.vnc.listen = strdup("127.0.0.1");
         b_info->u.hvm.vnc.display = 0;
         b_info->u.hvm.vnc.findunused = 1;
-        b_info->u.hvm.keymap = NULL;
-        b_info->u.hvm.sdl.enable = 0;
-        b_info->u.hvm.sdl.opengl = 0;
-        b_info->u.hvm.nographic = 0;
         b_info->u.hvm.serial = NULL;
-        b_info->u.hvm.boot = strdup("cda");
         b_info->u.hvm.usb = 0;
         b_info->u.hvm.usbdevice = NULL;
         b_info->u.hvm.xen_platform_pci = 1;
@@ -124,7 +110,47 @@ int libxl_init_build_info(libxl_ctx *ctx
         b_info->u.pv.slack_memkb = 8 * 1024;
         break;
     default:
-        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+        abort();
+    }
+}
+
+int libxl__domain_build_info_setdefault(libxl__gc *gc,
+                                        libxl_domain_build_info *b_info)
+{
+    if (!b_info->device_model_version)
+        b_info->device_model_version =
+            LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+
+    if (!b_info->max_vcpus)
+        b_info->max_vcpus = 1;
+    if (!b_info->cur_vcpus)
+        b_info->cur_vcpus = 1;
+
+    if (!b_info->cpumap.size) {
+        if (libxl_cpumap_alloc(CTX, &b_info->cpumap))
+            return ERROR_NOMEM;
+        libxl_cpumap_set_any(&b_info->cpumap);
+    }
+
+    switch (b_info->type) {
+    case LIBXL_DOMAIN_TYPE_HVM:
+        if (!b_info->u.hvm.boot) {
+            b_info->u.hvm.boot = strdup("cda");
+            if (!b_info->u.hvm.boot) return ERROR_NOMEM;
+        }
+
+        if (b_info->u.hvm.vnc.enable) {
+            if (!b_info->u.hvm.vnc.listen) {
+                b_info->u.hvm.vnc.listen = strdup("127.0.0.1");
+                if (!b_info->u.hvm.vnc.listen) return ERROR_NOMEM;
+            }
+        }
+
+        break;
+    case LIBXL_DOMAIN_TYPE_PV:
+        break;
+    default:
+        LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
                    "invalid domain type %s in create info",
                    libxl_domain_type_to_string(b_info->type));
         return ERROR_INVAL;
@@ -487,6 +513,8 @@ static int do_domain_create(libxl__gc *g
             goto error_out;
     }
 
+    ret = libxl__domain_build_info_setdefault(gc, &d_config->b_info);
+    if (ret) goto error_out;
 
     for (i = 0; i < d_config->num_disks; i++) {
         ret = libxl__device_disk_set_backend(gc, &d_config->disks[i]);
diff -r 53d4e198863a -r af0d151cd2d5 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Mon Feb 13 15:48:39 2012 +0000
+++ b/tools/libxl/libxl_dm.c	Mon Feb 13 15:48:46 2012 +0000
@@ -707,13 +707,12 @@ static int libxl__create_stubdom(libxl__
 
     libxl_uuid_generate(&dm_config.c_info.uuid);
 
-    memset(&dm_config.b_info, 0x00, sizeof(libxl_domain_build_info));
-    dm_config.b_info.type = dm_config.c_info.type;
+    libxl_domain_build_info_init(&dm_config.b_info, &dm_config.c_info);
+
     dm_config.b_info.max_vcpus = 1;
     dm_config.b_info.max_memkb = 32 * 1024;
     dm_config.b_info.target_memkb = dm_config.b_info.max_memkb;
 
-    dm_config.b_info.type = LIBXL_DOMAIN_TYPE_PV;
     dm_config.b_info.u.pv.kernel.path = libxl__abs_path(gc, "ioemu-stubdom.gz",
                                               libxl_xenfirmwaredir_path());
     dm_config.b_info.u.pv.cmdline = libxl__sprintf(gc, " -d %d", guest_domid);
@@ -736,6 +735,8 @@ static int libxl__create_stubdom(libxl__
 
     ret = libxl__domain_create_info_setdefault(gc, &dm_config.c_info);
     if (ret) goto out;
+    ret = libxl__domain_build_info_setdefault(gc, &dm_config.b_info);
+    if (ret) goto out;
 
     libxl__vfb_and_vkb_from_hvm_guest_config(gc, guest_config, &vfb, &vkb);
     dm_config.vfbs = &vfb;
diff -r 53d4e198863a -r af0d151cd2d5 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Mon Feb 13 15:48:39 2012 +0000
+++ b/tools/libxl/libxl_internal.h	Mon Feb 13 15:48:46 2012 +0000
@@ -189,6 +189,8 @@ libxl__ev_xswatch *libxl__watch_slot_con
  */
 _hidden int libxl__domain_create_info_setdefault(libxl__gc *gc,
                                         libxl_domain_create_info *c_info);
+_hidden int libxl__domain_build_info_setdefault(libxl__gc *gc,
+                                        libxl_domain_build_info *b_info);
 
 struct libxl__evgen_domain_death {
     uint32_t domid;
diff -r 53d4e198863a -r af0d151cd2d5 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Mon Feb 13 15:48:39 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Mon Feb 13 15:48:46 2012 +0000
@@ -586,8 +586,7 @@ static void parse_config_data(const char
         exit(1);
     }
 
-    if (libxl_init_build_info(ctx, b_info, c_info))
-        exit(1);
+    libxl_domain_build_info_init(b_info, c_info);
 
     /* the following is the actual config parsing with overriding values in the structures */
     if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
@@ -601,6 +600,11 @@ static void parse_config_data(const char
     if (!xlu_cfg_get_list (config, "cpus", &cpus, 0, 1)) {
         int i, n_cpus = 0;
 
+        if (libxl_cpumap_alloc(ctx, &b_info->cpumap)) {
+            fprintf(stderr, "Unable to allocate cpumap\n");
+            exit(1);
+        }
+
         libxl_cpumap_set_none(&b_info->cpumap);
         while ((buf = xlu_cfg_get_listitem(cpus, n_cpus)) != NULL) {
             i = atoi(buf);
@@ -615,6 +619,11 @@ static void parse_config_data(const char
     else if (!xlu_cfg_get_string (config, "cpus", &buf, 0)) {
         char *buf2 = strdup(buf);
 
+        if (libxl_cpumap_alloc(ctx, &b_info->cpumap)) {
+            fprintf(stderr, "Unable to allocate cpumap\n");
+            exit(1);
+        }
+
         libxl_cpumap_set_none(&b_info->cpumap);
         if (vcpupin_parse(buf2, &b_info->cpumap))
             exit(1);

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 04 of 23] libxl: use an explicit LIBXL_TIMER_MODE_DEFAULT value
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (2 preceding siblings ...)
  2012-02-13 16:52 ` [PATCH 03 of 23] libxl: provide _init and _setdefault for libxl_domain_build_info Ian Campbell
@ 2012-02-13 16:52 ` Ian Campbell
  2012-02-20 19:16   ` Ian Jackson
  2012-02-13 16:52 ` [PATCH 05 of 23] libxl: drop 8M slack for PV guests Ian Campbell
                   ` (18 subsequent siblings)
  22 siblings, 1 reply; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:52 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329148132 0
# Node ID d50d692ae52bca2d409415245e2fcd909259461a
# Parent  af0d151cd2d524f13a13e2fb85e727e8733dcd81
libxl: use an explicit LIBXL_TIMER_MODE_DEFAULT value

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

diff -r af0d151cd2d5 -r d50d692ae52b tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Mon Feb 13 15:48:46 2012 +0000
+++ b/tools/libxl/libxl.h	Mon Feb 13 15:48:52 2012 +0000
@@ -252,7 +252,7 @@ typedef LIBXL_TAILQ_ENTRY(struct libxl_e
 
 typedef struct libxl__ctx libxl_ctx;
 
-#define LIBXL_TIMER_MODE_DEFAULT LIBXL_TIMER_MODE_NO_DELAY_FOR_MISSED_TICKS
+#define LIBXL_TIMER_MODE_DEFAULT -1
 
 #include "_libxl_types.h"
 
diff -r af0d151cd2d5 -r d50d692ae52b tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Mon Feb 13 15:48:46 2012 +0000
+++ b/tools/libxl/libxl_create.c	Mon Feb 13 15:48:52 2012 +0000
@@ -93,7 +93,7 @@ void libxl_domain_build_info_init(libxl_
         b_info->u.hvm.viridian = 0;
         b_info->u.hvm.hpet = 1;
         b_info->u.hvm.vpt_align = 1;
-        b_info->u.hvm.timer_mode = 1;
+        b_info->u.hvm.timer_mode = LIBXL_TIMER_MODE_DEFAULT;
         b_info->u.hvm.nested_hvm = 0;
         b_info->u.hvm.no_incr_generationid = 0;
 
@@ -134,6 +134,10 @@ int libxl__domain_build_info_setdefault(
 
     switch (b_info->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
+        if (b_info->u.hvm.timer_mode == LIBXL_TIMER_MODE_DEFAULT)
+            b_info->u.hvm.timer_mode =
+                LIBXL_TIMER_MODE_NO_DELAY_FOR_MISSED_TICKS;
+
         if (!b_info->u.hvm.boot) {
             b_info->u.hvm.boot = strdup("cda");
             if (!b_info->u.hvm.boot) return ERROR_NOMEM;

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 05 of 23] libxl: drop 8M slack for PV guests
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (3 preceding siblings ...)
  2012-02-13 16:52 ` [PATCH 04 of 23] libxl: use an explicit LIBXL_TIMER_MODE_DEFAULT value Ian Campbell
@ 2012-02-13 16:52 ` Ian Campbell
  2012-02-20 19:15   ` Ian Jackson
  2012-02-13 16:52 ` [PATCH 06 of 23] libxl: allow specification of testidl random seed Ian Campbell
                   ` (17 subsequent siblings)
  22 siblings, 1 reply; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:52 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329148132 0
# Node ID 192eefb9e00e048333b01ee732a1b07071b90e7e
# Parent  d50d692ae52bca2d409415245e2fcd909259461a
libxl: drop 8M slack for PV guests.

As far as I can tell this serves no purpose. I think it relates to the old
8M to "account for backend allocations" which we used to add. This leaves a bit
of unpopulated space in the Pseudo-physical address space which can be used by
backends when mapping foreign memory. However 8M is not representative of that
any more and modern kernels do not operate in this way anyway.

I suspect an argument could be made for removing this from the libxl API
altogether but instead lets just set the overhead to 0.

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

diff -r d50d692ae52b -r 192eefb9e00e tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl_create.c	Mon Feb 13 15:48:52 2012 +0000
@@ -107,7 +107,7 @@ void libxl_domain_build_info_init(libxl_
         b_info->u.hvm.xen_platform_pci = 1;
         break;
     case LIBXL_DOMAIN_TYPE_PV:
-        b_info->u.pv.slack_memkb = 8 * 1024;
+        b_info->u.pv.slack_memkb = 0;
         break;
     default:
         abort();

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 06 of 23] libxl: allow specification of testidl random seed
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (4 preceding siblings ...)
  2012-02-13 16:52 ` [PATCH 05 of 23] libxl: drop 8M slack for PV guests Ian Campbell
@ 2012-02-13 16:52 ` Ian Campbell
  2012-02-20 19:18   ` Ian Jackson
  2012-02-13 16:52 ` [PATCH 07 of 23] libxl: introduce a descriminating default value for memkb fields Ian Campbell
                   ` (16 subsequent siblings)
  22 siblings, 1 reply; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:52 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329148132 0
# Node ID 1ace05e41c39f3620b423056004b709965ff6d80
# Parent  192eefb9e00e048333b01ee732a1b07071b90e7e
libxl: allow specification of testidl random seed.

Useful if you are interested in before/after results of changing the IDL or
generator.

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

diff -r 192eefb9e00e -r 1ace05e41c39 tools/libxl/gentest.py
--- a/tools/libxl/gentest.py	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/gentest.py	Mon Feb 13 15:48:52 2012 +0000
@@ -1,5 +1,6 @@
 #!/usr/bin/python
 
+import os
 import sys
 import re
 import random
@@ -72,7 +73,7 @@ if __name__ == '__main__':
         print >>sys.stderr, "Usage: gentest.py <idl> <implementation>"
         sys.exit(1)
 
-    random.seed()
+    random.seed(os.getenv('LIBXL_TESTIDL_SEED'))
 
     (builtins,types) = idl.parse(sys.argv[1])

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 07 of 23] libxl: introduce a descriminating default value for memkb fields
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (5 preceding siblings ...)
  2012-02-13 16:52 ` [PATCH 06 of 23] libxl: allow specification of testidl random seed Ian Campbell
@ 2012-02-13 16:52 ` Ian Campbell
  2012-02-20 19:24   ` Ian Jackson
  2012-02-13 16:52 ` [PATCH 08 of 23] libxl: disk: use _init/_setdefault Ian Campbell
                   ` (15 subsequent siblings)
  22 siblings, 1 reply; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:52 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329148132 0
# Node ID 2fef3eddec04dd3a56d651d15adcd1f72a201a1e
# Parent  1ace05e41c39f3620b423056004b709965ff6d80
libxl: introduce a descriminating default value for memkb fields.

Consitently make such fields 64 bit values as used by dominfo.

It is not clear if ->video_memkb and/or shadow_memkb shouldn't be part of
->u.hvm but I have not changed that here.

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

diff -r 1ace05e41c39 -r 2fef3eddec04 tools/libxl/gentest.py
--- a/tools/libxl/gentest.py	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/gentest.py	Mon Feb 13 15:48:52 2012 +0000
@@ -197,6 +197,7 @@ static void libxl_string_list_rand_init(
 }
 """)
     for ty in builtins + types:
+        if isinstance(ty, idl.Number): continue
         if ty.typename not in handcoded:
             f.write("static void %s_rand_init(%s);\n" % \
                     (ty.typename,
diff -r 1ace05e41c39 -r 2fef3eddec04 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl.h	Mon Feb 13 15:48:52 2012 +0000
@@ -253,6 +253,7 @@ typedef LIBXL_TAILQ_ENTRY(struct libxl_e
 typedef struct libxl__ctx libxl_ctx;
 
 #define LIBXL_TIMER_MODE_DEFAULT -1
+#define LIBXL_MEMKB_DEFAULT ~0ULL
 
 #include "_libxl_types.h"
 
diff -r 1ace05e41c39 -r 2fef3eddec04 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl_create.c	Mon Feb 13 15:48:52 2012 +0000
@@ -70,19 +70,20 @@ void libxl_domain_build_info_init(libxl_
                                   const libxl_domain_create_info *c_info)
 {
     memset(b_info, '\0', sizeof(*b_info));
-    b_info->max_memkb = 32 * 1024;
-    b_info->target_memkb = b_info->max_memkb;
     b_info->disable_migrate = 0;
     b_info->cpuid = NULL;
-    b_info->shadow_memkb = 0;
     b_info->type = c_info->type;
 
+    b_info->max_memkb = LIBXL_MEMKB_DEFAULT;
+    b_info->target_memkb = LIBXL_MEMKB_DEFAULT;
+    b_info->shadow_memkb = LIBXL_MEMKB_DEFAULT;
+    b_info->video_memkb =  LIBXL_MEMKB_DEFAULT;
+
     b_info->device_model_stubdomain = false;
     b_info->device_model = NULL;
 
     switch (b_info->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
-        b_info->video_memkb = 8 * 1024;
         b_info->u.hvm.firmware = NULL;
         b_info->u.hvm.pae = 1;
         b_info->u.hvm.apic = 1;
@@ -132,8 +133,17 @@ int libxl__domain_build_info_setdefault(
         libxl_cpumap_set_any(&b_info->cpumap);
     }
 
+    if (b_info->max_memkb == LIBXL_MEMKB_DEFAULT)
+        b_info->max_memkb = 32 * 1024;
+    if (b_info->target_memkb == LIBXL_MEMKB_DEFAULT)
+        b_info->target_memkb = b_info->max_memkb;
+
     switch (b_info->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
+        if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
+            b_info->shadow_memkb = 0;
+        if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
+            b_info->video_memkb = 8 * 1024;
         if (b_info->u.hvm.timer_mode == LIBXL_TIMER_MODE_DEFAULT)
             b_info->u.hvm.timer_mode =
                 LIBXL_TIMER_MODE_NO_DELAY_FOR_MISSED_TICKS;
@@ -152,6 +162,10 @@ int libxl__domain_build_info_setdefault(
 
         break;
     case LIBXL_DOMAIN_TYPE_PV:
+        if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
+            b_info->shadow_memkb = 0;
+        if (b_info->u.pv.slack_memkb == LIBXL_MEMKB_DEFAULT)
+            b_info->u.pv.slack_memkb = 0;
         break;
     default:
         LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
diff -r 1ace05e41c39 -r 2fef3eddec04 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl_dom.c	Mon Feb 13 15:48:52 2012 +0000
@@ -129,11 +129,12 @@ int libxl__build_post(libxl__gc *gc, uin
 
     ents = libxl__calloc(gc, 12 + (info->max_vcpus * 2) + 2, sizeof(char *));
     ents[0] = "memory/static-max";
-    ents[1] = libxl__sprintf(gc, "%d", info->max_memkb);
+    ents[1] = libxl__sprintf(gc, "%"PRId64, info->max_memkb);
     ents[2] = "memory/target";
-    ents[3] = libxl__sprintf(gc, "%d", info->target_memkb - info->video_memkb);
+    ents[3] = libxl__sprintf(gc, "%"PRId64,
+                             info->target_memkb - info->video_memkb);
     ents[4] = "memory/videoram";
-    ents[5] = libxl__sprintf(gc, "%d", info->video_memkb);
+    ents[5] = libxl__sprintf(gc, "%"PRId64, info->video_memkb);
     ents[6] = "domid";
     ents[7] = libxl__sprintf(gc, "%d", domid);
     ents[8] = "store/port";
diff -r 1ace05e41c39 -r 2fef3eddec04 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl_types.idl	Mon Feb 13 15:48:52 2012 +0000
@@ -18,6 +18,12 @@ libxl_file_reference = Builtin("file_ref
 libxl_hwcap = Builtin("hwcap", passby=PASS_BY_REFERENCE)
 
 #
+# Specific integer types
+#
+
+MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
+
+#
 # Constants / Enumerations
 #
 
@@ -147,9 +153,9 @@ libxl_dominfo = Struct("dominfo",[
     # Otherwise set to a value guaranteed not to clash with any valid
     # SHUTDOWN_* constant.
     ("shutdown_reason", uint8),
-    ("current_memkb",   uint64),
-    ("shared_memkb", uint64),
-    ("max_memkb",   uint64),
+    ("current_memkb",   MemKB),
+    ("shared_memkb", MemKB),
+    ("max_memkb",   MemKB),
     ("cpu_time",    uint64),
     ("vcpu_max_id", uint32),
     ("vcpu_online", uint32),
@@ -205,10 +211,10 @@ libxl_domain_build_info = Struct("domain
     ("cur_vcpus",       integer),
     ("cpumap",          libxl_cpumap),
     ("tsc_mode",        libxl_tsc_mode),
-    ("max_memkb",       uint32),
-    ("target_memkb",    uint32),
-    ("video_memkb",     uint32),
-    ("shadow_memkb",    uint32),
+    ("max_memkb",       MemKB),
+    ("target_memkb",    MemKB),
+    ("video_memkb",     MemKB),
+    ("shadow_memkb",    MemKB),
     ("disable_migrate", bool),
     ("cpuid",           libxl_cpuid_policy_list),
     ("type",            libxl_domain_type),
@@ -262,7 +268,7 @@ libxl_domain_build_info = Struct("domain
                                        ("xen_platform_pci", bool),
                                        ])),
                  ("pv", Struct(None, [("kernel", libxl_file_reference),
-                                      ("slack_memkb", uint32),
+                                      ("slack_memkb", MemKB),
                                       ("bootloader", string),
                                       ("bootloader_args", libxl_string_list),
                                       ("cmdline", string),
diff -r 1ace05e41c39 -r 2fef3eddec04 tools/libxl/xl_sxp.c
--- a/tools/libxl/xl_sxp.c	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/xl_sxp.c	Mon Feb 13 15:48:52 2012 +0000
@@ -21,6 +21,7 @@
 #include "libxl_osdeps.h"
 
 #include <stdlib.h>
+#include <inttypes.h>
 
 #include "libxl.h"
 #include "libxl_utils.h"
@@ -68,8 +69,8 @@ void printf_info_sexp(int domid, libxl_d
     printf("\t(build_info)\n");
     printf("\t(max_vcpus %d)\n", b_info->max_vcpus);
     printf("\t(tsc_mode %s)\n", libxl_tsc_mode_to_string(b_info->tsc_mode));
-    printf("\t(max_memkb %d)\n", b_info->max_memkb);
-    printf("\t(target_memkb %d)\n", b_info->target_memkb);
+    printf("\t(max_memkb %"PRId64")\n", b_info->max_memkb);
+    printf("\t(target_memkb %"PRId64")\n", b_info->target_memkb);
     printf("\t(nomigrate %d)\n", b_info->disable_migrate);
 
     if (c_info->type == LIBXL_DOMAIN_TYPE_PV && b_info->u.pv.bootloader) {
@@ -88,8 +89,8 @@ void printf_info_sexp(int domid, libxl_d
     case LIBXL_DOMAIN_TYPE_HVM:
         printf("\t\t(hvm\n");
         printf("\t\t\t(firmware %s)\n", b_info->u.hvm.firmware);
-        printf("\t\t\t(video_memkb %d)\n", b_info->video_memkb);
-        printf("\t\t\t(shadow_memkb %d)\n", b_info->shadow_memkb);
+        printf("\t\t\t(video_memkb %"PRId64")\n", b_info->video_memkb);
+        printf("\t\t\t(shadow_memkb %"PRId64")\n", b_info->shadow_memkb);
         printf("\t\t\t(pae %d)\n", b_info->u.hvm.pae);
         printf("\t\t\t(apic %d)\n", b_info->u.hvm.apic);
         printf("\t\t\t(acpi %d)\n", b_info->u.hvm.acpi);

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 08 of 23] libxl: disk: use _init/_setdefault
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (6 preceding siblings ...)
  2012-02-13 16:52 ` [PATCH 07 of 23] libxl: introduce a descriminating default value for memkb fields Ian Campbell
@ 2012-02-13 16:52 ` Ian Campbell
  2012-02-13 16:52 ` [PATCH 09 of 23] libxl: nic: " Ian Campbell
                   ` (14 subsequent siblings)
  22 siblings, 0 replies; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:52 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329148132 0
# Node ID 575cccbd3dc1c74cafb319fe82998c5fa619b428
# Parent  2fef3eddec04dd3a56d651d15adcd1f72a201a1e
libxl: disk: use _init/_setdefault

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

diff -r 2fef3eddec04 -r 575cccbd3dc1 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl.c	Mon Feb 13 15:48:52 2012 +0000
@@ -1185,10 +1185,19 @@ int libxl_vncviewer_exec(libxl_ctx *ctx,
 
 /******************************************************************************/
 
-int libxl_device_disk_init(libxl_ctx *ctx, libxl_device_disk *disk)
+void libxl_device_disk_init(libxl_device_disk *disk)
 {
     memset(disk, 0x00, sizeof(libxl_device_disk));
-    return 0;
+}
+
+int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
+{
+    int rc;
+
+    rc = libxl__device_disk_set_backend(gc, disk);
+    if (rc) return rc;
+
+    return rc;
 }
 
 static int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
@@ -1240,10 +1249,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
     libxl__device device;
     int major, minor, rc;
 
-    rc = libxl__device_disk_set_backend(gc, disk);
-    if (rc) goto out;
-
-    rc = libxl__device_disk_set_backend(gc, disk);
+    rc = libxl__device_disk_setdefault(gc, disk);
     if (rc) goto out;
 
     front = flexarray_make(16, 1);
@@ -1395,7 +1401,7 @@ static void libxl__device_disk_from_xs_b
     unsigned int len;
     char *tmp;
 
-    libxl_device_disk_init(ctx, disk);
+    libxl_device_disk_init(disk);
 
     tmp = xs_read(ctx->xsh, XBT_NULL,
                   libxl__sprintf(gc, "%s/params", be_path), &len);
@@ -1439,7 +1445,7 @@ int libxl_devid_to_device_disk(libxl_ctx
     char *dompath, *path;
     int rc = ERROR_FAIL;
 
-    libxl_device_disk_init(ctx, disk);
+    libxl_device_disk_init(disk);
 
     dompath = libxl__xs_get_dompath(gc, domid);
     if (!dompath) {
@@ -1603,7 +1609,7 @@ char * libxl_device_disk_local_attach(li
     char *ret = NULL;
     int rc;
 
-    rc = libxl__device_disk_set_backend(gc, disk);
+    rc = libxl__device_disk_setdefault(gc, disk);
     if (rc) goto out;
 
     switch (disk->backend) {
diff -r 2fef3eddec04 -r 575cccbd3dc1 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl.h	Mon Feb 13 15:48:52 2012 +0000
@@ -502,7 +502,7 @@ libxl_vminfo * libxl_list_vm(libxl_ctx *
  */
 
 /* Disks */
-int libxl_device_disk_init(libxl_ctx *ctx, libxl_device_disk *disk);
+void libxl_device_disk_init(libxl_device_disk *disk);
 int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk);
 int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t domid,
                              libxl_device_disk *disk,
diff -r 2fef3eddec04 -r 575cccbd3dc1 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl_create.c	Mon Feb 13 15:48:52 2012 +0000
@@ -535,7 +535,7 @@ static int do_domain_create(libxl__gc *g
     if (ret) goto error_out;
 
     for (i = 0; i < d_config->num_disks; i++) {
-        ret = libxl__device_disk_set_backend(gc, &d_config->disks[i]);
+        ret = libxl__device_disk_setdefault(gc, &d_config->disks[i]);
         if (ret) goto error_out;
     }
 
diff -r 2fef3eddec04 -r 575cccbd3dc1 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl_internal.h	Mon Feb 13 15:48:52 2012 +0000
@@ -191,6 +191,8 @@ _hidden int libxl__domain_create_info_se
                                         libxl_domain_create_info *c_info);
 _hidden int libxl__domain_build_info_setdefault(libxl__gc *gc,
                                         libxl_domain_build_info *b_info);
+_hidden int libxl__device_disk_setdefault(libxl__gc *gc,
+                                          libxl_device_disk *disk);
 
 struct libxl__evgen_domain_death {
     uint32_t domid;
diff -r 2fef3eddec04 -r 575cccbd3dc1 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Mon Feb 13 15:48:52 2012 +0000
@@ -384,7 +384,7 @@ static void parse_disk_config_multistrin
 {
     int e;
 
-    libxl_device_disk_init(ctx, disk);
+    libxl_device_disk_init(disk);
 
     if (!*config) {
         *config = xlu_cfg_init(stderr, "command line");

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 09 of 23] libxl: nic: use _init/_setdefault
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (7 preceding siblings ...)
  2012-02-13 16:52 ` [PATCH 08 of 23] libxl: disk: use _init/_setdefault Ian Campbell
@ 2012-02-13 16:52 ` Ian Campbell
  2012-02-13 16:52 ` [PATCH 10 of 23] libxl: vfb/vkb: " Ian Campbell
                   ` (13 subsequent siblings)
  22 siblings, 0 replies; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:52 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329148132 0
# Node ID 69c16a2a8927d81acc0cf0e127a5a4b7a9a1a886
# Parent  575cccbd3dc1c74cafb319fe82998c5fa619b428
libxl: nic: use _init/_setdefault

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

diff -r 575cccbd3dc1 -r 69c16a2a8927 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl.c	Mon Feb 13 15:48:52 2012 +0000
@@ -1676,32 +1676,43 @@ int libxl_device_disk_local_detach(libxl
 }
 
 /******************************************************************************/
-int libxl_device_nic_init(libxl_ctx *ctx, libxl_device_nic *nic)
+void libxl_device_nic_init(libxl_device_nic *nic)
 {
-    const uint8_t *r;
-    libxl_uuid uuid;
-
-    libxl_uuid_generate(&uuid);
-    r = libxl_uuid_bytearray(&uuid);
     memset(nic, '\0', sizeof(*nic));
-
-    nic->backend_domid = 0;
-    nic->devid = -1;
-    nic->mtu = 1492;
-    nic->model = strdup("rtl8139");
-    nic->mac[0] = 0x00;
-    nic->mac[1] = 0x16;
-    nic->mac[2] = 0x3e;
-    nic->mac[3] = r[0] & 0x7f;
-    nic->mac[4] = r[1];
-    nic->mac[5] = r[2];
-    nic->ifname = NULL;
-    nic->bridge = strdup("xenbr0");
-    nic->ip = NULL;
-    if ( asprintf(&nic->script, "%s/vif-bridge",
-               libxl_xen_script_dir_path()) < 0 )
+}
+
+int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic)
+{
+    if (!nic->mtu)
+        nic->mtu = 1492;
+    if (!nic->model) {
+        nic->model = strdup("rtl8139");
+        if (!nic->model) return ERROR_NOMEM;
+    }
+    if (!nic->mac[0] && !nic->mac[1] && !nic->mac[2] &&
+        !nic->mac[3] && !nic->mac[4] && !nic->mac[5]) {
+        const uint8_t *r;
+        libxl_uuid uuid;
+
+        libxl_uuid_generate(&uuid);
+        r = libxl_uuid_bytearray(&uuid);
+
+        nic->mac[0] = 0x00;
+        nic->mac[1] = 0x16;
+        nic->mac[2] = 0x3e;
+        nic->mac[3] = r[0] & 0x7f;
+        nic->mac[4] = r[1];
+        nic->mac[5] = r[2];
+    }
+    if (!nic->bridge) {
+        nic->bridge = strdup("xenbr0");
+        if (!nic->bridge) return ERROR_NOMEM;
+    }
+    if ( !nic->script && asprintf(&nic->script, "%s/vif-bridge",
+                                  libxl_xen_script_dir_path()) < 0 )
         return ERROR_FAIL;
-    nic->nictype = LIBXL_NIC_TYPE_IOEMU;
+    if (!nic->nictype)
+        nic->nictype = LIBXL_NIC_TYPE_IOEMU;
     return 0;
 }
 
@@ -1728,6 +1739,9 @@ int libxl_device_nic_add(libxl_ctx *ctx,
     char *dompath, **l;
     unsigned int nb, rc;
 
+    rc = libxl__device_nic_setdefault(gc, nic);
+    if (rc) goto out;
+
     front = flexarray_make(16, 1);
     if (!front) {
         rc = ERROR_NOMEM;
diff -r 575cccbd3dc1 -r 69c16a2a8927 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl.h	Mon Feb 13 15:48:52 2012 +0000
@@ -528,7 +528,7 @@ char * libxl_device_disk_local_attach(li
 int libxl_device_disk_local_detach(libxl_ctx *ctx, libxl_device_disk *disk);
 
 /* Network Interfaces */
-int libxl_device_nic_init(libxl_ctx *ctx, libxl_device_nic *nic);
+void libxl_device_nic_init(libxl_device_nic *nic);
 int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic);
 int libxl_device_nic_remove(libxl_ctx *ctx, uint32_t domid,
                             libxl_device_nic *nic,
diff -r 575cccbd3dc1 -r 69c16a2a8927 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl_internal.h	Mon Feb 13 15:48:52 2012 +0000
@@ -193,6 +193,7 @@ _hidden int libxl__domain_build_info_set
                                         libxl_domain_build_info *b_info);
 _hidden int libxl__device_disk_setdefault(libxl__gc *gc,
                                           libxl_device_disk *disk);
+_hidden int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic);
 
 struct libxl__evgen_domain_death {
     uint32_t domid;
diff -r 575cccbd3dc1 -r 69c16a2a8927 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Mon Feb 13 15:48:52 2012 +0000
@@ -842,7 +842,7 @@ static void parse_config_data(const char
 
             d_config->vifs = (libxl_device_nic *) realloc(d_config->vifs, sizeof (libxl_device_nic) * (d_config->num_vifs+1));
             nic = d_config->vifs + d_config->num_vifs;
-            CHK_ERRNO( libxl_device_nic_init(ctx, nic) );
+            libxl_device_nic_init(nic);
             nic->devid = d_config->num_vifs;
 
             if (default_vifscript) {
@@ -4602,7 +4602,7 @@ int main_networkattach(int argc, char **
         fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]);
         return 1;
     }
-    libxl_device_nic_init(ctx, &nic);
+    libxl_device_nic_init(&nic);
     for (argv += optind+1, argc -= optind+1; argc > 0; ++argv, --argc) {
         if (MATCH_OPTION("type", *argv, oparg)) {
             if (!strcmp("vif", oparg)) {

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 10 of 23] libxl: vfb/vkb: use _init/_setdefault
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (8 preceding siblings ...)
  2012-02-13 16:52 ` [PATCH 09 of 23] libxl: nic: " Ian Campbell
@ 2012-02-13 16:52 ` Ian Campbell
  2012-02-13 16:52 ` [PATCH 11 of 23] libxl: make libxl_device_console internal Ian Campbell
                   ` (12 subsequent siblings)
  22 siblings, 0 replies; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:52 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329148132 0
# Node ID a72b3dca661fd1a0b40d8252d56017355afe3b38
# Parent  69c16a2a8927d81acc0cf0e127a5a4b7a9a1a886
libxl: vfb/vkb: use _init/_setdefault

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

diff -r 69c16a2a8927 -r a72b3dca661f tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl.c	Mon Feb 13 15:48:52 2012 +0000
@@ -2100,9 +2100,13 @@ out:
 }
 
 /******************************************************************************/
-int libxl_device_vkb_init(libxl_ctx *ctx, libxl_device_vkb *vkb)
+void libxl_device_vkb_init(libxl_device_vkb *vkb)
 {
     memset(vkb, 0x00, sizeof(libxl_device_vkb));
+}
+
+int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb)
+{
     return 0;
 }
 
@@ -2128,6 +2132,9 @@ int libxl_device_vkb_add(libxl_ctx *ctx,
     libxl__device device;
     int rc;
 
+    rc = libxl__device_vkb_setdefault(gc, vkb);
+    if (rc) goto out;
+
     front = flexarray_make(16, 1);
     if (!front) {
         rc = ERROR_NOMEM;
@@ -2205,12 +2212,11 @@ out:
 }
 
 /******************************************************************************/
-int libxl_device_vfb_init(libxl_ctx *ctx, libxl_device_vfb *vfb)
+void libxl_device_vfb_init(libxl_device_vfb *vfb)
 {
     memset(vfb, 0x00, sizeof(libxl_device_vfb));
     vfb->vnc.enable = 1;
     vfb->vnc.passwd = NULL;
-    vfb->vnc.listen = strdup("127.0.0.1");
     vfb->vnc.display = 0;
     vfb->vnc.findunused = 1;
     vfb->keymap = NULL;
@@ -2218,6 +2224,15 @@ int libxl_device_vfb_init(libxl_ctx *ctx
     vfb->sdl.opengl = 0;
     vfb->sdl.display = NULL;
     vfb->sdl.xauthority = NULL;
+}
+
+int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb)
+{
+    if (!vfb->vnc.listen) {
+        vfb->vnc.listen = strdup("127.0.0.1");
+        if (!vfb->vnc.listen) return ERROR_NOMEM;
+    }
+
     return 0;
 }
 
@@ -2242,6 +2257,9 @@ int libxl_device_vfb_add(libxl_ctx *ctx,
     libxl__device device;
     int rc;
 
+    rc = libxl__device_vfb_setdefault(gc, vfb);
+    if (rc) goto out;
+
     front = flexarray_make(16, 1);
     if (!front) {
         rc = ERROR_NOMEM;
diff -r 69c16a2a8927 -r a72b3dca661f tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl.h	Mon Feb 13 15:48:52 2012 +0000
@@ -540,7 +540,7 @@ int libxl_device_nic_getinfo(libxl_ctx *
                               libxl_device_nic *nic, libxl_nicinfo *nicinfo);
 
 /* Keyboard */
-int libxl_device_vkb_init(libxl_ctx *ctx, libxl_device_vkb *vkb);
+void libxl_device_vkb_init(libxl_device_vkb *vkb);
 int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb);
 int libxl_device_vkb_remove(libxl_ctx *ctx, uint32_t domid,
                             libxl_device_vkb *vkb,
@@ -548,7 +548,7 @@ int libxl_device_vkb_remove(libxl_ctx *c
 int libxl_device_vkb_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb);
 
 /* Framebuffer */
-int libxl_device_vfb_init(libxl_ctx *ctx, libxl_device_vfb *vfb);
+void libxl_device_vfb_init(libxl_device_vfb *vfb);
 int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb);
 int libxl_device_vfb_remove(libxl_ctx *ctx, uint32_t domid,
                             libxl_device_vfb *vfb,
diff -r 69c16a2a8927 -r a72b3dca661f tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl_create.c	Mon Feb 13 15:48:52 2012 +0000
@@ -594,9 +594,7 @@ static int do_domain_create(libxl__gc *g
         libxl__device_console_add(gc, domid, &console, &state);
         libxl_device_console_dispose(&console);
 
-        ret = libxl_device_vkb_init(ctx, &vkb);
-        if ( ret )
-            goto error_out;
+        libxl_device_vkb_init(&vkb);
         libxl_device_vkb_add(ctx, domid, &vkb);
         libxl_device_vkb_dispose(&vkb);
 
diff -r 69c16a2a8927 -r a72b3dca661f tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl_dm.c	Mon Feb 13 15:48:52 2012 +0000
@@ -610,8 +610,8 @@ static int libxl__vfb_and_vkb_from_hvm_g
     if (b_info->type != LIBXL_DOMAIN_TYPE_HVM)
         return ERROR_INVAL;
 
-    memset(vfb, 0x00, sizeof(libxl_device_vfb));
-    memset(vkb, 0x00, sizeof(libxl_device_vkb));
+    libxl_device_vfb_init(vfb);
+    libxl_device_vkb_init(vkb);
 
     vfb->backend_domid = 0;
     vfb->devid = 0;
diff -r 69c16a2a8927 -r a72b3dca661f tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl_internal.h	Mon Feb 13 15:48:52 2012 +0000
@@ -194,6 +194,8 @@ _hidden int libxl__domain_build_info_set
 _hidden int libxl__device_disk_setdefault(libxl__gc *gc,
                                           libxl_device_disk *disk);
 _hidden int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic);
+_hidden int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb);
+_hidden int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb);
 
 struct libxl__evgen_domain_death {
     uint32_t domid;
diff -r 69c16a2a8927 -r a72b3dca661f tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Mon Feb 13 15:48:52 2012 +0000
@@ -933,12 +933,12 @@ skip:
 
             d_config->vfbs = (libxl_device_vfb *) realloc(d_config->vfbs, sizeof(libxl_device_vfb) * (d_config->num_vfbs + 1));
             vfb = d_config->vfbs + d_config->num_vfbs;
-            libxl_device_vfb_init(ctx, vfb);
+            libxl_device_vfb_init(vfb);
             vfb->devid = d_config->num_vfbs;
 
             d_config->vkbs = (libxl_device_vkb *) realloc(d_config->vkbs, sizeof(libxl_device_vkb) * (d_config->num_vkbs + 1));
             vkb = d_config->vkbs + d_config->num_vkbs;
-            libxl_device_vkb_init(ctx, vkb);
+            libxl_device_vkb_init(vkb);
             vkb->devid = d_config->num_vkbs;
 
             p = strtok(buf2, ",");

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 11 of 23] libxl: make libxl_device_console internal
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (9 preceding siblings ...)
  2012-02-13 16:52 ` [PATCH 10 of 23] libxl: vfb/vkb: " Ian Campbell
@ 2012-02-13 16:52 ` Ian Campbell
  2012-02-20 19:20   ` Ian Jackson
  2012-02-13 16:52 ` [PATCH 12 of 23] libxl: pci: use _init/_setdefault Ian Campbell
                   ` (11 subsequent siblings)
  22 siblings, 1 reply; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:52 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329148132 0
# Node ID b2934eb211360cdc6f996940410fd82d435ca059
# Parent  a72b3dca661fd1a0b40d8252d56017355afe3b38
libxl: make libxl_device_console internal

consoles are not directly exposed to users of the library and there are no API
functions for manipluating them (only the console_exec function). Rather than
commit to a particular API now make the type internal.

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

diff -r a72b3dca661f -r b2934eb21136 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl.c	Mon Feb 13 15:48:52 2012 +0000
@@ -2022,7 +2022,7 @@ int libxl_device_nic_getinfo(libxl_ctx *
 
 /******************************************************************************/
 int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
-                              libxl_device_console *console,
+                              libxl__device_console *console,
                               libxl__domain_build_state *state)
 {
     flexarray_t *front;
@@ -2069,7 +2069,7 @@ int libxl__device_console_add(libxl__gc 
     flexarray_append(front, "limit");
     flexarray_append(front, libxl__sprintf(gc, "%d", LIBXL_XENCONSOLE_LIMIT));
     flexarray_append(front, "type");
-    if (console->consback == LIBXL_CONSOLE_BACKEND_XENCONSOLED)
+    if (console->consback == LIBXL__CONSOLE_BACKEND_XENCONSOLED)
         flexarray_append(front, "xenconsoled");
     else
         flexarray_append(front, "ioemu");
diff -r a72b3dca661f -r b2934eb21136 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl_create.c	Mon Feb 13 15:48:52 2012 +0000
@@ -176,17 +176,16 @@ int libxl__domain_build_info_setdefault(
     return 0;
 }
 
-static int init_console_info(libxl_device_console *console, int dev_num)
+static int init_console_info(libxl__device_console *console, int dev_num)
 {
-    memset(console, 0x00, sizeof(libxl_device_console));
+    memset(console, 0x00, sizeof(libxl__device_console));
     console->devid = dev_num;
-    console->consback = LIBXL_CONSOLE_BACKEND_XENCONSOLED;
+    console->consback = LIBXL__CONSOLE_BACKEND_XENCONSOLED;
     console->output = strdup("pty");
-    if ( NULL == console->output )
+    if (!console->output)
         return ERROR_NOMEM;
     return 0;
 }
-
 int libxl__domain_build(libxl__gc *gc,
                         libxl_domain_build_info *info,
                         uint32_t domid,
@@ -585,14 +584,14 @@ static int do_domain_create(libxl__gc *g
     switch (d_config->c_info.type) {
     case LIBXL_DOMAIN_TYPE_HVM:
     {
-        libxl_device_console console;
+        libxl__device_console console;
         libxl_device_vkb vkb;
 
         ret = init_console_info(&console, 0);
         if ( ret )
             goto error_out;
         libxl__device_console_add(gc, domid, &console, &state);
-        libxl_device_console_dispose(&console);
+        libxl__device_console_dispose(&console);
 
         libxl_device_vkb_init(&vkb);
         libxl_device_vkb_add(ctx, domid, &vkb);
@@ -610,7 +609,7 @@ static int do_domain_create(libxl__gc *g
     case LIBXL_DOMAIN_TYPE_PV:
     {
         int need_qemu = 0;
-        libxl_device_console console;
+        libxl__device_console console;
 
         for (i = 0; i < d_config->num_vfbs; i++) {
             libxl_device_vfb_add(ctx, domid, &d_config->vfbs[i]);
@@ -626,10 +625,10 @@ static int do_domain_create(libxl__gc *g
                 d_config->num_disks, &d_config->disks[0]);
 
         if (need_qemu)
-             console.consback = LIBXL_CONSOLE_BACKEND_IOEMU;
+             console.consback = LIBXL__CONSOLE_BACKEND_IOEMU;
 
         libxl__device_console_add(gc, domid, &console, &state);
-        libxl_device_console_dispose(&console);
+        libxl__device_console_dispose(&console);
 
         if (need_qemu) {
             libxl__create_xenpv_qemu(gc, domid, d_config, &state, &dm_starting);
diff -r a72b3dca661f -r b2934eb21136 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl_dm.c	Mon Feb 13 15:48:52 2012 +0000
@@ -682,7 +682,7 @@ static int libxl__create_stubdom(libxl__
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     int i, num_console = STUBDOM_SPECIAL_CONSOLES, ret;
-    libxl_device_console *console;
+    libxl__device_console *console;
     libxl_domain_config dm_config;
     libxl_device_vfb vfb;
     libxl_device_vkb vkb;
@@ -814,7 +814,7 @@ retry_transaction:
     if (guest_config->b_info.u.hvm.serial)
         num_console++;
 
-    console = libxl__calloc(gc, num_console, sizeof(libxl_device_console));
+    console = libxl__calloc(gc, num_console, sizeof(libxl__device_console));
     if (!console) {
         ret = ERROR_NOMEM;
         goto out_free;
@@ -822,7 +822,7 @@ retry_transaction:
 
     for (i = 0; i < num_console; i++) {
         console[i].devid = i;
-        console[i].consback = LIBXL_CONSOLE_BACKEND_IOEMU;
+        console[i].consback = LIBXL__CONSOLE_BACKEND_IOEMU;
         /* STUBDOM_CONSOLE_LOGGING (console 0) is for minios logging
          * STUBDOM_CONSOLE_SAVE (console 1) is for writing the save file
          * STUBDOM_CONSOLE_RESTORE (console 2) is for reading the save file
@@ -1084,7 +1084,7 @@ out:
 }
 
 int libxl__need_xenpv_qemu(libxl__gc *gc,
-        int nr_consoles, libxl_device_console *consoles,
+        int nr_consoles, libxl__device_console *consoles,
         int nr_vfbs, libxl_device_vfb *vfbs,
         int nr_disks, libxl_device_disk *disks)
 {
@@ -1096,7 +1096,7 @@ int libxl__need_xenpv_qemu(libxl__gc *gc
     }
 
     for (i = 0; i < nr_consoles; i++) {
-        if (consoles[i].consback == LIBXL_CONSOLE_BACKEND_IOEMU) {
+        if (consoles[i].consback == LIBXL__CONSOLE_BACKEND_IOEMU) {
             ret = 1;
             goto out;
         }
diff -r a72b3dca661f -r b2934eb21136 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl_internal.h	Mon Feb 13 15:48:52 2012 +0000
@@ -661,7 +661,7 @@ _hidden int libxl__device_disk_dev_numbe
                                           int *pdisk, int *ppartition);
 
 _hidden int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
-                                      libxl_device_console *console,
+                                      libxl__device_console *console,
                                       libxl__domain_build_state *state);
 
 _hidden int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
@@ -904,7 +904,7 @@ _hidden int libxl__create_xenpv_qemu(lib
                               libxl__domain_build_state *state,
                               libxl__spawner_starting **starting_r);
 _hidden int libxl__need_xenpv_qemu(libxl__gc *gc,
-        int nr_consoles, libxl_device_console *consoles,
+        int nr_consoles, libxl__device_console *consoles,
         int nr_vfbs, libxl_device_vfb *vfbs,
         int nr_disks, libxl_device_disk *disks);
   /* Caller must either: pass starting_r==0, or on successful
diff -r a72b3dca661f -r b2934eb21136 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl_types.idl	Mon Feb 13 15:48:52 2012 +0000
@@ -42,11 +42,6 @@ libxl_console_type = Enumeration("consol
     (2, "PV"),
     ])
 
-libxl_console_backend = Enumeration("console_backend", [
-    (1, "XENCONSOLED"),
-    (2, "IOEMU"),
-    ])
-
 libxl_disk_format = Enumeration("disk_format", [
     (0, "UNKNOWN"),
     (1, "QCOW"),
@@ -295,13 +290,6 @@ libxl_device_vkb = Struct("device_vkb", 
     ("devid", integer),
     ])
 
-libxl_device_console = Struct("device_console", [
-    ("backend_domid", libxl_domid),
-    ("devid", integer),
-    ("consback", libxl_console_backend),
-    ("output", string),
-    ])
-
 libxl_device_disk = Struct("device_disk", [
     ("backend_domid", libxl_domid),
     ("pdev_path", string),
diff -r a72b3dca661f -r b2934eb21136 tools/libxl/libxl_types_internal.idl
--- a/tools/libxl/libxl_types_internal.idl	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl_types_internal.idl	Mon Feb 13 15:48:52 2012 +0000
@@ -1,5 +1,7 @@
 namespace("libxl__")
 
+libxl_domid = Builtin("domid", namespace="libxl_", json_fn = "yajl_gen_integer")
+
 libxl__qmp_message_type = Enumeration("qmp_message_type", [
     (1, "QMP"),
     (2, "return"),
@@ -17,3 +19,15 @@ libxl__device_kind = Enumeration("device
     (6, "VKBD"),
     (7, "CONSOLE"),
     ])
+
+libxl__console_backend = Enumeration("console_backend", [
+    (1, "XENCONSOLED"),
+    (2, "IOEMU"),
+    ])
+
+libxl__device_console = Struct("device_console", [
+    ("backend_domid", libxl_domid),
+    ("devid", integer),
+    ("consback", libxl__console_backend),
+    ("output", string),
+    ])

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 12 of 23] libxl: pci: use _init/_setdefault
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (10 preceding siblings ...)
  2012-02-13 16:52 ` [PATCH 11 of 23] libxl: make libxl_device_console internal Ian Campbell
@ 2012-02-13 16:52 ` Ian Campbell
  2012-02-13 16:52 ` [PATCH 13 of 23] libxl: add new "defbool" built in type Ian Campbell
                   ` (10 subsequent siblings)
  22 siblings, 0 replies; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:52 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329148133 0
# Node ID cb563f911986275f93ea8dd003b69d44d8745adf
# Parent  b2934eb211360cdc6f996940410fd82d435ca059
libxl: pci: use _init/_setdefault

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

diff -r b2934eb21136 -r cb563f911986 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl.h	Mon Feb 13 15:48:53 2012 +0000
@@ -556,7 +556,7 @@ int libxl_device_vfb_remove(libxl_ctx *c
 int libxl_device_vfb_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb);
 
 /* PCI Passthrough */
-int libxl_device_pci_init(libxl_ctx *ctx, libxl_device_pci *pci);
+void libxl_device_pci_init(libxl_device_pci *pci);
 int libxl_device_pci_add(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
 int libxl_device_pci_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
 int libxl_device_pci_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
diff -r b2934eb21136 -r cb563f911986 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl_internal.h	Mon Feb 13 15:48:53 2012 +0000
@@ -196,6 +196,7 @@ _hidden int libxl__device_disk_setdefaul
 _hidden int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic);
 _hidden int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb);
 _hidden int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb);
+_hidden int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci);
 
 struct libxl__evgen_domain_death {
     uint32_t domid;
diff -r b2934eb21136 -r cb563f911986 tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/libxl_pci.c	Mon Feb 13 15:48:53 2012 +0000
@@ -765,6 +765,16 @@ static int libxl__device_pci_reset(libxl
     return -1;
 }
 
+void libxl_device_pci_init(libxl_device_pci *pci)
+{
+    memset(pci, '\0', sizeof(*pci));
+}
+
+int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci)
+{
+    return 0;
+}
+
 int libxl_device_pci_add(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev)
 {
     GC_INIT(ctx);
@@ -782,6 +792,9 @@ int libxl__device_pci_add(libxl__gc *gc,
     int num_assigned, i, rc;
     int stubdomid = 0;
 
+    rc = libxl__device_pci_setdefault(gc, pcidev);
+    if (rc) goto out;
+
     rc = get_all_assigned_devices(gc, &assigned, &num_assigned);
     if ( rc ) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot determine if device is assigned, refusing to continue");
diff -r b2934eb21136 -r cb563f911986 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Mon Feb 13 15:48:52 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Mon Feb 13 15:48:53 2012 +0000
@@ -1014,7 +1014,7 @@ skip_vfb:
 
             d_config->pcidevs = (libxl_device_pci *) realloc(d_config->pcidevs, sizeof (libxl_device_pci) * (d_config->num_pcidevs + 1));
             pcidev = d_config->pcidevs + d_config->num_pcidevs;
-            memset(pcidev, 0x00, sizeof(libxl_device_pci));
+            libxl_device_pci_init(pcidev);
 
             pcidev->msitranslate = pci_msitranslate;
             pcidev->power_mgmt = pci_power_mgmt;

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 13 of 23] libxl: add new "defbool" built in type
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (11 preceding siblings ...)
  2012-02-13 16:52 ` [PATCH 12 of 23] libxl: pci: use _init/_setdefault Ian Campbell
@ 2012-02-13 16:52 ` Ian Campbell
  2012-02-20 19:14   ` Ian Jackson
  2012-02-13 16:53 ` [PATCH 14 of 23] libxl: make boolean members of libxl_domain_create_info into libxl_defbool Ian Campbell
                   ` (9 subsequent siblings)
  22 siblings, 1 reply; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:52 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329148133 0
# Node ID 41738bf7d28dd74018a160712da69bfc02b7f4b5
# Parent  cb563f911986275f93ea8dd003b69d44d8745adf
libxl: add new "defbool" built in type.

This type is a but like a "boolean" but with a third state "default" (so really
I suppose it's a tristate).

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff -r cb563f911986 -r 41738bf7d28d tools/libxl/gentest.py
--- a/tools/libxl/gentest.py	Mon Feb 13 15:48:53 2012 +0000
+++ b/tools/libxl/gentest.py	Mon Feb 13 15:48:53 2012 +0000
@@ -20,7 +20,8 @@ def randomize_case(s):
 def randomize_enum(e):
     return random.choice([v.name for v in e.values])
 
-handcoded = ["libxl_cpumap", "libxl_key_value_list",
+handcoded = ["libxl_defbool", # Temp until a user appears in the next patch
+             "libxl_cpumap", "libxl_key_value_list",
              "libxl_cpuid_policy_list", "libxl_file_reference",
              "libxl_string_list"]
 
@@ -55,6 +56,8 @@ def gen_rand_init(ty, v, indent = "    "
               ty.pass_arg(v, parent is None))
     elif ty.typename in ["bool"]:
         s += "%s = rand() %% 2;\n" % v
+    elif ty.typename in ["libxl_defbool"]:
+        s += "libxl_defbool_set(%s, !!rand() %% 1);\n" % v
     elif ty.typename in ["char *"]:
         s += "%s = rand_str();\n" % v
     elif ty.private:
diff -r cb563f911986 -r 41738bf7d28d tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Feb 13 15:48:53 2012 +0000
+++ b/tools/libxl/libxl.c	Mon Feb 13 15:48:53 2012 +0000
@@ -184,6 +184,47 @@ void libxl_key_value_list_dispose(libxl_
     free(kvl);
 }
 
+#define LIBXL__DEFBOOL_DEFAULT (0)
+#define LIBXL__DEFBOOL_FALSE (-1)
+#define LIBXL__DEFBOOL_TRUE (1)
+
+void libxl_defbool_set(libxl_defbool *db, bool b)
+{
+    db->val = b ? LIBXL__DEFBOOL_TRUE : LIBXL__DEFBOOL_FALSE;
+}
+
+void libxl_defbool_unset(libxl_defbool *db)
+{
+    db->val = LIBXL__DEFBOOL_DEFAULT;
+}
+
+bool libxl_defbool_is_default(libxl_defbool db)
+{
+    return !db.val;
+}
+
+void libxl_defbool_setdefault(libxl_defbool *db, bool b)
+{
+    if (libxl_defbool_is_default(*db))
+        libxl_defbool_set(db, b);
+}
+
+bool libxl_defbool_val(libxl_defbool db)
+{
+    assert(!libxl_defbool_is_default(db));
+    return db.val > 0;
+}
+
+const char *libxl_defbool_to_string(libxl_defbool b)
+{
+    if (b.val < 0)
+        return "False";
+    else if (b.val > 0)
+        return "True";
+    else
+        return "<default>";
+}
+
 /******************************************************************************/
 
 
diff -r cb563f911986 -r 41738bf7d28d tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Mon Feb 13 15:48:53 2012 +0000
+++ b/tools/libxl/libxl.h	Mon Feb 13 15:48:53 2012 +0000
@@ -250,6 +250,30 @@ typedef struct {
 struct libxl_event;
 typedef LIBXL_TAILQ_ENTRY(struct libxl_event) libxl_ev_link;
 
+/*
+ * A boolean variable with an explicit default state.
+ *
+ * Users should treat this struct as opaque and use the following
+ * defined macros and accessor functions.
+ *
+ * To allow users of the library to naively select all defaults this
+ * state is represented as 0. False is < 0 and True is > 0.
+ */
+typedef struct {
+    int val;
+} libxl_defbool;
+
+void libxl_defbool_set(libxl_defbool *db, bool b);
+/* Resets to default */
+void libxl_defbool_unset(libxl_defbool *db);
+/* Sets db only if it is currently == default */
+void libxl_defbool_setdefault(libxl_defbool *db, bool b);
+bool libxl_defbool_is_default(libxl_defbool db);
+/* db must not be == default */
+bool libxl_defbool_val(libxl_defbool db);
+
+const char *libxl_defbool_to_string(libxl_defbool b);
+
 typedef struct libxl__ctx libxl_ctx;
 
 #define LIBXL_TIMER_MODE_DEFAULT -1
diff -r cb563f911986 -r 41738bf7d28d tools/libxl/libxl_json.c
--- a/tools/libxl/libxl_json.c	Mon Feb 13 15:48:53 2012 +0000
+++ b/tools/libxl/libxl_json.c	Mon Feb 13 15:48:53 2012 +0000
@@ -85,6 +85,12 @@ yajl_gen_status libxl__yajl_gen_enum(yaj
 /*
  * YAJL generators for builtin libxl types.
  */
+yajl_gen_status libxl_defbool_gen_json(yajl_gen hand,
+                                       libxl_defbool *db)
+{
+    return libxl__yajl_gen_asciiz(hand, libxl_defbool_to_string(*db));
+}
+
 yajl_gen_status libxl_uuid_gen_json(yajl_gen hand,
                                     libxl_uuid *uuid)
 {
diff -r cb563f911986 -r 41738bf7d28d tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Mon Feb 13 15:48:53 2012 +0000
+++ b/tools/libxl/libxl_types.idl	Mon Feb 13 15:48:53 2012 +0000
@@ -5,6 +5,8 @@
 
 namespace("libxl_")
 
+libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE)
+
 libxl_domid = Builtin("domid", json_fn = "yajl_gen_integer", autogenerate_json = False)
 libxl_uuid = Builtin("uuid", passby=PASS_BY_REFERENCE)
 libxl_mac = Builtin("mac", passby=PASS_BY_REFERENCE)
diff -r cb563f911986 -r 41738bf7d28d tools/libxl/libxlu_cfg.c
--- a/tools/libxl/libxlu_cfg.c	Mon Feb 13 15:48:53 2012 +0000
+++ b/tools/libxl/libxlu_cfg.c	Mon Feb 13 15:48:53 2012 +0000
@@ -237,6 +237,17 @@ int xlu_cfg_get_long(const XLU_Config *c
     return 0;
 }
 
+int xlu_cfg_get_defbool(const XLU_Config *cfg, const char *n, libxl_defbool *b,
+                     int dont_warn)
+{
+    int ret;
+    long l;
+
+    ret = xlu_cfg_get_long(cfg, n, &l, dont_warn);
+    if (ret) return ret;
+    libxl_defbool_set(b, !!l);
+    return 0;
+}
 
 int xlu_cfg_get_list(const XLU_Config *cfg, const char *n,
                      XLU_ConfigList **list_r, int *entries_r, int dont_warn) {
diff -r cb563f911986 -r 41738bf7d28d tools/libxl/libxlutil.h
--- a/tools/libxl/libxlutil.h	Mon Feb 13 15:48:53 2012 +0000
+++ b/tools/libxl/libxlutil.h	Mon Feb 13 15:48:53 2012 +0000
@@ -52,6 +52,8 @@ int xlu_cfg_replace_string(const XLU_Con
                            char **value_r, int dont_warn);
 int xlu_cfg_get_long(const XLU_Config*, const char *n, long *value_r,
                      int dont_warn);
+int xlu_cfg_get_defbool(const XLU_Config*, const char *n, libxl_defbool *b,
+                     int dont_warn);
 
 int xlu_cfg_get_list(const XLU_Config*, const char *n,
                      XLU_ConfigList **list_r /* may be 0 */,
diff -r cb563f911986 -r 41738bf7d28d tools/ocaml/libs/xl/genwrap.py
--- a/tools/ocaml/libs/xl/genwrap.py	Mon Feb 13 15:48:53 2012 +0000
+++ b/tools/ocaml/libs/xl/genwrap.py	Mon Feb 13 15:48:53 2012 +0000
@@ -10,6 +10,7 @@ builtins = {
     "int":                  ("int",                    "%(c)s = Int_val(%(o)s)",            "Val_int(%(c)s)"  ),
     "char *":               ("string",                 "%(c)s = dup_String_val(gc, %(o)s)", "caml_copy_string(%(c)s)"),
     "libxl_domid":          ("domid",                  "%(c)s = Int_val(%(o)s)",            "Val_int(%(c)s)"  ),
+    "libxl_defbool":        ("bool option",            "%(c)s = Defbool_val(%(o)s)",        "Val_defbool(%(c)s)" ),
     "libxl_uuid":           ("int array",              "Uuid_val(gc, lg, &%(c)s, %(o)s)",   "Val_uuid(&%(c)s)"),
     "libxl_key_value_list": ("(string * string) list", None,                                None),
     "libxl_mac":            ("int array",              "Mac_val(gc, lg, &%(c)s, %(o)s)",    "Val_mac(&%(c)s)"),
diff -r cb563f911986 -r 41738bf7d28d tools/ocaml/libs/xl/xenlight_stubs.c
--- a/tools/ocaml/libs/xl/xenlight_stubs.c	Mon Feb 13 15:48:53 2012 +0000
+++ b/tools/ocaml/libs/xl/xenlight_stubs.c	Mon Feb 13 15:48:53 2012 +0000
@@ -195,6 +195,33 @@ static int Uuid_val(caml_gc *gc, struct 
 	CAMLreturn(0);
 }
 
+static value Val_defbool(libxl_defbool c_val)
+{
+	CAMLparam0();
+	CAMLlocal1(v);
+
+	if (libxl_defbool_is_default(c_val))
+		v = Val_none;
+	else {
+		bool b = libxl_defbool_val(c_val);
+		v = Val_some(b ? Val_bool(true) : Val_bool(false));
+	}
+	CAMLreturn(v);
+}
+
+static libxl_defbool Defbool_val(value v)
+{
+	CAMLparam1(v);
+	libxl_defbool db;
+	if (v == Val_none)
+		libxl_defbool_unset(&db);
+	else {
+		bool b = Bool_val(Some_val(v));
+		libxl_defbool_set(&db, b);
+	}
+	return db;
+}
+
 static value Val_hwcap(libxl_hwcap *c_val)
 {
 	CAMLparam0();
diff -r cb563f911986 -r 41738bf7d28d tools/python/genwrap.py
--- a/tools/python/genwrap.py	Mon Feb 13 15:48:53 2012 +0000
+++ b/tools/python/genwrap.py	Mon Feb 13 15:48:53 2012 +0000
@@ -4,11 +4,13 @@ import sys,os
 
 import idl
 
-(TYPE_BOOL, TYPE_INT, TYPE_UINT, TYPE_STRING, TYPE_AGGREGATE) = range(5)
+(TYPE_DEFBOOL, TYPE_BOOL, TYPE_INT, TYPE_UINT, TYPE_STRING, TYPE_AGGREGATE) = range(6)
 
 def py_type(ty):
     if ty == idl.bool:
         return TYPE_BOOL
+    if ty.typename == "libxl_defbool":
+        return TYPE_DEFBOOL
     if isinstance(ty, idl.Enumeration):
         return TYPE_UINT
     if isinstance(ty, idl.Number):
@@ -44,6 +46,8 @@ def py_decls(ty):
         for f in ty.fields:
             if py_type(f.type) is not None:
                 continue
+            if py_type(f.type) == TYPE_DEFBOOL:
+                continue
             if ty.marshal_out():
                 l.append('_hidden PyObject *attrib__%s_get(%s *%s);'%(\
                     fsanitize(f.type.typename), f.type.typename, f.name))
@@ -62,6 +66,8 @@ def py_attrib_get(ty, f):
         l.append('    ret = (self->obj.%s) ? Py_True : Py_False;'%f.name)
         l.append('    Py_INCREF(ret);')
         l.append('    return ret;')
+    elif t == TYPE_DEFBOOL:
+        l.append('    return genwrap__defbool_get(&self->obj.%s);'%f.name)
     elif t == TYPE_INT:
         l.append('    return genwrap__ll_get(self->obj.%s);'%f.name)
     elif t == TYPE_UINT:
@@ -85,6 +91,8 @@ def py_attrib_set(ty, f):
     if t == TYPE_BOOL:
         l.append('    self->obj.%s = (NULL == v || Py_None == v || Py_False == v) ? 0 : 1;'%f.name)
         l.append('    return 0;')
+    elif t == TYPE_DEFBOOL:
+        l.append('    return genwrap__defbool_set(v, &self->obj.%s);'%f.name)
     elif t == TYPE_UINT or t == TYPE_INT:
         l.append('    %slong long tmp;'%(t == TYPE_UINT and 'unsigned ' or ''))
         l.append('    int ret;')
@@ -275,6 +283,8 @@ _hidden PyObject *genwrap__ull_get(unsig
 _hidden int genwrap__ull_set(PyObject *v, unsigned long long *val, unsigned long long mask);
 _hidden PyObject *genwrap__ll_get(long long val);
 _hidden int genwrap__ll_set(PyObject *v, long long *val, long long mask);
+_hidden PyObject *genwrap__defbool_get(libxl_defbool *db);
+_hidden int genwrap__defbool_set(PyObject *v, libxl_defbool *db);
 
 """ % " ".join(sys.argv))
     for ty in [ty for ty in types if isinstance(ty, idl.Aggregate)]:
diff -r cb563f911986 -r 41738bf7d28d tools/python/xen/lowlevel/xl/xl.c
--- a/tools/python/xen/lowlevel/xl/xl.c	Mon Feb 13 15:48:53 2012 +0000
+++ b/tools/python/xen/lowlevel/xl/xl.c	Mon Feb 13 15:48:53 2012 +0000
@@ -156,6 +156,21 @@ int genwrap__ll_set(PyObject *v, long lo
     return 0;
 }
 
+PyObject *genwrap__defbool_get(libxl_defbool *db)
+{
+    PyObject *ret;
+    ret = libxl_defbool_val(*db) ? Py_True : Py_False;
+    Py_INCREF(ret);
+    return ret;
+}
+
+int genwrap__defbool_set(PyObject *v, libxl_defbool *db)
+{
+    bool val = !(NULL == v || Py_None == v || Py_False == v);
+    libxl_defbool_set(db, val);
+    return 0;
+}
+
 static int fixed_bytearray_set(PyObject *v, uint8_t *ptr, size_t len)
 {
     char *tmp;

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 14 of 23] libxl: make boolean members of libxl_domain_create_info into libxl_defbool
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (12 preceding siblings ...)
  2012-02-13 16:52 ` [PATCH 13 of 23] libxl: add new "defbool" built in type Ian Campbell
@ 2012-02-13 16:53 ` Ian Campbell
  2012-02-20 19:18   ` Ian Jackson
  2012-02-13 16:53 ` [PATCH 15 " Ian Campbell
                   ` (8 subsequent siblings)
  22 siblings, 1 reply; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:53 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329148140 0
# Node ID a20f15f1889ba1dda88d64509694818a0793cf8a
# Parent  41738bf7d28dd74018a160712da69bfc02b7f4b5
libxl: make boolean members of libxl_domain_create_info into libxl_defbool

This allows them to be set via the _init/_setdefault methods.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff -r 41738bf7d28d -r a20f15f1889b tools/libxl/gentest.py
--- a/tools/libxl/gentest.py	Mon Feb 13 15:48:53 2012 +0000
+++ b/tools/libxl/gentest.py	Mon Feb 13 15:49:00 2012 +0000
@@ -20,8 +20,7 @@ def randomize_case(s):
 def randomize_enum(e):
     return random.choice([v.name for v in e.values])
 
-handcoded = ["libxl_defbool", # Temp until a user appears in the next patch
-             "libxl_cpumap", "libxl_key_value_list",
+handcoded = ["libxl_cpumap", "libxl_key_value_list",
              "libxl_cpuid_policy_list", "libxl_file_reference",
              "libxl_string_list"]
 
diff -r 41738bf7d28d -r a20f15f1889b tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Mon Feb 13 15:48:53 2012 +0000
+++ b/tools/libxl/libxl_create.c	Mon Feb 13 15:49:00 2012 +0000
@@ -53,8 +53,6 @@ void libxl_domain_config_dispose(libxl_d
 void libxl_domain_create_info_init(libxl_domain_create_info *c_info)
 {
     memset(c_info, '\0', sizeof(*c_info));
-    c_info->hap = 1;
-    c_info->oos = 1;
 }
 
 int libxl__domain_create_info_setdefault(libxl__gc *gc,
@@ -63,6 +61,11 @@ int libxl__domain_create_info_setdefault
     if (!c_info->type)
         return ERROR_INVAL;
 
+    if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) {
+        libxl_defbool_setdefault(&c_info->hap, true);
+        libxl_defbool_setdefault(&c_info->oos, true);
+    }
+
     return 0;
 }
 
@@ -365,8 +368,8 @@ int libxl__domain_make(libxl__gc *gc, li
     flags = 0;
     if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
         flags |= XEN_DOMCTL_CDF_hvm_guest;
-        flags |= info->hap ? XEN_DOMCTL_CDF_hap : 0;
-        flags |= info->oos ? 0 : XEN_DOMCTL_CDF_oos_off;
+        flags |= libxl_defbool_val(info->hap) ? XEN_DOMCTL_CDF_hap : 0;
+        flags |= libxl_defbool_val(info->oos) ? 0 : XEN_DOMCTL_CDF_oos_off;
     }
     *domid = -1;
 
@@ -518,6 +521,9 @@ static int do_domain_create(libxl__gc *g
     ret = libxl__domain_create_info_setdefault(gc, &d_config->c_info);
     if (ret) goto error_out;
 
+    ret = libxl__domain_create_info_setdefault(gc, &d_config->c_info);
+    if (ret) goto error_out;
+
     ret = libxl__domain_make(gc, &d_config->c_info, &domid);
     if (ret) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot make domain: %d", ret);
diff -r 41738bf7d28d -r a20f15f1889b tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Mon Feb 13 15:48:53 2012 +0000
+++ b/tools/libxl/libxl_types.idl	Mon Feb 13 15:49:00 2012 +0000
@@ -188,8 +188,8 @@ libxl_version_info = Struct("version_inf
 
 libxl_domain_create_info = Struct("domain_create_info",[
     ("type",         libxl_domain_type),
-    ("hap",          bool),
-    ("oos",          bool),
+    ("hap",          libxl_defbool),
+    ("oos",          libxl_defbool),
     ("ssidref",      uint32),
     ("name",         string),
     ("uuid",         libxl_uuid),
diff -r 41738bf7d28d -r a20f15f1889b tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Mon Feb 13 15:48:53 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Mon Feb 13 15:49:00 2012 +0000
@@ -557,8 +557,7 @@ static void parse_config_data(const char
         !strncmp(buf, "hvm", strlen(buf)))
         c_info->type = LIBXL_DOMAIN_TYPE_HVM;
 
-    if (!xlu_cfg_get_long (config, "hap", &l, 0))
-        c_info->hap = l;
+    xlu_cfg_get_defbool(config, "hap", &c_info->hap, 0);
 
     if (xlu_cfg_replace_string (config, "name", &c_info->name, 0)) {
         fprintf(stderr, "Domain name must be specified.\n");
@@ -574,8 +573,7 @@ static void parse_config_data(const char
         libxl_uuid_generate(&c_info->uuid);
     }
 
-    if (!xlu_cfg_get_long(config, "oos", &l, 0))
-        c_info->oos = l;
+    xlu_cfg_get_defbool(config, "oos", &c_info->oos, 0);
 
     if (!xlu_cfg_get_string (config, "pool", &buf, 0)) {
         c_info->poolid = -1;
diff -r 41738bf7d28d -r a20f15f1889b tools/libxl/xl_sxp.c
--- a/tools/libxl/xl_sxp.c	Mon Feb 13 15:48:53 2012 +0000
+++ b/tools/libxl/xl_sxp.c	Mon Feb 13 15:49:00 2012 +0000
@@ -41,8 +41,8 @@ void printf_info_sexp(int domid, libxl_d
     printf("(domain\n\t(domid %d)\n", domid);
     printf("\t(create_info)\n");
     printf("\t(hvm %d)\n", c_info->type == LIBXL_DOMAIN_TYPE_HVM);
-    printf("\t(hap %d)\n", c_info->hap);
-    printf("\t(oos %d)\n", c_info->oos);
+    printf("\t(hap %s)\n", libxl_defbool_to_string(c_info->hap));
+    printf("\t(oos %s)\n", libxl_defbool_to_string(c_info->oos));
     printf("\t(ssidref %d)\n", c_info->ssidref);
     printf("\t(name %s)\n", c_info->name);

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 15 of 23] libxl: make boolean members of libxl_domain_create_info into libxl_defbool
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (13 preceding siblings ...)
  2012-02-13 16:53 ` [PATCH 14 of 23] libxl: make boolean members of libxl_domain_create_info into libxl_defbool Ian Campbell
@ 2012-02-13 16:53 ` Ian Campbell
  2012-02-20 19:25   ` Ian Jackson
  2012-02-13 16:53 ` [PATCH 16 of 23] libxl: switch generation id control to libxl_defbool Ian Campbell
                   ` (7 subsequent siblings)
  22 siblings, 1 reply; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:53 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329148141 0
# Node ID ca432f4ad9f58b0a0a87045a680e37a123bbda11
# Parent  a20f15f1889ba1dda88d64509694818a0793cf8a
libxl: make boolean members of libxl_domain_create_info into libxl_defbool

This allows them to be set via the _init/_setdefault methods.

This just covers the obvious ones.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff -r a20f15f1889b -r ca432f4ad9f5 tools/libxl/libxl_bootloader.c
--- a/tools/libxl/libxl_bootloader.c	Mon Feb 13 15:49:00 2012 +0000
+++ b/tools/libxl/libxl_bootloader.c	Mon Feb 13 15:49:01 2012 +0000
@@ -352,6 +352,9 @@ int libxl_run_bootloader(libxl_ctx *ctx,
     if (info->type != LIBXL_DOMAIN_TYPE_PV || !info->u.pv.bootloader)
         goto out;
 
+    rc = libxl__domain_build_info_setdefault(gc, info);
+    if (rc) goto out;
+
     rc = ERROR_INVAL;
     if (!disk)
         goto out;
diff -r a20f15f1889b -r ca432f4ad9f5 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Mon Feb 13 15:49:00 2012 +0000
+++ b/tools/libxl/libxl_create.c	Mon Feb 13 15:49:01 2012 +0000
@@ -73,7 +73,6 @@ void libxl_domain_build_info_init(libxl_
                                   const libxl_domain_create_info *c_info)
 {
     memset(b_info, '\0', sizeof(*b_info));
-    b_info->disable_migrate = 0;
     b_info->cpuid = NULL;
     b_info->type = c_info->type;
 
@@ -88,17 +87,7 @@ void libxl_domain_build_info_init(libxl_
     switch (b_info->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
         b_info->u.hvm.firmware = NULL;
-        b_info->u.hvm.pae = 1;
-        b_info->u.hvm.apic = 1;
-        b_info->u.hvm.acpi = 1;
-        b_info->u.hvm.acpi_s3 = 1;
-        b_info->u.hvm.acpi_s4 = 1;
-        b_info->u.hvm.nx = 1;
-        b_info->u.hvm.viridian = 0;
-        b_info->u.hvm.hpet = 1;
-        b_info->u.hvm.vpt_align = 1;
         b_info->u.hvm.timer_mode = LIBXL_TIMER_MODE_DEFAULT;
-        b_info->u.hvm.nested_hvm = 0;
         b_info->u.hvm.no_incr_generationid = 0;
 
         b_info->u.hvm.stdvga = 0;
@@ -106,9 +95,7 @@ void libxl_domain_build_info_init(libxl_
         b_info->u.hvm.vnc.display = 0;
         b_info->u.hvm.vnc.findunused = 1;
         b_info->u.hvm.serial = NULL;
-        b_info->u.hvm.usb = 0;
         b_info->u.hvm.usbdevice = NULL;
-        b_info->u.hvm.xen_platform_pci = 1;
         break;
     case LIBXL_DOMAIN_TYPE_PV:
         b_info->u.pv.slack_memkb = 0;
@@ -141,6 +128,8 @@ int libxl__domain_build_info_setdefault(
     if (b_info->target_memkb == LIBXL_MEMKB_DEFAULT)
         b_info->target_memkb = b_info->max_memkb;
 
+    libxl_defbool_setdefault(&b_info->disable_migrate, false);
+
     switch (b_info->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
         if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
@@ -151,6 +140,19 @@ int libxl__domain_build_info_setdefault(
             b_info->u.hvm.timer_mode =
                 LIBXL_TIMER_MODE_NO_DELAY_FOR_MISSED_TICKS;
 
+        libxl_defbool_setdefault(&b_info->u.hvm.pae, true);
+        libxl_defbool_setdefault(&b_info->u.hvm.apic, true);
+        libxl_defbool_setdefault(&b_info->u.hvm.acpi, true);
+        libxl_defbool_setdefault(&b_info->u.hvm.acpi_s3, true);
+        libxl_defbool_setdefault(&b_info->u.hvm.acpi_s4, true);
+        libxl_defbool_setdefault(&b_info->u.hvm.nx, true);
+        libxl_defbool_setdefault(&b_info->u.hvm.viridian, false);
+        libxl_defbool_setdefault(&b_info->u.hvm.hpet, true);
+        libxl_defbool_setdefault(&b_info->u.hvm.vpt_align, true);
+        libxl_defbool_setdefault(&b_info->u.hvm.nested_hvm, false);
+        libxl_defbool_setdefault(&b_info->u.hvm.usb, false);
+        libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci, true);
+
         if (!b_info->u.hvm.boot) {
             b_info->u.hvm.boot = strdup("cda");
             if (!b_info->u.hvm.boot) return ERROR_NOMEM;
@@ -165,6 +167,7 @@ int libxl__domain_build_info_setdefault(
 
         break;
     case LIBXL_DOMAIN_TYPE_PV:
+        libxl_defbool_setdefault(&b_info->u.pv.e820_host, false);
         if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
             b_info->shadow_memkb = 0;
         if (b_info->u.pv.slack_memkb == LIBXL_MEMKB_DEFAULT)
@@ -220,11 +223,11 @@ int libxl__domain_build(libxl__gc *gc,
 
         localents = libxl__calloc(gc, 7, sizeof(char *));
         localents[0] = "platform/acpi";
-        localents[1] = (info->u.hvm.acpi) ? "1" : "0";
+        localents[1] = libxl_defbool_val(info->u.hvm.acpi) ? "1" : "0";
         localents[2] = "platform/acpi_s3";
-        localents[3] = (info->u.hvm.acpi_s3) ? "1" : "0";
+        localents[3] = libxl_defbool_val(info->u.hvm.acpi_s3) ? "1" : "0";
         localents[4] = "platform/acpi_s4";
-        localents[5] = (info->u.hvm.acpi_s4) ? "1" : "0";
+        localents[5] = libxl_defbool_val(info->u.hvm.acpi_s4) ? "1" : "0";
 
         break;
     case LIBXL_DOMAIN_TYPE_PV:
@@ -426,7 +429,6 @@ retry_transaction:
     xs_rm(ctx->xsh, t, dom_path);
     libxl__xs_mkdir(gc, t, dom_path, roperm, ARRAY_SIZE(roperm));
 
-
     xs_rm(ctx->xsh, t, vm_path);
     libxl__xs_mkdir(gc, t, vm_path, roperm, ARRAY_SIZE(roperm));
 
@@ -673,7 +675,7 @@ static int do_domain_create(libxl__gc *g
     }
 
     if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV &&
-        d_config->b_info.u.pv.e820_host) {
+        libxl_defbool_val(d_config->b_info.u.pv.e820_host)) {
         int rc;
         rc = libxl__e820_alloc(gc, domid, d_config);
         if (rc)
diff -r a20f15f1889b -r ca432f4ad9f5 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Mon Feb 13 15:49:00 2012 +0000
+++ b/tools/libxl/libxl_dm.c	Mon Feb 13 15:49:01 2012 +0000
@@ -192,7 +192,7 @@ static char ** libxl__build_device_model
         if (b_info->u.hvm.boot) {
             flexarray_vappend(dm_args, "-boot", b_info->u.hvm.boot, NULL);
         }
-        if (b_info->u.hvm.usb || b_info->u.hvm.usbdevice) {
+        if (libxl_defbool_val(b_info->u.hvm.usb) || b_info->u.hvm.usbdevice) {
             flexarray_append(dm_args, "-usb");
             if (b_info->u.hvm.usbdevice) {
                 flexarray_vappend(dm_args,
@@ -202,7 +202,7 @@ static char ** libxl__build_device_model
         if (b_info->u.hvm.soundhw) {
             flexarray_vappend(dm_args, "-soundhw", b_info->u.hvm.soundhw, NULL);
         }
-        if (b_info->u.hvm.acpi) {
+        if (libxl_defbool_val(b_info->u.hvm.acpi)) {
             flexarray_append(dm_args, "-acpi");
         }
         if (b_info->max_vcpus > 1) {
@@ -427,7 +427,7 @@ static char ** libxl__build_device_model
             flexarray_vappend(dm_args, "-boot",
                     libxl__sprintf(gc, "order=%s", b_info->u.hvm.boot), NULL);
         }
-        if (b_info->u.hvm.usb || b_info->u.hvm.usbdevice) {
+        if (libxl_defbool_val(b_info->u.hvm.usb) || b_info->u.hvm.usbdevice) {
             flexarray_append(dm_args, "-usb");
             if (b_info->u.hvm.usbdevice) {
                 flexarray_vappend(dm_args,
@@ -437,7 +437,7 @@ static char ** libxl__build_device_model
         if (b_info->u.hvm.soundhw) {
             flexarray_vappend(dm_args, "-soundhw", b_info->u.hvm.soundhw, NULL);
         }
-        if (!b_info->u.hvm.acpi) {
+        if (!libxl_defbool_val(b_info->u.hvm.acpi)) {
             flexarray_append(dm_args, "-no-acpi");
         }
         if (b_info->max_vcpus > 1) {
@@ -939,7 +939,7 @@ int libxl__create_device_model(libxl__gc
         b_info->device_model_version
         == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL)
         libxl__xs_write(gc, XBT_NULL, libxl__sprintf(gc, "%s/disable_pf", path),
-                        "%d", !b_info->u.hvm.xen_platform_pci);
+                    "%d", !libxl_defbool_val(b_info->u.hvm.xen_platform_pci));
 
     libxl_create_logfile(ctx,
                          libxl__sprintf(gc, "qemu-dm-%s", c_info->name),
diff -r a20f15f1889b -r ca432f4ad9f5 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Mon Feb 13 15:49:00 2012 +0000
+++ b/tools/libxl/libxl_dom.c	Mon Feb 13 15:49:01 2012 +0000
@@ -88,7 +88,7 @@ int libxl__build_pre(libxl__gc *gc, uint
         abort();
     }
     xc_domain_set_tsc_info(ctx->xch, domid, tsc_mode, 0, 0, 0);
-    if ( info->disable_migrate )
+    if (libxl_defbool_val(info->disable_migrate))
         xc_domain_disable_migrate(ctx->xch, domid);
 
     if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
@@ -292,7 +292,7 @@ static int hvm_build_set_params(xc_inter
         return -1;
 
     va_hvm = (struct hvm_info_table *)(va_map + HVM_INFO_OFFSET);
-    va_hvm->apic_mode = info->u.hvm.apic;
+    va_hvm->apic_mode = libxl_defbool_val(info->u.hvm.apic);
     va_hvm->nr_vcpus = info->max_vcpus;
     memcpy(va_hvm->vcpu_online, &info->cur_vcpus, sizeof(info->cur_vcpus));
     for (i = 0, sum = 0; i < va_hvm->length; i++)
@@ -302,14 +302,19 @@ static int hvm_build_set_params(xc_inter
 
     xc_get_hvm_param(handle, domid, HVM_PARAM_STORE_PFN, store_mfn);
     xc_get_hvm_param(handle, domid, HVM_PARAM_CONSOLE_PFN, console_mfn);
-    xc_set_hvm_param(handle, domid, HVM_PARAM_PAE_ENABLED, info->u.hvm.pae);
+    xc_set_hvm_param(handle, domid, HVM_PARAM_PAE_ENABLED,
+                     libxl_defbool_val(info->u.hvm.pae));
 #if defined(__i386__) || defined(__x86_64__)
-    xc_set_hvm_param(handle, domid, HVM_PARAM_VIRIDIAN, info->u.hvm.viridian);
-    xc_set_hvm_param(handle, domid, HVM_PARAM_HPET_ENABLED, (unsigned long) info->u.hvm.hpet);
+    xc_set_hvm_param(handle, domid, HVM_PARAM_VIRIDIAN,
+                     libxl_defbool_val(info->u.hvm.viridian));
+    xc_set_hvm_param(handle, domid, HVM_PARAM_HPET_ENABLED,
+                     libxl_defbool_val(info->u.hvm.hpet));
 #endif
     xc_set_hvm_param(handle, domid, HVM_PARAM_TIMER_MODE, timer_mode(info));
-    xc_set_hvm_param(handle, domid, HVM_PARAM_VPT_ALIGN, (unsigned long) info->u.hvm.vpt_align);
-    xc_set_hvm_param(handle, domid, HVM_PARAM_NESTEDHVM, info->u.hvm.nested_hvm);
+    xc_set_hvm_param(handle, domid, HVM_PARAM_VPT_ALIGN,
+                     libxl_defbool_val(info->u.hvm.vpt_align));
+    xc_set_hvm_param(handle, domid, HVM_PARAM_NESTEDHVM,
+                     libxl_defbool_val(info->u.hvm.nested_hvm));
     xc_set_hvm_param(handle, domid, HVM_PARAM_STORE_EVTCHN, store_evtchn);
     xc_set_hvm_param(handle, domid, HVM_PARAM_CONSOLE_EVTCHN, console_evtchn);
 
@@ -400,7 +405,7 @@ int libxl__domain_restore_common(libxl__
     case LIBXL_DOMAIN_TYPE_HVM:
         hvm = 1;
         superpages = 1;
-        pae = info->u.hvm.pae;
+        pae = libxl_defbool_val(info->u.hvm.pae);
         no_incr_generationid = info->u.hvm.no_incr_generationid;
         break;
     case LIBXL_DOMAIN_TYPE_PV:
diff -r a20f15f1889b -r ca432f4ad9f5 tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c	Mon Feb 13 15:49:00 2012 +0000
+++ b/tools/libxl/libxl_pci.c	Mon Feb 13 15:49:01 2012 +0000
@@ -1378,7 +1378,7 @@ int libxl__e820_alloc(libxl__gc *gc, uin
         return ERROR_INVAL;
 
     b_info = &d_config->b_info;
-    if (!b_info->u.pv.e820_host)
+    if (!libxl_defbool_val(b_info->u.pv.e820_host))
         return ERROR_INVAL;
 
     rc = xc_get_machine_memory_map(ctx->xch, map, E820MAX);
diff -r a20f15f1889b -r ca432f4ad9f5 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Mon Feb 13 15:49:00 2012 +0000
+++ b/tools/libxl/libxl_types.idl	Mon Feb 13 15:49:01 2012 +0000
@@ -212,7 +212,7 @@ libxl_domain_build_info = Struct("domain
     ("target_memkb",    MemKB),
     ("video_memkb",     MemKB),
     ("shadow_memkb",    MemKB),
-    ("disable_migrate", bool),
+    ("disable_migrate", libxl_defbool),
     ("cpuid",           libxl_cpuid_policy_list),
     ("type",            libxl_domain_type),
     
@@ -230,19 +230,19 @@ libxl_domain_build_info = Struct("domain
     ("extra_hvm",        libxl_string_list),
 
     ("u", KeyedUnion(None, libxl_domain_type, "type",
-                [("hvm", Struct(None, [("firmware", string),
-                                       ("pae", bool),
-                                       ("apic", bool),
-                                       ("acpi", bool),
-                                       ("acpi_s3", bool),
-                                       ("acpi_s4", bool),
-                                       ("nx", bool),
-                                       ("viridian", bool),
-                                       ("timeoffset", string),
-                                       ("hpet", bool),
-                                       ("vpt_align", bool),
-                                       ("timer_mode", libxl_timer_mode),
-                                       ("nested_hvm", bool),
+                [("hvm", Struct(None, [("firmware",         string),
+                                       ("pae",              libxl_defbool),
+                                       ("apic",             libxl_defbool),
+                                       ("acpi",             libxl_defbool),
+                                       ("acpi_s3",          libxl_defbool),
+                                       ("acpi_s4",          libxl_defbool),
+                                       ("nx",               libxl_defbool),
+                                       ("viridian",         libxl_defbool),
+                                       ("timeoffset",       string),
+                                       ("hpet",             libxl_defbool),
+                                       ("vpt_align",        libxl_defbool),
+                                       ("timer_mode",       libxl_timer_mode),
+                                       ("nested_hvm",       libxl_defbool),
                                        ("no_incr_generationid", bool),
                                        ("nographic",        bool),
                                        ("stdvga",           bool),
@@ -256,13 +256,13 @@ libxl_domain_build_info = Struct("domain
                                        
                                        ("serial",           string),
                                        ("boot",             string),
-                                       ("usb",              bool),
+                                       ("usb",              libxl_defbool),
                                        # usbdevice:
                                        # - "tablet" for absolute mouse,
                                        # - "mouse" for PS/2 protocol relative mouse
                                        ("usbdevice",        string),
                                        ("soundhw",          string),
-                                       ("xen_platform_pci", bool),
+                                       ("xen_platform_pci", libxl_defbool),
                                        ])),
                  ("pv", Struct(None, [("kernel", libxl_file_reference),
                                       ("slack_memkb", MemKB),
@@ -272,7 +272,7 @@ libxl_domain_build_info = Struct("domain
                                       ("ramdisk", libxl_file_reference),
                                       ("features", string, {'const': True}),
                                       # Use host's E820 for PCI passthrough.
-                                      ("e820_host", bool),
+                                      ("e820_host", libxl_defbool),
                                       ])),
                  ])),
     ],
diff -r a20f15f1889b -r ca432f4ad9f5 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Mon Feb 13 15:49:00 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Mon Feb 13 15:49:01 2012 +0000
@@ -673,8 +673,7 @@ static void parse_config_data(const char
         : libxl_get_required_shadow_memory(b_info->max_memkb,
                                            b_info->max_vcpus);
 
-    if (!xlu_cfg_get_long (config, "nomigrate", &l, 0))
-        b_info->disable_migrate = l;
+    xlu_cfg_get_defbool(config, "nomigrate", &b_info->disable_migrate, 0);
 
     if (!xlu_cfg_get_long(config, "tsc_mode", &l, 1)) {
         const char *s = libxl_tsc_mode_to_string(l);
@@ -710,24 +709,16 @@ static void parse_config_data(const char
 
         xlu_cfg_replace_string (config, "firmware_override",
                                 &b_info->u.hvm.firmware, 0);
-        if (!xlu_cfg_get_long (config, "pae", &l, 0))
-            b_info->u.hvm.pae = l;
-        if (!xlu_cfg_get_long (config, "apic", &l, 0))
-            b_info->u.hvm.apic = l;
-        if (!xlu_cfg_get_long (config, "acpi", &l, 0))
-            b_info->u.hvm.acpi = l;
-        if (!xlu_cfg_get_long (config, "acpi_s3", &l, 0))
-            b_info->u.hvm.acpi_s3 = l;
-        if (!xlu_cfg_get_long (config, "acpi_s4", &l, 0))
-            b_info->u.hvm.acpi_s4 = l;
-        if (!xlu_cfg_get_long (config, "nx", &l, 0))
-            b_info->u.hvm.nx = l;
-        if (!xlu_cfg_get_long (config, "viridian", &l, 0))
-            b_info->u.hvm.viridian = l;
-        if (!xlu_cfg_get_long (config, "hpet", &l, 0))
-            b_info->u.hvm.hpet = l;
-        if (!xlu_cfg_get_long (config, "vpt_align", &l, 0))
-            b_info->u.hvm.vpt_align = l;
+
+        xlu_cfg_get_defbool(config, "pae", &b_info->u.hvm.pae, 0);
+        xlu_cfg_get_defbool(config, "apic", &b_info->u.hvm.apic, 0);
+        xlu_cfg_get_defbool(config, "acpi", &b_info->u.hvm.acpi, 0);
+        xlu_cfg_get_defbool(config, "acpi_s3", &b_info->u.hvm.acpi_s3, 0);
+        xlu_cfg_get_defbool(config, "acpi_s4", &b_info->u.hvm.acpi_s4, 0);
+        xlu_cfg_get_defbool(config, "nx", &b_info->u.hvm.nx, 0);
+        xlu_cfg_get_defbool(config, "viridian", &b_info->u.hvm.viridian, 0);
+        xlu_cfg_get_defbool(config, "hpet", &b_info->u.hvm.hpet, 0);
+        xlu_cfg_get_defbool(config, "vpt_align", &b_info->u.hvm.vpt_align, 0);
 
         if (!xlu_cfg_get_long(config, "timer_mode", &l, 1)) {
             const char *s = libxl_timer_mode_to_string(l);
@@ -752,8 +743,7 @@ static void parse_config_data(const char
             }
         }
 
-        if (!xlu_cfg_get_long (config, "nestedhvm", &l, 0))
-            b_info->u.hvm.nested_hvm = l;
+        xlu_cfg_get_defbool(config, "nestedhvm", &b_info->u.hvm.nested_hvm, 0);
         break;
     case LIBXL_DOMAIN_TYPE_PV:
     {
@@ -990,19 +980,10 @@ skip_vfb:
 
     /* To be reworked (automatically enabled) once the auto ballooning
      * after guest starts is done (with PCI devices passed in). */
-    if (!xlu_cfg_get_long (config, "e820_host", &l, 0)) {
-        switch (c_info->type) {
-        case LIBXL_DOMAIN_TYPE_HVM:
-            fprintf(stderr, "Can't do e820_host in HVM mode!");
-            break;
-        case LIBXL_DOMAIN_TYPE_PV:
-            if (l)
-                b_info->u.pv.e820_host = true;
-            break;
-        default:
-            abort();
-        }
-    }
+    if (c_info->type == LIBXL_DOMAIN_TYPE_PV) {
+        xlu_cfg_get_defbool(config, "e820_host", &b_info->u.pv.e820_host, 0);
+    }
+
     if (!xlu_cfg_get_list (config, "pci", &pcis, 0, 0)) {
         int i;
         d_config->num_pcidevs = 0;
@@ -1020,7 +1001,7 @@ skip_vfb:
                 d_config->num_pcidevs++;
         }
         if (d_config->num_pcidevs && c_info->type == LIBXL_DOMAIN_TYPE_PV)
-            b_info->u.pv.e820_host = true;
+            libxl_defbool_set(&b_info->u.pv.e820_host, true);
     }
 
     switch (xlu_cfg_get_list(config, "cpuid", &cpuids, 0, 1)) {
@@ -1212,12 +1193,12 @@ skip_vfb:
             b_info->u.hvm.gfx_passthru = l;
         xlu_cfg_replace_string (config, "serial", &b_info->u.hvm.serial, 0);
         xlu_cfg_replace_string (config, "boot", &b_info->u.hvm.boot, 0);
-        if (!xlu_cfg_get_long (config, "usb", &l, 0))
-            b_info->u.hvm.usb = l;
-        xlu_cfg_replace_string (config, "usbdevice", &b_info->u.hvm.usbdevice, 0);
+        xlu_cfg_get_defbool(config, "usb", &b_info->u.hvm.usb, 0);
+        xlu_cfg_replace_string (config, "usbdevice",
+                                &b_info->u.hvm.usbdevice, 0);
         xlu_cfg_replace_string (config, "soundhw", &b_info->u.hvm.soundhw, 0);
-        if (!xlu_cfg_get_long (config, "xen_platform_pci", &l, 0))
-            b_info->u.hvm.xen_platform_pci = l;
+        xlu_cfg_get_defbool(config, "xen_platform_pci",
+                            &b_info->u.hvm.xen_platform_pci, 0);
     }
 
     xlu_cfg_destroy(config);
diff -r a20f15f1889b -r ca432f4ad9f5 tools/libxl/xl_sxp.c
--- a/tools/libxl/xl_sxp.c	Mon Feb 13 15:49:00 2012 +0000
+++ b/tools/libxl/xl_sxp.c	Mon Feb 13 15:49:01 2012 +0000
@@ -71,7 +71,8 @@ void printf_info_sexp(int domid, libxl_d
     printf("\t(tsc_mode %s)\n", libxl_tsc_mode_to_string(b_info->tsc_mode));
     printf("\t(max_memkb %"PRId64")\n", b_info->max_memkb);
     printf("\t(target_memkb %"PRId64")\n", b_info->target_memkb);
-    printf("\t(nomigrate %d)\n", b_info->disable_migrate);
+    printf("\t(nomigrate %s)\n",
+           libxl_defbool_to_string(b_info->disable_migrate));
 
     if (c_info->type == LIBXL_DOMAIN_TYPE_PV && b_info->u.pv.bootloader) {
         int i;
@@ -91,16 +92,22 @@ void printf_info_sexp(int domid, libxl_d
         printf("\t\t\t(firmware %s)\n", b_info->u.hvm.firmware);
         printf("\t\t\t(video_memkb %"PRId64")\n", b_info->video_memkb);
         printf("\t\t\t(shadow_memkb %"PRId64")\n", b_info->shadow_memkb);
-        printf("\t\t\t(pae %d)\n", b_info->u.hvm.pae);
-        printf("\t\t\t(apic %d)\n", b_info->u.hvm.apic);
-        printf("\t\t\t(acpi %d)\n", b_info->u.hvm.acpi);
-        printf("\t\t\t(nx %d)\n", b_info->u.hvm.nx);
-        printf("\t\t\t(viridian %d)\n", b_info->u.hvm.viridian);
-        printf("\t\t\t(hpet %d)\n", b_info->u.hvm.hpet);
-        printf("\t\t\t(vpt_align %d)\n", b_info->u.hvm.vpt_align);
+        printf("\t\t\t(pae %s)\n", libxl_defbool_to_string(b_info->u.hvm.pae));
+        printf("\t\t\t(apic %s)\n",
+               libxl_defbool_to_string(b_info->u.hvm.apic));
+        printf("\t\t\t(acpi %s)\n",
+               libxl_defbool_to_string(b_info->u.hvm.acpi));
+        printf("\t\t\t(nx %s)\n", libxl_defbool_to_string(b_info->u.hvm.nx));
+        printf("\t\t\t(viridian %s)\n",
+               libxl_defbool_to_string(b_info->u.hvm.viridian));
+        printf("\t\t\t(hpet %s)\n",
+               libxl_defbool_to_string(b_info->u.hvm.hpet));
+        printf("\t\t\t(vpt_align %s)\n",
+               libxl_defbool_to_string(b_info->u.hvm.vpt_align));
         printf("\t\t\t(timer_mode %s)\n",
                libxl_timer_mode_to_string(b_info->u.hvm.timer_mode));
-        printf("\t\t\t(nestedhvm %d)\n", b_info->u.hvm.nested_hvm);
+        printf("\t\t\t(nestedhvm %s)\n",
+               libxl_defbool_to_string(b_info->u.hvm.nested_hvm));
         printf("\t\t\t(no_incr_generationid %d)\n",
                     b_info->u.hvm.no_incr_generationid);
 
@@ -125,7 +132,7 @@ void printf_info_sexp(int domid, libxl_d
         printf("\t\t\t(gfx_passthru %d)\n", b_info->u.hvm.gfx_passthru);
         printf("\t\t\t(serial %s)\n", b_info->u.hvm.serial);
         printf("\t\t\t(boot %s)\n", b_info->u.hvm.boot);
-        printf("\t\t\t(usb %d)\n", b_info->u.hvm.usb);
+        printf("\t\t\t(usb %s)\n", libxl_defbool_to_string(b_info->u.hvm.usb));
         printf("\t\t\t(usbdevice %s)\n", b_info->u.hvm.usbdevice);
         printf("\t\t)\n");
         break;
@@ -134,7 +141,8 @@ void printf_info_sexp(int domid, libxl_d
         printf("\t\t\t(kernel %s)\n", b_info->u.pv.kernel.path);
         printf("\t\t\t(cmdline %s)\n", b_info->u.pv.cmdline);
         printf("\t\t\t(ramdisk %s)\n", b_info->u.pv.ramdisk.path);
-        printf("\t\t\t(e820_host %d)\n", b_info->u.pv.e820_host);
+        printf("\t\t\t(e820_host %s)\n",
+               libxl_defbool_to_string(b_info->u.pv.e820_host));
         printf("\t\t)\n");
         break;
     default:

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 16 of 23] libxl: switch generation id control to libxl_defbool
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (14 preceding siblings ...)
  2012-02-13 16:53 ` [PATCH 15 " Ian Campbell
@ 2012-02-13 16:53 ` Ian Campbell
  2012-02-20 19:16   ` Ian Jackson
  2012-02-21 11:01   ` Ian Campbell
  2012-02-13 16:53 ` [PATCH 17 of 23] libxl: use defbool for graphics related options Ian Campbell
                   ` (6 subsequent siblings)
  22 siblings, 2 replies; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:53 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329150447 0
# Node ID 42c448aa641537798f4b09a6db55f3a5ee4082cd
# Parent  ca432f4ad9f58b0a0a87045a680e37a123bbda11
libxl: switch generation id control to libxl_defbool.

This allows it to be set via the _init/_setdefault methods.

Also switch the sense of the variable in the libxl API, since once you add
defbool to the mix the double negatives become even more confusing.

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

diff -r ca432f4ad9f5 -r 42c448aa6415 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Mon Feb 13 15:49:01 2012 +0000
+++ b/tools/libxl/libxl_create.c	Mon Feb 13 16:27:27 2012 +0000
@@ -88,7 +88,6 @@ void libxl_domain_build_info_init(libxl_
     case LIBXL_DOMAIN_TYPE_HVM:
         b_info->u.hvm.firmware = NULL;
         b_info->u.hvm.timer_mode = LIBXL_TIMER_MODE_DEFAULT;
-        b_info->u.hvm.no_incr_generationid = 0;
 
         b_info->u.hvm.stdvga = 0;
         b_info->u.hvm.vnc.enable = 1;
@@ -150,6 +149,8 @@ int libxl__domain_build_info_setdefault(
         libxl_defbool_setdefault(&b_info->u.hvm.hpet, true);
         libxl_defbool_setdefault(&b_info->u.hvm.vpt_align, true);
         libxl_defbool_setdefault(&b_info->u.hvm.nested_hvm, false);
+        libxl_defbool_setdefault(&b_info->u.hvm.incr_generationid, false);
+
         libxl_defbool_setdefault(&b_info->u.hvm.usb, false);
         libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci, true);
 
diff -r ca432f4ad9f5 -r 42c448aa6415 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Mon Feb 13 15:49:01 2012 +0000
+++ b/tools/libxl/libxl_dom.c	Mon Feb 13 16:27:27 2012 +0000
@@ -406,7 +406,7 @@ int libxl__domain_restore_common(libxl__
         hvm = 1;
         superpages = 1;
         pae = libxl_defbool_val(info->u.hvm.pae);
-        no_incr_generationid = info->u.hvm.no_incr_generationid;
+        no_incr_generationid = !libxl_defbool_val(info->u.hvm.incr_generationid);
         break;
     case LIBXL_DOMAIN_TYPE_PV:
         hvm = 0;
diff -r ca432f4ad9f5 -r 42c448aa6415 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Mon Feb 13 15:49:01 2012 +0000
+++ b/tools/libxl/libxl_types.idl	Mon Feb 13 16:27:27 2012 +0000
@@ -243,7 +243,7 @@ libxl_domain_build_info = Struct("domain
                                        ("vpt_align",        libxl_defbool),
                                        ("timer_mode",       libxl_timer_mode),
                                        ("nested_hvm",       libxl_defbool),
-                                       ("no_incr_generationid", bool),
+                                       ("incr_generationid",libxl_defbool),
                                        ("nographic",        bool),
                                        ("stdvga",           bool),
                                        ("vnc",              libxl_vnc_info),
diff -r ca432f4ad9f5 -r 42c448aa6415 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Mon Feb 13 15:49:01 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Mon Feb 13 16:27:27 2012 +0000
@@ -1350,7 +1350,7 @@ struct domain_create {
     const char *restore_file;
     int migrate_fd; /* -1 means none */
     char **migration_domname_r; /* from malloc */
-    int no_incr_generationid;
+    int incr_generationid;
 };
 
 static int freemem(libxl_domain_build_info *b_info)
@@ -1601,8 +1601,8 @@ static int create_domain(struct domain_c
     }
 
     if (d_config.c_info.type == LIBXL_DOMAIN_TYPE_HVM)
-        d_config.b_info.u.hvm.no_incr_generationid =
-            dom_info->no_incr_generationid;
+        libxl_defbool_set(&d_config.b_info.u.hvm.incr_generationid,
+                          dom_info->incr_generationid);
 
     if (debug || dom_info->dryrun)
         printf_info(default_output_format, -1, &d_config);
@@ -2877,7 +2877,7 @@ static void migrate_receive(int debug, i
     dom_info.restore_file = "incoming migration stream";
     dom_info.migrate_fd = 0; /* stdin */
     dom_info.migration_domname_r = &migration_domname;
-    dom_info.no_incr_generationid = 1;
+    dom_info.incr_generationid = 0;
 
     rc = create_domain(&dom_info);
     if (rc < 0) {
@@ -3002,6 +3002,7 @@ int main_restore(int argc, char **argv)
     dom_info.restore_file = checkpoint_file;
     dom_info.migrate_fd = -1;
     dom_info.console_autoconnect = console_autoconnect;
+    dom_info.incr_generationid = 1;
 
     rc = create_domain(&dom_info);
     if (rc < 0)
@@ -3384,6 +3385,7 @@ int main_create(int argc, char **argv)
     dom_info.extra_config = extra_config;
     dom_info.migrate_fd = -1;
     dom_info.console_autoconnect = console_autoconnect;
+    dom_info.incr_generationid = 0;
 
     rc = create_domain(&dom_info);
     if (rc < 0)
diff -r ca432f4ad9f5 -r 42c448aa6415 tools/libxl/xl_sxp.c
--- a/tools/libxl/xl_sxp.c	Mon Feb 13 15:49:01 2012 +0000
+++ b/tools/libxl/xl_sxp.c	Mon Feb 13 16:27:27 2012 +0000
@@ -108,8 +108,8 @@ void printf_info_sexp(int domid, libxl_d
                libxl_timer_mode_to_string(b_info->u.hvm.timer_mode));
         printf("\t\t\t(nestedhvm %s)\n",
                libxl_defbool_to_string(b_info->u.hvm.nested_hvm));
-        printf("\t\t\t(no_incr_generationid %d)\n",
-                    b_info->u.hvm.no_incr_generationid);
+        printf("\t\t\t(no_incr_generationid %s)\n",
+               libxl_defbool_to_string(b_info->u.hvm.incr_generationid));
 
         printf("\t\t\t(stdvga %d)\n", b_info->u.hvm.stdvga);
         printf("\t\t\t(vnc %d)\n", b_info->u.hvm.vnc.enable);

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 17 of 23] libxl: use defbool for graphics related options
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (15 preceding siblings ...)
  2012-02-13 16:53 ` [PATCH 16 of 23] libxl: switch generation id control to libxl_defbool Ian Campbell
@ 2012-02-13 16:53 ` Ian Campbell
  2012-02-20 19:22   ` Ian Jackson
  2012-02-13 16:53 ` [PATCH 18 of 23] libxl: do not explicitly initialise members of build info to zero Ian Campbell
                   ` (5 subsequent siblings)
  22 siblings, 1 reply; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:53 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329150453 0
# Node ID d27c92eeeddc5e4c190a88ce282fb0155405a65e
# Parent  42c448aa641537798f4b09a6db55f3a5ee4082cd
libxl: use defbool for graphics related options

This allows them to be set via the _init/_setdefault methods.

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

diff -r 42c448aa6415 -r d27c92eeeddc tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Feb 13 16:27:27 2012 +0000
+++ b/tools/libxl/libxl.c	Mon Feb 13 16:27:33 2012 +0000
@@ -2256,22 +2256,23 @@ out:
 void libxl_device_vfb_init(libxl_device_vfb *vfb)
 {
     memset(vfb, 0x00, sizeof(libxl_device_vfb));
-    vfb->vnc.enable = 1;
-    vfb->vnc.passwd = NULL;
-    vfb->vnc.display = 0;
-    vfb->vnc.findunused = 1;
-    vfb->keymap = NULL;
-    vfb->sdl.enable = 0;
-    vfb->sdl.opengl = 0;
-    vfb->sdl.display = NULL;
-    vfb->sdl.xauthority = NULL;
 }
 
 int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb)
 {
-    if (!vfb->vnc.listen) {
-        vfb->vnc.listen = strdup("127.0.0.1");
-        if (!vfb->vnc.listen) return ERROR_NOMEM;
+    libxl_defbool_setdefault(&vfb->vnc.enable, true);
+    if (libxl_defbool_val(vfb->vnc.enable)) {
+        if (!vfb->vnc.listen) {
+            vfb->vnc.listen = strdup("127.0.0.1");
+            if (!vfb->vnc.listen) return ERROR_NOMEM;
+        }
+
+        libxl_defbool_setdefault(&vfb->vnc.findunused, true);
+    }
+
+    libxl_defbool_setdefault(&vfb->sdl.enable, false);
+    if (libxl_defbool_val(vfb->sdl.enable)) {
+        libxl_defbool_setdefault(&vfb->sdl.opengl, false);
     }
 
     return 0;
@@ -2320,17 +2321,17 @@ int libxl_device_vfb_add(libxl_ctx *ctx,
     flexarray_append_pair(back, "state", libxl__sprintf(gc, "%d", 1));
     flexarray_append_pair(back, "domain", libxl__domid_to_name(gc, domid));
     flexarray_append_pair(back, "vnc",
-                          libxl__sprintf(gc, "%d", vfb->vnc.enable));
+                          libxl_defbool_val(vfb->vnc.enable) ? "1" : "0");
     flexarray_append_pair(back, "vnclisten", vfb->vnc.listen);
     flexarray_append_pair(back, "vncpasswd", vfb->vnc.passwd);
     flexarray_append_pair(back, "vncdisplay",
                           libxl__sprintf(gc, "%d", vfb->vnc.display));
     flexarray_append_pair(back, "vncunused",
-                          libxl__sprintf(gc, "%d", vfb->vnc.findunused));
+                          libxl_defbool_val(vfb->vnc.findunused) ? "1" : "0");
     flexarray_append_pair(back, "sdl",
-                          libxl__sprintf(gc, "%d", vfb->sdl.enable));
+                          libxl_defbool_val(vfb->sdl.enable) ? "1" : "0");
     flexarray_append_pair(back, "opengl",
-                          libxl__sprintf(gc, "%d", vfb->sdl.opengl));
+                          libxl_defbool_val(vfb->sdl.opengl) ? "1" : "0");
     if (vfb->sdl.xauthority) {
         flexarray_append_pair(back, "xauthority", vfb->sdl.xauthority);
     }
diff -r 42c448aa6415 -r d27c92eeeddc tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Mon Feb 13 16:27:27 2012 +0000
+++ b/tools/libxl/libxl_create.c	Mon Feb 13 16:27:33 2012 +0000
@@ -89,10 +89,7 @@ void libxl_domain_build_info_init(libxl_
         b_info->u.hvm.firmware = NULL;
         b_info->u.hvm.timer_mode = LIBXL_TIMER_MODE_DEFAULT;
 
-        b_info->u.hvm.stdvga = 0;
-        b_info->u.hvm.vnc.enable = 1;
         b_info->u.hvm.vnc.display = 0;
-        b_info->u.hvm.vnc.findunused = 1;
         b_info->u.hvm.serial = NULL;
         b_info->u.hvm.usbdevice = NULL;
         break;
@@ -159,13 +156,32 @@ int libxl__domain_build_info_setdefault(
             if (!b_info->u.hvm.boot) return ERROR_NOMEM;
         }
 
-        if (b_info->u.hvm.vnc.enable) {
+        libxl_defbool_setdefault(&b_info->u.hvm.stdvga, false);
+        libxl_defbool_setdefault(&b_info->u.hvm.vnc.enable, true);
+        if (libxl_defbool_val(b_info->u.hvm.vnc.enable)) {
+            libxl_defbool_setdefault(&b_info->u.hvm.vnc.findunused, true);
             if (!b_info->u.hvm.vnc.listen) {
                 b_info->u.hvm.vnc.listen = strdup("127.0.0.1");
                 if (!b_info->u.hvm.vnc.listen) return ERROR_NOMEM;
             }
         }
 
+        libxl_defbool_setdefault(&b_info->u.hvm.sdl.enable, false);
+        if (libxl_defbool_val(b_info->u.hvm.sdl.enable)) {
+            libxl_defbool_setdefault(&b_info->u.hvm.sdl.opengl, false);
+        }
+
+        libxl_defbool_setdefault(&b_info->u.hvm.spice.enable, false);
+        if (libxl_defbool_val(b_info->u.hvm.spice.enable)) {
+            libxl_defbool_setdefault(&b_info->u.hvm.spice.disable_ticketing,
+                                     false);
+            libxl_defbool_setdefault(&b_info->u.hvm.spice.agent_mouse, true);
+        }
+
+        libxl_defbool_setdefault(&b_info->u.hvm.nographic, false);
+
+        libxl_defbool_setdefault(&b_info->u.hvm.gfx_passthru, false);
+
         break;
     case LIBXL_DOMAIN_TYPE_PV:
         libxl_defbool_setdefault(&b_info->u.pv.e820_host, false);
diff -r 42c448aa6415 -r d27c92eeeddc tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Mon Feb 13 16:27:27 2012 +0000
+++ b/tools/libxl/libxl_dm.c	Mon Feb 13 16:27:33 2012 +0000
@@ -81,7 +81,7 @@ static const libxl_vnc_info *dm_vnc(cons
     } else if (guest_config->num_vfbs > 0) {
         vnc = &guest_config->vfbs[0].vnc;
     }
-    return vnc && vnc->enable ? vnc : NULL;
+    return vnc && libxl_defbool_val(vnc->enable) ? vnc : NULL;
 }
 
 static const libxl_sdl_info *dm_sdl(const libxl_domain_config *guest_config)
@@ -92,7 +92,7 @@ static const libxl_sdl_info *dm_sdl(cons
     } else if (guest_config->num_vfbs > 0) {
         sdl = &guest_config->vfbs[0].sdl;
     }
-    return sdl && sdl->enable ? sdl : NULL;
+    return sdl && libxl_defbool_val(sdl->enable) ? sdl : NULL;
 }
 
 static const char *dm_keymap(const libxl_domain_config *guest_config)
@@ -154,13 +154,13 @@ static char ** libxl__build_device_model
         flexarray_append(dm_args, "-vnc");
         flexarray_append(dm_args, vncarg);
 
-        if (vnc->findunused) {
+        if (libxl_defbool_val(vnc->findunused)) {
             flexarray_append(dm_args, "-vncunused");
         }
     }
     if (sdl) {
         flexarray_append(dm_args, "-sdl");
-        if (!sdl->opengl) {
+        if (!libxl_defbool_val(sdl->opengl)) {
             flexarray_append(dm_args, "-disable-opengl");
         }
         /* XXX sdl->{display,xauthority} into $DISPLAY/$XAUTHORITY */
@@ -175,7 +175,7 @@ static char ** libxl__build_device_model
             flexarray_vappend(dm_args, "-serial", b_info->u.hvm.serial, NULL);
         }
 
-        if (b_info->u.hvm.nographic && (!sdl && !vnc)) {
+        if (libxl_defbool_val(b_info->u.hvm.nographic) && (!sdl && !vnc)) {
             flexarray_append(dm_args, "-nographic");
         }
 
@@ -185,7 +185,7 @@ static char ** libxl__build_device_model
                                    libxl__sizekb_to_mb(b_info->video_memkb)),
                     NULL);
         }
-        if (b_info->u.hvm.stdvga) {
+        if (libxl_defbool_val(b_info->u.hvm.stdvga)) {
             flexarray_append(dm_args, "-std-vga");
         }
 
@@ -238,7 +238,7 @@ static char ** libxl__build_device_model
         if ( ioemu_vifs == 0 ) {
             flexarray_vappend(dm_args, "-net", "none", NULL);
         }
-        if (b_info->u.hvm.gfx_passthru) {
+        if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
             flexarray_append(dm_args, "-gfx_passthru");
         }
     } else {
@@ -291,7 +291,7 @@ static char *dm_spice_options(libxl__gc 
         return NULL;
     }
 
-    if (!spice->disable_ticketing) {
+    if (!libxl_defbool_val(spice->disable_ticketing)) {
         if (!spice->passwd) {
             LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
                        "spice ticketing is enabled but missing password");
@@ -307,12 +307,12 @@ static char *dm_spice_options(libxl__gc 
                          spice->port, spice->tls_port);
     if (spice->host)
         opt = libxl__sprintf(gc, "%s,addr=%s", opt, spice->host);
-    if (spice->disable_ticketing)
+    if (libxl_defbool_val(spice->disable_ticketing))
         opt = libxl__sprintf(gc, "%s,disable-ticketing", opt);
     else
         opt = libxl__sprintf(gc, "%s,password=%s", opt, spice->passwd);
     opt = libxl__sprintf(gc, "%s,agent-mouse=%s", opt,
-                         spice->agent_mouse ? "on" : "off");
+                         libxl_defbool_val(spice->agent_mouse) ? "on" : "off");
     return opt;
 }
 
@@ -379,11 +379,11 @@ static char ** libxl__build_device_model
         if (strchr(listen, ':') != NULL)
             flexarray_append(dm_args,
                     libxl__sprintf(gc, "%s%s", listen,
-                        vnc->findunused ? ",to=99" : ""));
+                        libxl_defbool_val(vnc->findunused) ? ",to=99" : ""));
         else
             flexarray_append(dm_args,
                     libxl__sprintf(gc, "%s:%d%s", listen, display,
-                        vnc->findunused ? ",to=99" : ""));
+                        libxl_defbool_val(vnc->findunused) ? ",to=99" : ""));
     }
     if (sdl) {
         flexarray_append(dm_args, "-sdl");
@@ -405,11 +405,11 @@ static char ** libxl__build_device_model
             flexarray_vappend(dm_args, "-serial", b_info->u.hvm.serial, NULL);
         }
 
-        if (b_info->u.hvm.nographic && (!sdl && !vnc)) {
+        if (libxl_defbool_val(b_info->u.hvm.nographic) && (!sdl && !vnc)) {
             flexarray_append(dm_args, "-nographic");
         }
 
-        if (b_info->u.hvm.spice.enable) {
+        if (libxl_defbool_val(b_info->u.hvm.spice.enable)) {
             const libxl_spice_info *spice = &b_info->u.hvm.spice;
             char *spiceoptions = dm_spice_options(gc, spice);
             if (!spiceoptions)
@@ -419,7 +419,7 @@ static char ** libxl__build_device_model
             flexarray_append(dm_args, spiceoptions);
         }
 
-        if (b_info->u.hvm.stdvga) {
+        if (libxl_defbool_val(b_info->u.hvm.stdvga)) {
                 flexarray_vappend(dm_args, "-vga", "std", NULL);
         }
 
@@ -479,7 +479,7 @@ static char ** libxl__build_device_model
             flexarray_append(dm_args, "-net");
             flexarray_append(dm_args, "none");
         }
-        if (b_info->u.hvm.gfx_passthru) {
+        if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
             flexarray_append(dm_args, "-gfx_passthru");
         }
     } else {
diff -r 42c448aa6415 -r d27c92eeeddc tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Mon Feb 13 16:27:27 2012 +0000
+++ b/tools/libxl/libxl_types.idl	Mon Feb 13 16:27:33 2012 +0000
@@ -106,31 +106,31 @@ libxl_timer_mode = Enumeration("timer_mo
 # Complex libxl types
 #
 libxl_vnc_info = Struct("vnc_info", [
-    ("enable",        bool),
+    ("enable",        libxl_defbool),
     # "address:port" that should be listened on
     ("listen",        string),
     ("passwd",        string),
     ("display",       integer),
     # If set then try to find an unused port
-    ("findunused",    bool),
+    ("findunused",    libxl_defbool),
     ])
 
 libxl_spice_info = Struct("spice_info", [
-    ("enable",            bool),
+    ("enable",      libxl_defbool),
     # At least one of spice port or spicetls_post must be given
     ("port",        integer),
     ("tls_port",    integer),
     # Interface to bind to
     ("host",        string),
     # enable client connection with no password
-    ("disable_ticketing", bool),
+    ("disable_ticketing", libxl_defbool),
     ("passwd",      string),
-    ("agent_mouse", bool),
+    ("agent_mouse", libxl_defbool),
     ])
 
 libxl_sdl_info = Struct("sdl_info", [
-    ("enable",        bool),
-    ("opengl",        bool),
+    ("enable",        libxl_defbool),
+    ("opengl",        libxl_defbool),
     ("display",       string),
     ("xauthority",    string),
     ])
@@ -244,15 +244,15 @@ libxl_domain_build_info = Struct("domain
                                        ("timer_mode",       libxl_timer_mode),
                                        ("nested_hvm",       libxl_defbool),
                                        ("incr_generationid",libxl_defbool),
-                                       ("nographic",        bool),
-                                       ("stdvga",           bool),
+                                       ("nographic",        libxl_defbool),
+                                       ("stdvga",           libxl_defbool),
                                        ("vnc",              libxl_vnc_info),
                                        # keyboard layout, default is en-us keyboard
                                        ("keymap",           string),
                                        ("sdl",              libxl_sdl_info),
                                        ("spice",            libxl_spice_info),
                                        
-                                       ("gfx_passthru",     bool),
+                                       ("gfx_passthru",     libxl_defbool),
                                        
                                        ("serial",           string),
                                        ("boot",             string),
diff -r 42c448aa6415 -r d27c92eeeddc tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Mon Feb 13 16:27:27 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Mon Feb 13 16:27:33 2012 +0000
@@ -939,7 +939,7 @@ skip:
                     break;
                 *p2 = '\0';
                 if (!strcmp(p, "vnc")) {
-                    vfb->vnc.enable = atoi(p2 + 1);
+                    libxl_defbool_set(&vfb->vnc.enable, atoi(p2 + 1));
                 } else if (!strcmp(p, "vnclisten")) {
                     free(vfb->vnc.listen);
                     vfb->vnc.listen = strdup(p2 + 1);
@@ -949,14 +949,14 @@ skip:
                 } else if (!strcmp(p, "vncdisplay")) {
                     vfb->vnc.display = atoi(p2 + 1);
                 } else if (!strcmp(p, "vncunused")) {
-                    vfb->vnc.findunused = atoi(p2 + 1);
+                    libxl_defbool_set(&vfb->vnc.findunused, atoi(p2 + 1));
                 } else if (!strcmp(p, "keymap")) {
                     free(vfb->keymap);
                     vfb->keymap = strdup(p2 + 1);
                 } else if (!strcmp(p, "sdl")) {
-                    vfb->sdl.enable = atoi(p2 + 1);
+                    libxl_defbool_set(&vfb->sdl.enable, atoi(p2 + 1));
                 } else if (!strcmp(p, "opengl")) {
-                    vfb->sdl.opengl = atoi(p2 + 1);
+                    libxl_defbool_set(&vfb->sdl.opengl, atoi(p2 + 1));
                 } else if (!strcmp(p, "display")) {
                     free(vfb->sdl.display);
                     vfb->sdl.display = strdup(p2 + 1);
@@ -1156,41 +1156,35 @@ skip_vfb:
 #undef parse_extra_args
 
     if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) {
-        if (!xlu_cfg_get_long (config, "stdvga", &l, 0))
-            b_info->u.hvm.stdvga = l;
-        if (!xlu_cfg_get_long (config, "vnc", &l, 0))
-            b_info->u.hvm.vnc.enable = l;
-        xlu_cfg_replace_string (config, "vnclisten", &b_info->u.hvm.vnc.listen, 0);
-        xlu_cfg_replace_string (config, "vncpasswd", &b_info->u.hvm.vnc.passwd, 0);
+        xlu_cfg_get_defbool(config, "stdvga", &b_info->u.hvm.stdvga, 0);
+        xlu_cfg_get_defbool(config, "vnc", &b_info->u.hvm.vnc.enable, 0);
+        xlu_cfg_replace_string (config, "vnclisten",
+                                &b_info->u.hvm.vnc.listen, 0);
+        xlu_cfg_replace_string (config, "vncpasswd",
+                                &b_info->u.hvm.vnc.passwd, 0);
         if (!xlu_cfg_get_long (config, "vncdisplay", &l, 0))
             b_info->u.hvm.vnc.display = l;
-        if (!xlu_cfg_get_long (config, "vncunused", &l, 0))
-            b_info->u.hvm.vnc.findunused = l;
+        xlu_cfg_get_defbool(config, "vncunused",
+                            &b_info->u.hvm.vnc.findunused, 0);
         xlu_cfg_replace_string (config, "keymap", &b_info->u.hvm.keymap, 0);
-        if (!xlu_cfg_get_long (config, "sdl", &l, 0))
-            b_info->u.hvm.sdl.enable = l;
-        if (!xlu_cfg_get_long (config, "opengl", &l, 0))
-            b_info->u.hvm.sdl.opengl = l;
-        if (!xlu_cfg_get_long (config, "spice", &l, 0))
-            b_info->u.hvm.spice.enable = l;
+        xlu_cfg_get_defbool(config, "sdl", &b_info->u.hvm.sdl.enable, 0);
+        xlu_cfg_get_defbool(config, "opengl", &b_info->u.hvm.sdl.opengl, 0);
+        xlu_cfg_get_defbool (config, "spice", &b_info->u.hvm.spice.enable, 0);
         if (!xlu_cfg_get_long (config, "spiceport", &l, 0))
             b_info->u.hvm.spice.port = l;
         if (!xlu_cfg_get_long (config, "spicetls_port", &l, 0))
             b_info->u.hvm.spice.tls_port = l;
         xlu_cfg_replace_string (config, "spicehost",
                                 &b_info->u.hvm.spice.host, 0);
-        if (!xlu_cfg_get_long (config, "spicedisable_ticketing", &l, 0))
-            b_info->u.hvm.spice.disable_ticketing = l;
+        xlu_cfg_get_defbool(config, "spicedisable_ticketing",
+                            &b_info->u.hvm.spice.disable_ticketing, 0);
         xlu_cfg_replace_string (config, "spicepasswd",
                                 &b_info->u.hvm.spice.passwd, 0);
-        if (!xlu_cfg_get_long (config, "spiceagent_mouse", &l, 0))
-            b_info->u.hvm.spice.agent_mouse = l;
-        else
-            b_info->u.hvm.spice.agent_mouse = 1;
-        if (!xlu_cfg_get_long (config, "nographic", &l, 0))
-            b_info->u.hvm.nographic = l;
-        if (!xlu_cfg_get_long (config, "gfx_passthru", &l, 0))
-            b_info->u.hvm.gfx_passthru = l;
+        xlu_cfg_get_defbool(config, "spiceagent_mouse",
+                            &b_info->u.hvm.spice.agent_mouse, 0);
+        xlu_cfg_get_defbool(config, "nographic", &b_info->u.hvm.nographic, 0);
+        xlu_cfg_get_defbool(config, "gfx_passthru", 
+                            &b_info->u.hvm.gfx_passthru, 0);
         xlu_cfg_replace_string (config, "serial", &b_info->u.hvm.serial, 0);
         xlu_cfg_replace_string (config, "boot", &b_info->u.hvm.boot, 0);
         xlu_cfg_get_defbool(config, "usb", &b_info->u.hvm.usb, 0);
diff -r 42c448aa6415 -r d27c92eeeddc tools/libxl/xl_sxp.c
--- a/tools/libxl/xl_sxp.c	Mon Feb 13 16:27:27 2012 +0000
+++ b/tools/libxl/xl_sxp.c	Mon Feb 13 16:27:33 2012 +0000
@@ -110,26 +110,34 @@ void printf_info_sexp(int domid, libxl_d
                libxl_defbool_to_string(b_info->u.hvm.nested_hvm));
         printf("\t\t\t(no_incr_generationid %s)\n",
                libxl_defbool_to_string(b_info->u.hvm.incr_generationid));
-
-        printf("\t\t\t(stdvga %d)\n", b_info->u.hvm.stdvga);
-        printf("\t\t\t(vnc %d)\n", b_info->u.hvm.vnc.enable);
+        printf("\t\t\t(stdvga %s)\n",
+               libxl_defbool_to_string(b_info->u.hvm.stdvga));
+        printf("\t\t\t(vnc %s)\n",
+               libxl_defbool_to_string(b_info->u.hvm.vnc.enable));
         printf("\t\t\t(vnclisten %s)\n", b_info->u.hvm.vnc.listen);
         printf("\t\t\t(vncdisplay %d)\n", b_info->u.hvm.vnc.display);
-        printf("\t\t\t(vncunused %d)\n", b_info->u.hvm.vnc.findunused);
+        printf("\t\t\t(vncunused %s)\n",
+               libxl_defbool_to_string(b_info->u.hvm.vnc.findunused));
         printf("\t\t\t(keymap %s)\n", b_info->u.hvm.keymap);
-        printf("\t\t\t(sdl %d)\n", b_info->u.hvm.sdl.enable);
-        printf("\t\t\t(opengl %d)\n", b_info->u.hvm.sdl.opengl);
-        printf("\t\t\t(nographic %d)\n", b_info->u.hvm.nographic);
-        printf("\t\t\t(spice %d)\n", b_info->u.hvm.spice.enable);
+        printf("\t\t\t(sdl %s)\n",
+               libxl_defbool_to_string(b_info->u.hvm.sdl.enable));
+        printf("\t\t\t(opengl %s)\n",
+               libxl_defbool_to_string(b_info->u.hvm.sdl.opengl));
+        printf("\t\t\t(nographic %s)\n",
+               libxl_defbool_to_string(b_info->u.hvm.nographic));
+        printf("\t\t\t(spice %s)\n",
+               libxl_defbool_to_string(b_info->u.hvm.spice.enable));
         printf("\t\t\t(spiceport %d)\n", b_info->u.hvm.spice.port);
         printf("\t\t\t(spicetls_port %d)\n", b_info->u.hvm.spice.tls_port);
         printf("\t\t\t(spicehost %s)\n", b_info->u.hvm.spice.host);
-        printf("\t\t\t(spicedisable_ticketing %d)\n",
-                    b_info->u.hvm.spice.disable_ticketing);
-        printf("\t\t\t(spiceagent_mouse %d)\n", b_info->u.hvm.spice.agent_mouse);
+        printf("\t\t\t(spicedisable_ticketing %s)\n",
+               libxl_defbool_to_string(b_info->u.hvm.spice.disable_ticketing));
+        printf("\t\t\t(spiceagent_mouse %s)\n",
+               libxl_defbool_to_string(b_info->u.hvm.spice.agent_mouse));
 
         printf("\t\t\t(device_model %s)\n", b_info->device_model ? : "default");
-        printf("\t\t\t(gfx_passthru %d)\n", b_info->u.hvm.gfx_passthru);
+        printf("\t\t\t(gfx_passthru %s)\n",
+               libxl_defbool_to_string(b_info->u.hvm.gfx_passthru));
         printf("\t\t\t(serial %s)\n", b_info->u.hvm.serial);
         printf("\t\t\t(boot %s)\n", b_info->u.hvm.boot);
         printf("\t\t\t(usb %s)\n", libxl_defbool_to_string(b_info->u.hvm.usb));
@@ -204,13 +212,17 @@ void printf_info_sexp(int domid, libxl_d
         printf("\t\t\t(backend_domid %d)\n", d_config->vfbs[i].backend_domid);
         printf("\t\t\t(frontend_domid %d)\n", domid);
         printf("\t\t\t(devid %d)\n", d_config->vfbs[i].devid);
-        printf("\t\t\t(vnc %d)\n", d_config->vfbs[i].vnc.enable);
+        printf("\t\t\t(vnc %s)\n",
+               libxl_defbool_to_string(d_config->vfbs[i].vnc.enable));
         printf("\t\t\t(vnclisten %s)\n", d_config->vfbs[i].vnc.listen);
         printf("\t\t\t(vncdisplay %d)\n", d_config->vfbs[i].vnc.display);
-        printf("\t\t\t(vncunused %d)\n", d_config->vfbs[i].vnc.findunused);
+        printf("\t\t\t(vncunused %s)\n",
+               libxl_defbool_to_string(d_config->vfbs[i].vnc.findunused));
         printf("\t\t\t(keymap %s)\n", d_config->vfbs[i].keymap);
-        printf("\t\t\t(sdl %d)\n", d_config->vfbs[i].sdl.enable);
-        printf("\t\t\t(opengl %d)\n", d_config->vfbs[i].sdl.opengl);
+        printf("\t\t\t(sdl %s)\n",
+               libxl_defbool_to_string(d_config->vfbs[i].sdl.enable));
+        printf("\t\t\t(opengl %s)\n",
+               libxl_defbool_to_string(d_config->vfbs[i].sdl.opengl));
         printf("\t\t\t(display %s)\n", d_config->vfbs[i].sdl.display);
         printf("\t\t\t(xauthority %s)\n", d_config->vfbs[i].sdl.xauthority);
         printf("\t\t)\n");

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 18 of 23] libxl: do not explicitly initialise members of build info to zero
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (16 preceding siblings ...)
  2012-02-13 16:53 ` [PATCH 17 of 23] libxl: use defbool for graphics related options Ian Campbell
@ 2012-02-13 16:53 ` Ian Campbell
  2012-02-20 19:20   ` Ian Jackson
  2012-02-13 16:53 ` [PATCH 19 of 23] libxl: switch device model selection over to libxl_defbool Ian Campbell
                   ` (4 subsequent siblings)
  22 siblings, 1 reply; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:53 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329150453 0
# Node ID ba3ab3558a5d84fef2676b82d641a04615a9d786
# Parent  d27c92eeeddc5e4c190a88ce282fb0155405a65e
libxl: do not explicitly initialise members of build info to zero

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

diff -r d27c92eeeddc -r ba3ab3558a5d tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Mon Feb 13 16:27:33 2012 +0000
+++ b/tools/libxl/libxl_create.c	Mon Feb 13 16:27:33 2012 +0000
@@ -73,7 +73,6 @@ void libxl_domain_build_info_init(libxl_
                                   const libxl_domain_create_info *c_info)
 {
     memset(b_info, '\0', sizeof(*b_info));
-    b_info->cpuid = NULL;
     b_info->type = c_info->type;
 
     b_info->max_memkb = LIBXL_MEMKB_DEFAULT;
@@ -86,12 +85,7 @@ void libxl_domain_build_info_init(libxl_
 
     switch (b_info->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
-        b_info->u.hvm.firmware = NULL;
         b_info->u.hvm.timer_mode = LIBXL_TIMER_MODE_DEFAULT;
-
-        b_info->u.hvm.vnc.display = 0;
-        b_info->u.hvm.serial = NULL;
-        b_info->u.hvm.usbdevice = NULL;
         break;
     case LIBXL_DOMAIN_TYPE_PV:
         b_info->u.pv.slack_memkb = 0;

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 19 of 23] libxl: switch device model selection over to libxl_defbool
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (17 preceding siblings ...)
  2012-02-13 16:53 ` [PATCH 18 of 23] libxl: do not explicitly initialise members of build info to zero Ian Campbell
@ 2012-02-13 16:53 ` Ian Campbell
  2012-02-13 16:53 ` [PATCH 20 of 23] libxl: add libxl_domain_build_info_init_type Ian Campbell
                   ` (3 subsequent siblings)
  22 siblings, 0 replies; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:53 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329150454 0
# Node ID 871a9c864b558f554f6d72cef59cfc8e6af2d81a
# Parent  ba3ab3558a5d84fef2676b82d641a04615a9d786
libxl: switch device model selection over to libxl_defbool

This allows it to be set via the _init/_setdefault methods.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
[since last v -- ERROR_INVAL on stubdomains + !traditional qemu]

diff -r ba3ab3558a5d -r 871a9c864b55 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Feb 13 16:27:33 2012 +0000
+++ b/tools/libxl/libxl.c	Mon Feb 13 16:27:34 2012 +0000
@@ -2713,7 +2713,7 @@ int libxl_domain_need_memory(libxl_ctx *
     switch (b_info->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
         *need_memkb += b_info->shadow_memkb + LIBXL_HVM_EXTRA_MEMORY;
-        if (b_info->device_model_stubdomain)
+        if (libxl_defbool_val(b_info->device_model_stubdomain))
             *need_memkb += 32 * 1024;
         break;
     case LIBXL_DOMAIN_TYPE_PV:
diff -r ba3ab3558a5d -r 871a9c864b55 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Mon Feb 13 16:27:33 2012 +0000
+++ b/tools/libxl/libxl_create.c	Mon Feb 13 16:27:34 2012 +0000
@@ -80,9 +80,6 @@ void libxl_domain_build_info_init(libxl_
     b_info->shadow_memkb = LIBXL_MEMKB_DEFAULT;
     b_info->video_memkb =  LIBXL_MEMKB_DEFAULT;
 
-    b_info->device_model_stubdomain = false;
-    b_info->device_model = NULL;
-
     switch (b_info->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
         b_info->u.hvm.timer_mode = LIBXL_TIMER_MODE_DEFAULT;
@@ -102,6 +99,17 @@ int libxl__domain_build_info_setdefault(
         b_info->device_model_version =
             LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
 
+    libxl_defbool_setdefault(&b_info->device_model_stubdomain, false);
+
+    if (b_info->type == LIBXL_DOMAIN_TYPE_HVM &&
+        b_info->device_model_version !=
+            LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL &&
+        libxl_defbool_val(b_info->device_model_stubdomain)) {
+        LIBXL__LOG(CTX, XTL_ERROR,
+            "device model stubdomains require \"qemu-xen-traditional\"");
+        return ERROR_INVAL;
+    }
+
     if (!b_info->max_vcpus)
         b_info->max_vcpus = 1;
     if (!b_info->cur_vcpus)
diff -r ba3ab3558a5d -r 871a9c864b55 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Mon Feb 13 16:27:33 2012 +0000
+++ b/tools/libxl/libxl_dm.c	Mon Feb 13 16:27:34 2012 +0000
@@ -39,7 +39,7 @@ const char *libxl__domain_device_model(l
     libxl_ctx *ctx = libxl__gc_owner(gc);
     const char *dm;
 
-    if (info->device_model_stubdomain)
+    if (libxl_defbool_val(info->device_model_stubdomain))
         return NULL;
 
     if (info->device_model) {
@@ -905,7 +905,7 @@ int libxl__create_device_model(libxl__gc
     char **pass_stuff;
     const char *dm;
 
-    if (b_info->device_model_stubdomain) {
+    if (libxl_defbool_val(b_info->device_model_stubdomain)) {
         rc = libxl__create_stubdom(gc, domid, guest_config, state, starting_r);
         goto out;
     }
diff -r ba3ab3558a5d -r 871a9c864b55 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Mon Feb 13 16:27:33 2012 +0000
+++ b/tools/libxl/libxl_types.idl	Mon Feb 13 16:27:34 2012 +0000
@@ -217,8 +217,8 @@ libxl_domain_build_info = Struct("domain
     ("type",            libxl_domain_type),
     
     ("device_model_version", libxl_device_model_version),
-    ("device_model_stubdomain", bool),
-    # you set device_model you must set device_model_version too
+    ("device_model_stubdomain", libxl_defbool),
+    # if you set device_model you must set device_model_version too
     ("device_model",     string),
     ("device_model_ssidref", uint32),
 
diff -r ba3ab3558a5d -r 871a9c864b55 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Mon Feb 13 16:27:33 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Mon Feb 13 16:27:34 2012 +0000
@@ -1118,8 +1118,8 @@ skip_vfb:
         }
     } else if (b_info->device_model)
         fprintf(stderr, "WARNING: device model override given without specific DM version\n");
-    if (!xlu_cfg_get_long (config, "device_model_stubdomain_override", &l, 0))
-        b_info->device_model_stubdomain = l;
+    xlu_cfg_get_defbool (config, "device_model_stubdomain_override",
+                         &b_info->device_model_stubdomain, 0);
 
     if (!xlu_cfg_get_string (config, "device_model_stubdomain_seclabel",
                              &buf, 0)) {

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 20 of 23] libxl: add libxl_domain_build_info_init_type
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (18 preceding siblings ...)
  2012-02-13 16:53 ` [PATCH 19 of 23] libxl: switch device model selection over to libxl_defbool Ian Campbell
@ 2012-02-13 16:53 ` Ian Campbell
  2012-02-20 19:22   ` Ian Jackson
  2012-02-20 19:24   ` Ian Jackson
  2012-02-13 16:53 ` [PATCH 21 of 23] libxl: Make IDL KeyedUnion keyvar an idl.Field Ian Campbell
                   ` (2 subsequent siblings)
  22 siblings, 2 replies; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:53 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329150454 0
# Node ID 1adeebf9e6bd3d74a2f6e35937d4c6ada4212ccd
# Parent  871a9c864b558f554f6d72cef59cfc8e6af2d81a
libxl: add libxl_domain_build_info_init_type

Use instead of parameterising libxl_domain_build_info_init. This allows callers
to initialise a libxl_domain_build_info but not to commit to a particular type
later on (e.g. after they've parsed the domain config)

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

diff -r 871a9c864b55 -r 1adeebf9e6bd tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Mon Feb 13 16:27:34 2012 +0000
+++ b/tools/libxl/libxl.h	Mon Feb 13 16:27:34 2012 +0000
@@ -389,8 +389,9 @@ int libxl_ctx_postfork(libxl_ctx *ctx);
 
 /* domain related functions */
 void libxl_domain_create_info_init(libxl_domain_create_info *c_info);
-void libxl_domain_build_info_init(libxl_domain_build_info *b_info,
-                          const libxl_domain_create_info *c_info);
+void libxl_domain_build_info_init(libxl_domain_build_info *b_info);
+void libxl_domain_build_info_init_type(libxl_domain_build_info *b_info,
+                                       libxl_domain_type type);
 typedef int (*libxl_console_ready)(libxl_ctx *ctx, uint32_t domid, void *priv);
 int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid);
 int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid, int restore_fd);
diff -r 871a9c864b55 -r 1adeebf9e6bd tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Mon Feb 13 16:27:34 2012 +0000
+++ b/tools/libxl/libxl_create.c	Mon Feb 13 16:27:34 2012 +0000
@@ -69,23 +69,29 @@ int libxl__domain_create_info_setdefault
     return 0;
 }
 
-void libxl_domain_build_info_init(libxl_domain_build_info *b_info,
-                                  const libxl_domain_create_info *c_info)
+void libxl_domain_build_info_init(libxl_domain_build_info *b_info)
 {
     memset(b_info, '\0', sizeof(*b_info));
-    b_info->type = c_info->type;
+    b_info->type = -1;
 
     b_info->max_memkb = LIBXL_MEMKB_DEFAULT;
     b_info->target_memkb = LIBXL_MEMKB_DEFAULT;
     b_info->shadow_memkb = LIBXL_MEMKB_DEFAULT;
     b_info->video_memkb =  LIBXL_MEMKB_DEFAULT;
 
+}
+
+void libxl_domain_build_info_init_type(libxl_domain_build_info *b_info,
+                                       libxl_domain_type type)
+{
+    assert(b_info->type == -1);
+    b_info->type = type;
     switch (b_info->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
         b_info->u.hvm.timer_mode = LIBXL_TIMER_MODE_DEFAULT;
         break;
     case LIBXL_DOMAIN_TYPE_PV:
-        b_info->u.pv.slack_memkb = 0;
+        b_info->u.pv.slack_memkb = LIBXL_MEMKB_DEFAULT;
         break;
     default:
         abort();
diff -r 871a9c864b55 -r 1adeebf9e6bd tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Mon Feb 13 16:27:34 2012 +0000
+++ b/tools/libxl/libxl_dm.c	Mon Feb 13 16:27:34 2012 +0000
@@ -707,7 +707,8 @@ static int libxl__create_stubdom(libxl__
 
     libxl_uuid_generate(&dm_config.c_info.uuid);
 
-    libxl_domain_build_info_init(&dm_config.b_info, &dm_config.c_info);
+    libxl_domain_build_info_init(&dm_config.b_info);
+    libxl_domain_build_info_init_type(&dm_config.b_info, LIBXL_DOMAIN_TYPE_PV);
 
     dm_config.b_info.max_vcpus = 1;
     dm_config.b_info.max_memkb = 32 * 1024;
diff -r 871a9c864b55 -r 1adeebf9e6bd tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Mon Feb 13 16:27:34 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Mon Feb 13 16:27:34 2012 +0000
@@ -584,7 +584,8 @@ static void parse_config_data(const char
         exit(1);
     }
 
-    libxl_domain_build_info_init(b_info, c_info);
+    libxl_domain_build_info_init(b_info);
+    libxl_domain_build_info_init_type(b_info, c_info->type);
 
     /* the following is the actual config parsing with overriding values in the structures */
     if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
@@ -701,7 +702,7 @@ static void parse_config_data(const char
     if (!xlu_cfg_get_long (config, "videoram", &l, 0))
         b_info->video_memkb = l * 1024;
 
-    switch(c_info->type) {
+    switch(b_info->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
         if (!xlu_cfg_get_string (config, "kernel", &buf, 0))
             fprintf(stderr, "WARNING: ignoring \"kernel\" directive for HVM guest. "

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 21 of 23] libxl: Make IDL KeyedUnion keyvar an idl.Field
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (19 preceding siblings ...)
  2012-02-13 16:53 ` [PATCH 20 of 23] libxl: add libxl_domain_build_info_init_type Ian Campbell
@ 2012-02-13 16:53 ` Ian Campbell
  2012-02-20 19:17   ` Ian Jackson
  2012-02-13 16:53 ` [PATCH 22 of 23] libxl: idl: generate KeyedUnion key member as part of the KeyedUnion Ian Campbell
  2012-02-13 16:53 ` [PATCH 23 of 23] libxl: autogenerate libxl_FOO_init and libxl_FOO_init_FIELD Ian Campbell
  22 siblings, 1 reply; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:53 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329150454 0
# Node ID 0751a69b3ef2264844500c47151b5e16a358e02d
# Parent  1adeebf9e6bd3d74a2f6e35937d4c6ada4212ccd
libxl: Make IDL KeyedUnion keyvar an idl.Field

This is more logical than having keyvar_name and keyvar_type members.

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

diff -r 1adeebf9e6bd -r 0751a69b3ef2 tools/libxl/gentest.py
--- a/tools/libxl/gentest.py	Mon Feb 13 16:27:34 2012 +0000
+++ b/tools/libxl/gentest.py	Mon Feb 13 16:27:34 2012 +0000
@@ -31,7 +31,7 @@ def gen_rand_init(ty, v, indent = "    "
     elif isinstance(ty, idl.KeyedUnion):
         if parent is None:
             raise Exception("KeyedUnion type must have a parent")
-        s += "switch (%s) {\n" % (parent + ty.keyvar_name)
+        s += "switch (%s) {\n" % (parent + ty.keyvar.name)
         for f in ty.fields:
             (nparent,fexpr) = ty.member(v, f, parent is None)
             s += "case %s:\n" % f.enumname
diff -r 1adeebf9e6bd -r 0751a69b3ef2 tools/libxl/gentypes.py
--- a/tools/libxl/gentypes.py	Mon Feb 13 16:27:34 2012 +0000
+++ b/tools/libxl/gentypes.py	Mon Feb 13 16:27:34 2012 +0000
@@ -56,7 +56,7 @@ def libxl_C_type_dispose(ty, v, indent =
     if isinstance(ty, idl.KeyedUnion):
         if parent is None:
             raise Exception("KeyedUnion type must have a parent")
-        s += "switch (%s) {\n" % (parent + ty.keyvar_name)
+        s += "switch (%s) {\n" % (parent + ty.keyvar.name)
         for f in ty.fields:
             (nparent,fexpr) = ty.member(v, f, parent is None)
             s += "case %s:\n" % f.enumname
@@ -86,7 +86,7 @@ def libxl_C_type_gen_json(ty, v, indent 
     elif isinstance(ty, idl.KeyedUnion):
         if parent is None:
             raise Exception("KeyedUnion type must have a parent")
-        s += "switch (%s) {\n" % (parent + ty.keyvar_name)
+        s += "switch (%s) {\n" % (parent + ty.keyvar.name)
         for f in ty.fields:
             (nparent,fexpr) = ty.member(v, f, parent is None)
             s += "case %s:\n" % f.enumname
diff -r 1adeebf9e6bd -r 0751a69b3ef2 tools/libxl/idl.py
--- a/tools/libxl/idl.py	Mon Feb 13 16:27:34 2012 +0000
+++ b/tools/libxl/idl.py	Mon Feb 13 16:27:34 2012 +0000
@@ -206,8 +206,9 @@ class KeyedUnion(Aggregate):
         if not isinstance(keyvar_type, Enumeration):
             raise ValueError
 
-        self.keyvar_name = keyvar_name
-        self.keyvar_type = keyvar_type
+        kv_kwargs = dict([(x.lstrip('keyvar_'),y) for (x,y) in kwargs.items() if x.startswith('keyvar_')])
+        
+        self.keyvar = Field(keyvar_type, keyvar_name, **kv_kwargs)
 
         for f in fields:
             # (name, enum, type)
diff -r 1adeebf9e6bd -r 0751a69b3ef2 tools/libxl/idl.txt
--- a/tools/libxl/idl.txt	Mon Feb 13 16:27:34 2012 +0000
+++ b/tools/libxl/idl.txt	Mon Feb 13 16:27:34 2012 +0000
@@ -128,10 +128,9 @@ idl.KeyedUnion
  where the currently valid member of the union can be determined based
  upon another member in the containing type.
 
- The KeyedUnion.keyvar_name must contain the name of the member of the
+ The KeyedUnion.keyvar contains an idl.type the member of the
  containing type which determines the valid member of the union. The
- member referenced by KeyedUnion.keyvar_name has type
- KeyedUnion.keyvar_type which must be an instance of the Enumeration type.
+ must be an instance of the Enumeration type.
 
 Standard Types
 --------------

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 22 of 23] libxl: idl: generate KeyedUnion key member as part of the KeyedUnion
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (20 preceding siblings ...)
  2012-02-13 16:53 ` [PATCH 21 of 23] libxl: Make IDL KeyedUnion keyvar an idl.Field Ian Campbell
@ 2012-02-13 16:53 ` Ian Campbell
  2012-02-13 17:08   ` Ian Jackson
  2012-02-13 16:53 ` [PATCH 23 of 23] libxl: autogenerate libxl_FOO_init and libxl_FOO_init_FIELD Ian Campbell
  22 siblings, 1 reply; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:53 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329150454 0
# Node ID ba485f9fce317ba31a96b407f5712ba62239e223
# Parent  0751a69b3ef2264844500c47151b5e16a358e02d
libxl: idl: generate KeyedUnion key member as part of the KeyedUnion

Rather than specifying it twice in the IDL.

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

diff -r 0751a69b3ef2 -r ba485f9fce31 tools/libxl/gentypes.py
--- a/tools/libxl/gentypes.py	Mon Feb 13 16:27:34 2012 +0000
+++ b/tools/libxl/gentypes.py	Mon Feb 13 16:27:34 2012 +0000
@@ -32,6 +32,9 @@ def libxl_C_type_define(ty, indent = "")
             s += "} %s" % ty.typename
 
     elif isinstance(ty, idl.Aggregate):
+        if isinstance(ty, idl.KeyedUnion):
+            s += libxl_C_instance_of(ty.keyvar.type, ty.keyvar.name) + ";\n"
+            
         if ty.typename is None:
             s += "%s {\n" % ty.kind
         else:
diff -r 0751a69b3ef2 -r ba485f9fce31 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Mon Feb 13 16:27:34 2012 +0000
+++ b/tools/libxl/libxl_types.idl	Mon Feb 13 16:27:34 2012 +0000
@@ -214,7 +214,6 @@ libxl_domain_build_info = Struct("domain
     ("shadow_memkb",    MemKB),
     ("disable_migrate", libxl_defbool),
     ("cpuid",           libxl_cpuid_policy_list),
-    ("type",            libxl_domain_type),
     
     ("device_model_version", libxl_device_model_version),
     ("device_model_stubdomain", libxl_defbool),
@@ -421,7 +420,6 @@ libxl_event = Struct("event",[
     ("domid",    libxl_domid),
     ("domuuid",  libxl_uuid),
     ("for_user", libxl_ev_user),
-    ("type",     libxl_event_type),
     ("u", KeyedUnion(None, libxl_event_type, "type",
           [("domain_shutdown", Struct(None, [
                                              ("shutdown_reason", uint8),

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 23 of 23] libxl: autogenerate libxl_FOO_init and libxl_FOO_init_FIELD
  2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
                   ` (21 preceding siblings ...)
  2012-02-13 16:53 ` [PATCH 22 of 23] libxl: idl: generate KeyedUnion key member as part of the KeyedUnion Ian Campbell
@ 2012-02-13 16:53 ` Ian Campbell
  2012-02-20 19:24   ` Ian Jackson
  22 siblings, 1 reply; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 16:53 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329150454 0
# Node ID 50c87c40031ca91265812881b18cecae7cdde138
# Parent  ba485f9fce317ba31a96b407f5712ba62239e223
libxl: autogenerate libxl_FOO_init and libxl_FOO_init_FIELD

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

diff -r ba485f9fce31 -r 50c87c40031c tools/libxl/gentypes.py
--- a/tools/libxl/gentypes.py	Mon Feb 13 16:27:34 2012 +0000
+++ b/tools/libxl/gentypes.py	Mon Feb 13 16:27:34 2012 +0000
@@ -78,6 +78,88 @@ def libxl_C_type_dispose(ty, v, indent =
         s = indent + s
     return s.replace("\n", "\n%s" % indent).rstrip(indent)
 
+def libxl_init_members(ty, nesting = 0):
+    """Returns a list of members of ty which require a separate init"""
+
+    if isinstance(ty, idl.Aggregate):
+        return [f for f in ty.fields if not f.const and isinstance(f.type,idl.KeyedUnion)]
+    else:
+        return []
+    
+def _libxl_C_type_init(ty, v, indent = "    ", parent = None, subinit=False):
+    s = ""
+    if isinstance(ty, idl.KeyedUnion):
+        if parent is None:
+            raise Exception("KeyedUnion type must have a parent")
+        if subinit:
+            s += "switch (%s) {\n" % (parent + ty.keyvar.name)
+            for f in ty.fields:
+                (nparent,fexpr) = ty.member(v, f, parent is None)
+                s += "case %s:\n" % f.enumname
+                s += _libxl_C_type_init(f.type, fexpr, "    ", nparent)
+                s += "    break;\n"
+            s += "}\n"
+        else:
+            if ty.keyvar.init_val:
+                s += "%s = %s;\n" % (parent + ty.keyvar.name, ty.keyvar.init_val)
+            elif ty.keyvar.type.init_val:
+                s += "%s = %s;\n" % (parent + ty.keyvar.name, ty.keyvar.type.init_val)
+    elif isinstance(ty, idl.Struct) and (parent is None or ty.init_fn is None):
+        for f in [f for f in ty.fields if not f.const]:
+            (nparent,fexpr) = ty.member(v, f, parent is None)
+            if f.init_val is not None:
+                s += "%s = %s;\n" % (fexpr, f.init_val)
+            else:
+                s += _libxl_C_type_init(f.type, fexpr, "", nparent)
+    else:
+        if ty.init_val is not None:
+            s += "%s = %s;\n" % (ty.pass_arg(v, parent is None), ty.init_val)
+        elif ty.init_fn is not None:
+            s += "%s(%s);\n" % (ty.init_fn, ty.pass_arg(v, parent is None))
+
+    if s != "":
+        s = indent + s
+    return s.replace("\n", "\n%s" % indent).rstrip(indent)
+
+def libxl_C_type_init(ty):
+    s = ""
+    s += "void %s(%s)\n" % (ty.init_fn, ty.make_arg("p", passby=idl.PASS_BY_REFERENCE))
+    s += "{\n"
+    s += "    memset(p, '\\0', sizeof(*p));\n"
+    s += _libxl_C_type_init(ty, "p")
+    s += "}\n"
+    s += "\n"
+    return s
+
+def libxl_C_type_member_init(ty, field):
+    if not isinstance(field.type, idl.KeyedUnion):
+        raise Exception("Only KeyedUnion is supported for member init")
+
+    ku = field.type
+    
+    s = ""
+    s += "void %s(%s, %s)\n" % (ty.init_fn + "_" + ku.keyvar.name,
+                                ty.make_arg("p", passby=idl.PASS_BY_REFERENCE),
+                                ku.keyvar.type.make_arg(ku.keyvar.name))
+    s += "{\n"
+    
+    if ku.keyvar.init_val:
+        init_val = ku.keyvar.init_val
+    elif ku.keyvar.type.init_val:
+        init_val = ku.keyvar.type.init_val
+    else:
+        init_val = None
+        
+    if init_val is not None:
+        (nparent,fexpr) = ty.member(ty.pass_arg("p"), ku.keyvar, isref=True)
+        s += "    assert(%s == %s);\n" % (fexpr, init_val)
+        s += "    %s = %s;\n" % (fexpr, ku.keyvar.name)
+    (nparent,fexpr) = ty.member(ty.pass_arg("p"), field, isref=True)
+    s += _libxl_C_type_init(ku, fexpr, parent=nparent, subinit=True)
+    s += "}\n"
+    s += "\n"
+    return s
+
 def libxl_C_type_gen_json(ty, v, indent = "    ", parent = None):
     s = ""
     if parent is None:
@@ -199,6 +281,15 @@ if __name__ == '__main__':
         f.write(libxl_C_type_define(ty) + ";\n")
         if ty.dispose_fn is not None:
             f.write("void %s(%s);\n" % (ty.dispose_fn, ty.make_arg("p")))
+        if ty.init_fn is not None:
+            f.write("void %s(%s);\n" % (ty.init_fn, ty.make_arg("p")))
+            for field in libxl_init_members(ty):
+                if not isinstance(field.type, idl.KeyedUnion):
+                    raise Exception("Only KeyedUnion is supported for member init")
+                ku = field.type
+                f.write("void %s(%s, %s);\n" % (ty.init_fn + "_" + ku.keyvar.name,
+                                               ty.make_arg("p"),
+                                               ku.keyvar.type.make_arg(ku.keyvar.name)))
         if ty.json_fn is not None:
             f.write("char *%s_to_json(libxl_ctx *ctx, %s);\n" % (ty.typename, ty.make_arg("p")))
         if isinstance(ty, idl.Enumeration):
@@ -227,7 +318,7 @@ if __name__ == '__main__':
 
 """ % (header_json_define, header_json_define, " ".join(sys.argv)))
 
-    for ty in [ty for ty in types+builtins if ty.json_fn is not None]:
+    for ty in [ty for ty in types if ty.json_fn is not None]:
         f.write("yajl_gen_status %s_gen_json(yajl_gen hand, %s);\n" % (ty.typename, ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
 
     f.write("\n")
@@ -264,6 +355,11 @@ if __name__ == '__main__':
         f.write("    memset(p, LIBXL_DTOR_POISON, sizeof(*p));\n")
         f.write("}\n")
         f.write("\n")
+        
+    for ty in [t for t in types if t.init_fn is not None and t.autogenerate_init_fn]:
+        f.write(libxl_C_type_init(ty))
+        for field in libxl_init_members(ty):
+            f.write(libxl_C_type_member_init(ty, field))
 
     for ty in [t for t in types if isinstance(t,idl.Enumeration)]:
         f.write("const char *%s_to_string(%s e)\n" % (ty.typename, ty.typename))
diff -r ba485f9fce31 -r 50c87c40031c tools/libxl/idl.py
--- a/tools/libxl/idl.py	Mon Feb 13 16:27:34 2012 +0000
+++ b/tools/libxl/idl.py	Mon Feb 13 16:27:34 2012 +0000
@@ -51,6 +51,10 @@ class Type(object):
 
         self.autogenerate_dispose_fn = kwargs.setdefault('autogenerate_dispose_fn', True)
 
+        self.init_fn = kwargs.setdefault('init_fn', None)
+        self.init_val = kwargs.setdefault('init_val', None)
+        self.autogenerate_init_fn = kwargs.setdefault('autogenerate_init_fn', False)
+
         if self.typename is not None and not self.private:
             self.json_fn = kwargs.setdefault('json_fn', self.typename + "_gen_json")
         else:
@@ -144,12 +148,20 @@ class Field(object):
         self.name = name
         self.const = kwargs.setdefault('const', False)
         self.enumname = kwargs.setdefault('enumname', None)
+        self.init_val = kwargs.setdefault('init_val', None)
 
 class Aggregate(Type):
     """A type containing a collection of other types"""
     def __init__(self, kind, typename, fields, **kwargs):
         Type.__init__(self, typename, **kwargs)
 
+        if self.typename is not None and self.dir in [DIR_IN, DIR_BOTH]:
+            self.init_fn = kwargs.setdefault('init_fn', self.typename + "_init")
+        else:
+            self.init_fn = kwargs.setdefault('init_fn', None)
+
+        self.autogenerate_init_fn = kwargs.setdefault('autogenerate_init_fn', True)
+
         self.kind = kind
 
         self.fields = []
diff -r ba485f9fce31 -r 50c87c40031c tools/libxl/idl.txt
--- a/tools/libxl/idl.txt	Mon Feb 13 16:27:34 2012 +0000
+++ b/tools/libxl/idl.txt	Mon Feb 13 16:27:34 2012 +0000
@@ -44,6 +44,22 @@ Type.autogenerate_dispose_fn: (default: 
  Indicates if the above named Type.dispose_fn should be
  autogenerated.
 
+Type.init_val: (default: None)
+
+ C expression for the value to initialise instances of this type to.
+
+ If present takes precendence over init_fn (see below).
+
+Type.init_fn: (default: typename + "_init" if dir in [IN, BOTH] and
+                        type != None)
+
+ The name of the C function which will initialist Type.
+
+Type.autogenerate_init_fn: (default: True if dir in [IN, BOTH])
+
+ Indicates if the above named Type.init_fn should be
+ autogenerated.
+
 Type.json_fn: (default: typename + "_gen_json" or None if type == None)
 
  The name of the C function which will generate a YAJL data structure
@@ -105,10 +121,13 @@ idl.Aggregate
 
  Each field has the following properties:
 
-  Field.type    The type of the member (a idl.Type).
-  Field.name    The name of the member (can be None for anonymous
-                fields).
-  Field.const   Boolean, true if the member is const.
+  Field.type     The type of the member (a idl.Type).
+  Field.name     The name of the member (can be None for anonymous
+                 fields).
+  Field.const    Boolean, true if the member is const.
+  Field.init_val The initialisation value for this field. Takes
+                 precendence over both Field.type.init_val and
+                 Field.type.init_fn.
 
 idl.Struct
 
diff -r ba485f9fce31 -r 50c87c40031c tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Feb 13 16:27:34 2012 +0000
+++ b/tools/libxl/libxl.c	Mon Feb 13 16:27:34 2012 +0000
@@ -1226,11 +1226,6 @@ int libxl_vncviewer_exec(libxl_ctx *ctx,
 
 /******************************************************************************/
 
-void libxl_device_disk_init(libxl_device_disk *disk)
-{
-    memset(disk, 0x00, sizeof(libxl_device_disk));
-}
-
 int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
 {
     int rc;
@@ -1717,10 +1712,6 @@ int libxl_device_disk_local_detach(libxl
 }
 
 /******************************************************************************/
-void libxl_device_nic_init(libxl_device_nic *nic)
-{
-    memset(nic, '\0', sizeof(*nic));
-}
 
 int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic)
 {
@@ -2141,10 +2132,6 @@ out:
 }
 
 /******************************************************************************/
-void libxl_device_vkb_init(libxl_device_vkb *vkb)
-{
-    memset(vkb, 0x00, sizeof(libxl_device_vkb));
-}
 
 int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb)
 {
@@ -2253,10 +2240,6 @@ out:
 }
 
 /******************************************************************************/
-void libxl_device_vfb_init(libxl_device_vfb *vfb)
-{
-    memset(vfb, 0x00, sizeof(libxl_device_vfb));
-}
 
 int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb)
 {
diff -r ba485f9fce31 -r 50c87c40031c tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Mon Feb 13 16:27:34 2012 +0000
+++ b/tools/libxl/libxl.h	Mon Feb 13 16:27:34 2012 +0000
@@ -388,10 +388,6 @@ int libxl_ctx_free(libxl_ctx *ctx /* 0 i
 int libxl_ctx_postfork(libxl_ctx *ctx);
 
 /* domain related functions */
-void libxl_domain_create_info_init(libxl_domain_create_info *c_info);
-void libxl_domain_build_info_init(libxl_domain_build_info *b_info);
-void libxl_domain_build_info_init_type(libxl_domain_build_info *b_info,
-                                       libxl_domain_type type);
 typedef int (*libxl_console_ready)(libxl_ctx *ctx, uint32_t domid, void *priv);
 int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid);
 int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid, int restore_fd);
@@ -527,7 +523,6 @@ libxl_vminfo * libxl_list_vm(libxl_ctx *
  */
 
 /* Disks */
-void libxl_device_disk_init(libxl_device_disk *disk);
 int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk);
 int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t domid,
                              libxl_device_disk *disk,
@@ -553,7 +548,6 @@ char * libxl_device_disk_local_attach(li
 int libxl_device_disk_local_detach(libxl_ctx *ctx, libxl_device_disk *disk);
 
 /* Network Interfaces */
-void libxl_device_nic_init(libxl_device_nic *nic);
 int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic);
 int libxl_device_nic_remove(libxl_ctx *ctx, uint32_t domid,
                             libxl_device_nic *nic,
@@ -565,7 +559,6 @@ int libxl_device_nic_getinfo(libxl_ctx *
                               libxl_device_nic *nic, libxl_nicinfo *nicinfo);
 
 /* Keyboard */
-void libxl_device_vkb_init(libxl_device_vkb *vkb);
 int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb);
 int libxl_device_vkb_remove(libxl_ctx *ctx, uint32_t domid,
                             libxl_device_vkb *vkb,
@@ -573,7 +566,6 @@ int libxl_device_vkb_remove(libxl_ctx *c
 int libxl_device_vkb_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb);
 
 /* Framebuffer */
-void libxl_device_vfb_init(libxl_device_vfb *vfb);
 int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb);
 int libxl_device_vfb_remove(libxl_ctx *ctx, uint32_t domid,
                             libxl_device_vfb *vfb,
@@ -581,7 +573,6 @@ int libxl_device_vfb_remove(libxl_ctx *c
 int libxl_device_vfb_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb);
 
 /* PCI Passthrough */
-void libxl_device_pci_init(libxl_device_pci *pci);
 int libxl_device_pci_add(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
 int libxl_device_pci_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
 int libxl_device_pci_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
diff -r ba485f9fce31 -r 50c87c40031c tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Mon Feb 13 16:27:34 2012 +0000
+++ b/tools/libxl/libxl_create.c	Mon Feb 13 16:27:34 2012 +0000
@@ -50,11 +50,6 @@ void libxl_domain_config_dispose(libxl_d
     libxl_domain_build_info_dispose(&d_config->b_info);
 }
 
-void libxl_domain_create_info_init(libxl_domain_create_info *c_info)
-{
-    memset(c_info, '\0', sizeof(*c_info));
-}
-
 int libxl__domain_create_info_setdefault(libxl__gc *gc,
                                          libxl_domain_create_info *c_info)
 {
@@ -69,35 +64,6 @@ int libxl__domain_create_info_setdefault
     return 0;
 }
 
-void libxl_domain_build_info_init(libxl_domain_build_info *b_info)
-{
-    memset(b_info, '\0', sizeof(*b_info));
-    b_info->type = -1;
-
-    b_info->max_memkb = LIBXL_MEMKB_DEFAULT;
-    b_info->target_memkb = LIBXL_MEMKB_DEFAULT;
-    b_info->shadow_memkb = LIBXL_MEMKB_DEFAULT;
-    b_info->video_memkb =  LIBXL_MEMKB_DEFAULT;
-
-}
-
-void libxl_domain_build_info_init_type(libxl_domain_build_info *b_info,
-                                       libxl_domain_type type)
-{
-    assert(b_info->type == -1);
-    b_info->type = type;
-    switch (b_info->type) {
-    case LIBXL_DOMAIN_TYPE_HVM:
-        b_info->u.hvm.timer_mode = LIBXL_TIMER_MODE_DEFAULT;
-        break;
-    case LIBXL_DOMAIN_TYPE_PV:
-        b_info->u.pv.slack_memkb = LIBXL_MEMKB_DEFAULT;
-        break;
-    default:
-        abort();
-    }
-}
-
 int libxl__domain_build_info_setdefault(libxl__gc *gc,
                                         libxl_domain_build_info *b_info)
 {
diff -r ba485f9fce31 -r 50c87c40031c tools/libxl/libxl_json.h
--- a/tools/libxl/libxl_json.h	Mon Feb 13 16:27:34 2012 +0000
+++ b/tools/libxl/libxl_json.h	Mon Feb 13 16:27:34 2012 +0000
@@ -22,6 +22,20 @@
 #  include <yajl/yajl_version.h>
 #endif
 
+yajl_gen_status libxl_defbool_gen_json(yajl_gen hand, libxl_defbool *p);
+yajl_gen_status libxl_domid_gen_json(yajl_gen hand, libxl_domid *p);
+yajl_gen_status libxl_uuid_gen_json(yajl_gen hand, libxl_uuid *p);
+yajl_gen_status libxl_mac_gen_json(yajl_gen hand, libxl_mac *p);
+yajl_gen_status libxl_cpumap_gen_json(yajl_gen hand, libxl_cpumap *p);
+yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
+                                                 libxl_cpuid_policy_list *p);
+yajl_gen_status libxl_string_list_gen_json(yajl_gen hand, libxl_string_list *p);
+yajl_gen_status libxl_key_value_list_gen_json(yajl_gen hand,
+                                              libxl_key_value_list *p);
+yajl_gen_status libxl_file_reference_gen_json(yajl_gen hand,
+                                              libxl_file_reference *p);
+yajl_gen_status libxl_hwcap_gen_json(yajl_gen hand, libxl_hwcap *p);
+
 #include <_libxl_types_json.h>
 
 /* YAJL version check */
diff -r ba485f9fce31 -r 50c87c40031c tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c	Mon Feb 13 16:27:34 2012 +0000
+++ b/tools/libxl/libxl_pci.c	Mon Feb 13 16:27:34 2012 +0000
@@ -765,11 +765,6 @@ static int libxl__device_pci_reset(libxl
     return -1;
 }
 
-void libxl_device_pci_init(libxl_device_pci *pci)
-{
-    memset(pci, '\0', sizeof(*pci));
-}
-
 int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci)
 {
     return 0;
diff -r ba485f9fce31 -r 50c87c40031c tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Mon Feb 13 16:27:34 2012 +0000
+++ b/tools/libxl/libxl_types.idl	Mon Feb 13 16:27:34 2012 +0000
@@ -100,7 +100,7 @@ libxl_timer_mode = Enumeration("timer_mo
     (1, "no_delay_for_missed_ticks"),
     (2, "no_missed_ticks_pending"),
     (3, "one_missed_tick_pending"),
-    ])
+    ], init_val = "LIBXL_TIMER_MODE_DEFAULT")
 
 #
 # Complex libxl types
@@ -157,19 +157,19 @@ libxl_dominfo = Struct("dominfo",[
     ("vcpu_max_id", uint32),
     ("vcpu_online", uint32),
     ("cpupool",     uint32),
-    ], dispose_fn=None)
+    ], dispose_fn=None, dir=DIR_OUT)
 
 libxl_cpupoolinfo = Struct("cpupoolinfo", [
     ("poolid",      uint32),
     ("sched_id",    uint32),
     ("n_dom",       uint32),
     ("cpumap",      libxl_cpumap)
-    ])
+    ], dir=DIR_OUT)
 
 libxl_vminfo = Struct("vminfo", [
     ("uuid", libxl_uuid),
     ("domid", libxl_domid),
-    ], dispose_fn=None)
+    ], dispose_fn=None, dir=DIR_OUT)
 
 libxl_version_info = Struct("version_info", [
     ("xen_version_major", integer),
@@ -184,7 +184,7 @@ libxl_version_info = Struct("version_inf
     ("virt_start",        uint64),
     ("pagesize",          integer),
     ("commandline",       string),
-    ])
+    ], dir=DIR_OUT)
 
 libxl_domain_create_info = Struct("domain_create_info",[
     ("type",         libxl_domain_type),
@@ -196,7 +196,9 @@ libxl_domain_create_info = Struct("domai
     ("xsdata",       libxl_key_value_list),
     ("platformdata", libxl_key_value_list),
     ("poolid",       uint32),
-    ])
+    ], dir=DIR_IN)
+
+MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
 
 # Instances of libxl_file_reference contained in this struct which
 # have been mapped (with libxl_file_reference_map) will be unmapped
@@ -273,8 +275,8 @@ libxl_domain_build_info = Struct("domain
                                       # Use host's E820 for PCI passthrough.
                                       ("e820_host", libxl_defbool),
                                       ])),
-                 ])),
-    ],
+                 ], keyvar_init_val = "-1")),
+    ], dir=DIR_IN
 )
 
 libxl_device_vfb = Struct("device_vfb", [
@@ -336,7 +338,7 @@ libxl_diskinfo = Struct("diskinfo", [
     ("state", integer),
     ("evtch", integer),
     ("rref", integer),
-    ])
+    ], dir=DIR_OUT)
 
 libxl_nicinfo = Struct("nicinfo", [
     ("backend", string),
@@ -348,7 +350,7 @@ libxl_nicinfo = Struct("nicinfo", [
     ("evtch", integer),
     ("rref_tx", integer),
     ("rref_rx", integer),
-    ])
+    ], dir=DIR_OUT)
 
 libxl_vcpuinfo = Struct("vcpuinfo", [
     ("vcpuid", uint32),
@@ -358,7 +360,7 @@ libxl_vcpuinfo = Struct("vcpuinfo", [
     ("running", bool),
     ("vcpu_time", uint64), # total vcpu time ran (ns)
     ("cpumap", libxl_cpumap), # current cpu's affinities
-    ])
+    ], dir=DIR_OUT)
 
 libxl_physinfo = Struct("physinfo", [
     ("threads_per_core", uint32),
@@ -383,16 +385,16 @@ libxl_cputopology = Struct("cputopology"
     ("core", uint32),
     ("socket", uint32),
     ("node", uint32),
-    ])
+    ], dir=DIR_OUT)
 
 libxl_sched_credit = Struct("sched_credit", [
     ("weight", integer),
     ("cap", integer),
-    ], dispose_fn=None)
+    ], dispose_fn=None, init_fn=None)
 
 libxl_sched_credit2 = Struct("sched_credit2", [
     ("weight", integer),
-    ], dispose_fn=None)
+    ], dispose_fn=None, init_fn=None)
 
 libxl_sched_sedf = Struct("sched_sedf", [
     ("period", integer),
@@ -400,7 +402,7 @@ libxl_sched_sedf = Struct("sched_sedf", 
     ("latency", integer),
     ("extratime", integer),
     ("weight", integer),
-    ], dispose_fn=None)
+    ], dispose_fn=None, init_fn=None)
 
 libxl_event_type = Enumeration("event_type", [
     (1, "DOMAIN_SHUTDOWN"),

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 22 of 23] libxl: idl: generate KeyedUnion key member as part of the KeyedUnion
  2012-02-13 16:53 ` [PATCH 22 of 23] libxl: idl: generate KeyedUnion key member as part of the KeyedUnion Ian Campbell
@ 2012-02-13 17:08   ` Ian Jackson
  2012-02-13 17:13     ` Ian Campbell
  0 siblings, 1 reply; 55+ messages in thread
From: Ian Jackson @ 2012-02-13 17:08 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

Ian Campbell writes ("[Xen-devel] [PATCH 22 of 23] libxl: idl: generate KeyedUnion key member as part of the KeyedUnion"):
> libxl: idl: generate KeyedUnion key member as part of the KeyedUnion
> 
> Rather than specifying it twice in the IDL.
...        else:
> diff -r 0751a69b3ef2 -r ba485f9fce31 tools/libxl/libxl_types.idl
> --- a/tools/libxl/libxl_types.idl	Mon Feb 13 16:27:34 2012 +0000
> +++ b/tools/libxl/libxl_types.idl	Mon Feb 13 16:27:34 2012 +0000
> @@ -214,7 +214,6 @@ libxl_domain_build_info = Struct("domain
>      ("shadow_memkb",    MemKB),
>      ("disable_migrate", libxl_defbool),
>      ("cpuid",           libxl_cpuid_policy_list),
> -    ("type",            libxl_domain_type),
>      
>      ("device_model_version", libxl_device_model_version),

If I'm not mistaken, an effect of this is to move the type field
around in the C structure - ie, it's not ABI compatible.  This is OK
at this stage, I think, but I just wanted to check that that was what
you meant.

Ian.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 22 of 23] libxl: idl: generate KeyedUnion key member as part of the KeyedUnion
  2012-02-13 17:08   ` Ian Jackson
@ 2012-02-13 17:13     ` Ian Campbell
  0 siblings, 0 replies; 55+ messages in thread
From: Ian Campbell @ 2012-02-13 17:13 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

On Mon, 2012-02-13 at 17:08 +0000, Ian Jackson wrote:
> Ian Campbell writes ("[Xen-devel] [PATCH 22 of 23] libxl: idl: generate KeyedUnion key member as part of the KeyedUnion"):
> > libxl: idl: generate KeyedUnion key member as part of the KeyedUnion
> > 
> > Rather than specifying it twice in the IDL.
> ...        else:
> > diff -r 0751a69b3ef2 -r ba485f9fce31 tools/libxl/libxl_types.idl
> > --- a/tools/libxl/libxl_types.idl	Mon Feb 13 16:27:34 2012 +0000
> > +++ b/tools/libxl/libxl_types.idl	Mon Feb 13 16:27:34 2012 +0000
> > @@ -214,7 +214,6 @@ libxl_domain_build_info = Struct("domain
> >      ("shadow_memkb",    MemKB),
> >      ("disable_migrate", libxl_defbool),
> >      ("cpuid",           libxl_cpuid_policy_list),
> > -    ("type",            libxl_domain_type),
> >      
> >      ("device_model_version", libxl_device_model_version),
> 
> If I'm not mistaken, an effect of this is to move the type field
> around in the C structure - ie, it's not ABI compatible.  This is OK
> at this stage, I think, but I just wanted to check that that was what
> you meant.

It was.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 01 of 23] libxl: Document _init/_dispose/_setdefault functions
  2012-02-13 16:52 ` [PATCH 01 of 23] libxl: Document _init/_dispose/_setdefault functions Ian Campbell
@ 2012-02-20 19:08   ` Ian Jackson
  2012-02-21 10:18     ` Ian Campbell
  0 siblings, 1 reply; 55+ messages in thread
From: Ian Jackson @ 2012-02-20 19:08 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, ian.jackson

Ian Campbell writes ("[Xen-devel] [PATCH 01 of 23] libxl: Document _init/_dispose/_setdefault functions"):
> + * void libxl_<type>_init_<subfield>(<type> *p, subfield):
> + *
> + *    Initialise those parts of "p" which are not initialised by the
> + *    main init function due to the unknown value of "subfield". Sets
> + *    p->subfield as well as initialising any fields to their default
> + *    values.
> + *
> + *    p->subfield must not have been previously initialised.
> + *
> + *    This method is provided for any aggregate type which is used as
> + *    an input parameter.

This final sentence needs a qualification I think.

> + * void libxl_<type>_dispose(instance *p):
> + *
> + *    Frees any dynamically allocated memory used by the members of
> + *    "p" but not the storage used by "p" itself (this allows for the
> + *    allocation of arrays of types and for the composition of types).
> + *
> + *    In general this method is only provided for types where memory
> + *    can be dynamically allocated as part of that type.

Is whether dynamic allocation may happen a very stable part of the
libxl API ?  If we add a dynamically allocated member to one of these
structs, don't we inevitably introduce a memory leak into all
callers ?

I think it would be better to provide a dispose function corresponding
to every init function.

Ian.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 02 of 23] libxl: provide _init and _setdefault for libxl_domain_create_info
  2012-02-13 16:52 ` [PATCH 02 of 23] libxl: provide _init and _setdefault for libxl_domain_create_info Ian Campbell
@ 2012-02-20 19:09   ` Ian Jackson
  0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2012-02-20 19:09 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, ian.jackson

Ian Campbell writes ("[Xen-devel] [PATCH 02 of 23] libxl: provide _init and _setdefault for libxl_domain_create_info"):
> libxl: provide _init and _setdefault for libxl_domain_create_info.
> 
> Some fields require further scaffolding before they can use the
> _init/_setdefault scheme and are handled in later patches.
...
> -int libxl_init_create_info(libxl_ctx *ctx, libxl_domain_create_info *c_info)
> +void libxl_domain_create_info_init(libxl_domain_create_info *c_info)
>  {
>      memset(c_info, '\0', sizeof(*c_info));
> -    c_info->xsdata = NULL;
> -    c_info->platformdata = NULL;
>      c_info->hap = 1;
> -    c_info->type = LIBXL_DOMAIN_TYPE_HVM;
>      c_info->oos = 1;
> -    c_info->ssidref = 0;
> -    c_info->poolid = 0;
> +}

Is it really best to do it like this, rather than generate this
function automatically ?  AFAICT the magic "use default" values are a
property of the member types so _init is entirely formulaic.

Ian.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 13 of 23] libxl: add new "defbool" built in type
  2012-02-13 16:52 ` [PATCH 13 of 23] libxl: add new "defbool" built in type Ian Campbell
@ 2012-02-20 19:14   ` Ian Jackson
  2012-02-21 10:24     ` Ian Campbell
  0 siblings, 1 reply; 55+ messages in thread
From: Ian Jackson @ 2012-02-20 19:14 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, ian.jackson

Ian Campbell writes ("[Xen-devel] [PATCH 13 of 23] libxl: add new "defbool" built in type"):
> libxl: add new "defbool" built in type.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 05 of 23] libxl: drop 8M slack for PV guests
  2012-02-13 16:52 ` [PATCH 05 of 23] libxl: drop 8M slack for PV guests Ian Campbell
@ 2012-02-20 19:15   ` Ian Jackson
  2012-02-21 10:10     ` Stefano Stabellini
  0 siblings, 1 reply; 55+ messages in thread
From: Ian Jackson @ 2012-02-20 19:15 UTC (permalink / raw)
  To: Ian Campbell, stefano.stabellini; +Cc: xen-devel

Ian Campbell writes ("[Xen-devel] [PATCH 05 of 23] libxl: drop 8M slack for PV guests"):
> libxl: drop 8M slack for PV guests.
> 
> As far as I can tell this serves no purpose. I think it relates to the old
> 8M to "account for backend allocations" which we used to add. This leaves a bit
> of unpopulated space in the Pseudo-physical address space which can be used by
> backends when mapping foreign memory. However 8M is not representative of that
> any more and modern kernels do not operate in this way anyway.
> 
> I suspect an argument could be made for removing this from the libxl API
> altogether but instead lets just set the overhead to 0.

I think this is plausible but I'd like to hear from Stefano, who iirc
may have some knowledge about the reason for this 8Mb.

Ian.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 04 of 23] libxl: use an explicit LIBXL_TIMER_MODE_DEFAULT value
  2012-02-13 16:52 ` [PATCH 04 of 23] libxl: use an explicit LIBXL_TIMER_MODE_DEFAULT value Ian Campbell
@ 2012-02-20 19:16   ` Ian Jackson
  0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2012-02-20 19:16 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, ian.jackson

Ian Campbell writes ("[Xen-devel] [PATCH 04 of 23] libxl: use an explicit LIBXL_TIMER_MODE_DEFAULT value"):
> libxl: use an explicit LIBXL_TIMER_MODE_DEFAULT value

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 16 of 23] libxl: switch generation id control to libxl_defbool
  2012-02-13 16:53 ` [PATCH 16 of 23] libxl: switch generation id control to libxl_defbool Ian Campbell
@ 2012-02-20 19:16   ` Ian Jackson
  2012-02-21 11:01   ` Ian Campbell
  1 sibling, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2012-02-20 19:16 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, ian.jackson

Ian Campbell writes ("[Xen-devel] [PATCH 16 of 23] libxl: switch generation id control to libxl_defbool"):
> libxl: switch generation id control to libxl_defbool.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 21 of 23] libxl: Make IDL KeyedUnion keyvar an idl.Field
  2012-02-13 16:53 ` [PATCH 21 of 23] libxl: Make IDL KeyedUnion keyvar an idl.Field Ian Campbell
@ 2012-02-20 19:17   ` Ian Jackson
  0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2012-02-20 19:17 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, ian.jackson

Ian Campbell writes ("[Xen-devel] [PATCH 21 of 23] libxl: Make IDL KeyedUnion keyvar an idl.Field"):
> libxl: Make IDL KeyedUnion keyvar an idl.Field

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 14 of 23] libxl: make boolean members of libxl_domain_create_info into libxl_defbool
  2012-02-13 16:53 ` [PATCH 14 of 23] libxl: make boolean members of libxl_domain_create_info into libxl_defbool Ian Campbell
@ 2012-02-20 19:18   ` Ian Jackson
  0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2012-02-20 19:18 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, ian.jackson

Ian Campbell writes ("[Xen-devel] [PATCH 14 of 23] libxl: make boolean members of libxl_domain_create_info into libxl_defbool"):
> libxl: make boolean members of libxl_domain_create_info into libxl_defbool

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 06 of 23] libxl: allow specification of testidl random seed
  2012-02-13 16:52 ` [PATCH 06 of 23] libxl: allow specification of testidl random seed Ian Campbell
@ 2012-02-20 19:18   ` Ian Jackson
  0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2012-02-20 19:18 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, ian.jackson

Ian Campbell writes ("[Xen-devel] [PATCH 06 of 23] libxl: allow specification of testidl random seed"):
> libxl: allow specification of testidl random seed.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 11 of 23] libxl: make libxl_device_console internal
  2012-02-13 16:52 ` [PATCH 11 of 23] libxl: make libxl_device_console internal Ian Campbell
@ 2012-02-20 19:20   ` Ian Jackson
  2012-02-21 10:23     ` Ian Campbell
  0 siblings, 1 reply; 55+ messages in thread
From: Ian Jackson @ 2012-02-20 19:20 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

Ian Campbell writes ("[Xen-devel] [PATCH 11 of 23] libxl: make libxl_device_console internal"):
> libxl: make libxl_device_console internal
> 
> consoles are not directly exposed to users of the library and there are no API
> functions for manipluating them (only the console_exec function). Rather than
> commit to a particular API now make the type internal.

Isn't it likely that at some future point we will re-expose this
difference, for example by providing a different set of access
functions ?

The fact that all you can do with a console is exec the xenconsole
client is pretty crazy.

Ian.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 18 of 23] libxl: do not explicitly initialise members of build info to zero
  2012-02-13 16:53 ` [PATCH 18 of 23] libxl: do not explicitly initialise members of build info to zero Ian Campbell
@ 2012-02-20 19:20   ` Ian Jackson
  0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2012-02-20 19:20 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, ian.jackson

Ian Campbell writes ("[Xen-devel] [PATCH 18 of 23] libxl: do not explicitly initialise members of build info to zero"):
> libxl: do not explicitly initialise members of build info to zero

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 20 of 23] libxl: add libxl_domain_build_info_init_type
  2012-02-13 16:53 ` [PATCH 20 of 23] libxl: add libxl_domain_build_info_init_type Ian Campbell
@ 2012-02-20 19:22   ` Ian Jackson
  2012-02-20 19:24   ` Ian Jackson
  1 sibling, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2012-02-20 19:22 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

Ian Campbell writes ("[Xen-devel] [PATCH 20 of 23] libxl: add libxl_domain_build_info_init_type"):
> libxl: add libxl_domain_build_info_init_type

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 17 of 23] libxl: use defbool for graphics related options
  2012-02-13 16:53 ` [PATCH 17 of 23] libxl: use defbool for graphics related options Ian Campbell
@ 2012-02-20 19:22   ` Ian Jackson
  0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2012-02-20 19:22 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

Ian Campbell writes ("[Xen-devel] [PATCH 17 of 23] libxl: use defbool for graphics related options"):
> libxl: use defbool for graphics related options

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 23 of 23] libxl: autogenerate libxl_FOO_init and libxl_FOO_init_FIELD
  2012-02-13 16:53 ` [PATCH 23 of 23] libxl: autogenerate libxl_FOO_init and libxl_FOO_init_FIELD Ian Campbell
@ 2012-02-20 19:24   ` Ian Jackson
  2012-02-21 10:11     ` Ian Campbell
  0 siblings, 1 reply; 55+ messages in thread
From: Ian Jackson @ 2012-02-20 19:24 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

Ian Campbell writes ("[Xen-devel] [PATCH 23 of 23] libxl: autogenerate libxl_FOO_init and libxl_FOO_init_FIELD"):
> libxl: autogenerate libxl_FOO_init and libxl_FOO_init_FIELD

Ah!  Here's what I was looking for.  Excellent.

I would like to see the autogenerated code this makes and it would be
much more convenient for me to try out this series if you could push
it to a hg or git tree somewhere for me to fetch.

Thanks,
Ian.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 20 of 23] libxl: add libxl_domain_build_info_init_type
  2012-02-13 16:53 ` [PATCH 20 of 23] libxl: add libxl_domain_build_info_init_type Ian Campbell
  2012-02-20 19:22   ` Ian Jackson
@ 2012-02-20 19:24   ` Ian Jackson
  1 sibling, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2012-02-20 19:24 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

Ian Campbell writes ("[Xen-devel] [PATCH 20 of 23] libxl: add libxl_domain_build_info_init_type"):
> libxl: add libxl_domain_build_info_init_type

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 07 of 23] libxl: introduce a descriminating default value for memkb fields
  2012-02-13 16:52 ` [PATCH 07 of 23] libxl: introduce a descriminating default value for memkb fields Ian Campbell
@ 2012-02-20 19:24   ` Ian Jackson
  0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2012-02-20 19:24 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, ian.jackson

Ian Campbell writes ("[Xen-devel] [PATCH 07 of 23] libxl: introduce a descriminating default value for memkb fields"):
> libxl: introduce a descriminating default value for memkb fields.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 15 of 23] libxl: make boolean members of libxl_domain_create_info into libxl_defbool
  2012-02-13 16:53 ` [PATCH 15 " Ian Campbell
@ 2012-02-20 19:25   ` Ian Jackson
  2012-02-21 10:58     ` Ian Campbell
  0 siblings, 1 reply; 55+ messages in thread
From: Ian Jackson @ 2012-02-20 19:25 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

Ian Campbell writes ("[Xen-devel] [PATCH 15 of 23] libxl: make boolean members of libxl_domain_create_info into libxl_defbool"):
> libxl: make boolean members of libxl_domain_create_info into libxl_defbool

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

> +        libxl_defbool_setdefault(&b_info->u.hvm.pae, true);
> +        libxl_defbool_setdefault(&b_info->u.hvm.apic, true);
> +        libxl_defbool_setdefault(&b_info->u.hvm.acpi, true);
> +        libxl_defbool_setdefault(&b_info->u.hvm.acpi_s3, true);
> +        libxl_defbool_setdefault(&b_info->u.hvm.acpi_s4, true);
> +        libxl_defbool_setdefault(&b_info->u.hvm.nx, true);
> +        libxl_defbool_setdefault(&b_info->u.hvm.viridian, false);
> +        libxl_defbool_setdefault(&b_info->u.hvm.hpet, true);
> +        libxl_defbool_setdefault(&b_info->u.hvm.vpt_align, true);
> +        libxl_defbool_setdefault(&b_info->u.hvm.nested_hvm, false);
> +        libxl_defbool_setdefault(&b_info->u.hvm.usb, false);
> +        libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci, true);

Maybe this would be easier to read like this:

 +        libxl_defbool_setdefault(&b_info->u.hvm.nested_hvm,       false);
 +        libxl_defbool_setdefault(&b_info->u.hvm.usb,              false);
 +        libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci, true);

Ian.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 05 of 23] libxl: drop 8M slack for PV guests
  2012-02-20 19:15   ` Ian Jackson
@ 2012-02-21 10:10     ` Stefano Stabellini
  2012-02-21 10:10       ` Ian Campbell
  0 siblings, 1 reply; 55+ messages in thread
From: Stefano Stabellini @ 2012-02-21 10:10 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, Dave Scott, Ian Campbell, Stefano Stabellini

On Mon, 20 Feb 2012, Ian Jackson wrote:
> Ian Campbell writes ("[Xen-devel] [PATCH 05 of 23] libxl: drop 8M slack for PV guests"):
> > libxl: drop 8M slack for PV guests.
> > 
> > As far as I can tell this serves no purpose. I think it relates to the old
> > 8M to "account for backend allocations" which we used to add. This leaves a bit
> > of unpopulated space in the Pseudo-physical address space which can be used by
> > backends when mapping foreign memory. However 8M is not representative of that
> > any more and modern kernels do not operate in this way anyway.
> > 
> > I suspect an argument could be made for removing this from the libxl API
> > altogether but instead lets just set the overhead to 0.
> 
> I think this is plausible but I'd like to hear from Stefano, who iirc
> may have some knowledge about the reason for this 8Mb.

It comes from XenD (and XAPI too, if I recall correctly), see this
comment in tools/python/xen/xend/image.py:


class X86_Linux_ImageHandler(LinuxImageHandler):

    def buildDomain(self):
        # set physical mapping limit
        # add an 8MB slack to balance backend allocations.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 05 of 23] libxl: drop 8M slack for PV guests
  2012-02-21 10:10     ` Stefano Stabellini
@ 2012-02-21 10:10       ` Ian Campbell
  2012-02-21 10:53         ` David Vrabel
  2012-02-21 15:27         ` Dan Magenheimer
  0 siblings, 2 replies; 55+ messages in thread
From: Ian Campbell @ 2012-02-21 10:10 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: xen-devel, Ian Jackson, Dave Scott

On Tue, 2012-02-21 at 10:10 +0000, Stefano Stabellini wrote:
> On Mon, 20 Feb 2012, Ian Jackson wrote:
> > Ian Campbell writes ("[Xen-devel] [PATCH 05 of 23] libxl: drop 8M slack for PV guests"):
> > > libxl: drop 8M slack for PV guests.
> > > 
> > > As far as I can tell this serves no purpose. I think it relates to the old
> > > 8M to "account for backend allocations" which we used to add. This leaves a bit
> > > of unpopulated space in the Pseudo-physical address space which can be used by
> > > backends when mapping foreign memory. However 8M is not representative of that
> > > any more and modern kernels do not operate in this way anyway.
> > > 
> > > I suspect an argument could be made for removing this from the libxl API
> > > altogether but instead lets just set the overhead to 0.
> > 
> > I think this is plausible but I'd like to hear from Stefano, who iirc
> > may have some knowledge about the reason for this 8Mb.
> 
> It comes from XenD (and XAPI too, if I recall correctly), see this
> comment in tools/python/xen/xend/image.py:

That doesn't answer the "why" though.

I think I should just be more assertive since I believe I do actually
know why the 8 is there (or certainly nobody appears to know any
better). I'll update the changelog to read:

        This serves no purpose. It relates to the old 8M to "account for
        backend allocations" which we used to add. This leaves a bit of
        unpopulated space in the Pseudo-physical address space which can
        be used by backends when mapping foreign memory. However 8M is
        not representative of that any more and modern kernels do not
        operate in this way anyway.

Ian.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 23 of 23] libxl: autogenerate libxl_FOO_init and libxl_FOO_init_FIELD
  2012-02-20 19:24   ` Ian Jackson
@ 2012-02-21 10:11     ` Ian Campbell
  0 siblings, 0 replies; 55+ messages in thread
From: Ian Campbell @ 2012-02-21 10:11 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

On Mon, 2012-02-20 at 19:24 +0000, Ian Jackson wrote:
> Ian Campbell writes ("[Xen-devel] [PATCH 23 of 23] libxl: autogenerate libxl_FOO_init and libxl_FOO_init_FIELD"):
> > libxl: autogenerate libxl_FOO_init and libxl_FOO_init_FIELD
> 
> Ah!  Here's what I was looking for.  Excellent.
> 
> I would like to see the autogenerated code this makes and it would be
> much more convenient for me to try out this series if you could push
> it to a hg or git tree somewhere for me to fetch.

I'll process your acks and other comments and resend including a git
url.

Ian.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 01 of 23] libxl: Document _init/_dispose/_setdefault functions
  2012-02-20 19:08   ` Ian Jackson
@ 2012-02-21 10:18     ` Ian Campbell
  0 siblings, 0 replies; 55+ messages in thread
From: Ian Campbell @ 2012-02-21 10:18 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

On Mon, 2012-02-20 at 19:08 +0000, Ian Jackson wrote:
> Ian Campbell writes ("[Xen-devel] [PATCH 01 of 23] libxl: Document _init/_dispose/_setdefault functions"):
> > + * void libxl_<type>_init_<subfield>(<type> *p, subfield):
> > + *
> > + *    Initialise those parts of "p" which are not initialised by the
> > + *    main init function due to the unknown value of "subfield". Sets
> > + *    p->subfield as well as initialising any fields to their default
> > + *    values.
> > + *
> > + *    p->subfield must not have been previously initialised.
> > + *
> > + *    This method is provided for any aggregate type which is used as
> > + *    an input parameter.
> 
> This final sentence needs a qualification I think.

I struggled a bit to express what I meant.

What I was trying to say is that they are produced only for types
declared in the idl with dir=IN|BOTH. There are some output only types
which a user cannot reasonably create.

Maybe I should just declare that the distinction is irrelevant and
always generate an _init? (I think I introduced it because of some issue
I was having with the code generation, I'll try again and see if I trip
over it again)

> > + * void libxl_<type>_dispose(instance *p):
> > + *
> > + *    Frees any dynamically allocated memory used by the members of
> > + *    "p" but not the storage used by "p" itself (this allows for the
> > + *    allocation of arrays of types and for the composition of types).
> > + *
> > + *    In general this method is only provided for types where memory
> > + *    can be dynamically allocated as part of that type.
> 
> Is whether dynamic allocation may happen a very stable part of the
> libxl API ?  If we add a dynamically allocated member to one of these
> structs, don't we inevitably introduce a memory leak into all
> callers ?
> 
> I think it would be better to provide a dispose function corresponding
> to every init function.

I think the _dispose functions should be a strict superset since you
need to be able to dispose of output-only functions too. I _think_ that
means we should have a _dispose for every type.

I expect we'll uncover a mass of latent bugs whenever we add dynamic
allocation to a type which previously did not do so. But at least the
bug fix will be compatible with older versions of the library too, so I
will go ahead and add a dispose everywhere.

Ian.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 11 of 23] libxl: make libxl_device_console internal
  2012-02-20 19:20   ` Ian Jackson
@ 2012-02-21 10:23     ` Ian Campbell
  2012-02-21 12:31       ` Ian Jackson
  0 siblings, 1 reply; 55+ messages in thread
From: Ian Campbell @ 2012-02-21 10:23 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

On Mon, 2012-02-20 at 19:20 +0000, Ian Jackson wrote:
> Ian Campbell writes ("[Xen-devel] [PATCH 11 of 23] libxl: make libxl_device_console internal"):
> > libxl: make libxl_device_console internal
> > 
> > consoles are not directly exposed to users of the library and there are no API
> > functions for manipluating them (only the console_exec function). Rather than
> > commit to a particular API now make the type internal.
> 
> Isn't it likely that at some future point we will re-expose this
> difference, for example by providing a different set of access
> functions ?

Very likely. However I didn't want to design or commit to an API in the
total absence of users.

When a user does come along it is much easier to add a completely new
API rather than to fix an existing broken one. It's easier to do this in
a manner which users of the library can cope with in a compatible way
e.g. adding a new API is easier to check for with ./configure.

> The fact that all you can do with a console is exec the xenconsole
> client is pretty crazy.

Yup.

Ian.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 13 of 23] libxl: add new "defbool" built in type
  2012-02-20 19:14   ` Ian Jackson
@ 2012-02-21 10:24     ` Ian Campbell
  0 siblings, 0 replies; 55+ messages in thread
From: Ian Campbell @ 2012-02-21 10:24 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

On Mon, 2012-02-20 at 19:14 +0000, Ian Jackson wrote:
> Ian Campbell writes ("[Xen-devel] [PATCH 13 of 23] libxl: add new "defbool" built in type"):
> > libxl: add new "defbool" built in type.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Again ;-)

Ian.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 05 of 23] libxl: drop 8M slack for PV guests
  2012-02-21 10:10       ` Ian Campbell
@ 2012-02-21 10:53         ` David Vrabel
  2012-02-21 10:59           ` Ian Campbell
  2012-02-21 15:27         ` Dan Magenheimer
  1 sibling, 1 reply; 55+ messages in thread
From: David Vrabel @ 2012-02-21 10:53 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, Ian Jackson, Dave Scott, Stefano Stabellini

On 21/02/12 10:10, Ian Campbell wrote:
> On Tue, 2012-02-21 at 10:10 +0000, Stefano Stabellini wrote:
>> On Mon, 20 Feb 2012, Ian Jackson wrote:
>>> Ian Campbell writes ("[Xen-devel] [PATCH 05 of 23] libxl: drop 8M slack for PV guests"):
>>>> libxl: drop 8M slack for PV guests.
>>>>
>>>> As far as I can tell this serves no purpose. I think it relates to the old
>>>> 8M to "account for backend allocations" which we used to add. This leaves a bit
>>>> of unpopulated space in the Pseudo-physical address space which can be used by
>>>> backends when mapping foreign memory. However 8M is not representative of that
>>>> any more and modern kernels do not operate in this way anyway.
>>>>
>>>> I suspect an argument could be made for removing this from the libxl API
>>>> altogether but instead lets just set the overhead to 0.
>>>
>>> I think this is plausible but I'd like to hear from Stefano, who iirc
>>> may have some knowledge about the reason for this 8Mb.
>>
>> It comes from XenD (and XAPI too, if I recall correctly), see this
>> comment in tools/python/xen/xend/image.py:
> 
> That doesn't answer the "why" though.
> 
> I think I should just be more assertive since I believe I do actually
> know why the 8 is there (or certainly nobody appears to know any
> better). I'll update the changelog to read:
> 
>         This serves no purpose. It relates to the old 8M to "account for
>         backend allocations" which we used to add. This leaves a bit of
>         unpopulated space in the Pseudo-physical address space which can
>         be used by backends when mapping foreign memory. However 8M is
>         not representative of that any more and modern kernels do not
>         operate in this way anyway.

Should a similar fix be applied to Xen as well for dom0?

Daid

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 15 of 23] libxl: make boolean members of libxl_domain_create_info into libxl_defbool
  2012-02-20 19:25   ` Ian Jackson
@ 2012-02-21 10:58     ` Ian Campbell
  0 siblings, 0 replies; 55+ messages in thread
From: Ian Campbell @ 2012-02-21 10:58 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

On Mon, 2012-02-20 at 19:25 +0000, Ian Jackson wrote:
> Ian Campbell writes ("[Xen-devel] [PATCH 15 of 23] libxl: make boolean members of libxl_domain_create_info into libxl_defbool"):
> > libxl: make boolean members of libxl_domain_create_info into libxl_defbool

NB: should have said build_info here.

> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> > +        libxl_defbool_setdefault(&b_info->u.hvm.pae, true);
> > +        libxl_defbool_setdefault(&b_info->u.hvm.apic, true);
> > +        libxl_defbool_setdefault(&b_info->u.hvm.acpi, true);
> > +        libxl_defbool_setdefault(&b_info->u.hvm.acpi_s3, true);
> > +        libxl_defbool_setdefault(&b_info->u.hvm.acpi_s4, true);
> > +        libxl_defbool_setdefault(&b_info->u.hvm.nx, true);
> > +        libxl_defbool_setdefault(&b_info->u.hvm.viridian, false);
> > +        libxl_defbool_setdefault(&b_info->u.hvm.hpet, true);
> > +        libxl_defbool_setdefault(&b_info->u.hvm.vpt_align, true);
> > +        libxl_defbool_setdefault(&b_info->u.hvm.nested_hvm, false);
> > +        libxl_defbool_setdefault(&b_info->u.hvm.usb, false);
> > +        libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci, true);
> 
> Maybe this would be easier to read like this:
> 
>  +        libxl_defbool_setdefault(&b_info->u.hvm.nested_hvm,       false);
>  +        libxl_defbool_setdefault(&b_info->u.hvm.usb,              false);
>  +        libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci, true);

Seems like an improvement. I'll make that change.

Ian.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 05 of 23] libxl: drop 8M slack for PV guests
  2012-02-21 10:53         ` David Vrabel
@ 2012-02-21 10:59           ` Ian Campbell
  0 siblings, 0 replies; 55+ messages in thread
From: Ian Campbell @ 2012-02-21 10:59 UTC (permalink / raw)
  To: David Vrabel; +Cc: xen-devel, Ian Jackson, Dave Scott, Stefano Stabellini

On Tue, 2012-02-21 at 10:53 +0000, David Vrabel wrote:
> On 21/02/12 10:10, Ian Campbell wrote:
> > On Tue, 2012-02-21 at 10:10 +0000, Stefano Stabellini wrote:
> >> On Mon, 20 Feb 2012, Ian Jackson wrote:
> >>> Ian Campbell writes ("[Xen-devel] [PATCH 05 of 23] libxl: drop 8M slack for PV guests"):
> >>>> libxl: drop 8M slack for PV guests.
> >>>>
> >>>> As far as I can tell this serves no purpose. I think it relates to the old
> >>>> 8M to "account for backend allocations" which we used to add. This leaves a bit
> >>>> of unpopulated space in the Pseudo-physical address space which can be used by
> >>>> backends when mapping foreign memory. However 8M is not representative of that
> >>>> any more and modern kernels do not operate in this way anyway.
> >>>>
> >>>> I suspect an argument could be made for removing this from the libxl API
> >>>> altogether but instead lets just set the overhead to 0.
> >>>
> >>> I think this is plausible but I'd like to hear from Stefano, who iirc
> >>> may have some knowledge about the reason for this 8Mb.
> >>
> >> It comes from XenD (and XAPI too, if I recall correctly), see this
> >> comment in tools/python/xen/xend/image.py:
> > 
> > That doesn't answer the "why" though.
> > 
> > I think I should just be more assertive since I believe I do actually
> > know why the 8 is there (or certainly nobody appears to know any
> > better). I'll update the changelog to read:
> > 
> >         This serves no purpose. It relates to the old 8M to "account for
> >         backend allocations" which we used to add. This leaves a bit of
> >         unpopulated space in the Pseudo-physical address space which can
> >         be used by backends when mapping foreign memory. However 8M is
> >         not representative of that any more and modern kernels do not
> >         operate in this way anyway.
> 
> Should a similar fix be applied to Xen as well for dom0?

I don't think it is harmful to have the extra 8, but yes we could
consider doing so.

Ian.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 16 of 23] libxl: switch generation id control to libxl_defbool
  2012-02-13 16:53 ` [PATCH 16 of 23] libxl: switch generation id control to libxl_defbool Ian Campbell
  2012-02-20 19:16   ` Ian Jackson
@ 2012-02-21 11:01   ` Ian Campbell
  1 sibling, 0 replies; 55+ messages in thread
From: Ian Campbell @ 2012-02-21 11:01 UTC (permalink / raw)
  To: xen-devel; +Cc: Paul Durrant, Ian Jackson

On Mon, 2012-02-13 at 16:53 +0000, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell <ian.campbell@citrix.com>
> # Date 1329150447 0
> # Node ID 42c448aa641537798f4b09a6db55f3a5ee4082cd
> # Parent  ca432f4ad9f58b0a0a87045a680e37a123bbda11
> libxl: switch generation id control to libxl_defbool.
> 
> This allows it to be set via the _init/_setdefault methods.
> 
> Also switch the sense of the variable in the libxl API, since once you add
> defbool to the mix the double negatives become even more confusing.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Paul Durrant <Paul.Durrant@citrix.com>

hg email doesn't obey these -- cc'ing Paul this time.

> diff -r ca432f4ad9f5 -r 42c448aa6415 tools/libxl/libxl_create.c
> --- a/tools/libxl/libxl_create.c	Mon Feb 13 15:49:01 2012 +0000
> +++ b/tools/libxl/libxl_create.c	Mon Feb 13 16:27:27 2012 +0000
> @@ -88,7 +88,6 @@ void libxl_domain_build_info_init(libxl_
>      case LIBXL_DOMAIN_TYPE_HVM:
>          b_info->u.hvm.firmware = NULL;
>          b_info->u.hvm.timer_mode = LIBXL_TIMER_MODE_DEFAULT;
> -        b_info->u.hvm.no_incr_generationid = 0;
>  
>          b_info->u.hvm.stdvga = 0;
>          b_info->u.hvm.vnc.enable = 1;
> @@ -150,6 +149,8 @@ int libxl__domain_build_info_setdefault(
>          libxl_defbool_setdefault(&b_info->u.hvm.hpet, true);
>          libxl_defbool_setdefault(&b_info->u.hvm.vpt_align, true);
>          libxl_defbool_setdefault(&b_info->u.hvm.nested_hvm, false);
> +        libxl_defbool_setdefault(&b_info->u.hvm.incr_generationid, false);
> +
>          libxl_defbool_setdefault(&b_info->u.hvm.usb, false);
>          libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci, true);
>  
> diff -r ca432f4ad9f5 -r 42c448aa6415 tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c	Mon Feb 13 15:49:01 2012 +0000
> +++ b/tools/libxl/libxl_dom.c	Mon Feb 13 16:27:27 2012 +0000
> @@ -406,7 +406,7 @@ int libxl__domain_restore_common(libxl__
>          hvm = 1;
>          superpages = 1;
>          pae = libxl_defbool_val(info->u.hvm.pae);
> -        no_incr_generationid = info->u.hvm.no_incr_generationid;
> +        no_incr_generationid = !libxl_defbool_val(info->u.hvm.incr_generationid);
>          break;
>      case LIBXL_DOMAIN_TYPE_PV:
>          hvm = 0;
> diff -r ca432f4ad9f5 -r 42c448aa6415 tools/libxl/libxl_types.idl
> --- a/tools/libxl/libxl_types.idl	Mon Feb 13 15:49:01 2012 +0000
> +++ b/tools/libxl/libxl_types.idl	Mon Feb 13 16:27:27 2012 +0000
> @@ -243,7 +243,7 @@ libxl_domain_build_info = Struct("domain
>                                         ("vpt_align",        libxl_defbool),
>                                         ("timer_mode",       libxl_timer_mode),
>                                         ("nested_hvm",       libxl_defbool),
> -                                       ("no_incr_generationid", bool),
> +                                       ("incr_generationid",libxl_defbool),
>                                         ("nographic",        bool),
>                                         ("stdvga",           bool),
>                                         ("vnc",              libxl_vnc_info),
> diff -r ca432f4ad9f5 -r 42c448aa6415 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c	Mon Feb 13 15:49:01 2012 +0000
> +++ b/tools/libxl/xl_cmdimpl.c	Mon Feb 13 16:27:27 2012 +0000
> @@ -1350,7 +1350,7 @@ struct domain_create {
>      const char *restore_file;
>      int migrate_fd; /* -1 means none */
>      char **migration_domname_r; /* from malloc */
> -    int no_incr_generationid;
> +    int incr_generationid;
>  };
>  
>  static int freemem(libxl_domain_build_info *b_info)
> @@ -1601,8 +1601,8 @@ static int create_domain(struct domain_c
>      }
>  
>      if (d_config.c_info.type == LIBXL_DOMAIN_TYPE_HVM)
> -        d_config.b_info.u.hvm.no_incr_generationid =
> -            dom_info->no_incr_generationid;
> +        libxl_defbool_set(&d_config.b_info.u.hvm.incr_generationid,
> +                          dom_info->incr_generationid);
>  
>      if (debug || dom_info->dryrun)
>          printf_info(default_output_format, -1, &d_config);
> @@ -2877,7 +2877,7 @@ static void migrate_receive(int debug, i
>      dom_info.restore_file = "incoming migration stream";
>      dom_info.migrate_fd = 0; /* stdin */
>      dom_info.migration_domname_r = &migration_domname;
> -    dom_info.no_incr_generationid = 1;
> +    dom_info.incr_generationid = 0;
>  
>      rc = create_domain(&dom_info);
>      if (rc < 0) {
> @@ -3002,6 +3002,7 @@ int main_restore(int argc, char **argv)
>      dom_info.restore_file = checkpoint_file;
>      dom_info.migrate_fd = -1;
>      dom_info.console_autoconnect = console_autoconnect;
> +    dom_info.incr_generationid = 1;
>  
>      rc = create_domain(&dom_info);
>      if (rc < 0)
> @@ -3384,6 +3385,7 @@ int main_create(int argc, char **argv)
>      dom_info.extra_config = extra_config;
>      dom_info.migrate_fd = -1;
>      dom_info.console_autoconnect = console_autoconnect;
> +    dom_info.incr_generationid = 0;
>  
>      rc = create_domain(&dom_info);
>      if (rc < 0)
> diff -r ca432f4ad9f5 -r 42c448aa6415 tools/libxl/xl_sxp.c
> --- a/tools/libxl/xl_sxp.c	Mon Feb 13 15:49:01 2012 +0000
> +++ b/tools/libxl/xl_sxp.c	Mon Feb 13 16:27:27 2012 +0000
> @@ -108,8 +108,8 @@ void printf_info_sexp(int domid, libxl_d
>                 libxl_timer_mode_to_string(b_info->u.hvm.timer_mode));
>          printf("\t\t\t(nestedhvm %s)\n",
>                 libxl_defbool_to_string(b_info->u.hvm.nested_hvm));
> -        printf("\t\t\t(no_incr_generationid %d)\n",
> -                    b_info->u.hvm.no_incr_generationid);
> +        printf("\t\t\t(no_incr_generationid %s)\n",
> +               libxl_defbool_to_string(b_info->u.hvm.incr_generationid));
>  
>          printf("\t\t\t(stdvga %d)\n", b_info->u.hvm.stdvga);
>          printf("\t\t\t(vnc %d)\n", b_info->u.hvm.vnc.enable);
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 11 of 23] libxl: make libxl_device_console internal
  2012-02-21 10:23     ` Ian Campbell
@ 2012-02-21 12:31       ` Ian Jackson
  0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2012-02-21 12:31 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

Ian Campbell writes ("Re: [Xen-devel] [PATCH 11 of 23] libxl: make libxl_device_console internal"):
> On Mon, 2012-02-20 at 19:20 +0000, Ian Jackson wrote:
> > Ian Campbell writes ("[Xen-devel] [PATCH 11 of 23] libxl: make libxl_device_console internal"):
> > > libxl: make libxl_device_console internal
...
> > Isn't it likely that at some future point we will re-expose this
> > difference, for example by providing a different set of access
> > functions ?
> 
> Very likely. However I didn't want to design or commit to an API in the
> total absence of users.
> 
> When a user does come along it is much easier to add a completely new
> API rather than to fix an existing broken one. It's easier to do this in
> a manner which users of the library can cope with in a compatible way
> e.g. adding a new API is easier to check for with ./configure.

Good point.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Ian.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 05 of 23] libxl: drop 8M slack for PV guests
  2012-02-21 10:10       ` Ian Campbell
  2012-02-21 10:53         ` David Vrabel
@ 2012-02-21 15:27         ` Dan Magenheimer
  1 sibling, 0 replies; 55+ messages in thread
From: Dan Magenheimer @ 2012-02-21 15:27 UTC (permalink / raw)
  To: Ian Campbell, Stefano Stabellini; +Cc: xen-devel, Ian Jackson, Dave Scott

> From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
> Subject: Re: [Xen-devel] [PATCH 05 of 23] libxl: drop 8M slack for PV guests
> 
> On Tue, 2012-02-21 at 10:10 +0000, Stefano Stabellini wrote:
> > On Mon, 20 Feb 2012, Ian Jackson wrote:
> > > Ian Campbell writes ("[Xen-devel] [PATCH 05 of 23] libxl: drop 8M slack for PV guests"):
> > > > libxl: drop 8M slack for PV guests.
> > > >
> > > > As far as I can tell this serves no purpose. I think it relates to the old
> > > > 8M to "account for backend allocations" which we used to add. This leaves a bit
> > > > of unpopulated space in the Pseudo-physical address space which can be used by
> > > > backends when mapping foreign memory. However 8M is not representative of that
> > > > any more and modern kernels do not operate in this way anyway.
> > > >
> > > > I suspect an argument could be made for removing this from the libxl API
> > > > altogether but instead lets just set the overhead to 0.
> > >
> > > I think this is plausible but I'd like to hear from Stefano, who iirc
> > > may have some knowledge about the reason for this 8Mb.
> >
> > It comes from XenD (and XAPI too, if I recall correctly), see this
> > comment in tools/python/xen/xend/image.py:
> 
> That doesn't answer the "why" though.
> 
> I think I should just be more assertive since I believe I do actually
> know why the 8 is there (or certainly nobody appears to know any
> better). I'll update the changelog to read:
> 
>         This serves no purpose. It relates to the old 8M to "account for
>         backend allocations" which we used to add. This leaves a bit of
>         unpopulated space in the Pseudo-physical address space which can
>         be used by backends when mapping foreign memory. However 8M is
>         not representative of that any more and modern kernels do not
>         operate in this way anyway.

Hmmm... this adds a small, but not trivial, performance advantage to xl
over xm/xend, at least for small memory guests.  Might be a nice
second-tier bullet for the 4.2/xl release notes... "better performance"
always encourages people to move from old-to-new quicker even when
it is hard to quantify or measure.

^ permalink raw reply	[flat|nested] 55+ messages in thread

end of thread, other threads:[~2012-02-21 15:27 UTC | newest]

Thread overview: 55+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-13 16:52 [PATCH 00 of 23] libxl: improved handling for default values in API Ian Campbell
2012-02-13 16:52 ` [PATCH 01 of 23] libxl: Document _init/_dispose/_setdefault functions Ian Campbell
2012-02-20 19:08   ` Ian Jackson
2012-02-21 10:18     ` Ian Campbell
2012-02-13 16:52 ` [PATCH 02 of 23] libxl: provide _init and _setdefault for libxl_domain_create_info Ian Campbell
2012-02-20 19:09   ` Ian Jackson
2012-02-13 16:52 ` [PATCH 03 of 23] libxl: provide _init and _setdefault for libxl_domain_build_info Ian Campbell
2012-02-13 16:52 ` [PATCH 04 of 23] libxl: use an explicit LIBXL_TIMER_MODE_DEFAULT value Ian Campbell
2012-02-20 19:16   ` Ian Jackson
2012-02-13 16:52 ` [PATCH 05 of 23] libxl: drop 8M slack for PV guests Ian Campbell
2012-02-20 19:15   ` Ian Jackson
2012-02-21 10:10     ` Stefano Stabellini
2012-02-21 10:10       ` Ian Campbell
2012-02-21 10:53         ` David Vrabel
2012-02-21 10:59           ` Ian Campbell
2012-02-21 15:27         ` Dan Magenheimer
2012-02-13 16:52 ` [PATCH 06 of 23] libxl: allow specification of testidl random seed Ian Campbell
2012-02-20 19:18   ` Ian Jackson
2012-02-13 16:52 ` [PATCH 07 of 23] libxl: introduce a descriminating default value for memkb fields Ian Campbell
2012-02-20 19:24   ` Ian Jackson
2012-02-13 16:52 ` [PATCH 08 of 23] libxl: disk: use _init/_setdefault Ian Campbell
2012-02-13 16:52 ` [PATCH 09 of 23] libxl: nic: " Ian Campbell
2012-02-13 16:52 ` [PATCH 10 of 23] libxl: vfb/vkb: " Ian Campbell
2012-02-13 16:52 ` [PATCH 11 of 23] libxl: make libxl_device_console internal Ian Campbell
2012-02-20 19:20   ` Ian Jackson
2012-02-21 10:23     ` Ian Campbell
2012-02-21 12:31       ` Ian Jackson
2012-02-13 16:52 ` [PATCH 12 of 23] libxl: pci: use _init/_setdefault Ian Campbell
2012-02-13 16:52 ` [PATCH 13 of 23] libxl: add new "defbool" built in type Ian Campbell
2012-02-20 19:14   ` Ian Jackson
2012-02-21 10:24     ` Ian Campbell
2012-02-13 16:53 ` [PATCH 14 of 23] libxl: make boolean members of libxl_domain_create_info into libxl_defbool Ian Campbell
2012-02-20 19:18   ` Ian Jackson
2012-02-13 16:53 ` [PATCH 15 " Ian Campbell
2012-02-20 19:25   ` Ian Jackson
2012-02-21 10:58     ` Ian Campbell
2012-02-13 16:53 ` [PATCH 16 of 23] libxl: switch generation id control to libxl_defbool Ian Campbell
2012-02-20 19:16   ` Ian Jackson
2012-02-21 11:01   ` Ian Campbell
2012-02-13 16:53 ` [PATCH 17 of 23] libxl: use defbool for graphics related options Ian Campbell
2012-02-20 19:22   ` Ian Jackson
2012-02-13 16:53 ` [PATCH 18 of 23] libxl: do not explicitly initialise members of build info to zero Ian Campbell
2012-02-20 19:20   ` Ian Jackson
2012-02-13 16:53 ` [PATCH 19 of 23] libxl: switch device model selection over to libxl_defbool Ian Campbell
2012-02-13 16:53 ` [PATCH 20 of 23] libxl: add libxl_domain_build_info_init_type Ian Campbell
2012-02-20 19:22   ` Ian Jackson
2012-02-20 19:24   ` Ian Jackson
2012-02-13 16:53 ` [PATCH 21 of 23] libxl: Make IDL KeyedUnion keyvar an idl.Field Ian Campbell
2012-02-20 19:17   ` Ian Jackson
2012-02-13 16:53 ` [PATCH 22 of 23] libxl: idl: generate KeyedUnion key member as part of the KeyedUnion Ian Campbell
2012-02-13 17:08   ` Ian Jackson
2012-02-13 17:13     ` Ian Campbell
2012-02-13 16:53 ` [PATCH 23 of 23] libxl: autogenerate libxl_FOO_init and libxl_FOO_init_FIELD Ian Campbell
2012-02-20 19:24   ` Ian Jackson
2012-02-21 10:11     ` Ian Campbell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.