Patchwork [XEN,RFC,V2,14/17] xl-parsing: Parse new device_models option

login
register
mail settings
Submitter Julien Grall
Date Aug. 22, 2012, 12:32 p.m.
Message ID <557fe87e4a6c0defdc6549e23e8e5e7b2ebb7a9f.1345552068.git.julien.grall@citrix.com>
Download mbox | patch
Permalink /patch/179393/
State New
Headers show

Comments

Julien Grall - Aug. 22, 2012, 12:32 p.m.
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' ]

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

Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
 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
Ian Campbell - Aug. 23, 2012, 1:35 p.m.
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@citrix.com>
> ---
>  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);            \
Julien Grall - Aug. 24, 2012, 1:12 p.m.
On 08/23/2012 02:35 PM, Ian Campbell wrote:
> 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' ]
>>      
> iirc you can give multiple vifs -- what does that syntax look like?
>
>    
vifs=nic1;nic2

> 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?
>
>    
It's used for logging and in qemu log filename. It's not a mandatory.
>> Signed-off-by: Julien Grall<julien.grall@citrix.com>
>> ---
>>   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.
>    
It's nearly the same, except it's skip blank at the beginning
of a value.
For instance if we have 'foo;   bar', the function will return
['foo', 'bar'].

Patch

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)
+{
+    char *s, *saveptr;
+    const char *p;
+    libxl_string_list sl;
+
+    int i = 0, nr = 0;
+
+    s = strdup(str);
+    if (s == NULL) {
+        fprintf(stderr, "xlu_dm: unable to allocate memory\n");
+        exit(-1);
+    }
+
+    /* Count number of entries */
+    p = strtok_r(s, delim, &saveptr);
+    do {
+        nr++;
+    } while ((p = strtok_r(NULL, delim, &saveptr)));
+
+    free(s);
+
+    s = strdup(str);
+
+    sl = malloc((nr+1) * sizeof (char *));
+    if (sl == NULL) {
+        fprintf(stderr, "xlu_dm: unable to allocate memory\n");
+        exit(-1);
+    }
+
+    p = strtok_r(s, delim, &saveptr);
+    do {
+        assert(i < nr);
+        // Skip blank
+        while (*p == ' ')
+            p++;
+        sl[i] = strdup(p);
+        i++;
+    } while ((p = strtok_r(NULL, delim, &saveptr)));
+    sl[i] = NULL;
+
+    *psl = sl;
+
+    free(s);
+}
+
+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"))
+                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);            \