All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/21] libxl/xl: add PVH guest type
@ 2017-09-07 10:16 Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 01/21] libxl: add is_default checkers for string and timer_mode types Roger Pau Monne
                   ` (20 more replies)
  0 siblings, 21 replies; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: boris.ostrovsky

Hello,

This series adds a new PVH guest type to libxl/xl, this supersedes the
current PVHv2 implementation that relies on using the "none" device
model version.

As part of this series a new xl option is also implemented, called
"type" that supersedes the current "builder" option. A "firmware"
option is also introduced in order to have a uniform way of loading
firmwares for all guest types (HVM, PV and PVH).

Patch 1 adds a default value checker for the string and timer_mode
types, and patch 2 introduces IDL code to generate a helper function
that copies fields marked as deprecated to their new positions. This
is already used to move some PV/HVM specific fields from
domain_build_info into the top level of the structure. Finally patch 3
switches libxl to use the new field position.

Patches 4 and 5 introduce the new type
and firmware options. Patch 6 introduces the PVH guest type to libxl.

Patches from 7 to 19 add PVH support to all the needed functions, this
could be considered a single patch, but I've tried to split it in
order to ease the review. The current split is done on a per file
basis.

Finally patch 20 adds PVH support to xl and patch 21 removes the
device model version "none".

This implementation is based on the discussion held during the
Budapest XenSummit [0].

The full series can be found at:

git://xenbits.xen.org/people/royger/xen.git pvh_tools_v2

I've tested the creation and migration of PVH guests, which seems to
be all OK. I've also run an osstest flight with this series, the
output can be seen at:

http://osstest.xs.citrite.net/~osstest/testlogs/logs/72069/

(Only accessible from the Citrix internal network, sorry)

Thanks, Roger.

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

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

* [PATCH v2 01/21] libxl: add is_default checkers for string and timer_mode types
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-19 14:48   ` Ian Jackson
  2017-09-20 16:12   ` Wei Liu
  2017-09-07 10:16 ` [PATCH v2 02/21] libxl: introduce a way to mark fields as deprecated in the idl Roger Pau Monne
                   ` (19 subsequent siblings)
  20 siblings, 2 replies; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

Those types are missing a helper to check whether a definition of the
type holds the default value. This will be required by a later patch
that will implement deprecation of fields inside of a libxl type.

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>
---
Changes since v1:
 - New in this version.
---
 tools/libxl/idl.py           |  3 ++-
 tools/libxl/libxl_internal.h | 10 ++++++++++
 tools/libxl/libxl_types.idl  |  3 ++-
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/idl.py b/tools/libxl/idl.py
index 437049ebb9..a4a084e1ce 100644
--- a/tools/libxl/idl.py
+++ b/tools/libxl/idl.py
@@ -302,7 +302,8 @@ string = Builtin("char *", namespace = None, copy_fn = "libxl_string_copy", disp
                  json_gen_fn = "libxl__string_gen_json",
                  json_parse_type = "JSON_STRING | JSON_NULL",
                  json_parse_fn = "libxl__string_parse_json",
-                 autogenerate_json = False)
+                 autogenerate_json = False,
+                 check_default_fn="libxl__string_is_default")
 
 class Array(Type):
     """An array of the same type"""
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 724750967c..06d3b25418 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -4350,6 +4350,16 @@ static inline bool libxl__acpi_defbool_val(const libxl_domain_build_info *b_info
     return libxl_defbool_val(b_info->acpi) &&
            libxl_defbool_val(b_info->u.hvm.acpi);
 }
+
+static inline bool libxl__timer_mode_is_default(libxl_timer_mode *tm)
+{
+    return *tm == LIBXL_TIMER_MODE_DEFAULT;
+}
+
+static inline bool libxl__string_is_default(char **s)
+{
+    return *s == NULL;
+}
 #endif
 
 /*
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 173d70acec..267c4f3804 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -173,7 +173,8 @@ libxl_timer_mode = Enumeration("timer_mode", [
     (1, "no_delay_for_missed_ticks"),
     (2, "no_missed_ticks_pending"),
     (3, "one_missed_tick_pending"),
-    ], init_val = "LIBXL_TIMER_MODE_DEFAULT")
+    ], init_val = "LIBXL_TIMER_MODE_DEFAULT",
+       check_default_fn = "libxl__timer_mode_is_default")
 
 libxl_bios_type = Enumeration("bios_type", [
     (0, "unknown"),
-- 
2.11.0 (Apple Git-81)


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

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

* [PATCH v2 02/21] libxl: introduce a way to mark fields as deprecated in the idl
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 01/21] libxl: add is_default checkers for string and timer_mode types Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-20 15:46   ` Ian Jackson
  2017-09-07 10:16 ` [PATCH v2 03/21] libxl/xl: use the new domain_build_info fields position Roger Pau Monne
                   ` (18 subsequent siblings)
  20 siblings, 1 reply; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

The deprecation involves generating a function that copies the
deprecated fields into it's new location if the new location has not
been set.

The fields that are going to be shared between PVH and HVM or between
PVH and PV are moved to the top level of libxl_domain_build_info, and
the old locations are marked as deprecated.

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>
---
Changes since v1:
 - New in this version.
---
 tools/libxl/gentypes.py     | 56 +++++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/idl.py          |  2 ++
 tools/libxl/libxl_types.idl | 17 +++++++++-----
 3 files changed, 69 insertions(+), 6 deletions(-)

diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
index 4ea7091e6b..c2d34bcd30 100644
--- a/tools/libxl/gentypes.py
+++ b/tools/libxl/gentypes.py
@@ -261,6 +261,47 @@ def libxl_C_type_gen_map_key(f, parent, indent = ""):
         s = indent + s
     return s.replace("\n", "\n%s" % indent).rstrip(indent)
 
+def libxl_C_type_copy_deprecated(field, v, indent = "    ", vparent = None):
+    s = ""
+
+    if isinstance(field.type, idl.KeyedUnion):
+        if vparent is None:
+            raise Exception("KeyedUnion type must have a parent")
+        s += "switch (%s) {\n" % (vparent + field.type.keyvar.name)
+        for f in [f for f in field.type.fields if not f.const]:
+            (vnparent,vfexpr) = ty.member(v, f, vparent is None)
+            s += "case %s:\n" % f.enumname
+            if f.type is not None:
+                s += libxl_C_type_copy_deprecated(f, vfexpr, indent, vnparent)
+            s+= "    break;\n"
+        s+="}\n";
+    elif isinstance(field.type, idl.Array) and field.deprecated_by:
+        raise Exception("Array type is not supported for deprecation")
+    elif isinstance(field.type, idl.Struct) and field.type.copy_fn is None:
+        for f in [f for f in field.type.fields if not f.const]:
+            (vnparent,vfexpr) = ty.member(v, f, vparent is None)
+            s += libxl_C_type_copy_deprecated(f, vfexpr, "", vnparent)
+    elif field.deprecated_by is not None:
+        if field.type.check_default_fn is None:
+            raise Exception("Deprecated field %s type doesn't have a default value checker" % field.name)
+        field_val = field.type.pass_arg(v, vparent is None,
+                                        passby=idl.PASS_BY_VALUE)
+        field_ptr = field.type.pass_arg(v, vparent is None,
+                                        passby=idl.PASS_BY_REFERENCE)
+
+        s+= "if (%s(&p->%s))\n" % (field.type.check_default_fn,
+                                   field.deprecated_by)
+        s+= "    "
+        if field.type.copy_fn is not None:
+            s += "%s(ctx, &p->%s, %s);\n" % (field.type.copy_fn,
+                                             field.deprecated_by, field_ptr)
+        else:
+            s += "p->%s = %s;\n" % (field.deprecated_by, field_val)
+
+    if s != "":
+        s = indent + s
+    return s.replace("\n", "\n%s" % indent).rstrip(indent)
+
 def get_init_val(f):
     if f.init_val is not None:
         return f.init_val
@@ -543,6 +584,10 @@ if __name__ == '__main__':
         f.write(libxl_C_type_define(ty) + ";\n")
         if ty.dispose_fn is not None:
             f.write("%svoid %s(%s);\n" % (ty.hidden(), ty.dispose_fn, ty.make_arg("p")))
+        if ty.copy_deprecated_fn is not None:
+            f.write("%sint %s(libxl_ctx *ctx, %s);\n" % (ty.hidden(),
+                                                         ty.copy_deprecated_fn,
+                                                         ty.make_arg("p")))
         if ty.copy_fn is not None:
             f.write("%svoid %s(libxl_ctx *ctx, %s, const %s);\n" % (ty.hidden(), ty.copy_fn,
                                               ty.make_arg("dst"), ty.make_arg("src")))
@@ -657,6 +702,17 @@ if __name__ == '__main__':
         f.write("}\n")
         f.write("\n")
         
+    for ty in [t for t in types if t.copy_deprecated_fn]:
+        f.write("int %s(libxl_ctx *ctx, %s)\n" % (ty.copy_deprecated_fn,
+            ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
+        f.write("{\n")
+        for field in [field for field in ty.fields if not field.const]:
+            (vnparent,vfexpr) = ty.member("p", field, True)
+            f.write(libxl_C_type_copy_deprecated(field, vfexpr, vparent = vnparent))
+        f.write("    return 0;\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):
diff --git a/tools/libxl/idl.py b/tools/libxl/idl.py
index a4a084e1ce..687cb8c2ec 100644
--- a/tools/libxl/idl.py
+++ b/tools/libxl/idl.py
@@ -72,6 +72,7 @@ class Type(object):
         self.autogenerate_init_fn = kwargs.setdefault('autogenerate_init_fn', False)
 
         self.check_default_fn = kwargs.setdefault('check_default_fn', None)
+        self.copy_deprecated_fn = kwargs.setdefault('copy_deprecated_fn', None)
 
         if self.typename is not None and not self.private:
             self.json_gen_fn = kwargs.setdefault('json_gen_fn', self.typename + "_gen_json")
@@ -193,6 +194,7 @@ class Field(object):
         self.const = kwargs.setdefault('const', False)
         self.enumname = kwargs.setdefault('enumname', None)
         self.init_val = kwargs.setdefault('init_val', None)
+        self.deprecated_by = kwargs.setdefault('deprecated_by', None)
 
 class Aggregate(Type):
     """A type containing a collection of other types"""
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 267c4f3804..fe82d683b4 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -507,11 +507,16 @@ 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),
-                                       ("apic",             libxl_defbool),
+                                       ("apic",             libxl_defbool, {'deprecated_by': 'apic'}),
                                        # The following acpi field is deprecated.
                                        # Please use the unified acpi field above
                                        # which works for both x86 and ARM.
@@ -527,8 +532,8 @@ libxl_domain_build_info = Struct("domain_build_info",[
                                        ("hpet",             libxl_defbool),
                                        ("vpt_align",        libxl_defbool),
                                        ("mmio_hole_memkb",  MemKB),
-                                       ("timer_mode",       libxl_timer_mode),
-                                       ("nested_hvm",       libxl_defbool),
+                                       ("timer_mode",       libxl_timer_mode, {'deprecated_by': 'timer_mode'}),
+                                       ("nested_hvm",       libxl_defbool, {'deprecated_by': 'nested_hvm'}),
                                        # The u.hvm.altp2m field is used solely
                                        # for x86 HVM guests and is maintained
                                        # for legacy purposes.
@@ -569,8 +574,8 @@ libxl_domain_build_info = Struct("domain_build_info",[
                                        ])),
                  ("pv", Struct(None, [("kernel", string),
                                       ("slack_memkb", MemKB),
-                                      ("bootloader", string),
-                                      ("bootloader_args", libxl_string_list),
+                                      ("bootloader", string, {'deprecated_by': 'bootloader'}),
+                                      ("bootloader_args", libxl_string_list, {'deprecated_by': 'bootloader_args'}),
                                       ("cmdline", string),
                                       ("ramdisk", string),
                                       ("features", string, {'const': True}),
@@ -587,7 +592,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
     # supported by x86 HVM and ARM support is planned.
     ("altp2m", libxl_altp2m_mode),
 
-    ], dir=DIR_IN
+    ], dir=DIR_IN, copy_deprecated_fn="libxl__domain_build_info_copy_deprecated"
 )
 
 libxl_device_vfb = Struct("device_vfb", [
-- 
2.11.0 (Apple Git-81)


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

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

* [PATCH v2 03/21] libxl/xl: use the new domain_build_info fields position
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 01/21] libxl: add is_default checkers for string and timer_mode types Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 02/21] libxl: introduce a way to mark fields as deprecated in the idl Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-20 14:48   ` Ian Jackson
  2017-09-07 10:16 ` [PATCH v2 04/21] xl: introduce a domain type option Roger Pau Monne
                   ` (17 subsequent siblings)
  20 siblings, 1 reply; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

This is required because those options will be used by the new PVH
guest type, and thus need to be shared between PV and HVM.

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     |  21 +++++---
 tools/libxl/libxl_dom.c        |   8 +--
 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 +++---
 8 files changed, 110 insertions(+), 76 deletions(-)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 812b7ea95d..e441479780 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -311,6 +311,22 @@
 #define LIBXL_HAVE_P9S 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 9123585b52..93682a960f 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -62,7 +62,7 @@ void libxl__rdm_setdefault(libxl__gc *gc, libxl_domain_build_info *b_info)
 int libxl__domain_build_info_setdefault(libxl__gc *gc,
                                         libxl_domain_build_info *b_info)
 {
-    int i;
+    int i, rc;
 
     if (b_info->type != LIBXL_DOMAIN_TYPE_HVM &&
         b_info->type != LIBXL_DOMAIN_TYPE_PV) {
@@ -70,6 +70,13 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
         return ERROR_INVAL;
     }
 
+    /* Copy deprecated options to it's new position. */
+    rc = libxl__domain_build_info_copy_deprecated(CTX, b_info);
+    if (rc) {
+        LOG(ERROR, "Unable to copy deprecated fields");
+        return rc;
+    }
+
     libxl_defbool_setdefault(&b_info->device_model_stubdomain, false);
 
     if (libxl_defbool_val(b_info->device_model_stubdomain) &&
@@ -91,7 +98,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
         if (b_info->device_model_version
                 == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
             const char *dm;
-            int rc;
 
             dm = libxl__domain_device_model(gc, b_info);
             rc = access(dm, X_OK);
@@ -304,12 +310,11 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
             break;
         }
 
-        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 (libxl__timer_mode_is_default(b_info->timer_mode))
+            b_info->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->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);
@@ -318,7 +323,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
         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->nested_hvm,               false);
         libxl_defbool_setdefault(&b_info->u.hvm.altp2m,             false);
         libxl_defbool_setdefault(&b_info->u.hvm.usb,                false);
         libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci,   true);
@@ -903,7 +908,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_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 176175676f..9350402308 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 02ddd2e90d..61f9a38573 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

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

* [PATCH v2 04/21] xl: introduce a domain type option
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
                   ` (2 preceding siblings ...)
  2017-09-07 10:16 ` [PATCH v2 03/21] libxl/xl: use the new domain_build_info fields position Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-20 14:50   ` Ian Jackson
  2017-09-07 10:16 ` [PATCH v2 05/21] xl: introduce a firmware option Roger Pau Monne
                   ` (16 subsequent siblings)
  20 siblings, 1 reply; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

Introduce a new type option to xl configuration files in order to
specify the domain type. This supersedes the current builder option.

The new option is documented in the xl.cfg man page, and the previous
builder option is marked as deprecated.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 docs/man/xl.cfg.pod.5.in | 22 ++++++++++++++++++++--
 tools/xl/xl_parse.c      | 20 +++++++++++++++++---
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
index 79cb2eaea7..ab53436da2 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -54,9 +54,9 @@ Pairs may be separated either by a newline or a semicolon.  Both
 of the following are valid:
 
   name="h0"
-  builder="hvm"
+  type="hvm"
 
-  name="h0"; builder="hvm"
+  name="h0"; type="hvm"
 
 =head1 OPTIONS
 
@@ -77,6 +77,24 @@ single host must be unique.
 
 =over 4
 
+=item B<type="pv">
+
+Specifies that this is to be a PV domain, suitable for hosting Xen-aware guest
+operating systems. This is the default.
+
+=item B<type="hvm">
+
+Specifies that this is to be an HVM domain. That is, a fully virtualised
+computer with emulated BIOS, disk and network peripherals, etc.
+
+=back
+
+=head3 Deprecated guest type selection
+
+Note that the builder option is being deprecated in favor of the type option.
+
+=over 4
+
 =item B<builder="generic">
 
 Specifies that this is to be a PV domain, suitable for hosting Xen-aware guest
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 61f9a38573..65297352bd 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -854,9 +854,23 @@ void parse_config_data(const char *config_source,
 
     libxl_defbool_set(&c_info->run_hotplug_scripts, run_hotplug_scripts);
     c_info->type = LIBXL_DOMAIN_TYPE_PV;
-    if (!xlu_cfg_get_string (config, "builder", &buf, 0) &&
-        !strncmp(buf, "hvm", strlen(buf)))
-        c_info->type = LIBXL_DOMAIN_TYPE_HVM;
+    if (!xlu_cfg_get_string (config, "builder", &buf, 0)) {
+        fprintf(stderr,
+                "The builder option is being deprecated, please use type instead.\n");
+        if (!strncmp(buf, "hvm", strlen(buf)))
+            c_info->type = LIBXL_DOMAIN_TYPE_HVM;
+    }
+
+    if (!xlu_cfg_get_string (config, "type", &buf, 0)) {
+        if (!strncmp(buf, "hvm", strlen(buf))) {
+            c_info->type = LIBXL_DOMAIN_TYPE_HVM;
+        } else if (!strncmp(buf, "pv", strlen(buf))) {
+            c_info->type = LIBXL_DOMAIN_TYPE_PV;
+        } else {
+            fprintf(stderr, "Invalid domain type %s.\n", buf);
+            exit(1);
+        }
+    }
 
     xlu_cfg_get_defbool(config, "hap", &c_info->hap, 0);
 
-- 
2.11.0 (Apple Git-81)


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

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

* [PATCH v2 05/21] xl: introduce a firmware option
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
                   ` (3 preceding siblings ...)
  2017-09-07 10:16 ` [PATCH v2 04/21] xl: introduce a domain type option Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-20 14:53   ` Ian Jackson
  2017-09-07 10:16 ` [PATCH v2 06/21] libxl: introduce a PVH guest type Roger Pau Monne
                   ` (15 subsequent siblings)
  20 siblings, 1 reply; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

The new firmware option aims to provide a coherent way to set the
firmware for the different kind of guests Xen supports.

For PV guests the available firmwares are pvgrub{32|64}, and for HVM
the following are supported: bios, uefi, seabios, rombios and ovmf.
Note that uefi maps to ovmf, and bios maps to the default firmware for
each device model.

The xl.cfg man page is updated to document the new feature.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 docs/man/xl.cfg.pod.5.in | 56 ++++++++++++++++++++++++++++++++++++++++++++++++
 tools/xl/xl_parse.c      | 43 +++++++++++++++++++++++++++++++++++++
 2 files changed, 99 insertions(+)

diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
index ab53436da2..a0f3c8ab2a 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -440,6 +440,62 @@ is guest specific).
 
 =back
 
+=head3 Non direct Kernel Boot
+
+Non direct kernel boot allows booting guests with a firmware. This can be used
+by all types of guests, although the selection of options is different
+depending on the guest type.
+
+This option provides the flexibly of letting the guest decide which kernel they
+want to boot, while preventing having to poke at the guest file system form the
+toolstack domain.
+
+=head4 PV guest options
+
+=over 4
+
+=item B<firmware="pvgrub32|pvgrub64">
+
+Boots a guest using a para-virtualized version of grub that runs inside of the
+guest. The bitness of the guest needs to be know, so that the right version of
+pvgrub can be selected.
+
+Note that xl expects to find the pvgrub32.bin and pvgrub64.bin binaries in
+F<@XENFIRMWAREDIR@>.
+
+=back
+
+=head4 HVM guest options
+
+=over 4
+
+=item B<firmware="bios">
+
+Boot the guest using the default BIOS firmware, which depends on the chosen
+device model.
+
+=item B<firmware="uefi">
+
+Boot the guest using the default UEFI firmware, currently OVMF.
+
+=item B<firmware="seabios">
+
+Boot the guest using the SeaBIOS BIOS firmware.
+
+=item B<firmware="rombios">
+
+Boot the guest using the ROMBIOS BIOS firmware.
+
+=item B<firmware="ovmf">
+
+Boot the guest using the OVMF UEFI firmware.
+
+=item B<firmware="PATH">
+
+Load the specified file as firmware for the guest.
+
+=back
+
 =head3 Other Options
 
 =over 4
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 65297352bd..00c939b423 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -1258,9 +1258,52 @@ void parse_config_data(const char *config_source,
             exit(-ERROR_FAIL);
         }
 
+        /*
+         * The firmware config option can be used as a simplification instead
+         * of setting bios or firmware_override. It has the following meanings
+         * for HVM guests:
+         *
+         *  - ovmf | seabios | rombios: maps directly into the "bios" option.
+         *  - uefi | bios: maps into one of the above options and is set in the
+         *    bios field.
+         *  - Anything else is treated as a path that is copied into firmware.
+         */
+        if (!xlu_cfg_get_string (config, "firmware", &buf, 0) &&
+            libxl_bios_type_from_string(buf, &b_info->u.hvm.bios)) {
+            if (!strncmp(buf, "uefi", strlen(buf)))
+                b_info->u.hvm.bios = LIBXL_BIOS_TYPE_OVMF;
+            else if (strncmp(buf, "bios", strlen(buf)))
+                /* Assume it's a path to a custom firmware. */
+                xlu_cfg_replace_string(config, "firmware",
+                                       &b_info->u.hvm.firmware, 0);
+            /*
+             * BIOS is the default, and will be chosen by libxl based on the
+             * device model specified.
+             */
+        }
+
         break;
     case LIBXL_DOMAIN_TYPE_PV:
     {
+        /*
+         * The firmware config option can be used as a simplification instead
+         * of directly setting kernel. It will be translated to
+         * XENFIRMWAREDIR/<string>.bin
+         */
+        if (!xlu_cfg_get_string (config, "firmware", &buf, 0)) {
+            if (b_info->kernel) {
+                fprintf(stderr, "ERROR: both kernel and firmware specified\n");
+                exit(1);
+            }
+            if (strncmp(buf, "pvgrub32", strlen(buf)) &&
+                strncmp(buf, "pvgrub64", strlen(buf))) {
+                fprintf(stderr,
+                        "ERROR: only pvgrub{32|64} supported as firmware options\n");
+                exit(1);
+            }
+
+            xasprintf(&b_info->kernel, XENFIRMWAREDIR "/%s.bin", buf);
+        }
         if (!b_info->bootloader && !b_info->kernel) {
             fprintf(stderr, "Neither kernel nor bootloader specified\n");
             exit(1);
-- 
2.11.0 (Apple Git-81)


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

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

* [PATCH v2 06/21] libxl: introduce a PVH guest type
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
                   ` (4 preceding siblings ...)
  2017-09-07 10:16 ` [PATCH v2 05/21] xl: introduce a firmware option Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 07/21] libxl: allow PVH guests to use a bootloader Roger Pau Monne
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

The new guest type is introduced to the libxl IDL. libxl__domain_make
is also modified to save the guest type, and libxl__domain_type is
expanded to fetch that information when detecting guest type.

This is required because the hypervisor only differentiates between PV
and HVM guests, so libxl needs some extra information in order to
differentiate between a HVM and a PVH guest.

The new PVH guest type and its options are documented on the xl.cfg
man page.

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>
---
Changes since v1:
 - it's -> its in the commit message.
 - Fix formatting of the nestedhvm man page option.
---
 docs/man/xl.cfg.pod.5.in    | 135 +++++++++++++++++++++++++++++++++++++++++---
 tools/libxl/libxl_create.c  |  21 +++++--
 tools/libxl/libxl_dom.c     |  28 ++++++++-
 tools/libxl/libxl_types.idl |   2 +
 4 files changed, 170 insertions(+), 16 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
index a0f3c8ab2a..c7cb014bcb 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -82,6 +82,12 @@ single host must be unique.
 Specifies that this is to be a PV domain, suitable for hosting Xen-aware guest
 operating systems. This is the default.
 
+=item B<type="pvh">
+
+Specifies that this is to be an PVH domain. That is a lightweight HVM-like
+guest without a device model and without many of the emulated devices available
+to HVM guests. Note that this mode requires a PVH aware kernel.
+
 =item B<type="hvm">
 
 Specifies that this is to be an HVM domain. That is, a fully virtualised
@@ -496,6 +502,11 @@ Load the specified file as firmware for the guest.
 
 =back
 
+=head4 PVH guest options
+
+Currently there's no firmware available for PVH guests, they should be booted
+using the B<Direct Kernel Boot> method or the B<bootloader> option.
+
 =head3 Other Options
 
 =over 4
@@ -1504,14 +1515,12 @@ x86 systems will continue to work using it.
 
 =item B<nestedhvm=BOOLEAN>
 
-Enable or disables guest access to hardware virtualisation features,
-e.g. it allows a guest Operating System to also function as a
-hypervisor. You may want this
-option if you want to run another hypervisor (including another copy
-of Xen) within a Xen guest or to support a guest Operating System
-which uses hardware virtualisation extensions (e.g. Windows XP
-compatibility mode on more modern Windows OS).
-This option is disabled by default.
+Enable or disables guest access to hardware virtualisation features, e.g. it
+allows a guest Operating System to also function as a hypervisor. You may want
+this option if you want to run another hypervisor (including another copy of
+Xen) within a Xen guest or to support a guest Operating System which uses
+hardware virtualisation extensions (e.g. Windows XP compatibility mode on more
+modern Windows OS). This option is disabled by default.
 
 =item B<cpuid="LIBXL_STRING"> or B<cpuid=[ "XEND_STRING", "XEND_STRING" ]>
 
@@ -2121,6 +2130,116 @@ See B<xen-pci-device-reservations(7)> for more information.
 
 =back
 
+=head2 PVH Guest Specific Options
+
+=over 4
+
+=item B<nestedhvm=BOOLEAN>
+
+Enable or disables guest access to hardware virtualisation features,
+e.g. it allows a guest Operating System to also function as a
+hypervisor. You may want this
+option if you want to run another hypervisor (including another copy
+of Xen) within a Xen guest or to support a guest Operating System
+which uses hardware virtualisation extensions (e.g. Windows XP
+compatibility mode on more modern Windows OS).
+This option is disabled by default.
+
+=item B<apic=BOOLEAN>
+
+Enable the local APIC emulation for the guest. The local APIC information will
+be exposed to the guest in the ACPI tables. This option is enabled by
+default.
+
+=item B<bootloader="PROGRAM">
+
+Run C<PROGRAM> to find the kernel image and ramdisk to use.  Normally
+C<PROGRAM> would be C<pygrub>, which is an emulation of
+grub/grub2/syslinux. Either B<kernel> or B<bootloader> must be specified
+for PV guests.
+
+=item B<bootloader_args=[ "ARG", "ARG", ...]>
+
+Append B<ARG>s to the arguments to the B<bootloader>
+program. Alternatively if the argument is a simple string then it will
+be split into words at whitespace B<(this second option is deprecated)>.
+
+=item B<timer_mode="MODE">
+
+Specifies the mode for Virtual Timers. The valid values are as follows:
+
+=over 4
+
+=item B<delay_for_missed_ticks>
+
+Delay for missed ticks. Do not advance a vCPU's time beyond the
+correct delivery time for interrupts that have been missed due to
+preemption. Deliver missed interrupts when the vCPU is rescheduled and
+advance the vCPU's virtual time stepwise for each one.
+
+=item B<no_delay_for_missed_ticks>
+
+No delay for missed ticks. As above, missed interrupts are delivered,
+but guest time always tracks wallclock (i.e., real) time while doing
+so.
+
+=item B<no_missed_ticks_pending>
+
+No missed interrupts are held pending. Instead, to ensure ticks are
+delivered at some non-zero rate, if we detect missed ticks then the
+internal tick alarm is not disabled if the vCPU is preempted during
+the next tick period.
+
+=item B<one_missed_tick_pending>
+
+One missed tick pending. Missed interrupts are collapsed
+together and delivered as one 'late tick'.  Guest time always tracks
+wallclock (i.e., real) time.
+
+=back
+
+=back
+
+=head3 Paging
+
+The following options control the mechanisms used to virtualise guest
+memory.  The defaults are selected to give the best results for the
+common cases so you should normally leave these options
+unspecified.
+
+=over 4
+
+=item B<hap=BOOLEAN>
+
+Turns "hardware assisted paging" (the use of the hardware nested page
+table feature) on or off.  This feature is called EPT (Extended Page
+Tables) by Intel and NPT (Nested Page Tables) or RVI (Rapid
+Virtualisation Indexing) by AMD. If turned
+off, Xen will run the guest in "shadow page table" mode where the
+guest's page table updates and/or TLB flushes etc. will be emulated.
+Use of HAP is the default when available.
+
+=item B<oos=BOOLEAN>
+
+Turns "out of sync pagetables" on or off.  When running in shadow page
+table mode, the guest's page table updates may be deferred as
+specified in the Intel/AMD architecture manuals.  However, this may
+expose unexpected bugs in the guest, or find bugs in Xen, so it is
+possible to disable this feature.  Use of out of sync page tables,
+when Xen thinks it appropriate, is the default.
+
+=item B<shadow_memory=MBYTES>
+
+Number of megabytes to set aside for shadowing guest pagetable pages
+(effectively acting as a cache of translated pages) or to use for HAP
+state. By default this is 1MB per guest vCPU plus 8KB per MB of guest
+RAM. You should not normally need to adjust this value. However, if you
+are not using hardware assisted paging (i.e. you are using shadow
+mode) and your guest workload consists of a very large number of
+similar processes then increasing this value may improve performance.
+
+=back
+
 =head2 Device-Model Options
 
 The following options control the selection of the device-model.  This
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 93682a960f..421e41b5fd 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -310,11 +310,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
             break;
         }
 
-        if (libxl__timer_mode_is_default(b_info->timer_mode))
-            b_info->timer_mode = LIBXL_TIMER_MODE_NO_DELAY_FOR_MISSED_TICKS;
-
         libxl_defbool_setdefault(&b_info->u.hvm.pae,                true);
-        libxl_defbool_setdefault(&b_info->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);
@@ -323,7 +319,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
         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->nested_hvm,               false);
         libxl_defbool_setdefault(&b_info->u.hvm.altp2m,             false);
         libxl_defbool_setdefault(&b_info->u.hvm.usb,                false);
         libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci,   true);
@@ -411,6 +406,16 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
             libxl_domain_type_to_string(b_info->type));
         return ERROR_INVAL;
     }
+
+    /* Configuration fields shared between PVH and HVM. */
+    if (b_info->type != LIBXL_DOMAIN_TYPE_PV) {
+        if (libxl__timer_mode_is_default(&b_info->timer_mode))
+            b_info->timer_mode = LIBXL_TIMER_MODE_NO_DELAY_FOR_MISSED_TICKS;
+
+        libxl_defbool_setdefault(&b_info->apic,                     true);
+        libxl_defbool_setdefault(&b_info->nested_hvm,               false);
+    }
+
     return 0;
 }
 
@@ -518,6 +523,7 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     int flags, ret, rc, nb_vm;
+    const char *dom_type;
     char *uuid_string;
     char *dom_path, *vm_path, *libxl_path;
     struct xs_permissions roperm[2];
@@ -708,6 +714,11 @@ retry_transaction:
 
     xs_write(ctx->xsh, t, GCSPRINTF("%s/control/platform-feature-multiprocessor-suspend", dom_path), "1", 1);
     xs_write(ctx->xsh, t, GCSPRINTF("%s/control/platform-feature-xs_reset_watches", dom_path), "1", 1);
+
+    dom_type = libxl_domain_type_to_string(info->type);
+    xs_write(ctx->xsh, t, GCSPRINTF("%s/type", libxl_path), dom_type,
+             strlen(dom_type));
+
     if (!xs_transaction_end(ctx->xsh, t, 0)) {
         if (errno == EAGAIN) {
             t = 0;
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 33213db388..5ef35e66bf 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -38,9 +38,31 @@ libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid)
         LOG(ERROR, "unable to get domain type for domid=%"PRIu32, domid);
         return LIBXL_DOMAIN_TYPE_INVALID;
     }
-    if (info.flags & XEN_DOMINF_hvm_guest)
-        return LIBXL_DOMAIN_TYPE_HVM;
-    else
+    if (info.flags & XEN_DOMINF_hvm_guest) {
+        const char *type_path = GCSPRINTF("%s/type",
+                                          libxl__xs_libxl_path(gc, domid));
+        const char *type;
+        libxl_domain_type t;
+        int rc;
+
+        rc = libxl__xs_read_mandatory(gc, XBT_NULL, type_path, &type);
+        if (rc) {
+            LOG(WARN,
+                "unable to get domain type for domid=%"PRIu32", assuming HVM",
+                domid);
+            return LIBXL_DOMAIN_TYPE_HVM;
+        }
+
+        rc = libxl_domain_type_from_string(type, &t);
+        if (rc) {
+            LOG(WARN,
+                "unable to get domain type for domid=%"PRIu32", assuming HVM",
+                domid);
+            return LIBXL_DOMAIN_TYPE_HVM;
+        }
+
+        return t;
+    } else
         return LIBXL_DOMAIN_TYPE_PV;
 }
 
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index fe82d683b4..18aa05b589 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -75,6 +75,7 @@ libxl_domain_type = Enumeration("domain_type", [
     (-1, "INVALID"),
     (1, "HVM"),
     (2, "PV"),
+    (3, "PVH"),
     ], init_val = "LIBXL_DOMAIN_TYPE_INVALID")
 
 libxl_rdm_reserve_strategy = Enumeration("rdm_reserve_strategy", [
@@ -582,6 +583,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
                                       # Use host's E820 for PCI passthrough.
                                       ("e820_host", libxl_defbool),
                                       ])),
+                 ("pvh", None),
                  ("invalid", None),
                  ], keyvar_init_val = "LIBXL_DOMAIN_TYPE_INVALID")),
 
-- 
2.11.0 (Apple Git-81)


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

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

* [PATCH v2 07/21] libxl: allow PVH guests to use a bootloader
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
                   ` (5 preceding siblings ...)
  2017-09-07 10:16 ` [PATCH v2 06/21] libxl: introduce a PVH guest type Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 08/21] libxl: set PVH guests to use the PV console Roger Pau Monne
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

Allow PVH guests to boot using a bootloader.

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_bootloader.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index a47bd8c25c..18e9ebd714 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -324,8 +324,8 @@ void libxl__bootloader_run(libxl__egc *egc, libxl__bootloader_state *bl)
 
     libxl__bootloader_init(bl);
 
-    if (info->type != LIBXL_DOMAIN_TYPE_PV) {
-        LOGD(DEBUG, domid, "not a PV domain, skipping bootloader");
+    if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
+        LOGD(DEBUG, domid, "not a PV/PVH domain, skipping bootloader");
         rc = 0;
         goto out_ok;
     }
-- 
2.11.0 (Apple Git-81)


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

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

* [PATCH v2 08/21] libxl: set PVH guests to use the PV console
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
                   ` (6 preceding siblings ...)
  2017-09-07 10:16 ` [PATCH v2 07/21] libxl: allow PVH guests to use a bootloader Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 09/21] libxl: add PVH support to domain creation Roger Pau Monne
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

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_console.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c
index 446e766911..5029365d2e 100644
--- a/tools/libxl/libxl_console.c
+++ b/tools/libxl/libxl_console.c
@@ -133,6 +133,7 @@ static int libxl__primary_console_find(libxl_ctx *ctx, uint32_t domid_vm,
             *cons_num = 0;
             *type = LIBXL_CONSOLE_TYPE_SERIAL;
             break;
+        case LIBXL_DOMAIN_TYPE_PVH:
         case LIBXL_DOMAIN_TYPE_PV:
             *domid = domid_vm;
             *cons_num = 0;
-- 
2.11.0 (Apple Git-81)


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

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

* [PATCH v2 09/21] libxl: add PVH support to domain creation
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
                   ` (7 preceding siblings ...)
  2017-09-07 10:16 ` [PATCH v2 08/21] libxl: set PVH guests to use the PV console Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 10/21] libxl: remove device model "none" support from disk related functions Roger Pau Monne
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

Remove the device model "none" support from domain creation and
introduce support for PVH.

This requires changing some of the HVM checks to be applied for both
HVM and PVH.

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_create.c | 69 ++++++++++++++++++++++++----------------------
 1 file changed, 36 insertions(+), 33 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 421e41b5fd..a158dec825 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -35,7 +35,7 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
         return ERROR_INVAL;
     }
 
-    if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) {
+    if (c_info->type != LIBXL_DOMAIN_TYPE_PV) {
         libxl_defbool_setdefault(&c_info->hap, true);
         libxl_defbool_setdefault(&c_info->oos, true);
     }
@@ -65,7 +65,8 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
     int i, rc;
 
     if (b_info->type != LIBXL_DOMAIN_TYPE_HVM &&
-        b_info->type != LIBXL_DOMAIN_TYPE_PV) {
+        b_info->type != LIBXL_DOMAIN_TYPE_PV &&
+        b_info->type != LIBXL_DOMAIN_TYPE_PVH) {
         LOG(ERROR, "invalid domain type");
         return ERROR_INVAL;
     }
@@ -126,8 +127,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
                 b_info->u.hvm.bios = LIBXL_BIOS_TYPE_ROMBIOS; break;
             case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
                 b_info->u.hvm.bios = LIBXL_BIOS_TYPE_SEABIOS; break;
-            case LIBXL_DEVICE_MODEL_VERSION_NONE:
-                break;
             default:
                 LOG(ERROR, "unknown device model version");
                 return ERROR_INVAL;
@@ -147,8 +146,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
                 return ERROR_INVAL;
             }
             break;
-        case LIBXL_DEVICE_MODEL_VERSION_NONE:
-            break;
         default:abort();
         }
 
@@ -228,10 +225,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
             b_info->u.hvm.mmio_hole_memkb = 0;
 
         if (b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_UNKNOWN) {
-            if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_NONE)
-                b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_NONE;
-            else
-                b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+            b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
         }
 
         if (!b_info->u.hvm.hdtype)
@@ -265,12 +259,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
                 break;
             }
             break;
-        case LIBXL_DEVICE_MODEL_VERSION_NONE:
-            if (b_info->u.hvm.vga.kind != LIBXL_VGA_INTERFACE_TYPE_NONE) {
-                LOG(ERROR,
-        "guests without a device model cannot have an emulated video card");
-                return ERROR_INVAL;
-            }
             b_info->video_memkb = 0;
             break;
         case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
@@ -401,6 +389,8 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
             b_info->u.pv.cmdline = NULL;
         }
         break;
+    case LIBXL_DOMAIN_TYPE_PVH:
+        break;
     default:
         LOG(ERROR, "invalid domain type %s in create info",
             libxl_domain_type_to_string(b_info->type));
@@ -509,6 +499,17 @@ int libxl__domain_build(libxl__gc *gc,
         }
 
         break;
+    case LIBXL_DOMAIN_TYPE_PVH:
+        ret = libxl__build_hvm(gc, domid, d_config, state);
+        if (ret)
+            goto out;
+
+        vments = libxl__calloc(gc, 3, sizeof(char *));
+        vments[0] = "start_time";
+        vments[1] = GCSPRINTF("%lu.%02d", start_time.tv_sec,
+                              (int)start_time.tv_usec/10000);
+
+        break;
     default:
         ret = ERROR_INVAL;
         goto out;
@@ -543,7 +544,7 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
     }
 
     flags = 0;
-    if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
+    if (info->type != LIBXL_DOMAIN_TYPE_PV) {
         flags |= XEN_DOMCTL_CDF_hvm_guest;
         flags |= libxl_defbool_val(info->hap) ? XEN_DOMCTL_CDF_hap : 0;
         flags |= libxl_defbool_val(info->oos) ? 0 : XEN_DOMCTL_CDF_oos_off;
@@ -870,7 +871,7 @@ static void initiate_domain_create(libxl__egc *egc,
     /* If target_memkb is smaller than max_memkb, the subsequent call
      * to libxc when building HVM domain will enable PoD mode.
      */
-    pod_enabled = (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM) &&
+    pod_enabled = (d_config->c_info.type != LIBXL_DOMAIN_TYPE_PV) &&
         (d_config->b_info.target_memkb < d_config->b_info.max_memkb);
 
     /* We cannot have PoD and PCI device assignment at the same time
@@ -879,7 +880,7 @@ static void initiate_domain_create(libxl__egc *egc,
      * guest. To stay on the safe side, we disable PCI device
      * assignment when PoD is enabled.
      */
-    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM &&
+    if (d_config->c_info.type != LIBXL_DOMAIN_TYPE_PV &&
         d_config->num_pcidevs && pod_enabled) {
         ret = ERROR_INVAL;
         LOGD(ERROR, domid,
@@ -918,18 +919,20 @@ static void initiate_domain_create(libxl__egc *egc,
         goto error_out;
     }
 
-    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM &&
+    if (d_config->c_info.type != LIBXL_DOMAIN_TYPE_PV &&
         (libxl_defbool_val(d_config->b_info.nested_hvm) &&
-        (libxl_defbool_val(d_config->b_info.u.hvm.altp2m) ||
+        ((d_config->c_info.type != LIBXL_DOMAIN_TYPE_HVM &&
+          libxl_defbool_val(d_config->b_info.u.hvm.altp2m)) ||
         (d_config->b_info.altp2m != LIBXL_ALTP2M_MODE_DISABLED)))) {
         ret = ERROR_INVAL;
         LOGD(ERROR, domid, "nestedhvm and altp2mhvm cannot be used together");
         goto error_out;
     }
 
-    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM &&
-        (libxl_defbool_val(d_config->b_info.u.hvm.altp2m) ||
-        (d_config->b_info.altp2m != LIBXL_ALTP2M_MODE_DISABLED)) &&
+    if (((d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM &&
+         libxl_defbool_val(d_config->b_info.u.hvm.altp2m)) ||
+        (d_config->c_info.type != LIBXL_DOMAIN_TYPE_PV &&
+         d_config->b_info.altp2m != LIBXL_ALTP2M_MODE_DISABLED)) &&
         pod_enabled) {
         ret = ERROR_INVAL;
         LOGD(ERROR, domid, "Cannot enable PoD and ALTP2M at the same time");
@@ -1112,7 +1115,7 @@ static void domcreate_bootloader_done(libxl__egc *egc,
             crs->domid = domid;
             crs->send_back_fd = dcs->send_back_fd;
             crs->recv_fd = restore_fd;
-            crs->hvm = (info->type == LIBXL_DOMAIN_TYPE_HVM);
+            crs->hvm = (info->type != LIBXL_DOMAIN_TYPE_PV);
             crs->callback = libxl__colo_restore_setup_done;
             libxl__colo_restore_setup(egc, crs);
             break;
@@ -1193,6 +1196,12 @@ static void domcreate_stream_done(libxl__egc *egc,
             vments[i++] = (char *) state->pv_cmdline;
         }
         break;
+    case LIBXL_DOMAIN_TYPE_PVH:
+        vments = libxl__calloc(gc, 3, sizeof(char *));
+        vments[0] = "start_time";
+        vments[1] = GCSPRINTF("%lu.%02d", start_time.tv_sec,
+                              (int)start_time.tv_usec/10000);
+        break;
     default:
         ret = ERROR_INVAL;
         goto out;
@@ -1357,12 +1366,6 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
         libxl__device_console_add(gc, domid, &console, state, &device);
         libxl__device_console_dispose(&console);
 
-        if (d_config->b_info.device_model_version ==
-            LIBXL_DEVICE_MODEL_VERSION_NONE) {
-            domcreate_devmodel_started(egc, &dcs->sdss.dm, 0);
-            return;
-        }
-
         libxl_device_vkb_init(&vkb);
         libxl__device_vkb_add(gc, domid, &vkb);
         libxl_device_vkb_dispose(&vkb);
@@ -1384,6 +1387,7 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
         return;
     }
     case LIBXL_DOMAIN_TYPE_PV:
+    case LIBXL_DOMAIN_TYPE_PVH:
     {
         libxl__device_console console;
         libxl__device device;
@@ -1690,8 +1694,7 @@ static void domain_soft_reset_cb(libxl__egc *egc,
         goto error;
     }
 
-    if (cdcs->dcs.guest_config->b_info.device_model_version !=
-        LIBXL_DEVICE_MODEL_VERSION_NONE) {
+    if (cdcs->dcs.guest_config->b_info.type == LIBXL_DOMAIN_TYPE_HVM) {
         savefile = GCSPRINTF(LIBXL_DEVICE_MODEL_SAVE_FILE".%d", dds->domid);
         restorefile = GCSPRINTF(LIBXL_DEVICE_MODEL_RESTORE_FILE".%d",
                                 dds->domid);
-- 
2.11.0 (Apple Git-81)


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

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

* [PATCH v2 10/21] libxl: remove device model "none" support from disk related functions
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
                   ` (8 preceding siblings ...)
  2017-09-07 10:16 ` [PATCH v2 09/21] libxl: add PVH support to domain creation Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 11/21] libxl: set device model for PVH guests Roger Pau Monne
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

CD-ROM backend selection was partially based on the device model, this
is no longer needed since the device model "none" is now removed, so
HVM guests always have a device model.

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_disk.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/tools/libxl/libxl_disk.c b/tools/libxl/libxl_disk.c
index 63de75c8fe..f6c61fdc09 100644
--- a/tools/libxl/libxl_disk.c
+++ b/tools/libxl/libxl_disk.c
@@ -166,9 +166,7 @@ int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk,
 
     /* Force Qdisk backend for CDROM devices of guests with a device model. */
     if (disk->is_cdrom != 0 &&
-        libxl__domain_type(gc, domid) == LIBXL_DOMAIN_TYPE_HVM &&
-        libxl__device_model_version_running(gc, domid) !=
-        LIBXL_DEVICE_MODEL_VERSION_NONE) {
+        libxl__domain_type(gc, domid) == LIBXL_DOMAIN_TYPE_HVM) {
         if (!(disk->backend == LIBXL_DISK_BACKEND_QDISK ||
               disk->backend == LIBXL_DISK_BACKEND_UNKNOWN)) {
             LOGD(ERROR, domid, "Backend for CD devices on HVM guests must be Qdisk");
@@ -777,12 +775,6 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
         goto out;
     }
 
-    if (dm_ver == LIBXL_DEVICE_MODEL_VERSION_NONE) {
-        LOGD(ERROR, domid, "Guests without a device model cannot use cd-insert");
-        rc = ERROR_FAIL;
-        goto out;
-    }
-
     disks = libxl_device_disk_list(ctx, domid, &num);
     for (i = 0; i < num; i++) {
         if (disks[i].is_cdrom && !strcmp(disk->vdev, disks[i].vdev))
-- 
2.11.0 (Apple Git-81)


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

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

* [PATCH v2 11/21] libxl: set device model for PVH guests
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
                   ` (9 preceding siblings ...)
  2017-09-07 10:16 ` [PATCH v2 10/21] libxl: remove device model "none" support from disk related functions Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 12/21] libxl: add PVH support to domain building Roger Pau Monne
                   ` (9 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

PVH guests use the same device model selection as PV guests, because
PVH guests only use the device model for the PV backends.

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_dm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index e0e6a99e67..c35cdcfef1 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -648,6 +648,7 @@ static int libxl__build_device_model_args_old(libxl__gc *gc,
         flexarray_append(dm_args, b_info->extra[i]);
     flexarray_append(dm_args, "-M");
     switch (b_info->type) {
+    case LIBXL_DOMAIN_TYPE_PVH:
     case LIBXL_DOMAIN_TYPE_PV:
         flexarray_append(dm_args, "xenpv");
         for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
@@ -1407,6 +1408,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
 
     flexarray_append(dm_args, "-machine");
     switch (b_info->type) {
+    case LIBXL_DOMAIN_TYPE_PVH:
     case LIBXL_DOMAIN_TYPE_PV:
         flexarray_append(dm_args, "xenpv");
         for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
-- 
2.11.0 (Apple Git-81)


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

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

* [PATCH v2 12/21] libxl: add PVH support to domain building
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
                   ` (10 preceding siblings ...)
  2017-09-07 10:16 ` [PATCH v2 11/21] libxl: set device model for PVH guests Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 13/21] libxl: add PVH support to domain save/suspend Roger Pau Monne
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

And remove device model "none" support.

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_dom.c | 148 ++++++++++++++++++++++++++++++------------------
 1 file changed, 94 insertions(+), 54 deletions(-)

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 5ef35e66bf..72ca5e266c 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -317,17 +317,31 @@ static int hvm_set_mca_capabilities(libxl__gc *gc, uint32_t domid,
 static void hvm_set_conf_params(xc_interface *handle, uint32_t domid,
                                 libxl_domain_build_info *const info)
 {
-    xc_hvm_param_set(handle, domid, HVM_PARAM_PAE_ENABLED,
-                    libxl_defbool_val(info->u.hvm.pae));
+    switch(info->type) {
+    case LIBXL_DOMAIN_TYPE_PVH:
+        xc_hvm_param_set(handle, domid, HVM_PARAM_PAE_ENABLED, true);
+        xc_hvm_param_set(handle, domid, HVM_PARAM_TIMER_MODE,
+                         timer_mode(info));
+        xc_hvm_param_set(handle, domid, HVM_PARAM_NESTEDHVM,
+                         libxl_defbool_val(info->nested_hvm));
+        break;
+    case LIBXL_DOMAIN_TYPE_HVM:
+        xc_hvm_param_set(handle, domid, HVM_PARAM_PAE_ENABLED,
+                         libxl_defbool_val(info->u.hvm.pae));
 #if defined(__i386__) || defined(__x86_64__)
-    xc_hvm_param_set(handle, domid, HVM_PARAM_HPET_ENABLED,
-                    libxl_defbool_val(info->u.hvm.hpet));
+        xc_hvm_param_set(handle, domid, HVM_PARAM_HPET_ENABLED,
+                         libxl_defbool_val(info->u.hvm.hpet));
 #endif
-    xc_hvm_param_set(handle, domid, HVM_PARAM_TIMER_MODE, timer_mode(info));
-    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->nested_hvm));
+        xc_hvm_param_set(handle, domid, HVM_PARAM_TIMER_MODE,
+                         timer_mode(info));
+        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->nested_hvm));
+        break;
+    default:
+        abort();
+    }
 }
 
 int libxl__build_pre(libxl__gc *gc, uint32_t domid,
@@ -467,9 +481,11 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
     state->store_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->store_domid);
     state->console_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->console_domid);
 
-    if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
+    if (info->type != LIBXL_DOMAIN_TYPE_PV)
         hvm_set_conf_params(ctx->xch, domid, info);
+
 #if defined(__i386__) || defined(__x86_64__)
+    if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
         rc = hvm_set_viridian_features(gc, domid, info);
         if (rc)
             return rc;
@@ -477,10 +493,10 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
         rc = hvm_set_mca_capabilities(gc, domid, info);
         if (rc)
             return rc;
-#endif
     }
+#endif
 
-    /* Alternate p2m support on x86 is available only for HVM guests. */
+    /* Alternate p2m support on x86 is available only for PVH/HVM guests. */
     if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
         /* The config parameter "altp2m" replaces the parameter "altp2mhvm". For
          * legacy reasons, both parameters are accepted on x86 HVM guests.
@@ -494,6 +510,9 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
         else
             xc_hvm_param_set(ctx->xch, domid, HVM_PARAM_ALTP2M,
                              info->altp2m);
+    } else if (info->type == LIBXL_DOMAIN_TYPE_PVH) {
+        xc_hvm_param_set(ctx->xch, domid, HVM_PARAM_ALTP2M,
+                         info->altp2m);
     }
 
     rc = libxl__arch_domain_create(gc, d_config, domid);
@@ -847,7 +866,7 @@ static int hvm_build_set_params(xc_interface *handle, uint32_t domid,
     uint64_t str_mfn, cons_mfn;
     int i;
 
-    if (info->device_model_version != LIBXL_DEVICE_MODEL_VERSION_NONE) {
+    if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
         va_map = xc_map_foreign_range(handle, domid,
                                       XC_PAGE_SIZE, PROT_READ | PROT_WRITE,
                                       HVM_INFO_PFN);
@@ -903,7 +922,7 @@ static int hvm_build_set_xs_values(libxl__gc *gc,
 
     /* Only one module can be passed. PVHv2 guests do not support this. */
     if (dom->acpi_modules[0].guest_addr_out && 
-        info->device_model_version !=LIBXL_DEVICE_MODEL_VERSION_NONE) {
+        info->type == LIBXL_DOMAIN_TYPE_HVM) {
         path = GCSPRINTF("/local/domain/%d/"HVM_XS_ACPI_PT_ADDRESS, domid);
 
         ret = libxl__xs_printf(gc, XBT_NULL, path, "0x%"PRIx64,
@@ -964,6 +983,7 @@ out:
 
 static int libxl__domain_firmware(libxl__gc *gc,
                                   libxl_domain_build_info *info,
+                                  libxl__domain_build_state *state,
                                   struct xc_dom_image *dom)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -973,39 +993,64 @@ static int libxl__domain_firmware(libxl__gc *gc,
     void *data;
     const char *bios_filename = NULL;
 
-    if (info->u.hvm.firmware)
-        firmware = info->u.hvm.firmware;
-    else {
-        switch (info->device_model_version)
-        {
-        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
-            firmware = "hvmloader";
-            break;
-        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
-            firmware = "hvmloader";
-            break;
-        case LIBXL_DEVICE_MODEL_VERSION_NONE:
-            if (info->kernel == NULL) {
-                LOG(ERROR, "no device model requested without a kernel");
+    if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
+        if (info->u.hvm.firmware) {
+            firmware = info->u.hvm.firmware;
+        } else {
+            switch (info->device_model_version)
+            {
+            case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+            case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+                firmware = "hvmloader";
+                break;
+            default:
+                LOG(ERROR, "invalid device model version %d",
+                    info->device_model_version);
                 rc = ERROR_FAIL;
                 goto out;
             }
-            break;
-        default:
-            LOG(ERROR, "invalid device model version %d",
-                info->device_model_version);
-            rc = ERROR_FAIL;
-            goto out;
         }
     }
 
-    if (info->kernel != NULL &&
-        info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_NONE) {
+    if (state->pv_kernel.path != NULL && info->type == LIBXL_DOMAIN_TYPE_PVH) {
         /* Try to load a kernel instead of the firmware. */
-        rc = xc_dom_kernel_file(dom, info->kernel);
-        if (rc == 0 && info->ramdisk != NULL)
-            rc = xc_dom_ramdisk_file(dom, info->ramdisk);
+        if (state->pv_kernel.mapped) {
+            rc = xc_dom_kernel_mem(dom, state->pv_kernel.data,
+                                   state->pv_kernel.size);
+            if (rc) {
+                LOGE(ERROR, "xc_dom_kernel_mem failed");
+                goto out;
+            }
+        } else {
+            rc = xc_dom_kernel_file(dom, state->pv_kernel.path);
+            if (rc) {
+                LOGE(ERROR, "xc_dom_kernel_file failed");
+                goto out;
+            }
+        }
+
+        if (state->pv_ramdisk.path && strlen(state->pv_ramdisk.path)) {
+            if (state->pv_ramdisk.mapped) {
+                rc = xc_dom_ramdisk_mem(dom, state->pv_ramdisk.data,
+                                        state->pv_ramdisk.size);
+                if (rc) {
+                    LOGE(ERROR, "xc_dom_ramdisk_mem failed");
+                    goto out;
+                }
+            } else {
+                rc = xc_dom_ramdisk_file(dom, state->pv_ramdisk.path);
+                if (rc) {
+                    LOGE(ERROR, "xc_dom_ramdisk_file failed");
+                    goto out;
+                }
+            }
+        }
     } else {
+        /*
+         * Only HVM guests should get here, PVH should always have a set
+         * kernel at this point.
+         */
+        assert(info->type == LIBXL_DOMAIN_TYPE_HVM);
         rc = xc_dom_kernel_file(dom, libxl__abs_path(gc, firmware,
                                                  libxl__xenfirmwaredir_path()));
     }
@@ -1015,7 +1060,8 @@ static int libxl__domain_firmware(libxl__gc *gc,
         goto out;
     }
 
-    if (info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
+    if (info->type == LIBXL_DOMAIN_TYPE_HVM &&
+        info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
         if (info->u.hvm.system_firmware) {
             bios_filename = info->u.hvm.system_firmware;
         } else {
@@ -1039,7 +1085,8 @@ static int libxl__domain_firmware(libxl__gc *gc,
         if (rc) goto out;
     }
 
-    if (info->u.hvm.smbios_firmware) {
+    if (info->type == LIBXL_DOMAIN_TYPE_HVM &&
+        info->u.hvm.smbios_firmware) {
         data = NULL;
         e = libxl_read_file_contents(ctx, info->u.hvm.smbios_firmware,
                                      &data, &datalen);
@@ -1057,14 +1104,8 @@ static int libxl__domain_firmware(libxl__gc *gc,
         }
     }
 
-    if (info->u.hvm.acpi_firmware) {
-
-        if (info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_NONE) {
-            LOGE(ERROR, "PVH guests do not allow loading ACPI modules");
-            rc = ERROR_FAIL;
-            goto out;
-        }
-
+    if (info->type == LIBXL_DOMAIN_TYPE_HVM &&
+        info->u.hvm.acpi_firmware) {
         data = NULL;
         e = libxl_read_file_contents(ctx, info->u.hvm.acpi_firmware,
                                      &data, &datalen);
@@ -1097,13 +1138,12 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
     uint64_t mmio_start, lowmem_end, highmem_end, mem_size;
     libxl_domain_build_info *const info = &d_config->b_info;
     struct xc_dom_image *dom = NULL;
-    bool device_model =
-        info->device_model_version != LIBXL_DEVICE_MODEL_VERSION_NONE ?
-        true : false;
+    bool device_model = info->type == LIBXL_DOMAIN_TYPE_HVM ? true : false;
 
     xc_dom_loginit(ctx->xch);
 
-    dom = xc_dom_allocate(ctx->xch, info->cmdline, NULL);
+    dom = xc_dom_allocate(ctx->xch, info->type == LIBXL_DOMAIN_TYPE_PVH ?
+                          state->pv_cmdline : info->cmdline, NULL);
     if (!dom) {
         LOGE(ERROR, "xc_dom_allocate failed");
         rc = ERROR_NOMEM;
@@ -1128,7 +1168,7 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
             dom->mmio_size = info->u.hvm.mmio_hole_memkb << 10;
     }
 
-    rc = libxl__domain_firmware(gc, info, dom);
+    rc = libxl__domain_firmware(gc, info, state, dom);
     if (rc != 0) {
         LOG(ERROR, "initializing domain firmware failed");
         goto out;
-- 
2.11.0 (Apple Git-81)


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

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

* [PATCH v2 13/21] libxl: add PVH support to domain save/suspend
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
                   ` (11 preceding siblings ...)
  2017-09-07 10:16 ` [PATCH v2 12/21] libxl: add PVH support to domain building Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 14/21] libxl: add PVH support to vpcu hotplug, domain destruction/pause and domain configuration Roger Pau Monne
                   ` (7 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

And remove the device model "none" support.

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_dom_save.c    | 9 ++++++---
 tools/libxl/libxl_dom_suspend.c | 8 +++-----
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/tools/libxl/libxl_dom_save.c b/tools/libxl/libxl_dom_save.c
index 77fe30e9c0..194bbdbc5d 100644
--- a/tools/libxl/libxl_dom_save.c
+++ b/tools/libxl/libxl_dom_save.c
@@ -158,6 +158,11 @@ void libxl__domain_suspend_common_switch_qemu_logdirty
     /* Convenience aliases. */
     libxl__logdirty_switch *const lds = &dss->logdirty;
 
+    if (dss->type == LIBXL_DOMAIN_TYPE_PVH) {
+        domain_suspend_switch_qemu_logdirty_done(egc, lds, 0);
+        return;
+    }
+
     lds->callback = domain_suspend_switch_qemu_logdirty_done;
     libxl__domain_common_switch_qemu_logdirty(egc, domid, enable, lds);
 }
@@ -176,9 +181,6 @@ void libxl__domain_common_switch_qemu_logdirty(libxl__egc *egc,
     case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
         domain_suspend_switch_qemu_xen_logdirty(egc, domid, enable, lds);
         break;
-    case LIBXL_DEVICE_MODEL_VERSION_NONE:
-        lds->callback(egc, lds, 0);
-        break;
     default:
         LOGD(ERROR, domid, "logdirty switch failed"
              ", no valid device model version found, abandoning suspend");
@@ -363,6 +365,7 @@ void libxl__domain_save(libxl__egc *egc, libxl__domain_save_state *dss)
     if (rc) goto out;
 
     switch (type) {
+    case LIBXL_DOMAIN_TYPE_PVH:
     case LIBXL_DOMAIN_TYPE_HVM: {
         dss->hvm = 1;
         break;
diff --git a/tools/libxl/libxl_dom_suspend.c b/tools/libxl/libxl_dom_suspend.c
index 6314a001d1..ca41107412 100644
--- a/tools/libxl/libxl_dom_suspend.c
+++ b/tools/libxl/libxl_dom_suspend.c
@@ -90,8 +90,6 @@ int libxl__domain_suspend_device_model(libxl__gc *gc,
         if (ret)
             unlink(filename);
         break;
-    case LIBXL_DEVICE_MODEL_VERSION_NONE:
-        break;
     default:
         return ERROR_INVAL;
     }
@@ -148,14 +146,14 @@ static void domain_suspend_callback_common(libxl__egc *egc,
     /* Convenience aliases */
     const uint32_t domid = dsps->domid;
 
-    if (dsps->type == LIBXL_DOMAIN_TYPE_HVM) {
+    if (dsps->type != LIBXL_DOMAIN_TYPE_PV) {
         xc_hvm_param_get(CTX->xch, domid, HVM_PARAM_CALLBACK_IRQ, &hvm_pvdrv);
         xc_hvm_param_get(CTX->xch, domid, HVM_PARAM_ACPI_S_STATE, &hvm_s_state);
     }
 
     if ((hvm_s_state == 0) && (dsps->guest_evtchn.port >= 0)) {
         LOGD(DEBUG, domid, "issuing %s suspend request via event channel",
-            dsps->type == LIBXL_DOMAIN_TYPE_HVM ? "PVHVM" : "PV");
+            dsps->type != LIBXL_DOMAIN_TYPE_PV ? "PVH/HVM" : "PV");
         ret = xenevtchn_notify(CTX->xce, dsps->guest_evtchn.port);
         if (ret < 0) {
             LOGD(ERROR, domid, "xenevtchn_notify failed ret=%d", ret);
@@ -190,7 +188,7 @@ static void domain_suspend_callback_common(libxl__egc *egc,
     }
 
     LOGD(DEBUG, domid, "issuing %s suspend request via XenBus control node",
-        dsps->type == LIBXL_DOMAIN_TYPE_HVM ? "PVHVM" : "PV");
+        dsps->type != LIBXL_DOMAIN_TYPE_PV ? "PVH/HVM" : "PV");
 
     libxl__domain_pvcontrol_write(gc, XBT_NULL, domid, "suspend");
 
-- 
2.11.0 (Apple Git-81)


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

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

* [PATCH v2 14/21] libxl: add PVH support to vpcu hotplug, domain destruction/pause and domain configuration
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
                   ` (12 preceding siblings ...)
  2017-09-07 10:16 ` [PATCH v2 13/21] libxl: add PVH support to domain save/suspend Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 15/21] libxl: add PVH support to memory functions Roger Pau Monne
                   ` (6 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

And remove support for device model "none".

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_domain.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/tools/libxl/libxl_domain.c b/tools/libxl/libxl_domain.c
index 08eccd082b..691158512d 100644
--- a/tools/libxl/libxl_domain.c
+++ b/tools/libxl/libxl_domain.c
@@ -571,14 +571,11 @@ int libxl_domain_unpause(libxl_ctx *ctx, uint32_t domid)
     }
 
     if (type == LIBXL_DOMAIN_TYPE_HVM) {
-        if (libxl__device_model_version_running(gc, domid) !=
-            LIBXL_DEVICE_MODEL_VERSION_NONE) {
-            rc = libxl__domain_resume_device_model(gc, domid);
-            if (rc < 0) {
-                LOGD(ERROR, domid, "Failed to unpause device model for domain:%d",
-                     rc);
-                goto out;
-            }
+        rc = libxl__domain_resume_device_model(gc, domid);
+        if (rc < 0) {
+            LOGD(ERROR, domid, "Failed to unpause device model for domain:%d",
+                 rc);
+            goto out;
         }
     }
     ret = xc_domain_unpause(ctx->xch, domid);
@@ -1012,6 +1009,7 @@ void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis)
             break;
         }
         /* fall through */
+    case LIBXL_DOMAIN_TYPE_PVH:
     case LIBXL_DOMAIN_TYPE_PV:
         dm_present = libxl__dm_active(gc, domid);
         break;
@@ -1349,7 +1347,6 @@ int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_bitmap *cpumap)
     case LIBXL_DOMAIN_TYPE_HVM:
         switch (libxl__device_model_version_running(gc, domid)) {
         case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
-        case LIBXL_DEVICE_MODEL_VERSION_NONE:
             break;
         case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
             rc = libxl__set_vcpuonline_qmp(gc, domid, cpumap, &info);
@@ -1358,6 +1355,7 @@ int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_bitmap *cpumap)
             rc = ERROR_INVAL;
         }
         break;
+    case LIBXL_DOMAIN_TYPE_PVH:
     case LIBXL_DOMAIN_TYPE_PV:
         break;
     default:
@@ -1584,7 +1582,6 @@ int libxl_retrieve_domain_configuration(libxl_ctx *ctx, uint32_t domid,
                                                    max_vcpus, map);
                 break;
             case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
-            case LIBXL_DEVICE_MODEL_VERSION_NONE:
                 rc = libxl__update_avail_vcpus_xenstore(gc, domid,
                                                         max_vcpus, map);
                 break;
@@ -1592,6 +1589,7 @@ int libxl_retrieve_domain_configuration(libxl_ctx *ctx, uint32_t domid,
                 abort();
             }
             break;
+        case LIBXL_DOMAIN_TYPE_PVH:
         case LIBXL_DOMAIN_TYPE_PV:
             rc = libxl__update_avail_vcpus_xenstore(gc, domid,
                                                     max_vcpus, map);
-- 
2.11.0 (Apple Git-81)


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

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

* [PATCH v2 15/21] libxl: add PVH support to memory functions
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
                   ` (13 preceding siblings ...)
  2017-09-07 10:16 ` [PATCH v2 14/21] libxl: add PVH support to vpcu hotplug, domain destruction/pause and domain configuration Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 16/21] libxl: PVH guests use PV nics Roger Pau Monne
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

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_mem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/libxl/libxl_mem.c b/tools/libxl/libxl_mem.c
index f5d2530d8c..e551e09fed 100644
--- a/tools/libxl/libxl_mem.c
+++ b/tools/libxl/libxl_mem.c
@@ -460,6 +460,7 @@ int libxl_domain_need_memory(libxl_ctx *ctx,
 
     *need_memkb = b_info->target_memkb;
     switch (b_info->type) {
+    case LIBXL_DOMAIN_TYPE_PVH:
     case LIBXL_DOMAIN_TYPE_HVM:
         *need_memkb += b_info->shadow_memkb + LIBXL_HVM_EXTRA_MEMORY;
         if (libxl_defbool_val(b_info->device_model_stubdomain))
-- 
2.11.0 (Apple Git-81)


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

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

* [PATCH v2 16/21] libxl: PVH guests use PV nics
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
                   ` (14 preceding siblings ...)
  2017-09-07 10:16 ` [PATCH v2 15/21] libxl: add PVH support to memory functions Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 17/21] libxl: remove device model "none" support from stream functions Roger Pau Monne
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

Remove device model "none" support from the nic functions.

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_nic.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl_nic.c b/tools/libxl/libxl_nic.c
index 4b6e8c0cb1..78afba2971 100644
--- a/tools/libxl/libxl_nic.c
+++ b/tools/libxl/libxl_nic.c
@@ -91,18 +91,17 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
     switch (libxl__domain_type(gc, domid)) {
     case LIBXL_DOMAIN_TYPE_HVM:
         if (!nic->nictype) {
-            if (hotplug ||
-                (libxl__device_model_version_running(gc, domid) ==
-                 LIBXL_DEVICE_MODEL_VERSION_NONE))
+            if (hotplug)
                 nic->nictype = LIBXL_NIC_TYPE_VIF;
             else
                 nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
         }
         break;
+    case LIBXL_DOMAIN_TYPE_PVH:
     case LIBXL_DOMAIN_TYPE_PV:
         if (nic->nictype == LIBXL_NIC_TYPE_VIF_IOEMU) {
             LOGD(ERROR, domid,
-                 "trying to create PV guest with an emulated interface");
+                 "trying to create PV or PVH guest with an emulated interface");
             return ERROR_INVAL;
         }
         nic->nictype = LIBXL_NIC_TYPE_VIF;
-- 
2.11.0 (Apple Git-81)


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

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

* [PATCH v2 17/21] libxl: remove device model "none" support from stream functions
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
                   ` (15 preceding siblings ...)
  2017-09-07 10:16 ` [PATCH v2 16/21] libxl: PVH guests use PV nics Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 18/21] libxl: add PVH support to USB Roger Pau Monne
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

Remove the usage of device model "none" in the migration stream
related functions.

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_stream_read.c  |  6 ++----
 tools/libxl/libxl_stream_write.c | 11 ++---------
 2 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
index 483875038c..fcb39ee7d5 100644
--- a/tools/libxl/libxl_stream_read.c
+++ b/tools/libxl/libxl_stream_read.c
@@ -584,8 +584,7 @@ static bool process_record(libxl__egc *egc,
         break;
 
     case REC_TYPE_EMULATOR_XENSTORE_DATA:
-        if (dcs->guest_config->b_info.device_model_version ==
-            LIBXL_DEVICE_MODEL_VERSION_NONE) {
+        if (dcs->guest_config->b_info.type != LIBXL_DOMAIN_TYPE_HVM) {
             rc = ERROR_FAIL;
             LOG(ERROR,
                 "Received a xenstore emulator record when none was expected");
@@ -613,8 +612,7 @@ static bool process_record(libxl__egc *egc,
         break;
 
     case REC_TYPE_EMULATOR_CONTEXT:
-        if (dcs->guest_config->b_info.device_model_version ==
-            LIBXL_DEVICE_MODEL_VERSION_NONE) {
+        if (dcs->guest_config->b_info.type != LIBXL_DOMAIN_TYPE_HVM) {
             rc = ERROR_FAIL;
             LOG(ERROR,
                 "Received an emulator context record when none was expected");
diff --git a/tools/libxl/libxl_stream_write.c b/tools/libxl/libxl_stream_write.c
index c96a6a2c38..634f3240d1 100644
--- a/tools/libxl/libxl_stream_write.c
+++ b/tools/libxl/libxl_stream_write.c
@@ -181,7 +181,6 @@ static void setup_emulator_write(libxl__egc *egc,
                                  sws_record_done_cb cb)
 {
     assert(stream->emu_sub_hdr.id != EMULATOR_UNKNOWN);
-    assert(stream->device_model_version != LIBXL_DEVICE_MODEL_VERSION_NONE);
     setup_generic_write(egc, stream, what, hdr, emu_hdr, body, cb);
 }
 
@@ -261,10 +260,6 @@ void libxl__stream_write_start(libxl__egc *egc,
             stream->emu_sub_hdr.id = EMULATOR_QEMU_UPSTREAM;
             break;
 
-        case LIBXL_DEVICE_MODEL_VERSION_NONE:
-            stream->emu_sub_hdr.id = EMULATOR_UNKNOWN;
-            break;
-
         default:
             rc = ERROR_FAIL;
             LOGD(ERROR, dss->domid, "Unknown emulator for HVM domain");
@@ -395,7 +390,7 @@ static void write_emulator_xenstore_record(libxl__egc *egc,
     char *buf = NULL;
     uint32_t len = 0;
 
-    if (stream->device_model_version == LIBXL_DEVICE_MODEL_VERSION_NONE) {
+    if (dss->type != LIBXL_DOMAIN_TYPE_HVM) {
         emulator_xenstore_record_done(egc, stream);
         return;
     }
@@ -449,9 +444,7 @@ static void write_emulator_context_record(libxl__egc *egc,
     struct stat st;
     int rc;
 
-    assert(dss->type == LIBXL_DOMAIN_TYPE_HVM);
-
-    if (stream->device_model_version == LIBXL_DEVICE_MODEL_VERSION_NONE) {
+    if (dss->type != LIBXL_DOMAIN_TYPE_HVM) {
         emulator_context_record_done(egc, stream);
         return;
     }
-- 
2.11.0 (Apple Git-81)


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

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

* [PATCH v2 18/21] libxl: add PVH support to USB
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
                   ` (16 preceding siblings ...)
  2017-09-07 10:16 ` [PATCH v2 17/21] libxl: remove device model "none" support from stream functions Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 19/21] libxl: add PVH support to x86 functions Roger Pau Monne
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

Add PVH support to usb related functions.

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_usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c
index d8948d5212..d6bdbb822b 100644
--- a/tools/libxl/libxl_usb.c
+++ b/tools/libxl/libxl_usb.c
@@ -45,13 +45,13 @@ static int libxl__device_usbctrl_setdefault(libxl__gc *gc, uint32_t domid,
     libxl_domain_type domtype = libxl__domain_type(gc, domid);
 
     if (usbctrl->type == LIBXL_USBCTRL_TYPE_AUTO) {
-        if (domtype == LIBXL_DOMAIN_TYPE_PV) {
+        if (domtype != LIBXL_DOMAIN_TYPE_HVM) {
             rc = usbback_is_loaded(gc);
             if (rc < 0)
                 goto out;
             usbctrl->type = rc ? LIBXL_USBCTRL_TYPE_PV
                                : LIBXL_USBCTRL_TYPE_QUSB;
-        } else if (domtype == LIBXL_DOMAIN_TYPE_HVM) {
+        } else {
             /* FIXME: See if we can detect PV frontend */
             usbctrl->type = LIBXL_USBCTRL_TYPE_DEVICEMODEL;
         }
-- 
2.11.0 (Apple Git-81)


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

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

* [PATCH v2 19/21] libxl: add PVH support to x86 functions
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
                   ` (17 preceding siblings ...)
  2017-09-07 10:16 ` [PATCH v2 18/21] libxl: add PVH support to USB Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 20/21] xl: add PVH as a guest type Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 21/21] libxl: remove device model "none" from IDL Roger Pau Monne
  20 siblings, 0 replies; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

This also includes the x86 ACPI related functions. Remove support for
device model "none"

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_x86.c      | 33 +++++++++++++++++----------------
 tools/libxl/libxl_x86_acpi.c |  3 +--
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index 442854c5c2..d321b8349c 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -7,20 +7,22 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
                                       libxl_domain_config *d_config,
                                       xc_domain_configuration_t *xc_config)
 {
-
-    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM) {
-        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.apic)) {
-            /*
-             * HVM guests without device model may want
-             * to have LAPIC emulation.
-             */
+    switch(d_config->c_info.type) {
+    case LIBXL_DOMAIN_TYPE_HVM:
+        xc_config->emulation_flags = XEN_X86_EMU_ALL;
+        break;
+    case LIBXL_DOMAIN_TYPE_PVH:
+        if (libxl_defbool_val(d_config->b_info.apic))
+            /* PVH guests may want to have LAPIC emulation. */
             xc_config->emulation_flags = XEN_X86_EMU_LAPIC;
-        }
-    } else {
+        else
+            xc_config->emulation_flags = 0;
+        break;
+    case LIBXL_DOMAIN_TYPE_PV:
         xc_config->emulation_flags = 0;
+        break;
+    default:
+        abort();
     }
 
     return 0;
@@ -266,7 +268,7 @@ static int libxl__e820_alloc(libxl__gc *gc, uint32_t domid,
     struct e820entry map[E820MAX];
     libxl_domain_build_info *b_info;
 
-    if (d_config == NULL || d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM)
+    if (d_config == NULL || d_config->c_info.type != LIBXL_DOMAIN_TYPE_PV)
         return ERROR_INVAL;
 
     b_info = &d_config->b_info;
@@ -338,7 +340,7 @@ int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
     if (rtc_timeoffset)
         xc_domain_set_time_offset(ctx->xch, domid, rtc_timeoffset);
 
-    if (d_config->b_info.type == LIBXL_DOMAIN_TYPE_HVM) {
+    if (d_config->b_info.type != LIBXL_DOMAIN_TYPE_PV) {
         unsigned long shadow = DIV_ROUNDUP(d_config->b_info.shadow_memkb,
                                            1024);
         xc_shadow_control(ctx->xch, domid, XEN_DOMCTL_SHADOW_OP_SET_ALLOCATION,
@@ -381,8 +383,7 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
 {
     int rc = 0;
 
-    if ((info->type == LIBXL_DOMAIN_TYPE_HVM) &&
-        (info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_NONE)) {
+    if (info->type == LIBXL_DOMAIN_TYPE_PVH) {
         rc = libxl__dom_load_acpi(gc, info, dom);
         if (rc != 0)
             LOGE(ERROR, "libxl_dom_load_acpi failed");
diff --git a/tools/libxl/libxl_x86_acpi.c b/tools/libxl/libxl_x86_acpi.c
index 9350402308..9a7c90467d 100644
--- a/tools/libxl/libxl_x86_acpi.c
+++ b/tools/libxl/libxl_x86_acpi.c
@@ -171,8 +171,7 @@ int libxl__dom_load_acpi(libxl__gc *gc,
     void *acpi_pages;
     unsigned long page_mask;
 
-    if ((b_info->type != LIBXL_DOMAIN_TYPE_HVM) ||
-        (b_info->device_model_version != LIBXL_DEVICE_MODEL_VERSION_NONE))
+    if (b_info->type != LIBXL_DOMAIN_TYPE_PVH)
         goto out;
 
     libxl_ctxt.page_size = XC_DOM_PAGE_SIZE(dom);
-- 
2.11.0 (Apple Git-81)


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

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

* [PATCH v2 20/21] xl: add PVH as a guest type
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
                   ` (18 preceding siblings ...)
  2017-09-07 10:16 ` [PATCH v2 19/21] libxl: add PVH support to x86 functions Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  2017-09-07 10:16 ` [PATCH v2 21/21] libxl: remove device model "none" from IDL Roger Pau Monne
  20 siblings, 0 replies; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

And remove device model "none".

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/xl/xl_parse.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 00c939b423..a2b3b13fc3 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -866,6 +866,8 @@ void parse_config_data(const char *config_source,
             c_info->type = LIBXL_DOMAIN_TYPE_HVM;
         } else if (!strncmp(buf, "pv", strlen(buf))) {
             c_info->type = LIBXL_DOMAIN_TYPE_PV;
+        } else if (!strncmp(buf, "pvh", strlen(buf))) {
+            c_info->type = LIBXL_DOMAIN_TYPE_PVH;
         } else {
             fprintf(stderr, "Invalid domain type %s.\n", buf);
             exit(1);
@@ -1283,6 +1285,7 @@ void parse_config_data(const char *config_source,
         }
 
         break;
+    case LIBXL_DOMAIN_TYPE_PVH:
     case LIBXL_DOMAIN_TYPE_PV:
     {
         /*
@@ -1960,8 +1963,6 @@ skip_usbdev:
         } else if (!strcmp(buf, "qemu-xen")) {
             b_info->device_model_version
                 = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
-        } else if (!strcmp(buf, "none")) {
-            b_info->device_model_version = LIBXL_DEVICE_MODEL_VERSION_NONE;
         } else {
             fprintf(stderr,
                     "Unknown device_model_version \"%s\" specified\n", buf);
-- 
2.11.0 (Apple Git-81)


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

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

* [PATCH v2 21/21] libxl: remove device model "none" from IDL
  2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
                   ` (19 preceding siblings ...)
  2017-09-07 10:16 ` [PATCH v2 20/21] xl: add PVH as a guest type Roger Pau Monne
@ 2017-09-07 10:16 ` Roger Pau Monne
  20 siblings, 0 replies; 34+ messages in thread
From: Roger Pau Monne @ 2017-09-07 10:16 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, boris.ostrovsky, Roger Pau Monne, Ian Jackson

And the xl.cfg man page documentation.

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>
---
 docs/man/xl.cfg.pod.5.in    | 5 -----
 tools/libxl/libxl.h         | 8 --------
 tools/libxl/libxl_types.idl | 1 -
 3 files changed, 14 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
index c7cb014bcb..82cb047971 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -2269,11 +2269,6 @@ This device-model is the default for Linux dom0.
 Use the device-model based upon the historical Xen fork of QEMU.
 This device-model is still the default for NetBSD dom0.
 
-=item B<none>
-
-Don't use any device model. This requires a kernel capable of booting
-without emulated devices.
-
 =back
 
 It is recommended to accept the default value for new guests.  If
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index e441479780..4aec148c36 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1001,14 +1001,6 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src);
 #define LIBXL_HAVE_GFX_PASSTHRU_KIND
 
 /*
- * LIBXL_HAVE_DEVICE_MODEL_VERSION_NONE
- *
- * In the case that LIBXL_HAVE_DEVICE_MODEL_VERSION_NONE is set libxl
- * allows the creation of HVM guests without a device model.
- */
-#define LIBXL_HAVE_DEVICE_MODEL_VERSION_NONE 1
-
-/*
  * LIBXL_HAVE_CHECKPOINTED_STREAM
  *
  * If this is defined, then libxl_checkpointed_stream exists.
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 18aa05b589..2ff54a3e0a 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -99,7 +99,6 @@ libxl_device_model_version = Enumeration("device_model_version", [
     (0, "UNKNOWN"),
     (1, "QEMU_XEN_TRADITIONAL"), # Historical qemu-xen device model (qemu-dm)
     (2, "QEMU_XEN"),             # Upstream based qemu-xen device model
-    (3, "NONE"),                 # No device model
     ])
 
 libxl_console_type = Enumeration("console_type", [
-- 
2.11.0 (Apple Git-81)


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

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

* Re: [PATCH v2 01/21] libxl: add is_default checkers for string and timer_mode types
  2017-09-07 10:16 ` [PATCH v2 01/21] libxl: add is_default checkers for string and timer_mode types Roger Pau Monne
@ 2017-09-19 14:48   ` Ian Jackson
  2017-09-20 16:12   ` Wei Liu
  1 sibling, 0 replies; 34+ messages in thread
From: Ian Jackson @ 2017-09-19 14:48 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel, boris.ostrovsky, Wei Liu

Roger Pau Monne writes ("[PATCH v2 01/21] libxl: add is_default checkers for string and timer_mode types"):
> Those types are missing a helper to check whether a definition of the
> type holds the default value. This will be required by a later patch
> that will implement deprecation of fields inside of a libxl type.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

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

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

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

* Re: [PATCH v2 03/21] libxl/xl: use the new domain_build_info fields position
  2017-09-07 10:16 ` [PATCH v2 03/21] libxl/xl: use the new domain_build_info fields position Roger Pau Monne
@ 2017-09-20 14:48   ` Ian Jackson
  0 siblings, 0 replies; 34+ messages in thread
From: Ian Jackson @ 2017-09-20 14:48 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel, boris.ostrovsky, Wei Liu

Roger Pau Monne writes ("[PATCH v2 03/21] libxl/xl: use the new domain_build_info fields position"):
> This is required because those options will be used by the new PVH
> guest type, and thus need to be shared between PV and HVM.
...
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index 02ddd2e90d..61f9a38573 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)) {

The code motion would benefit from being moved into a pre-patch.  Is
that easily possible ?  Obviously in the new location, in the
pre-patch it would still need to be in an if () to check the guest
type.

Ian.

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

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

* Re: [PATCH v2 04/21] xl: introduce a domain type option
  2017-09-07 10:16 ` [PATCH v2 04/21] xl: introduce a domain type option Roger Pau Monne
@ 2017-09-20 14:50   ` Ian Jackson
  2017-09-21 17:16     ` Roger Pau Monné
  0 siblings, 1 reply; 34+ messages in thread
From: Ian Jackson @ 2017-09-20 14:50 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel, boris.ostrovsky, Wei Liu

Roger Pau Monne writes ("[PATCH v2 04/21] xl: introduce a domain type option"):
> Introduce a new type option to xl configuration files in order to
> specify the domain type. This supersedes the current builder option.
...> 
> Th
>      libxl_defbool_set(&c_info->run_hotplug_scripts, run_hotplug_scripts);
>      c_info->type = LIBXL_DOMAIN_TYPE_PV;
> -    if (!xlu_cfg_get_string (config, "builder", &buf, 0) &&
> -        !strncmp(buf, "hvm", strlen(buf)))
> -        c_info->type = LIBXL_DOMAIN_TYPE_HVM;
> +    if (!xlu_cfg_get_string (config, "builder", &buf, 0)) {
> +        fprintf(stderr,
> +                "The builder option is being deprecated, please use type instead.\n");

Line length.  Probably best to shuffle the message to the left, rather
than linewrapping, for ease of grepping.

The error message would benefit from some `quotes' around the
parameter names.

It would be nice if you did this in a way that meant that a config
file which specified both `type="hvm"' and `builder="hvm"' did not
generate a warning.

And it ought to be an error to specify `type="pv"' with
`builder="hvm"' surely.

Ian.

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

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

* Re: [PATCH v2 05/21] xl: introduce a firmware option
  2017-09-07 10:16 ` [PATCH v2 05/21] xl: introduce a firmware option Roger Pau Monne
@ 2017-09-20 14:53   ` Ian Jackson
  0 siblings, 0 replies; 34+ messages in thread
From: Ian Jackson @ 2017-09-20 14:53 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel, boris.ostrovsky, Wei Liu

Roger Pau Monne writes ("[PATCH v2 05/21] xl: introduce a firmware option"):
> The new firmware option aims to provide a coherent way to set the
> firmware for the different kind of guests Xen supports.
...
> +=head3 Non direct Kernel Boot
> +
> +Non direct kernel boot allows booting guests with a firmware. This can be \
used
> +by all types of guests, although the selection of options is different
> +depending on the guest type.
> +
> +This option provides the flexibly of letting the guest decide which kernel\
 they
> +want to boot, while preventing having to poke at the guest file system for\
m the

Line length.  Observe the (simulated) wrap damage.

You should to consider using semantic linefeeds (semantic newlines).
That is, break the lines in the .pod file at sentence or phrase
boundaries.

I see the code has line length problems too (it wraps when I look at
it in my MUA and even more so when I try to quote it when replying).

After we've chatted about the idl compiler changes, can you fix the
long lines everywhere and resend ?

Thanks,
Ian.

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

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

* Re: [PATCH v2 02/21] libxl: introduce a way to mark fields as deprecated in the idl
  2017-09-07 10:16 ` [PATCH v2 02/21] libxl: introduce a way to mark fields as deprecated in the idl Roger Pau Monne
@ 2017-09-20 15:46   ` Ian Jackson
  2017-09-20 16:39     ` Wei Liu
  2017-09-22 16:02     ` Roger Pau Monné
  0 siblings, 2 replies; 34+ messages in thread
From: Ian Jackson @ 2017-09-20 15:46 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel, boris.ostrovsky, Wei Liu

Roger Pau Monne writes ("[PATCH v2 02/21] libxl: introduce a way to mark fields as deprecated in the idl"):
> The deprecation involves generating a function that copies the
> deprecated fields into it's new location if the new location has not
> been set.

Hi.  We had an IRL conversation which I will summarise.


The first issue is about the scoping and context of the deprecated_by
annotations.  The arrangement you have is that the field name in
deprecated_by is a (textual) reference to an (implicit) enclosing
struct which has copy_deprecated_fn specified.

This is kind of OK but it feels a bit limited and irregular to me.
The practical consequence is that this can be used to bring fields out
into the toplevel, but it is difficult to use it in other ways (for
example, to move a field from one substruct to another, or to
deprecate fields which are part of named substrutures rather than
anonymous ones and which might therefore appear in several places).

We discussed how this might be done better.  To me it seems like the
only really plausible alternative was to replace the
`deprecated_by' and `copy_deprecated_fn' annotations with a single
annotation in the parent structure, something like
  deprecated_fields=['u.hvm','u',['bootloader_args',
                                  'timer_mode', ...]
or maybe even
  deprecated_fields=['u.hvm','u',[('timer_mode_new_name','timer_mode')]]

I really don't want to ask you to implement this general scheme now,
but if you feel like it you could perhaps experiment and see how it
seems.


I have two related comments that do need addressing though: I think
your generate deprecated copy function should check that at most one
of the old and new fields is set.  And, then, to make default setting
idempotent (and avoid memory leaks etc.), it should clear the old
field.


Thanks,
Ian.

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

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

* Re: [PATCH v2 01/21] libxl: add is_default checkers for string and timer_mode types
  2017-09-07 10:16 ` [PATCH v2 01/21] libxl: add is_default checkers for string and timer_mode types Roger Pau Monne
  2017-09-19 14:48   ` Ian Jackson
@ 2017-09-20 16:12   ` Wei Liu
  1 sibling, 0 replies; 34+ messages in thread
From: Wei Liu @ 2017-09-20 16:12 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel, boris.ostrovsky, Ian Jackson, Wei Liu

On Thu, Sep 07, 2017 at 11:16:22AM +0100, Roger Pau Monne wrote:
> Those types are missing a helper to check whether a definition of the
> type holds the default value. This will be required by a later patch
> that will implement deprecation of fields inside of a libxl type.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

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

* Re: [PATCH v2 02/21] libxl: introduce a way to mark fields as deprecated in the idl
  2017-09-20 15:46   ` Ian Jackson
@ 2017-09-20 16:39     ` Wei Liu
  2017-09-21 10:32       ` Ian Jackson
  2017-09-22 16:02     ` Roger Pau Monné
  1 sibling, 1 reply; 34+ messages in thread
From: Wei Liu @ 2017-09-20 16:39 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, boris.ostrovsky, Wei Liu, Roger Pau Monne

On Wed, Sep 20, 2017 at 04:46:16PM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH v2 02/21] libxl: introduce a way to mark fields as deprecated in the idl"):
> > The deprecation involves generating a function that copies the
> > deprecated fields into it's new location if the new location has not
> > been set.
> 
> Hi.  We had an IRL conversation which I will summarise.
> 
> 
> The first issue is about the scoping and context of the deprecated_by
> annotations.  The arrangement you have is that the field name in
> deprecated_by is a (textual) reference to an (implicit) enclosing
> struct which has copy_deprecated_fn specified.
> 
> This is kind of OK but it feels a bit limited and irregular to me.
> The practical consequence is that this can be used to bring fields out
> into the toplevel, but it is difficult to use it in other ways (for
> example, to move a field from one substruct to another, or to
> deprecate fields which are part of named substrutures rather than
> anonymous ones and which might therefore appear in several places).
> 
> We discussed how this might be done better.  To me it seems like the
> only really plausible alternative was to replace the
> `deprecated_by' and `copy_deprecated_fn' annotations with a single
> annotation in the parent structure, something like
>   deprecated_fields=['u.hvm','u',['bootloader_args',
>                                   'timer_mode', ...]
> or maybe even
>   deprecated_fields=['u.hvm','u',[('timer_mode_new_name','timer_mode')]]

I know this may sound crazy but: do we need to consider the possibility
that one field in struct A is deprecated by one field in struct B?

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

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

* Re: [PATCH v2 02/21] libxl: introduce a way to mark fields as deprecated in the idl
  2017-09-20 16:39     ` Wei Liu
@ 2017-09-21 10:32       ` Ian Jackson
  0 siblings, 0 replies; 34+ messages in thread
From: Ian Jackson @ 2017-09-21 10:32 UTC (permalink / raw)
  To: Wei Liu; +Cc: xen-devel, boris.ostrovsky, Roger Pau Monne

Wei Liu writes ("Re: [PATCH v2 02/21] libxl: introduce a way to mark fields as deprecated in the idl"):
> On Wed, Sep 20, 2017 at 04:46:16PM +0100, Ian Jackson wrote:
...
> > We discussed how this might be done better.  To me it seems like the
> > only really plausible alternative was to replace the
> > `deprecated_by' and `copy_deprecated_fn' annotations with a single
> > annotation in the parent structure, something like
> >   deprecated_fields=['u.hvm','u',['bootloader_args',
> >                                   'timer_mode', ...]
> > or maybe even
> >   deprecated_fields=['u.hvm','u',[('timer_mode_new_name','timer_mode')]]
> 
> I know this may sound crazy but: do we need to consider the possibility
> that one field in struct A is deprecated by one field in struct B?

I don't think we can rule that out.  My proposed "deprecated_fields"
thing would be able to cope with that: the information about which
field to copy where is in the struct(s) that contain both A and B.

But, as I say, I don't want to insist on a more comprehensive solution
at this stage.  The IDL compiler language is not a stable interface so
we could choose to do this rework later, when the need arises.

Ian.

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

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

* Re: [PATCH v2 04/21] xl: introduce a domain type option
  2017-09-20 14:50   ` Ian Jackson
@ 2017-09-21 17:16     ` Roger Pau Monné
  2017-09-21 17:22       ` Ian Jackson
  0 siblings, 1 reply; 34+ messages in thread
From: Roger Pau Monné @ 2017-09-21 17:16 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, boris.ostrovsky, Wei Liu

On Wed, Sep 20, 2017 at 03:50:48PM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH v2 04/21] xl: introduce a domain type option"):
> > Introduce a new type option to xl configuration files in order to
> > specify the domain type. This supersedes the current builder option.
> ...> 
> > Th
> >      libxl_defbool_set(&c_info->run_hotplug_scripts, run_hotplug_scripts);
> >      c_info->type = LIBXL_DOMAIN_TYPE_PV;
> > -    if (!xlu_cfg_get_string (config, "builder", &buf, 0) &&
> > -        !strncmp(buf, "hvm", strlen(buf)))
> > -        c_info->type = LIBXL_DOMAIN_TYPE_HVM;
> > +    if (!xlu_cfg_get_string (config, "builder", &buf, 0)) {
> > +        fprintf(stderr,
> > +                "The builder option is being deprecated, please use type instead.\n");
> 
> Line length.  Probably best to shuffle the message to the left, rather
> than linewrapping, for ease of grepping.
> 
> The error message would benefit from some `quotes' around the
> parameter names.
> 
> It would be nice if you did this in a way that meant that a config
> file which specified both `type="hvm"' and `builder="hvm"' did not
> generate a warning.

Hm, really? I can do that but it seems weird because it doesn't
promote deprecation.

Thanks, Roger.

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

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

* Re: [PATCH v2 04/21] xl: introduce a domain type option
  2017-09-21 17:16     ` Roger Pau Monné
@ 2017-09-21 17:22       ` Ian Jackson
  0 siblings, 0 replies; 34+ messages in thread
From: Ian Jackson @ 2017-09-21 17:22 UTC (permalink / raw)
  To: Roger Pau Monné; +Cc: xen-devel, boris.ostrovsky, Wei Liu

Roger Pau Monné writes ("Re: [PATCH v2 04/21] xl: introduce a domain type option"):
> On Wed, Sep 20, 2017 at 03:50:48PM +0100, Ian Jackson wrote:
> > It would be nice if you did this in a way that meant that a config
> > file which specified both `type="hvm"' and `builder="hvm"' did not
> > generate a warning.
> 
> Hm, really? I can do that but it seems weird because it doesn't
> promote deprecation.

It does promote deprecation, surely.  Such a config file is compatible
with all past and all planned future versions of Xen.  In particular,
if we stop honouring builder= at all, it will still work.

Ian.

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

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

* Re: [PATCH v2 02/21] libxl: introduce a way to mark fields as deprecated in the idl
  2017-09-20 15:46   ` Ian Jackson
  2017-09-20 16:39     ` Wei Liu
@ 2017-09-22 16:02     ` Roger Pau Monné
  2017-09-22 16:09       ` Ian Jackson
  1 sibling, 1 reply; 34+ messages in thread
From: Roger Pau Monné @ 2017-09-22 16:02 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, boris.ostrovsky, Wei Liu

On Wed, Sep 20, 2017 at 04:46:16PM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH v2 02/21] libxl: introduce a way to mark fields as deprecated in the idl"):
> > The deprecation involves generating a function that copies the
> > deprecated fields into it's new location if the new location has not
> > been set.
> 
> Hi.  We had an IRL conversation which I will summarise.
> 
> 
> The first issue is about the scoping and context of the deprecated_by
> annotations.  The arrangement you have is that the field name in
> deprecated_by is a (textual) reference to an (implicit) enclosing
> struct which has copy_deprecated_fn specified.
> 
> This is kind of OK but it feels a bit limited and irregular to me.
> The practical consequence is that this can be used to bring fields out
> into the toplevel, but it is difficult to use it in other ways (for
> example, to move a field from one substruct to another, or to
> deprecate fields which are part of named substrutures rather than
> anonymous ones and which might therefore appear in several places).
> 
> We discussed how this might be done better.  To me it seems like the
> only really plausible alternative was to replace the
> `deprecated_by' and `copy_deprecated_fn' annotations with a single
> annotation in the parent structure, something like
>   deprecated_fields=['u.hvm','u',['bootloader_args',
>                                   'timer_mode', ...]
> or maybe even
>   deprecated_fields=['u.hvm','u',[('timer_mode_new_name','timer_mode')]]
> 
> I really don't want to ask you to implement this general scheme now,
> but if you feel like it you could perhaps experiment and see how it
> seems.

Hello,

Thanks for the review. I've fixed all the other comments on the series
and started an osstest flight, my aim is to post a new version of the
series using the same deprecation idl mechanism ASAP.

The motivation for doing this is that I think we really need stable
PVHv2 support in Xen, and it must be in the next release.

Regarding your proposal for deprecating fields, I was thinking of
something along the lines of:

deprecated_fields=[['u.hvm.timer_mode', 'timer_mode'],
                   ['u.pv.bootloader', 'booloader'],
                   ...]

Something like a list of tuples (I'm not sure this is the right
terminology in python?), where the first element is the deprecated
field, and the second one is the new position.

This has the extra complication that when parsing the types gentypes
needs to split the strings based on the '.' or '->' delimiters, and
then iterate inside of the structure object to find the actual
element. AFAICT this is doable, but not as simple as my current
approach.

Roger.

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

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

* Re: [PATCH v2 02/21] libxl: introduce a way to mark fields as deprecated in the idl
  2017-09-22 16:02     ` Roger Pau Monné
@ 2017-09-22 16:09       ` Ian Jackson
  0 siblings, 0 replies; 34+ messages in thread
From: Ian Jackson @ 2017-09-22 16:09 UTC (permalink / raw)
  To: Roger Pau Monné; +Cc: xen-devel, boris.ostrovsky, Wei Liu

Roger Pau Monné writes ("Re: [PATCH v2 02/21] libxl: introduce a way to mark fields as deprecated in the idl"):
> Thanks for the review. I've fixed all the other comments on the series
> and started an osstest flight, my aim is to post a new version of the
> series using the same deprecation idl mechanism ASAP.

I'll look forward to it.

> The motivation for doing this is that I think we really need stable
> PVHv2 support in Xen, and it must be in the next release.

I agree.


> Regarding your proposal for deprecating fields, I was thinking of
> something along the lines of:
> 
> deprecated_fields=[['u.hvm.timer_mode', 'timer_mode'],
>                    ['u.pv.bootloader', 'booloader'],
>                    ...]

That looks fairly nice, yes.

> Something like a list of tuples (I'm not sure this is the right
> terminology in python?), where the first element is the deprecated
> field, and the second one is the new position.
> 
> This has the extra complication that when parsing the types gentypes
> needs to split the strings based on the '.' or '->' delimiters, and
> then iterate inside of the structure object to find the actual
> element. AFAICT this is doable, but not as simple as my current
> approach.

Indeed.

Ian.

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

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

end of thread, other threads:[~2017-09-22 16:09 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-07 10:16 [PATCH v2 00/21] libxl/xl: add PVH guest type Roger Pau Monne
2017-09-07 10:16 ` [PATCH v2 01/21] libxl: add is_default checkers for string and timer_mode types Roger Pau Monne
2017-09-19 14:48   ` Ian Jackson
2017-09-20 16:12   ` Wei Liu
2017-09-07 10:16 ` [PATCH v2 02/21] libxl: introduce a way to mark fields as deprecated in the idl Roger Pau Monne
2017-09-20 15:46   ` Ian Jackson
2017-09-20 16:39     ` Wei Liu
2017-09-21 10:32       ` Ian Jackson
2017-09-22 16:02     ` Roger Pau Monné
2017-09-22 16:09       ` Ian Jackson
2017-09-07 10:16 ` [PATCH v2 03/21] libxl/xl: use the new domain_build_info fields position Roger Pau Monne
2017-09-20 14:48   ` Ian Jackson
2017-09-07 10:16 ` [PATCH v2 04/21] xl: introduce a domain type option Roger Pau Monne
2017-09-20 14:50   ` Ian Jackson
2017-09-21 17:16     ` Roger Pau Monné
2017-09-21 17:22       ` Ian Jackson
2017-09-07 10:16 ` [PATCH v2 05/21] xl: introduce a firmware option Roger Pau Monne
2017-09-20 14:53   ` Ian Jackson
2017-09-07 10:16 ` [PATCH v2 06/21] libxl: introduce a PVH guest type Roger Pau Monne
2017-09-07 10:16 ` [PATCH v2 07/21] libxl: allow PVH guests to use a bootloader Roger Pau Monne
2017-09-07 10:16 ` [PATCH v2 08/21] libxl: set PVH guests to use the PV console Roger Pau Monne
2017-09-07 10:16 ` [PATCH v2 09/21] libxl: add PVH support to domain creation Roger Pau Monne
2017-09-07 10:16 ` [PATCH v2 10/21] libxl: remove device model "none" support from disk related functions Roger Pau Monne
2017-09-07 10:16 ` [PATCH v2 11/21] libxl: set device model for PVH guests Roger Pau Monne
2017-09-07 10:16 ` [PATCH v2 12/21] libxl: add PVH support to domain building Roger Pau Monne
2017-09-07 10:16 ` [PATCH v2 13/21] libxl: add PVH support to domain save/suspend Roger Pau Monne
2017-09-07 10:16 ` [PATCH v2 14/21] libxl: add PVH support to vpcu hotplug, domain destruction/pause and domain configuration Roger Pau Monne
2017-09-07 10:16 ` [PATCH v2 15/21] libxl: add PVH support to memory functions Roger Pau Monne
2017-09-07 10:16 ` [PATCH v2 16/21] libxl: PVH guests use PV nics Roger Pau Monne
2017-09-07 10:16 ` [PATCH v2 17/21] libxl: remove device model "none" support from stream functions Roger Pau Monne
2017-09-07 10:16 ` [PATCH v2 18/21] libxl: add PVH support to USB Roger Pau Monne
2017-09-07 10:16 ` [PATCH v2 19/21] libxl: add PVH support to x86 functions Roger Pau Monne
2017-09-07 10:16 ` [PATCH v2 20/21] xl: add PVH as a guest type Roger Pau Monne
2017-09-07 10:16 ` [PATCH v2 21/21] libxl: remove device model "none" from IDL Roger Pau Monne

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.