Re: [Qemu-devel] [RFC PATCH V2 1/2] xen: pass kernel initrd to qemu

[Ian Campbell]

On Wed, 2014-06-04 at 15:34 +0800, Chunyan Liu wrote:

Please can you remember to CC Ian Jackson on these patches in the future
as a) he's a toolstack maintainer and b) he's the one doing work on
upstream qemu stubdom so he may have concerns about this stuff.

Thanks,
Ian.

> xen side patch to support xen HVM direct kernel boot:
> support 'kernel', 'ramdisk', 'root', 'extra' in HVM config file,
> parse config file, pass -kernel, -initrd, -append parameters to qemu.
> It's working with seabios and non-stubdom. Rombios and stubdom cases
> are currently not supported.
> 
> [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"
> disk=[ 'file:/mnt/images/bjz_04_sles11_sp2/disk0.raw,hda,w', ]
> 
> Signed-off-by: Chunyan Liu <address@hidden>
> ---
> Changes:
>   * update man page to document the new parameters for HVM guests (move them
>     from PV special options to general options) and note current limitation 
>   * rombios and stubdom are not working yet, add libxl error messages
>     to inform that.
>   * extract "parse commandline" code to a common helper for both HVM and
>     PV parse_config_data to use.
> 
>  docs/man/xl.cfg.pod.5       | 50 ++++++++++++++++++++++++----------------
>  tools/libxl/libxl_dm.c      | 15 ++++++++++++
>  tools/libxl/libxl_types.idl |  3 +++
>  tools/libxl/xl_cmdimpl.c    | 56 
> +++++++++++++++++++++++++++------------------
>  4 files changed, 82 insertions(+), 42 deletions(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index 0ca37bc..c585801 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -304,6 +304,34 @@ Action to take if the domain crashes.  Default is 
> C<destroy>.
>  
>  =back
>  
> +=head3 Direct Kernel Boot
> +
> +Currently, direct kernel boot can be supported by PV guests, and HVM guests
> +in limitation. For HVM guests, in case of stubdom-dm and old rombios,
> +direct kernel boot is not supported.
> +
> +=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
> @@ -647,20 +675,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", ...]>
>  
> @@ -668,16 +688,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_dm.c b/tools/libxl/libxl_dm.c
> index 51ab2bf..c2eaa54 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->u.hvm.kernel) {
> +            LOG(ERROR, "%s: direct kernel boot is not supported by %s",
> +                __func__, dm);
> +            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->u.hvm.kernel)
> +            flexarray_vappend(dm_args, "-kernel", b_info->u.hvm.kernel, 
> NULL);
> +
> +        if (b_info->u.hvm.ramdisk)
> +            flexarray_vappend(dm_args, "-initrd", b_info->u.hvm.ramdisk, 
> NULL);
> +
> +        if (b_info->u.hvm.cmdline)
> +            flexarray_vappend(dm_args, "-append", b_info->u.hvm.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 52f1aa9..a96b228 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -336,6 +336,9 @@ libxl_domain_build_info = Struct("domain_build_info",[
>      ("event_channels",   uint32),
>      ("u", KeyedUnion(None, libxl_domain_type, "type",
>                  [("hvm", Struct(None, [("firmware",         string),
> +                                       ("kernel",           string),
> +                                       ("cmdline",          string),
> +                                       ("ramdisk",          string),
>                                         ("bios",             libxl_bios_type),
>                                         ("pae",              libxl_defbool),
>                                         ("apic",             libxl_defbool),
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 5195914..c3cadbb 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -725,6 +725,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,
> @@ -1007,9 +1030,16 @@ static void parse_config_data(const char 
> *config_source,
>  
>      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 (!xlu_cfg_get_string (config, "kernel", &buf, 0)) {
> +            if (strstr(buf, "hvmloader"))
> +                fprintf(stderr, "WARNING: ignoring \"kernel\" directive for 
> HVM guest. "
> +                        "Use \"firmware_override\" instead if you really 
> want a non-default firmware\n");
> +            else
> +                b_info->u.hvm.kernel = strdup(buf);
> +        }
> +
> +        b_info->u.hvm.cmdline = parse_cmdline(config);
> +        xlu_cfg_replace_string (config, "ramdisk", &b_info->u.hvm.ramdisk, 
> 0);
>  
>          xlu_cfg_replace_string (config, "firmware_override",
>                                  &b_info->u.hvm.firmware, 0);
> @@ -1061,26 +1091,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))
> @@ -1108,7 +1120,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;
>      }