Re: [Xen-devel] [XEN][RFC PATCH V2 14/17] xl-parsing: Parse new device_models option

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-08-22 at 13:32 +0100, Julien Grall wrote:
> Add new option "device_models". The user can specify the capability of the
> QEMU (ui, vifs, ...). This option only works with QEMU upstream (qemu-xen).
> 
> For instance:
> device_models= [ 'name=all,vifs=nic1', 'name=qvga,ui', 'name=qide,ide' ]

Please can you patch docs/man/xl.cfg.pod.5 with a description of this
syntax. Possibly just a stub referencing
docs/man/xl-device-models.markdown in the same manner as
xl-disk-configuration.txt, xl-numa-placement.markdown,
xl-network-configuration.markdown etc.

iirc you can give multiple vifs -- what does that syntax look like?

I didn't ask before -- what does naming the dm give you? Is it just used
for ui things like logging or can you cross reference this in some way?

> Each device model can also take a path argument which override the default 
> one.
> It's usefull for debugging.

      useful
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> ---
>  tools/libxl/Makefile     |    2 +-
>  tools/libxl/libxlu_dm.c  |   96 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  tools/libxl/libxlutil.h  |    5 ++
>  tools/libxl/xl_cmdimpl.c |   29 +++++++++++++-
>  4 files changed, 130 insertions(+), 2 deletions(-)
>  create mode 100644 tools/libxl/libxlu_dm.c
> 
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index 47fb110..2b58721 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -79,7 +79,7 @@ AUTOINCS= libxlu_cfg_y.h libxlu_cfg_l.h _libxl_list.h 
> _paths.h \
>  AUTOSRCS= libxlu_cfg_y.c libxlu_cfg_l.c
>  AUTOSRCS += _libxl_save_msgs_callout.c _libxl_save_msgs_helper.c
>  LIBXLU_OBJS = libxlu_cfg_y.o libxlu_cfg_l.o libxlu_cfg.o \
> -     libxlu_disk_l.o libxlu_disk.o libxlu_vif.o libxlu_pci.o
> +     libxlu_disk_l.o libxlu_disk.o libxlu_vif.o libxlu_pci.o libxlu_dm.o
>  $(LIBXLU_OBJS): CFLAGS += $(CFLAGS_libxenctrl) # For xentoollog.h
>  
>  CLIENTS = xl testidl libxl-save-helper
> diff --git a/tools/libxl/libxlu_dm.c b/tools/libxl/libxlu_dm.c
> new file mode 100644
> index 0000000..9f0a347
> --- /dev/null
> +++ b/tools/libxl/libxlu_dm.c
> @@ -0,0 +1,96 @@
> +#include "libxl_osdeps.h" /* must come before any other headers */
> +#include <stdlib.h>
> +#include "libxlu_internal.h"
> +#include "libxlu_cfg_i.h"
> +
> +static void split_string_into_string_list(const char *str,
> +                                          const char *delim,
> +                                          libxl_string_list *psl)

Is this a cut-n-paste of the one in xl_cmdimpl.c or did it change?

Probably better to add this as a common utility function somewhere.

> +{
> [...]
> +}
> +
> +int xlu_dm_parse(XLU_Config *cfg, const char *spec,
> +                 libxl_dm *dm)
> +{
> +    char *buf = strdup(spec);
> +    char *p, *p2;
> +    int rc = 0;
> +
> +    p = strtok(buf, ",");
> +    if (!p)
> +        goto skip_dm;
> +    do {
> +        while (*p == ' ')
> +            p++;
> +        if ((p2 = strchr(p, '=')) == NULL) {
> +            if (!strcmp(p, "ui"))

libxl provides a libxl_BLAH_from_string for enums in the idl, which
might be helpful here?

> +                dm->capabilities |= LIBXL_DM_CAP_UI;
> +            else if (!strcmp(p, "ide"))
> +                dm->capabilities |= LIBXL_DM_CAP_IDE;
> +            else if (!strcmp(p, "serial"))
> +                dm->capabilities |= LIBXL_DM_CAP_SERIAL;
> +            else if (!strcmp(p, "audio"))
> +                dm->capabilities |= LIBXL_DM_CAP_AUDIO;
> +        } else {
> +            *p2 = '\0';
> +            if (!strcmp(p, "name"))
> +                dm->name = strdup(p2 + 1);
> +            else if (!strcmp(p, "path"))
> +                dm->path = strdup(p2 + 1);
> +            else if (!strcmp(p, "vifs"))
> +                split_string_into_string_list(p2 + 1, ";", &dm->vifs);
> +       }
> +    } while ((p = strtok(NULL, ",")) != NULL);
> +
> +    if (!dm->name && dm->path)
> +    {
> +        fprintf(stderr, "xl: Unable to parse device_deamon\n");
> +        exit(-ERROR_FAIL);
> +    }
> +skip_dm:
> +    free(buf);
> +
> +    return rc;
> +}
> diff --git a/tools/libxl/libxlutil.h b/tools/libxl/libxlutil.h
> index 0333e55..db22715 100644
> --- a/tools/libxl/libxlutil.h
> +++ b/tools/libxl/libxlutil.h
> @@ -93,6 +93,11 @@ int xlu_disk_parse(XLU_Config *cfg, int nspecs, const char 
> *const *specs,
>   */
>  int xlu_pci_parse_bdf(XLU_Config *cfg, libxl_device_pci *pcidev, const char 
> *str);
>  
> +/*
> + * Daemon specification parsing.
> + */
> +int xlu_dm_parse(XLU_Config *cfg, const char *spec,
> +                 libxl_dm *dm);
>  
>  /*
>   * Vif rate parsing.
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 138cd72..2a26fa4 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -561,7 +561,7 @@ static void parse_config_data(const char *config_source,
>      const char *buf;
>      long l;
>      XLU_Config *config;
> -    XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids;
> +    XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids, *dms;
>      int pci_power_mgmt = 0;
>      int pci_msitranslate = 0;
>      int pci_permissive = 0;
> @@ -995,6 +995,9 @@ static void parse_config_data(const char *config_source,
>                  } else if (!strcmp(p, "vifname")) {
>                      free(nic->ifname);
>                      nic->ifname = strdup(p2 + 1);
> +                } else if (!strcmp(p, "id")) {
> +                    free(nic->id);
> +                    nic->id = strdup(p2 + 1);
>                  } else if (!strcmp(p, "backend")) {
>                      if(libxl_name_to_domid(ctx, (p2 + 1), 
> &(nic->backend_domid))) {
>                          fprintf(stderr, "Specified backend domain does not 
> exist, defaulting to Dom0\n");
> @@ -1249,6 +1252,30 @@ skip_vfb:
>              }
>          }
>      }
> +
> +    d_config->num_dms = 0;
> +    d_config->dms = NULL;
> +
> +    if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
> +        && !xlu_cfg_get_list (config, "device_models", &dms, 0, 0)) {
> +        while ((buf = xlu_cfg_get_listitem (dms, d_config->num_dms)) != 
> NULL) {
> +            libxl_dm *dm;
> +            size_t size = sizeof (libxl_dm) * (d_config->num_dms + 1);
> +
> +            d_config->dms = (libxl_dm *)realloc (d_config->dms, size);
> +            if (!d_config->dms) {
> +                fprintf(stderr, "Can't realloc d_config->dms\n");
> +                exit (1);
> +            }
> +            dm = d_config->dms + d_config->num_dms;
> +            libxl_dm_init (dm);
> +            if (xlu_dm_parse(config, buf, dm)) {
> +                exit (-ERROR_FAIL);
> +            }
> +            d_config->num_dms++;
> +        }
> +    }
> +
>  #define parse_extra_args(type)                                            \
>      e = xlu_cfg_get_list_as_string_list(config, "device_model_args"#type, \
>                                      &b_info->extra##type, 0);            \



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