diff mbox

[RFC,V4,1/2] xen: pass kernel initrd to qemu

Message ID 1404198361-5795-2-git-send-email-cyliu@suse.com
State New
Headers show

Commit Message

Chunyan Liu July 1, 2014, 7:06 a.m. UTC
xen side patch to support xen HVM direct kernel boot:
support 'kernel', 'ramdisk', 'cmdline' (and 'root', 'extra' as well
which would be deprecated later) in HVM config file, parse config file,
pass -kernel, -initrd, -append parameters to qemu.

It's working with qemu-xen when using the default BIOS (seabios).

[config example]
kernel="/mnt/vmlinuz-3.0.13-0.27-default"
ramdisk="/mnt/initrd-3.0.13-0.27-default"
root="/dev/hda2"
extra="console=tty0 console=ttyS0"

Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
Changes to v3:
  address to Ian J's comments:
  - 'root' and 'extra' might be useful for future
    extension, so drop 'cmdline' replacing 'root' and 'extra'
    implementation introduced in v3, keep existing way.
  - move 'kernel','ramdisk','cmdline' to libxl_domain_build_info common
    areas, rather then adding to .u.hvm. But for compatibility, keep
    .u.pv.kernel, .u.pv.ramdisk, .u.pv.cmdline.
  - update libxl.h to indicate the new changes: add
    LIBXL_HAVE_BUILDINFO_KERNEL
    LIBXL_HAVE_BUILDINFO_RAMDISK 
    LIBXL_HAVE_BUILDINFO_CMDLINE
  - update description in man page to make it more clear

 docs/man/xl.cfg.pod.5       | 53 +++++++++++++++++++++++++----------------
 tools/libxl/libxl.h         | 38 ++++++++++++++++++++++++++++++
 tools/libxl/libxl_dm.c      | 15 ++++++++++++
 tools/libxl/libxl_types.idl |  3 +++
 tools/libxl/xl_cmdimpl.c    | 57 ++++++++++++++++++++++++++++-----------------
 5 files changed, 124 insertions(+), 42 deletions(-)

Comments

Ian Campbell July 2, 2014, 2:47 p.m. UTC | #1
On Tue, 2014-07-01 at 15:06 +0800, Chunyan Liu wrote:
> xen side patch to support xen HVM direct kernel boot:
> support 'kernel', 'ramdisk', 'cmdline' (and 'root', 'extra' as well
> which would be deprecated later) in HVM config file, parse config file,
> pass -kernel, -initrd, -append parameters to qemu.
> 
> It's working with qemu-xen when using the default BIOS (seabios).
> 
> [config example]
> kernel="/mnt/vmlinuz-3.0.13-0.27-default"
> ramdisk="/mnt/initrd-3.0.13-0.27-default"
> root="/dev/hda2"
> extra="console=tty0 console=ttyS0"
> 
> Signed-off-by: Chunyan Liu <cyliu@suse.com>
> ---
> Changes to v3:
>   address to Ian J's comments:
>   - 'root' and 'extra' might be useful for future
>     extension, so drop 'cmdline' replacing 'root' and 'extra'
>     implementation introduced in v3, keep existing way.
>   - move 'kernel','ramdisk','cmdline' to libxl_domain_build_info common
>     areas, rather then adding to .u.hvm. But for compatibility, keep
>     .u.pv.kernel, .u.pv.ramdisk, .u.pv.cmdline.

For compatibility you also need to propagate u.pv.* into the top level
options when they are set but the toplevel one isn't. The right place to
do this is probably libxl__domain_build_info_setdefaults.

The important thing is that applications which have not updated to the
new interface must continue to work.

> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 69ceac8..a45415d 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -493,6 +493,44 @@ typedef struct libxl__ctx libxl_ctx;
>   */
>  #define LIBXL_HAVE_DEVICE_PCI_SEIZE 1
>  
> +/*
> + * LIBXL_HAVE_BUILDINFO_KERNEL

In general we try and collapse multiple of these LIBXL_HAVE together if
the features were added at the same time. I think a single
LIBLX_HAVE_BUILDINFO_KERNEL or so to cover kernel, ramdisk and cmdline
would do.

> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index be041f2..123d84d 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -693,6 +693,29 @@ static void parse_top_level_sdl_options(XLU_Config *config,
>      xlu_cfg_replace_string (config, "xauthority", &sdl->xauthority, 0);
>  }
>  
> +static char *parse_cmdline(XLU_Config *config)
> +{
> +    char *cmdline = NULL;
> +    const char *root = NULL, *extra = "";
> +
> +    xlu_cfg_get_string (config, "root", &root, 0);
> +    xlu_cfg_get_string (config, "extra", &extra, 0);

I see you've dropped "cmdline" here, presumably because of Ian J's
comments. Sorry that we are disagreeing here with you caught in the
middle.

I still think cmdline= is a good improvement, but I suggest you wait
until Ian and I have agreed one way or the other before making any
change.

Sorry again.

Ian.
Ian Campbell July 2, 2014, 3:16 p.m. UTC | #2
On Tue, 2014-07-01 at 15:06 +0800, Chunyan Liu wrote:
> xen side patch to support xen HVM direct kernel boot:
> support 'kernel', 'ramdisk', 'cmdline' (and 'root', 'extra' as well
> which would be deprecated later) in HVM config file, parse config file,
> pass -kernel, -initrd, -append parameters to qemu.
> 
> It's working with qemu-xen when using the default BIOS (seabios).
> 
> [config example]
> kernel="/mnt/vmlinuz-3.0.13-0.27-default"
> ramdisk="/mnt/initrd-3.0.13-0.27-default"
> root="/dev/hda2"
> extra="console=tty0 console=ttyS0"

Is this example complete? I think it will create a PV guest which isn't
what you are trying to demonstrate.

Ian.
Chunyan Liu July 3, 2014, 2:34 a.m. UTC | #3
>>> On 7/2/2014 at 11:16 PM, in message
<1404314181.8137.7.camel@kazak.uk.xensource.com>, Ian Campbell
<Ian.Campbell@citrix.com> wrote: 
> On Tue, 2014-07-01 at 15:06 +0800, Chunyan Liu wrote: 
> > xen side patch to support xen HVM direct kernel boot: 
> > support 'kernel', 'ramdisk', 'cmdline' (and 'root', 'extra' as well 
> > which would be deprecated later) in HVM config file, parse config file, 
> > pass -kernel, -initrd, -append parameters to qemu. 
> >  
> > It's working with qemu-xen when using the default BIOS (seabios). 
> >  
> > [config example] 
> > kernel="/mnt/vmlinuz-3.0.13-0.27-default" 
> > ramdisk="/mnt/initrd-3.0.13-0.27-default" 
> > root="/dev/hda2" 
> > extra="console=tty0 console=ttyS0" 
>  
> Is this example complete? I think it will create a PV guest which isn't 
> what you are trying to demonstrate. 

This is a HVM example.
For PV guest, it already supports direct kernel boot before this patch, so
use as before. No changes in config file.
I'll update the examples after readding 'cmdline' parameter.

Thank you.
-Chunyan

>  
> Ian. 
>  
>  
> _______________________________________________ 
> Xen-devel mailing list 
> Xen-devel@lists.xen.org 
> http://lists.xen.org/xen-devel 
>  
>
Ian Campbell July 3, 2014, 8:57 a.m. UTC | #4
On Wed, 2014-07-02 at 20:34 -0600, Chun Yan Liu wrote:
> 
> >>> On 7/2/2014 at 11:16 PM, in message
> <1404314181.8137.7.camel@kazak.uk.xensource.com>, Ian Campbell
> <Ian.Campbell@citrix.com> wrote: 
> > On Tue, 2014-07-01 at 15:06 +0800, Chunyan Liu wrote: 
> > > xen side patch to support xen HVM direct kernel boot: 
> > > support 'kernel', 'ramdisk', 'cmdline' (and 'root', 'extra' as well 
> > > which would be deprecated later) in HVM config file, parse config file, 
> > > pass -kernel, -initrd, -append parameters to qemu. 
> > >  
> > > It's working with qemu-xen when using the default BIOS (seabios). 
> > >  
> > > [config example] 
> > > kernel="/mnt/vmlinuz-3.0.13-0.27-default" 
> > > ramdisk="/mnt/initrd-3.0.13-0.27-default" 
> > > root="/dev/hda2" 
> > > extra="console=tty0 console=ttyS0" 
> >  
> > Is this example complete? I think it will create a PV guest which isn't 
> > what you are trying to demonstrate. 
> 
> This is a HVM example.

But none of the parameters given would cause it to be so, would they? I
think if you were to paste exactly that into a cfg file you would end up
with a PV guest.

Specifically it is missing: builder="hvm"

Ian.
diff mbox

Patch

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index c087cbc..5833908 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -304,6 +304,37 @@  Action to take if the domain crashes.  Default is C<destroy>.
 
 =back
 
+=head3 Direct Kernel Boot
+
+Direct kernel boot allows booting directly from a kernel and initrd
+stored in the host physical machine OS, allowing command line arguments
+to be passed directly. PV guest direct kernel boot is supported. HVM
+guest direct kernel boot is supported with limitation (it's supported
+when using qemu-xen and default BIOS 'seabios'; not supported in case of
+stubdom-dm and old rombios.)
+
+=over 4
+
+=item B<kernel="PATHNAME">
+
+Load the specified file as the kernel image.
+
+=item B<ramdisk="PATHNAME">
+
+Load the specified file as the ramdisk.
+
+=item B<root="STRING">
+
+Append B<root="STRING"> to the kernel command line (Note: it is guest
+specific what meaning this has).
+
+=item B<extra="STRING">
+
+Append B<STRING> to the kernel command line. (Note: it is guest
+specific what meaning this has).
+
+=back
+
 =head3 Other Options
 
 =over 4
@@ -646,20 +677,12 @@  The following options apply only to Paravirtual guests.
 
 =over 4
 
-=item B<kernel="PATHNAME">
-
-Load the specified file as the kernel image.  Either B<kernel> or
-B<bootloader> must be specified for PV guests.
-
-=item B<ramdisk="PATHNAME">
-
-Load the specified file as the ramdisk.
-
 =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.
+grub/grub2/syslinux. Either B<kernel> or B<bootloader> must be specified
+for PV guests.
 
 =item B<bootloader_args=[ "ARG", "ARG", ...]>
 
@@ -667,16 +690,6 @@  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 (this second option is deprecated).
 
-=item B<root="STRING">
-
-Append B<root="STRING"> to the kernel command line (Note: it is guest
-specific what meaning this has).
-
-=item B<extra="STRING">
-
-Append B<STRING> to the kernel command line. Note: it is guest
-specific what meaning this has).
-
 =item B<e820_host=BOOLEAN>
 
 Selects whether to expose the host e820 (memory map) to the guest via
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 69ceac8..a45415d 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -493,6 +493,44 @@  typedef struct libxl__ctx libxl_ctx;
  */
 #define LIBXL_HAVE_DEVICE_PCI_SEIZE 1
 
+/*
+ * LIBXL_HAVE_BUILDINFO_KERNEL
+ *
+ * If this is defined, then the libxl_domain_build_info structure will
+ * contain 'kernel' string field. It is to indicate kernel image location,
+ * for both PV and HVM guest to do direct kernel boot. For compatibility,
+ * u.pv.kernel still exists, but it might be deprecated in future.
+ *
+ * If it is set, guest will be booted from the indicated kernel.
+ */
+#define LIBXL_HAVE_BUILDINFO_KERNEL 1
+
+/*
+ * LIBXL_HAVE_BUILDINFO_RAMDISK
+ *
+ * If this is defined, then the libxl_domain_build_info structure will
+ * contain 'ramdisk' string field. It is to indicate ramdisk location,
+ * for both PV and HVM guest to do direct kernel boot. For compatibility,
+ * u.pv.ramdisk still exists, but it might be deprecated in future.
+ *
+ * If it is set, guest will be booted from indicated kernel and ramdisk.
+ */
+#define LIBXL_HAVE_BUILDINFO_RAMDISK 1
+
+/*
+ * LIBXL_HAVE_BUILDINFO_CMDLINE
+ *
+ * If this is defined, then the libxl_domain_build_info structure will
+ * contain 'cmdline' string field. It is a string to indicate paramters
+ * which would be appened to kernel image, for both PV and HVM guest to
+ * do direct kernel boot. For compatibility, u.pv.cmdline still exists,
+ * but it might be deprecated in future.
+ *
+ * If it is set, guest will be booted from indicated kernel with
+ * parameters as indicated in 'cmdline'.
+ */
+#define LIBXL_HAVE_BUILDINFO_CMDLINE 1
+
 /* Functions annotated with LIBXL_EXTERNAL_CALLERS_ONLY may not be
  * called from within libxl itself. Callers outside libxl, who
  * do not #include libxl_internal.h, are fine. */
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index addacdb..4f1cbd2 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -196,6 +196,12 @@  static char ** libxl__build_device_model_args_old(libxl__gc *gc,
         int nr_set_cpus = 0;
         char *s;
 
+        if (b_info->kernel) {
+            LOG(ERROR, "HVM direct kernel boot is not supported by "
+                "qemu-xen-traditional");
+            return NULL;
+        }
+
         if (b_info->u.hvm.serial) {
             flexarray_vappend(dm_args, "-serial", b_info->u.hvm.serial, NULL);
         }
@@ -479,6 +485,15 @@  static char ** libxl__build_device_model_args_new(libxl__gc *gc,
     if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
         int ioemu_nics = 0;
 
+        if (b_info->kernel)
+            flexarray_vappend(dm_args, "-kernel", b_info->kernel, NULL);
+
+        if (b_info->ramdisk)
+            flexarray_vappend(dm_args, "-initrd", b_info->ramdisk, NULL);
+
+        if (b_info->cmdline)
+            flexarray_vappend(dm_args, "-append", b_info->cmdline, NULL);
+
         if (b_info->u.hvm.serial) {
             flexarray_vappend(dm_args, "-serial", b_info->u.hvm.serial, NULL);
         }
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 1018142..66ba18b 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -338,6 +338,9 @@  libxl_domain_build_info = Struct("domain_build_info",[
     ("iomem",            Array(libxl_iomem_range, "num_iomem")),
     ("claim_mode",	     libxl_defbool),
     ("event_channels",   uint32),
+    ("kernel",           string),
+    ("cmdline",          string),
+    ("ramdisk",          string),
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
                                        ("bios",             libxl_bios_type),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index be041f2..123d84d 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -693,6 +693,29 @@  static void parse_top_level_sdl_options(XLU_Config *config,
     xlu_cfg_replace_string (config, "xauthority", &sdl->xauthority, 0);
 }
 
+static char *parse_cmdline(XLU_Config *config)
+{
+    char *cmdline = NULL;
+    const char *root = NULL, *extra = "";
+
+    xlu_cfg_get_string (config, "root", &root, 0);
+    xlu_cfg_get_string (config, "extra", &extra, 0);
+
+    if (root) {
+        if (asprintf(&cmdline, "root=%s %s", root, extra) == -1)
+            cmdline = NULL;
+    } else {
+        cmdline = strdup(extra);
+    }
+
+    if ((root || extra) && !cmdline) {
+        fprintf(stderr, "Failed to allocate memory for cmdline\n");
+        exit(1);
+    }
+
+    return cmdline;
+}
+
 static void parse_config_data(const char *config_source,
                               const char *config_data,
                               int config_len,
@@ -942,13 +965,21 @@  static void parse_config_data(const char *config_source,
     if (!xlu_cfg_get_long(config, "max_event_channels", &l, 0))
         b_info->event_channels = l;
 
+    xlu_cfg_replace_string (config, "kernel", &b_info->kernel, 0);
+    xlu_cfg_replace_string (config, "ramdisk", &b_info->ramdisk, 0);
+    b_info->cmdline = parse_cmdline(config);
+
     xlu_cfg_get_defbool(config, "driver_domain", &c_info->driver_domain, 0);
 
     switch(b_info->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
-        if (!xlu_cfg_get_string (config, "kernel", &buf, 0))
-            fprintf(stderr, "WARNING: ignoring \"kernel\" directive for HVM guest. "
-                    "Use \"firmware_override\" instead if you really want a non-default firmware\n");
+        if (!strcmp(libxl_basename(b_info->kernel), "hvmloader")) {
+            fprintf(stderr, "WARNING: you seem to be using \"kernel\" "
+                    "directive to override HVM guest firmware. Ignore "
+                    "that. Use \"firmware_override\" instead if you "
+                    "really want a non-default firmware\n");
+            b_info->kernel = NULL;
+        }
 
         xlu_cfg_replace_string (config, "firmware_override",
                                 &b_info->u.hvm.firmware, 0);
@@ -1000,26 +1031,8 @@  static void parse_config_data(const char *config_source,
         break;
     case LIBXL_DOMAIN_TYPE_PV:
     {
-        char *cmdline = NULL;
-        const char *root = NULL, *extra = "";
-
         xlu_cfg_replace_string (config, "kernel", &b_info->u.pv.kernel, 0);
 
-        xlu_cfg_get_string (config, "root", &root, 0);
-        xlu_cfg_get_string (config, "extra", &extra, 0);
-
-        if (root) {
-            if (asprintf(&cmdline, "root=%s %s", root, extra) == -1)
-                cmdline = NULL;
-        } else {
-            cmdline = strdup(extra);
-        }
-
-        if ((root || extra) && !cmdline) {
-            fprintf(stderr, "Failed to allocate memory for cmdline\n");
-            exit(1);
-        }
-
         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))
@@ -1047,7 +1060,7 @@  static void parse_config_data(const char *config_source,
             exit(1);
         }
 
-        b_info->u.pv.cmdline = cmdline;
+        b_info->u.pv.cmdline = parse_cmdline(config);
         xlu_cfg_replace_string (config, "ramdisk", &b_info->u.pv.ramdisk, 0);
         break;
     }