xl: make bootloader_args a list

This is much more natural. Continue to support the old syntax in xl
but deprecate it.

[ Fixed up a long line in xl_cmdimpl.c. -iwj ]

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>

4 files changed, 81 insertions, 17 deletions

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 7748922e7d..33a3256c22 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -321,10 +321,11 @@ Run C<PROGRAM> to find the kernel image and ramdisk to use.  Normally
 C<PROGRAM> would be C<pygrub>, which is an emulation of
 grub/grub2/syslinux.
 
-=item B<bootloader_args=STRING>
+=item B<bootloader_args=[ "ARG", "ARG", ...]>
 
-Append B<STRING> (split into words at whitespace) to the arguments to
-the B<bootloader> program.  XXX this should be a list of strings.
+Append B<ARG>s to the arguments to the B<bootloader>
+program. Alternatively if the argument is a simple string then it will
+be split into words at whitespace (this second option is deprecated).
 
 =item B<root="STRING">
 
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index 9ca364e39d..47bb3a1bcf 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -58,13 +58,9 @@ static char **make_bootloader_args(libxl__gc *gc,
     flexarray_set(args, nr++, libxl__sprintf(gc, "--output-directory=%s", "/var/run/libxl/"));
 
     if (info->u.pv.bootloader_args) {
-        char *saveptr;
-        /* Operate on a duplicate since strtok modifes the argument */
-        char *dup = libxl__strdup(gc, info->u.pv.bootloader_args);
-        char *t = strtok_r(dup, " \t\n", &saveptr);
-        do {
-            flexarray_set(args, nr++, t);
-        } while ((t = strtok_r(NULL, " \t\n", &saveptr)));
+        char *p = info->u.pv.bootloader_args[0];
+        while (*(p++))
+            flexarray_set(args, nr++, p);
     }
 
     flexarray_set(args, nr++, disk);
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index dffc453c03..d59d2cb86d 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -187,7 +187,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
                  ("pv", Struct(None, [("kernel", libxl_file_reference),
                                       ("slack_memkb", uint32),
                                       ("bootloader", string),
-                                      ("bootloader_args", string),
+                                      ("bootloader_args", libxl_string_list),
                                       ("cmdline", string),
                                       ("ramdisk", libxl_file_reference),
                                       ("features", string, True),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 234a7b4757..f1e729c6a2 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -334,9 +334,14 @@ static void printf_info(int domid,
     printf("\t(nomigrate %d)\n", b_info->disable_migrate);
 
     if (c_info->type == LIBXL_DOMAIN_TYPE_PV && b_info->u.pv.bootloader) {
+        int i;
         printf("\t(bootloader %s)\n", b_info->u.pv.bootloader);
-        if (b_info->u.pv.bootloader_args)
-            printf("\t(bootloader_args %s)\n", b_info->u.pv.bootloader_args);
+        if (b_info->u.pv.bootloader_args) {
+            printf("\t(bootloader_args");
+            for (i=0; b_info->u.pv.bootloader_args[i]; i++)
+                printf(" %s", b_info->u.pv.bootloader_args[i]);
+            printf(")\n");
+        }
     }
 
     printf("\t(image\n");
@@ -515,6 +520,51 @@ static void parse_disk_config(XLU_Config **config, const char *spec,
     parse_disk_config_multistring(config, 1, &spec, disk);
 }
 
+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, "unable to allocate memory to parse bootloader args\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, "unable to allocate memory for bootloader args\n");
+        exit(-1);
+    }
+
+    p = strtok_r(s, delim, &saveptr);
+    do {
+        assert(i < nr);
+        sl[i] = strdup(p);
+        i++;
+    } while ((p = strtok_r(NULL, delim, &saveptr)));
+    sl[i] = NULL;
+
+    *psl = sl;
+
+    free(s);
+}
+
 static void parse_config_data(const char *configfile_filename_report,
                               const char *configfile_data,
                               int configfile_len,
@@ -739,10 +789,27 @@ static void parse_config_data(const char *configfile_filename_report,
             exit(1);
         }
 
-        xlu_cfg_replace_string (config, "bootloader",
-                                &b_info->u.pv.bootloader, 0);
-        xlu_cfg_replace_string (config, "bootloader_args",
-                                &b_info->u.pv.bootloader_args, 0);
+        xlu_cfg_replace_string (config, "bootloader", &b_info->u.pv.bootloader, 0);
+        switch (xlu_cfg_get_list_as_string_list(config, "bootloader_args",
+                                      &b_info->u.pv.bootloader_args, 1))
+        {
+
+        case 0: break; /* Success */
+        case ESRCH: break; /* Option not present */
+        case EINVAL:
+            if (!xlu_cfg_get_string(config, "bootloader_args", &buf, 0)) {
+
+                fprintf(stderr, "WARNING: Specifying \"bootloader_args\""
+                        " as a string is deprecated. "
+                        "Please use a list of arguments.\n");
+                split_string_into_string_list(buf, " \t\n",
+                                              &b_info->u.pv.bootloader_args);
+            }
+            break;
+        default:
+            fprintf(stderr,"xl: Unable to parse bootloader_args.\n");
+            exit(-ERROR_FAIL);
+        }
 
         if (!b_info->u.pv.bootloader && !b_info->u.pv.kernel.path) {
             fprintf(stderr, "Neither kernel nor bootloader specified\n");