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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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@xxxxxxxx>
> ---
> 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.


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