Re: [PATCH] tools/xl: Add stubdomain_cmdline option to xl.cfg

[Julien Grall <julien@xxxxxxx>]

Hi Scott,

On 20/07/2021 18:56, Scott Davis wrote:
> This adds an option to the xl domain configuration file syntax for specifying
> a kernel command line for device-model stubdomains. It is intended for use with
> Linux-based stubdomains.
May I ask why embedding the command line in the kernel would not be a 
solution? Do you expect it to change from stubdom to stubdom?
> Signed-off-by: Scott Davis <scott.davis@xxxxxxxxxx>
> ---
>   docs/man/xl.cfg.5.pod.in             | 4 ++++
>   tools/golang/xenlight/helpers.gen.go | 3 +++
>   tools/golang/xenlight/types.gen.go   | 1 +
>   tools/libs/light/libxl_dm.c          | 1 +
>   tools/libs/light/libxl_types.idl     | 1 +
>   tools/xl/xl_parse.c                  | 2 ++
>   6 files changed, 12 insertions(+)
>
> diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
> index 56370a37db..6c777cad5c 100644
> --- a/docs/man/xl.cfg.5.pod.in
> +++ b/docs/man/xl.cfg.5.pod.in
> @@ -2742,6 +2742,10 @@ In case of B<qemu-xen-traditional> it is expected to be 
> MiniOS-based stubdomain
>   image, in case of B<qemu-xen> it is expected to be Linux-based stubdomain
>   kernel.
>   
> +=item B<stubdomain_cmdline="STRING">
> +
> +Append B<STRING> to the device-model stubdomain kernel command line.
> +
>   =item B<stubdomain_ramdisk="PATH">
>   
>   Override the path to the ramdisk image used as device-model stubdomain.
> diff --git a/tools/golang/xenlight/helpers.gen.go 
> b/tools/golang/xenlight/helpers.gen.go
> index db82537b42..bfc1e7f312 100644
> --- a/tools/golang/xenlight/helpers.gen.go
> +++ b/tools/golang/xenlight/helpers.gen.go
> @@ -1018,6 +1018,7 @@ return fmt.Errorf("converting field DeviceModelStubdomain: 
> %v", err)
>   }
>   x.StubdomainMemkb = uint64(xc.stubdomain_memkb)
>   x.StubdomainKernel = C.GoString(xc.stubdomain_kernel)
> +x.StubdomainCmdline = C.GoString(xc.stubdomain_cmdline)
>   x.StubdomainRamdisk = C.GoString(xc.stubdomain_ramdisk)
>   x.DeviceModel = C.GoString(xc.device_model)
>   x.DeviceModelSsidref = uint32(xc.device_model_ssidref)
> @@ -1344,6 +1345,8 @@ return fmt.Errorf("converting field DeviceModelStubdomain: 
> %v", err)
>   xc.stubdomain_memkb = C.uint64_t(x.StubdomainMemkb)
>   if x.StubdomainKernel != "" {
>   xc.stubdomain_kernel = C.CString(x.StubdomainKernel)}
> +if x.StubdomainCmdline != "" {
> +xc.stubdomain_cmdline = C.CString(x.StubdomainCmdline)}
>   if x.StubdomainRamdisk != "" {
>   xc.stubdomain_ramdisk = C.CString(x.StubdomainRamdisk)}
>   if x.DeviceModel != "" {
> diff --git a/tools/golang/xenlight/types.gen.go 
> b/tools/golang/xenlight/types.gen.go
> index a214dd9df6..09a3bb67e2 100644
> --- a/tools/golang/xenlight/types.gen.go
> +++ b/tools/golang/xenlight/types.gen.go
> @@ -483,6 +483,7 @@ DeviceModelVersion DeviceModelVersion
>   DeviceModelStubdomain Defbool
>   StubdomainMemkb uint64
>   StubdomainKernel string
> +StubdomainCmdline string
>   StubdomainRamdisk string
>   DeviceModel string
>   DeviceModelSsidref uint32
> diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
> index dbd3c7f278..2d54596834 100644
> --- a/tools/libs/light/libxl_dm.c
> +++ b/tools/libs/light/libxl_dm.c
> @@ -2373,6 +2373,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc, 
> libxl__stub_dm_spawn_state *sdss)
>       }
>   
>       stubdom_state->pv_kernel.path = guest_config->b_info.stubdomain_kernel;
> +    stubdom_state->pv_cmdline = guest_config->b_info.stubdomain_cmdline;
>       stubdom_state->pv_ramdisk.path = guest_config->b_info.stubdomain_ramdisk;
>   
>       /* fixme: this function can leak the stubdom if it fails */
> diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
> index f45adddab0..e782e15cf2 100644
> --- a/tools/libs/light/libxl_types.idl
> +++ b/tools/libs/light/libxl_types.idl
> @@ -523,6 +523,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>       ("device_model_stubdomain", libxl_defbool),
>       ("stubdomain_memkb",   MemKB),
>       ("stubdomain_kernel",  string),
> +    ("stubdomain_cmdline", string),
Please add a LIBXL_HAVE_... in include/libxl.h. This is used by external 
toolstack (e.g. libvirt) to know whether a given version of libxl 
provide the field.
>       ("stubdomain_ramdisk", string),
>       # if you set device_model you must set device_model_version too
>       ("device_model",     string),
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index 9fb0791429..17dddb4cd5 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -2533,6 +2533,8 @@ skip_usbdev:
>   
>       xlu_cfg_replace_string (config, "stubdomain_kernel",
>                               &b_info->stubdomain_kernel, 0);
> +    xlu_cfg_replace_string (config, "stubdomain_cmdline",
> +                            &b_info->stubdomain_cmdline, 0);
>       xlu_cfg_replace_string (config, "stubdomain_ramdisk",
>                               &b_info->stubdomain_ramdisk, 0);
>       if (!xlu_cfg_get_long (config, "stubdomain_memory", &l, 0))
Cheers,

--
Julien Grall