Re: [Qemu-devel] [Xen-devel] [XEN][RFC PATCH 12/15] xl: Add interface to handle multiple device models

[Ian Campbell <Ian.Campbell@citrix.com>]

On Thu, 2012-03-22 at 15:59 +0000, Julien Grall wrote:
> This patch add a structure with contain all informations about
> a device model.
> 
> Signed-off-by: Julien Grall <julien.grall@citrix.com>
> ---
>  tools/libxl/libxl.h          |    4 ++--
>  tools/libxl/libxl_internal.h |    1 +
>  tools/libxl/libxl_types.idl  |   11 +++++++++++
>  3 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 6b69030..a347a34 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -357,9 +357,9 @@ typedef struct {
>  typedef struct {
>      libxl_domain_create_info c_info;
>      libxl_domain_build_info b_info;
> -
>      int num_disks, num_vifs, num_pcidevs, num_vfbs, num_vkbs;
> -
> +    int num_dms;
> +    libxl_dm *dms;
>      libxl_device_disk *disks;
>      libxl_device_nic *vifs;
>      libxl_device_pci *pcidevs;
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index e0a1070..247bdb9 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -767,6 +767,7 @@ typedef struct {
>      char *dom_path; /* from libxl_malloc, only for libxl_spawner_record_pid */
>      const char *pid_path; /* only for libxl_spawner_record_pid */
>      int domid;
> +    uint32_t dmid;
>      libxl__spawn_starting *for_spawn;
>  } libxl__spawner_starting;
>  
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 413a1a6..7e48817 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -37,6 +37,7 @@ libxl_domain_type = Enumeration("domain_type", [
>  libxl_device_model_version = Enumeration("device_model_version", [
>      (1, "QEMU_XEN_TRADITIONAL"), # Historical qemu-xen device model (qemu-dm)
>      (2, "QEMU_XEN"),             # Upstream based qemu-xen device model
> +    (3, "MULTIPLE_QEMU_XEN"),    # Handle multiple dm

Isn't this implicit in the provision or otherwise of num_dms?

>      ])
>  
>  libxl_console_type = Enumeration("console_type", [
> @@ -224,6 +225,15 @@ libxl_domain_create_info = Struct("domain_create_info",[
>  
>  MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
>  
> +libxl_dm = Struct("dm", [
> +    ("id",           uint32),
> +    ("name",         string),
> +    ("path",         string),
> +    ("pcis",         libxl_string_list),
> +    ("mmios",        libxl_string_list),
> +    ("pios",         libxl_string_list),
> +    ])

Why does the user of libxl need to know the id? can't that be internal
to the library?

What are name and path? I guess path is something to do with xenstore
but isn't that also internal to the libxl<->dm interface not something
the caller of libxl need be aware of?

I'm not sure what syntax "pcis", "mmios" and "pios" are going to have
but I expect that this would be better represent as actual
datastructures rather than encoding it as a string.

How are toolstack supposed to know the values for e.g. pcis?

All in all this seems like a very raw/low-level interface. Can libxl not
expose something a bit more meaningful to toolstack authors? For example
if we consider emulated disk controllers then perhaps the options are
      * Handled by the "primary" dm
      * Handled by a single disaggregated dm
      * Handled by multiple disaggregated dm's (one per disk controller)
Similarly for other classes or emulated device. Or maybe this should be
a flag on those actual devices (e.g. in libxl_device_FOO)?

> +
>  # Instances of libxl_file_reference contained in this struct which
>  # have been mapped (with libxl_file_reference_map) will be unmapped
>  # by libxl_domain_build/restore. If either of these are never called
> @@ -289,6 +299,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>                                         ("usbdevice",        string),
>                                         ("soundhw",          string),
>                                         ("xen_platform_pci", libxl_defbool),
> +                                       ("max_servers",     integer),

As a toolstack author how do I decide what number to use here?

Ian.