[PATCH 01/19] libxl/xl: move some HVM/PV specific fields of libxl_domain_build_info

[Roger Pau Monne <roger.pau@citrix.com>]

Move the HVM/PV sub-structure fields of libxl_domain_build_info into
the top-level structure. xl is also modified to start using those
fields.

This is required because those options will be used by the new PVH
guest type.

Defines are added in order to signal consumers that the fields are
available.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.h            |  16 ++++++
 tools/libxl/libxl_bootloader.c |  14 +++---
 tools/libxl/libxl_create.c     |  24 ++++++++-
 tools/libxl/libxl_dom.c        |   8 +--
 tools/libxl/libxl_types.idl    |  13 +++++
 tools/libxl/libxl_x86.c        |   2 +-
 tools/libxl/libxl_x86_acpi.c   |   2 +-
 tools/xl/xl_parse.c            | 107 +++++++++++++++++++++++------------------
 tools/xl/xl_sxp.c              |  16 +++---
 9 files changed, 133 insertions(+), 69 deletions(-)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 229e289750..3aee26ff53 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -306,6 +306,22 @@
 #define LIBXL_HAVE_BUILDINFO_HVM_ACPI_LAPTOP_SLATE 1
 
 /*
+ * LIBXL_HAVE_BUILDINFO_* indicates that libxl_domain_build_info has
+ * the field represented by the '*'. The original position of those
+ * fields is:
+ *  - u.hvm.timer_mode
+ *  - u.hvm.apic
+ *  - u.hvm.nested_hvm
+ *  - u.pv.bootloader
+ *  - u.pv.bootloader_args
+ */
+#define LIBXL_HAVE_BUILDINFO_TIMER_MODE 1
+#define LIBXL_HAVE_BUILDINFO_APIC 1
+#define LIBXL_HAVE_BUILDINFO_NESTED_HVM 1
+#define LIBXL_HAVE_BUILDINFO_BOOTLOADER 1
+#define LIBXL_HAVE_BUILDINFO_BOOTLOADER_ARGS 1
+
+/*
  * libxl ABI compatibility
  *
  * The only guarantee which libxl makes regarding ABI compatibility
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index c7c201262c..a47bd8c25c 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -51,7 +51,7 @@ static void make_bootloader_args(libxl__gc *gc, libxl__bootloader_state *bl,
 {
     const libxl_domain_build_info *info = bl->info;
 
-    bl->argsspace = 9 + libxl_string_list_length(&info->u.pv.bootloader_args);
+    bl->argsspace = 9 + libxl_string_list_length(&info->bootloader_args);
 
     GCNEW_ARRAY(bl->args, bl->argsspace);
 
@@ -70,8 +70,8 @@ static void make_bootloader_args(libxl__gc *gc, libxl__bootloader_state *bl,
     ARG("--output-format=simple0");
     ARG(GCSPRINTF("--output-directory=%s", bl->outputdir));
 
-    if (info->u.pv.bootloader_args) {
-        char **p = info->u.pv.bootloader_args;
+    if (info->bootloader_args) {
+        char **p = info->bootloader_args;
         while (*p) {
             ARG(*p);
             p++;
@@ -330,7 +330,7 @@ void libxl__bootloader_run(libxl__egc *egc, libxl__bootloader_state *bl)
         goto out_ok;
     }
 
-    if (!info->u.pv.bootloader) {
+    if (!info->bootloader) {
         LOGD(DEBUG, domid,
              "no bootloader configured, using user supplied kernel");
         bl->kernel->path = bl->info->kernel;
@@ -419,14 +419,14 @@ static void bootloader_disk_attached_cb(libxl__egc *egc,
     }
 
     LOGD(DEBUG, bl->domid,
-         "Config bootloader value: %s", info->u.pv.bootloader);
+         "Config bootloader value: %s", info->bootloader);
 
-    if ( !strcmp(info->u.pv.bootloader, "/usr/bin/pygrub") )
+    if ( !strcmp(info->bootloader, "/usr/bin/pygrub") )
         LOGD(WARN, bl->domid,
              "bootloader='/usr/bin/pygrub' is deprecated; use " \
              "bootloader='pygrub' instead");
 
-    bootloader = info->u.pv.bootloader;
+    bootloader = info->bootloader;
 
     /* If the full path is not specified, check in the libexec path */
     if ( bootloader[0] != '/' ) {
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 1158303e1a..2f03ebe586 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -406,6 +406,28 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
             libxl_domain_type_to_string(b_info->type));
         return ERROR_INVAL;
     }
+
+    if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
+        libxl_defbool_setdefault(&b_info->nested_hvm,
+                                 libxl_defbool_val(b_info->u.hvm.nested_hvm));
+        libxl_defbool_setdefault(&b_info->apic,
+                                 libxl_defbool_val(b_info->u.hvm.apic));
+    } else {
+        libxl_defbool_setdefault(&b_info->nested_hvm, false);
+        libxl_defbool_setdefault(&b_info->apic, true);
+    }
+
+    if (b_info->timer_mode == LIBXL_TIMER_MODE_DEFAULT)
+        b_info->timer_mode = b_info->type == LIBXL_DOMAIN_TYPE_HVM
+                             ? b_info->u.hvm.timer_mode
+                             : LIBXL_TIMER_MODE_NO_DELAY_FOR_MISSED_TICKS;
+
+    if (b_info->type == LIBXL_DOMAIN_TYPE_PV && !b_info->bootloader) {
+        assert(!b_info->bootloader_args);
+        b_info->bootloader = b_info->u.pv.bootloader;
+        b_info->bootloader_args = b_info->u.pv.bootloader_args;
+    }
+
     return 0;
 }
 
@@ -901,7 +923,7 @@ static void initiate_domain_create(libxl__egc *egc,
     }
 
     if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM &&
-        (libxl_defbool_val(d_config->b_info.u.hvm.nested_hvm) &&
+        (libxl_defbool_val(d_config->b_info.nested_hvm) &&
         (libxl_defbool_val(d_config->b_info.u.hvm.altp2m) ||
         (d_config->b_info.altp2m != LIBXL_ALTP2M_MODE_DISABLED)))) {
         ret = ERROR_INVAL;
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index f54fd49a73..33213db388 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -190,7 +190,7 @@ static int numa_place_domain(libxl__gc *gc, uint32_t domid,
 
 static unsigned long timer_mode(const libxl_domain_build_info *info)
 {
-    const libxl_timer_mode mode = info->u.hvm.timer_mode;
+    const libxl_timer_mode mode = info->timer_mode;
     assert(mode >= LIBXL_TIMER_MODE_DELAY_FOR_MISSED_TICKS &&
            mode <= LIBXL_TIMER_MODE_ONE_MISSED_TICK_PENDING);
     return ((unsigned long)mode);
@@ -305,7 +305,7 @@ static void hvm_set_conf_params(xc_interface *handle, uint32_t domid,
     xc_hvm_param_set(handle, domid, HVM_PARAM_VPT_ALIGN,
                     libxl_defbool_val(info->u.hvm.vpt_align));
     xc_hvm_param_set(handle, domid, HVM_PARAM_NESTEDHVM,
-                    libxl_defbool_val(info->u.hvm.nested_hvm));
+                    libxl_defbool_val(info->nested_hvm));
 }
 
 int libxl__build_pre(libxl__gc *gc, uint32_t domid,
@@ -833,7 +833,7 @@ static int hvm_build_set_params(xc_interface *handle, uint32_t domid,
             return ERROR_FAIL;
 
         va_hvm = (struct hvm_info_table *)(va_map + HVM_INFO_OFFSET);
-        va_hvm->apic_mode = libxl_defbool_val(info->u.hvm.apic);
+        va_hvm->apic_mode = libxl_defbool_val(info->apic);
         va_hvm->nr_vcpus = info->max_vcpus;
         memset(va_hvm->vcpu_online, 0, sizeof(va_hvm->vcpu_online));
         memcpy(va_hvm->vcpu_online, info->avail_vcpus.map, info->avail_vcpus.size);
@@ -1118,7 +1118,7 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
         dom->mmio_size = HVM_BELOW_4G_MMIO_LENGTH;
     else if (dom->mmio_size == 0 && !device_model) {
 #if defined(__i386__) || defined(__x86_64__)
-        if (libxl_defbool_val(info->u.hvm.apic)) {
+        if (libxl_defbool_val(info->apic)) {
             /* Make sure LAPIC_BASE_ADDRESS is below special pages */
             assert(((((X86_HVM_END_SPECIAL_REGION - X86_HVM_NR_SPECIAL_PAGES)
                       << XC_PAGE_SHIFT) - LAPIC_BASE_ADDRESS)) >= XC_PAGE_SIZE);
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 6e80d36256..bf1652d367 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -506,10 +506,17 @@ libxl_domain_build_info = Struct("domain_build_info",[
     # 65000 which is reserved by the toolstack.
     ("device_tree",      string),
     ("acpi",             libxl_defbool),
+    ("bootloader",       string),
+    ("bootloader_args",  libxl_string_list),
+    ("timer_mode",       libxl_timer_mode),
+    ("nested_hvm",       libxl_defbool),
+    ("apic",             libxl_defbool),
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
                                        ("bios",             libxl_bios_type),
                                        ("pae",              libxl_defbool),
+                                       # The apic field is deprecated, please
+                                       # use the unified apic field above.
                                        ("apic",             libxl_defbool),
                                        # The following acpi field is deprecated.
                                        # Please use the unified acpi field above
@@ -526,6 +533,9 @@ libxl_domain_build_info = Struct("domain_build_info",[
                                        ("hpet",             libxl_defbool),
                                        ("vpt_align",        libxl_defbool),
                                        ("mmio_hole_memkb",  MemKB),
+                                       # time_mode and nested_hvm fields are
+                                       # deprecated in favor of the unified
+                                       # fields present above.
                                        ("timer_mode",       libxl_timer_mode),
                                        ("nested_hvm",       libxl_defbool),
                                        # The u.hvm.altp2m field is used solely
@@ -568,6 +578,9 @@ libxl_domain_build_info = Struct("domain_build_info",[
                                        ])),
                  ("pv", Struct(None, [("kernel", string),
                                       ("slack_memkb", MemKB),
+                                      # bootloader and bootloader_args fields
+                                      # are deprecated in favor of the unified
+                                      # fields present above.
                                       ("bootloader", string),
                                       ("bootloader_args", libxl_string_list),
                                       ("cmdline", string),
diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index 455f6f0bed..442854c5c2 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -12,7 +12,7 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
         if (d_config->b_info.device_model_version !=
             LIBXL_DEVICE_MODEL_VERSION_NONE) {
             xc_config->emulation_flags = XEN_X86_EMU_ALL;
-        } else if (libxl_defbool_val(d_config->b_info.u.hvm.apic)) {
+        } else if (libxl_defbool_val(d_config->b_info.apic)) {
             /*
              * HVM guests without device model may want
              * to have LAPIC emulation.
diff --git a/tools/libxl/libxl_x86_acpi.c b/tools/libxl/libxl_x86_acpi.c
index c0a6e321ec..cd8f4f4779 100644
--- a/tools/libxl/libxl_x86_acpi.c
+++ b/tools/libxl/libxl_x86_acpi.c
@@ -112,7 +112,7 @@ static int init_acpi_config(libxl__gc *gc,
 
     hvminfo = libxl__zalloc(gc, sizeof(*hvminfo));
 
-    hvminfo->apic_mode = libxl_defbool_val(b_info->u.hvm.apic);
+    hvminfo->apic_mode = libxl_defbool_val(b_info->apic);
 
     if (dom->nr_vnodes) {
         unsigned int *vcpu_to_vnode, *vdistance;
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 5c2bf17222..f78255dfe5 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -1037,6 +1037,65 @@ void parse_config_data(const char *config_source,
     xlu_cfg_get_defbool(config, "driver_domain", &c_info->driver_domain, 0);
     xlu_cfg_get_defbool(config, "acpi", &b_info->acpi, 0);
 
+    xlu_cfg_replace_string (config, "bootloader", &b_info->bootloader, 0);
+    switch (xlu_cfg_get_list_as_string_list(config, "bootloader_args",
+                                            &b_info->bootloader_args, 1)) {
+    case 0:
+        break; /* Success */
+    case ESRCH: break; /* Option not present */
+    case EINVAL:
+        if (!xlu_cfg_get_string(config, "bootloader_args", &buf, 0)) {
+
+            fprintf(stderr, "WARNING: Specifying \"bootloader_args\""
+                    " as a string is deprecated. "
+                    "Please use a list of arguments.\n");
+            split_string_into_string_list(buf, " \t\n",
+                                          &b_info->bootloader_args);
+        }
+        break;
+    default:
+        fprintf(stderr,"xl: Unable to parse bootloader_args.\n");
+        exit(-ERROR_FAIL);
+    }
+
+    if (!xlu_cfg_get_long(config, "timer_mode", &l, 1)) {
+        const char *s = libxl_timer_mode_to_string(l);
+
+        if (b_info->type == LIBXL_DOMAIN_TYPE_PV) {
+            fprintf(stderr,
+                    "xl: \"timer_mode\" option is not supported for PV guests.\n");
+            exit(-ERROR_FAIL);
+        }
+
+        fprintf(stderr, "WARNING: specifying \"timer_mode\" as an integer is deprecated. "
+                "Please use the named parameter variant. %s%s%s\n",
+                s ? "e.g. timer_mode=\"" : "",
+                s ? s : "",
+                s ? "\"" : "");
+
+        if (l < LIBXL_TIMER_MODE_DELAY_FOR_MISSED_TICKS ||
+            l > LIBXL_TIMER_MODE_ONE_MISSED_TICK_PENDING) {
+            fprintf(stderr, "ERROR: invalid value %ld for \"timer_mode\"\n", l);
+            exit (1);
+        }
+        b_info->timer_mode = l;
+    } else if (!xlu_cfg_get_string(config, "timer_mode", &buf, 0)) {
+        if (b_info->type == LIBXL_DOMAIN_TYPE_PV) {
+            fprintf(stderr,
+                    "xl: \"timer_mode\" option is not supported for PV guests.\n");
+            exit(-ERROR_FAIL);
+        }
+
+        if (libxl_timer_mode_from_string(buf, &b_info->timer_mode)) {
+            fprintf(stderr, "ERROR: invalid value \"%s\" for \"timer_mode\"\n",
+                    buf);
+            exit (1);
+        }
+    }
+
+    xlu_cfg_get_defbool(config, "nestedhvm", &b_info->nested_hvm, 0);
+    xlu_cfg_get_defbool(config, "apic", &b_info->apic, 0);
+
     switch(b_info->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
         kernel_basename = libxl_basename(b_info->kernel);
@@ -1064,7 +1123,6 @@ void parse_config_data(const char *config_source,
                     "bios_path_override given without specific bios name\n");
 
         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_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, "acpi_laptop_slate", &b_info->u.hvm.acpi_laptop_slate, 0);
@@ -1134,29 +1192,6 @@ void parse_config_data(const char *config_source,
                 exit (1);
             }
         }
-        if (!xlu_cfg_get_long(config, "timer_mode", &l, 1)) {
-            const char *s = libxl_timer_mode_to_string(l);
-            fprintf(stderr, "WARNING: specifying \"timer_mode\" as an integer is deprecated. "
-                    "Please use the named parameter variant. %s%s%s\n",
-                    s ? "e.g. timer_mode=\"" : "",
-                    s ? s : "",
-                    s ? "\"" : "");
-
-            if (l < LIBXL_TIMER_MODE_DELAY_FOR_MISSED_TICKS ||
-                l > LIBXL_TIMER_MODE_ONE_MISSED_TICK_PENDING) {
-                fprintf(stderr, "ERROR: invalid value %ld for \"timer_mode\"\n", l);
-                exit (1);
-            }
-            b_info->u.hvm.timer_mode = l;
-        } else if (!xlu_cfg_get_string(config, "timer_mode", &buf, 0)) {
-            if (libxl_timer_mode_from_string(buf, &b_info->u.hvm.timer_mode)) {
-                fprintf(stderr, "ERROR: invalid value \"%s\" for \"timer_mode\"\n",
-                        buf);
-                exit (1);
-            }
-        }
-
-        xlu_cfg_get_defbool(config, "nestedhvm", &b_info->u.hvm.nested_hvm, 0);
 
         if (!xlu_cfg_get_defbool(config, "altp2mhvm", &b_info->u.hvm.altp2m, 0))
             fprintf(stderr, "WARNING: Specifying \"altp2mhvm\" is deprecated. "
@@ -1212,29 +1247,7 @@ void parse_config_data(const char *config_source,
         break;
     case LIBXL_DOMAIN_TYPE_PV:
     {
-        xlu_cfg_replace_string (config, "bootloader", &b_info->u.pv.bootloader, 0);
-        switch (xlu_cfg_get_list_as_string_list(config, "bootloader_args",
-                                      &b_info->u.pv.bootloader_args, 1))
-        {
-
-        case 0: break; /* Success */
-        case ESRCH: break; /* Option not present */
-        case EINVAL:
-            if (!xlu_cfg_get_string(config, "bootloader_args", &buf, 0)) {
-
-                fprintf(stderr, "WARNING: Specifying \"bootloader_args\""
-                        " as a string is deprecated. "
-                        "Please use a list of arguments.\n");
-                split_string_into_string_list(buf, " \t\n",
-                                              &b_info->u.pv.bootloader_args);
-            }
-            break;
-        default:
-            fprintf(stderr,"xl: Unable to parse bootloader_args.\n");
-            exit(-ERROR_FAIL);
-        }
-
-        if (!b_info->u.pv.bootloader && !b_info->kernel) {
+        if (!b_info->bootloader && !b_info->kernel) {
             fprintf(stderr, "Neither kernel nor bootloader specified\n");
             exit(1);
         }
diff --git a/tools/xl/xl_sxp.c b/tools/xl/xl_sxp.c
index e738bf2465..48758240e6 100644
--- a/tools/xl/xl_sxp.c
+++ b/tools/xl/xl_sxp.c
@@ -70,12 +70,12 @@ void printf_info_sexp(int domid, libxl_domain_config *d_config, FILE *fh)
     fprintf(fh, "\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) {
-        fprintf(fh, "\t(bootloader %s)\n", b_info->u.pv.bootloader);
-        if (b_info->u.pv.bootloader_args) {
+    if (c_info->type == LIBXL_DOMAIN_TYPE_PV && b_info->bootloader) {
+        fprintf(fh, "\t(bootloader %s)\n", b_info->bootloader);
+        if (b_info->bootloader_args) {
             fprintf(fh, "\t(bootloader_args");
-            for (i=0; b_info->u.pv.bootloader_args[i]; i++)
-                fprintf(fh, " %s", b_info->u.pv.bootloader_args[i]);
+            for (i=0; b_info->bootloader_args[i]; i++)
+                fprintf(fh, " %s", b_info->bootloader_args[i]);
             fprintf(fh, ")\n");
         }
     }
@@ -89,7 +89,7 @@ void printf_info_sexp(int domid, libxl_domain_config *d_config, FILE *fh)
         fprintf(fh, "\t\t\t(shadow_memkb %"PRId64")\n", b_info->shadow_memkb);
         fprintf(fh, "\t\t\t(pae %s)\n", libxl_defbool_to_string(b_info->u.hvm.pae));
         fprintf(fh, "\t\t\t(apic %s)\n",
-               libxl_defbool_to_string(b_info->u.hvm.apic));
+               libxl_defbool_to_string(b_info->apic));
         fprintf(fh, "\t\t\t(acpi %s)\n",
                libxl_defbool_to_string(b_info->u.hvm.acpi));
         fprintf(fh, "\t\t\t(nx %s)\n", libxl_defbool_to_string(b_info->u.hvm.nx));
@@ -100,9 +100,9 @@ void printf_info_sexp(int domid, libxl_domain_config *d_config, FILE *fh)
         fprintf(fh, "\t\t\t(vpt_align %s)\n",
                libxl_defbool_to_string(b_info->u.hvm.vpt_align));
         fprintf(fh, "\t\t\t(timer_mode %s)\n",
-               libxl_timer_mode_to_string(b_info->u.hvm.timer_mode));
+               libxl_timer_mode_to_string(b_info->timer_mode));
         fprintf(fh, "\t\t\t(nestedhvm %s)\n",
-               libxl_defbool_to_string(b_info->u.hvm.nested_hvm));
+               libxl_defbool_to_string(b_info->nested_hvm));
         fprintf(fh, "\t\t\t(stdvga %s)\n", b_info->u.hvm.vga.kind ==
                                       LIBXL_VGA_INTERFACE_TYPE_STD ?
                                       "True" : "False");
-- 
2.11.0 (Apple Git-81)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel