Re: [Xen-devel] [PATCH] Add a device_model_machine option for HVM domains to libxl.

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 17 Jun 2013, Paul Durrant wrote:
> The device-model machine type used for HVM domains is currently hardcoded to
> 'xenfv'. This patch adds a device_model_machine option, which defaults to
> 'xenfv' for comatibility, but allows the specification of an alternative
> machine type 'pc'.
> Note that use of the 'pc' machine type relies on patches that I have
> submitted to qemu-devel.
> 
> Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

NACK

I don't think that this kind of option should exposed to the user except
maybe for debugging purposed.
We need a way to automatically select the right one.


>  docs/man/xl.cfg.pod.5       |   27 +++++++++++++++++++++++++++
>  tools/libxl/libxl_create.c  |    3 +++
>  tools/libxl/libxl_dm.c      |   27 ++++++++++++++++++++++-----
>  tools/libxl/libxl_types.idl |    7 +++++++
>  tools/libxl/xl_cmdimpl.c    |    9 +++++++++
>  5 files changed, 68 insertions(+), 5 deletions(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index b7d64a6..e066778 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -966,6 +966,9 @@ improves performance and is strongly recommended when 
> available. PV
>  drivers are available for various Operating Systems including HVM
>  Linux L<http://wiki.xen.org/wiki/XenLinuxPVonHVMdrivers> and Microsoft
>  Windows L<http://wiki.xen.org/wiki/XenWindowsGplPv>.
> +This option only has effect when using the qemu-xen-traditional
> +device-model, or when a device_model_machine value of something other
> +than xenfv is specified when using the upstream qemu-xen device-model.
>  
>  =item B<viridian=BOOLEAN>
>  
> @@ -1197,6 +1200,30 @@ you have existing guests then, depending on the nature 
> of the guest
>  Operating System, you may wish to force them to use the device
>  model which they were installed with.
>  
> +=item B<device_model_machine="MACHINE-TYPE">
> +
> +Selects which machine type should be specified to the device-model.
> +This option only takes effect when using the upstream qemu-xen
> +device-model.
> +Valid values are:
> +
> +=over 4
> +
> +=item B<xenfv>
> +
> +This is the default and implies creation of the Xen platform PCI
> +device.
> +
> +=item B<pc>
> +
> +Creation of the Xen platform PCI device is controlled by the
> +xen_platform_pci option.
> +
> +=back
> +
> +Use of the pc machine type may not work with versions of
> +the qemu-xen device-model prior to 1.6.
> +
>  =item B<device_model_override="PATH">
>  
>  Override the path to the binary to be used as the device-model. The
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 0c32d0b..2b84e9e 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -246,6 +246,9 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>          libxl_defbool_setdefault(&b_info->u.hvm.usb,                false);
>          libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci,   true);
>  
> +        if (b_info->u.hvm.machine == LIBXL_DEVICE_MODEL_HVM_MACHINE_DEFAULT)
> +            b_info->u.hvm.machine = LIBXL_DEVICE_MODEL_HVM_MACHINE_XENFV;
> +
>          if (!b_info->u.hvm.boot) {
>              b_info->u.hvm.boot = strdup("cda");
>              if (!b_info->u.hvm.boot) return ERROR_NOMEM;
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index ac1f90e..eb2ca46 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -570,18 +570,31 @@ static char ** 
> libxl__build_device_model_args_new(libxl__gc *gc,
>      }
>      for (i = 0; b_info->extra && b_info->extra[i] != NULL; i++)
>          flexarray_append(dm_args, b_info->extra[i]);
> -    flexarray_append(dm_args, "-M");
> +
> +    flexarray_append(dm_args, "-machine");
>      switch (b_info->type) {
>      case LIBXL_DOMAIN_TYPE_PV:
>          flexarray_append(dm_args, "xenpv");
>          for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
>              flexarray_append(dm_args, b_info->extra_pv[i]);
>          break;
> -    case LIBXL_DOMAIN_TYPE_HVM:
> -        flexarray_append(dm_args, "xenfv");
> +    case LIBXL_DOMAIN_TYPE_HVM: {
> +        char *machine = (char 
> *)libxl_device_model_hvm_machine_to_string(b_info->u.hvm.machine);
> +
> +        if (b_info->u.hvm.machine != LIBXL_DEVICE_MODEL_HVM_MACHINE_XENFV) {
> +            flexarray_append(dm_args, libxl__sprintf(gc, 
> "type=%s,accel=xen", machine));
> +            if (libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
> +                flexarray_append(dm_args, "-device");
> +                flexarray_append(dm_args, "xen-platform");
> +            }
> +        } else {
> +            flexarray_append(dm_args, libxl__sprintf(gc, "type=%s", 
> machine));
> +        }
> +
>          for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
>              flexarray_append(dm_args, b_info->extra_hvm[i]);
>          break;
> +    }
>      default:
>          abort();
>      }
> @@ -748,11 +761,15 @@ static int libxl__write_stub_dmargs(libxl__gc *gc,
>      i = 1;
>      dmargs[0] = '\0';
>      while (args[i] != NULL) {
> -        if (strcmp(args[i], "-sdl") && strcmp(args[i], "-M") && 
> strcmp(args[i], "xenfv")) {
> +        if (!strcmp(args[i], "-sdl"))
> +            i++;
> +        else if (!strcmp(args[i], "-M"))
> +            i += 2;
> +        else {
>              strcat(dmargs, " ");
>              strcat(dmargs, args[i]);
> +            i++;
>          }
> -        i++;
>      }
>      path = libxl__sprintf(gc, "%s/image/dmargs", vm_path);
>  
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index d218a2d..bf31249 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -39,6 +39,12 @@ libxl_device_model_version = 
> Enumeration("device_model_version", [
>      (2, "QEMU_XEN"),             # Upstream based qemu-xen device model
>      ])
>  
> +libxl_device_model_hvm_machine = Enumeration("device_model_hvm_machine", [
> +    (0, "DEFAULT"),
> +    (1, "XENFV"),
> +    (2, "PC"),
> +    ])
> +
>  libxl_console_type = Enumeration("console_type", [
>      (1, "SERIAL"),
>      (2, "PV"),
> @@ -331,6 +337,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>                                         ("usbdevice",        string),
>                                         ("soundhw",          string),
>                                         ("xen_platform_pci", libxl_defbool),
> +                                       ("machine",          
> libxl_device_model_hvm_machine),
>                                         ("usbdevice_list",   
> libxl_string_list),
>                                         ])),
>                   ("pv", Struct(None, [("kernel", string),
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 8a478ba..2fe5532 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1526,6 +1526,15 @@ skip_vfb:
>              exit (1);
>  
>          }
> +
> +        b_info->u.hvm.machine = LIBXL_DEVICE_MODEL_HVM_MACHINE_DEFAULT;
> +        if (!xlu_cfg_get_string (config, "device_model_machine", &buf, 0)) {
> +            if (libxl_device_model_hvm_machine_from_string(buf, 
> &b_info->u.hvm.machine) < 0) {
> +                fprintf(stderr,
> +                    "Unknown device_model_machine type \"%s\" specified\n", 
> buf);
> +                exit (1);
> +            }
> +        }
>      }
>  
>      xlu_cfg_destroy(config);
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel
> 

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