[PATCH v2 17/18] xl: use libxl_retrieve_domain_configuration and JSON format

[Wei Liu <wei.liu2@citrix.com>]

Before this change, xl stores domain configuration in "xl" format, which
is in fact a verbatim copy of user supplied domain config.

Now libxl provides a new API to retrieve domain configuration, switch to
that new API, store configuration in JSON format.

Tests done so far (xl.{new,old} denotes xl with{,out} "libxl-json"
support):

1. xl.new create then xl.new save, hexdump saved file: domain config
   saved in JSON format
2. xl.new create, xl.new save then xl.old restore: failed on
   mandatory flag check
3. xl.new create, xl.new save then xl.new restore: succeeded
4. xl.old create, xl.old save then xl.new restore: succeeded
5. xl.new create then local migrate, receiving end xl.new: succeeded
6. xl.old create then local migrate, receiving end xl.new: succeeded

Note that "xl" config is still supported and handled when restarting a
domain. "xl" config file takes precedence over "libxl-json" in that
case, so that user who uses "config-update" to store new config file
won't have regression. All other scenarios (migration, domain listing
etc.) now use the new API.

Lastly, print out warning when users invoke "config-update" to
discourage them from using this command.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 docs/man/xl.pod.1         |    4 ++
 tools/libxl/xl_cmdimpl.c  |  124 ++++++++++++++++++++++++++++-----------------
 tools/libxl/xl_cmdtable.c |    4 +-
 3 files changed, 84 insertions(+), 48 deletions(-)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 30bd4bf..23add95 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -187,6 +187,10 @@ immediate effect but will be applied when the guest is next
 restarted. This command is useful to ensure that runtime modifications
 made to the guest will be preserved when the guest is restarted.
 
+Libxl now has improved capabilities to handle dynamic domain configuration
+changes and will preserve any changes made a runtime when necessary. Therefore
+it should not normally be necessary to use this command any more.
+
 I<configfile> has to be an absolute path to a file.
 
 B<OPTIONS>
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 01bce2f..c9c45bf 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -107,6 +107,8 @@ static const char migrate_report[]=
    *            from target to source
    */
 
+#define XL_MANDATORY_FLAG_JSON (1U << 0) /* config data is in JSON format */
+#define XL_MANDATORY_FLAG_ALL  (XL_MANDATORY_FLAG_JSON)
 struct save_file_header {
     char magic[32]; /* savefileheader_magic */
     /* All uint32_ts are in domain's byte order. */
@@ -1699,21 +1701,41 @@ skip_vfb:
 }
 
 static void reload_domain_config(uint32_t domid,
-                                 uint8_t **config_data, int *config_len)
+                                 libxl_domain_config *d_config)
 {
+    int rc;
     uint8_t *t_data;
     int ret, t_len;
+    libxl_domain_config d_config_new;
 
+    /* In case user has used "config-update" to store a new config
+     * file.
+     */
     ret = libxl_userdata_retrieve(ctx, domid, "xl", &t_data, &t_len);
-    if (ret) {
-        LOG("failed to retrieve guest configuration (rc=%d). "
-            "reusing old configuration", ret);
+    if (ret && errno != ENOENT) {
+        LOG("\"xl\" configuration found but failed to load\n");
+    }
+    if (t_len > 0) {
+        LOG("\"xl\" configuration found, using it\n");
+        libxl_domain_config_dispose(d_config);
+        parse_config_data("<updated>", (const char *)t_data,
+                          t_len, d_config);
+        free(t_data);
+        libxl_userdata_unlink(ctx, domid, "xl");
         return;
     }
 
-    free(*config_data);
-    *config_data = t_data;
-    *config_len = t_len;
+    libxl_domain_config_init(&d_config_new);
+    rc = libxl_retrieve_domain_configuration(ctx, domid, &d_config_new);
+    if (rc) {
+        LOG("failed to retrieve guest configuration (rc=%d). "
+            "reusing old configuration", rc);
+        libxl_domain_config_dispose(&d_config_new);
+    } else {
+        libxl_domain_config_dispose(d_config);
+        /* Steal allocations */
+        memcpy(d_config, &d_config_new, sizeof(libxl_domain_config));
+    }
 }
 
 /* Returns 1 if domain should be restarted,
@@ -1721,7 +1743,6 @@ static void reload_domain_config(uint32_t domid,
  * Can update r_domid if domain is destroyed etc */
 static int handle_domain_death(uint32_t *r_domid,
                                libxl_event *event,
-                               uint8_t **config_data, int *config_len,
                                libxl_domain_config *d_config)
 
 {
@@ -1779,13 +1800,12 @@ static int handle_domain_death(uint32_t *r_domid,
         break;
 
     case LIBXL_ACTION_ON_SHUTDOWN_RESTART_RENAME:
-        reload_domain_config(*r_domid, config_data, config_len);
+        reload_domain_config(*r_domid, d_config);
         restart = 2;
         break;
 
     case LIBXL_ACTION_ON_SHUTDOWN_RESTART:
-        reload_domain_config(*r_domid, config_data, config_len);
-
+        reload_domain_config(*r_domid, d_config);
         restart = 1;
         /* fall-through */
     case LIBXL_ACTION_ON_SHUTDOWN_DESTROY:
@@ -1982,6 +2002,7 @@ static uint32_t create_domain(struct domain_create *dom_info)
     const char *config_source = NULL;
     const char *restore_source = NULL;
     int migrate_fd = dom_info->migrate_fd;
+    bool config_in_json;
 
     int i;
     int need_daemon = daemonize;
@@ -2036,7 +2057,7 @@ static uint32_t create_domain(struct domain_create *dom_info)
                 restore_source, hdr.mandatory_flags, hdr.optional_flags,
                 hdr.optional_data_len);
 
-        badflags = hdr.mandatory_flags & ~( 0 /* none understood yet */ );
+        badflags = hdr.mandatory_flags & ~XL_MANDATORY_FLAG_ALL;
         if (badflags) {
             fprintf(stderr, "Savefile has mandatory flag(s) 0x%"PRIx32" "
                     "which are not supported; need newer xl\n",
@@ -2064,7 +2085,9 @@ static uint32_t create_domain(struct domain_create *dom_info)
         optdata_here = optdata_begin;
 
         if (OPTDATA_LEFT) {
-            fprintf(stderr, " Savefile contains xl domain config\n");
+            fprintf(stderr, " Savefile contains xl domain config%s\n",
+                    !!(hdr.mandatory_flags & XL_MANDATORY_FLAG_JSON)
+                    ? " in JSON format" : "");
             WITH_OPTDATA(4, {
                 memcpy(u32buf.b, optdata_here, 4);
                 config_len = u32buf.u32;
@@ -2104,6 +2127,7 @@ static uint32_t create_domain(struct domain_create *dom_info)
                 extra_config);
         }
         config_source=config_file;
+        config_in_json = false;
     } else {
         if (!config_data) {
             fprintf(stderr, "Config file not specified and"
@@ -2111,12 +2135,18 @@ static uint32_t create_domain(struct domain_create *dom_info)
             return ERROR_INVAL;
         }
         config_source = "<saved>";
+        config_in_json = !!(hdr.mandatory_flags & XL_MANDATORY_FLAG_JSON);
     }
 
     if (!dom_info->quiet)
         printf("Parsing config from %s\n", config_source);
 
-    parse_config_data(config_source, config_data, config_len, &d_config);
+    if (config_in_json) {
+        libxl_domain_config_from_json(ctx, &d_config,
+                                      (const char *)config_data);
+    } else {
+        parse_config_data(config_source, config_data, config_len, &d_config);
+    }
 
     if (migrate_fd >= 0) {
         if (d_config.c_info.name) {
@@ -2184,14 +2214,6 @@ start:
     if ( ret )
         goto error_out;
 
-    ret = libxl_userdata_store(ctx, domid, "xl",
-                                    config_data, config_len);
-    if (ret) {
-        perror("cannot save config file");
-        ret = ERROR_FAIL;
-        goto error_out;
-    }
-
     release_lock();
 
     if (!paused)
@@ -2248,9 +2270,7 @@ start:
             LOG("Domain %d has shut down, reason code %d 0x%x", domid,
                 event->u.domain_shutdown.shutdown_reason,
                 event->u.domain_shutdown.shutdown_reason);
-            switch (handle_domain_death(&domid, event,
-                                        (uint8_t **)&config_data, &config_len,
-                                        &d_config)) {
+            switch (handle_domain_death(&domid, event, &d_config)) {
             case 2:
                 if (!preserve_domain(&domid, event, &d_config)) {
                     /* If we fail then exit leaving the old domain in place. */
@@ -2287,12 +2307,6 @@ start:
                     d_config.c_info.name = strdup(common_domname);
                 }
 
-                /* Reparse the configuration in case it has changed */
-                libxl_domain_config_dispose(&d_config);
-                libxl_domain_config_init(&d_config);
-                parse_config_data(config_source, config_data, config_len,
-                                  &d_config);
-
                 /*
                  * XXX FIXME: If this sleep is not there then domain
                  * re-creation fails sometimes.
@@ -3071,9 +3085,7 @@ static void list_domains_details(const libxl_dominfo *info, int nb_domain)
 {
     libxl_domain_config d_config;
 
-    char *config_source;
-    uint8_t *data;
-    int i, len, rc;
+    int i, rc;
 
     yajl_gen hand = NULL;
     yajl_gen_status s;
@@ -3094,24 +3106,18 @@ static void list_domains_details(const libxl_dominfo *info, int nb_domain)
         s = yajl_gen_status_ok;
 
     for (i = 0; i < nb_domain; i++) {
+        libxl_domain_config_init(&d_config);
         /* no detailed info available on dom0 */
         if (info[i].domid == 0)
             continue;
-        rc = libxl_userdata_retrieve(ctx, info[i].domid, "xl", &data, &len);
+        rc = libxl_retrieve_domain_configuration(ctx, info[i].domid, &d_config);
         if (rc)
             continue;
-        if (len == 0)
-            continue;
-        CHK_SYSCALL(asprintf(&config_source, "<domid %d data>", info[i].domid));
-        libxl_domain_config_init(&d_config);
-        parse_config_data(config_source, (char *)data, len, &d_config);
         if (default_output_format == OUTPUT_FORMAT_JSON)
             s = printf_info_one_json(hand, info[i].domid, &d_config);
         else
             printf_info_sexp(info[i].domid, &d_config);
         libxl_domain_config_dispose(&d_config);
-        free(data);
-        free(config_source);
         if (s != yajl_gen_status_ok)
             goto out;
     }
@@ -3296,22 +3302,41 @@ static void save_domain_core_begin(uint32_t domid,
                                    int *config_len_r)
 {
     int rc;
+    libxl_domain_config d_config;
+    char *config_c = 0;
 
     /* configuration file in optional data: */
 
+    libxl_domain_config_init(&d_config);
+
     if (override_config_file) {
         void *config_v = 0;
         rc = libxl_read_file_contents(ctx, override_config_file,
                                       &config_v, config_len_r);
-        *config_data_r = config_v;
+        if (rc) {
+            fprintf(stderr, "unable to read overridden config file\n");
+            exit(2);
+        }
+        parse_config_data(override_config_file, config_v, *config_len_r,
+                          &d_config);
+        free(config_v);
     } else {
-        rc = libxl_userdata_retrieve(ctx, domid, "xl",
-                                     config_data_r, config_len_r);
+        rc = libxl_retrieve_domain_configuration(ctx, domid, &d_config);
+        if (rc) {
+            fprintf(stderr, "unable to retrieve domain configuration\n");
+            exit(2);
+        }
     }
-    if (rc) {
-        fputs("Unable to get config file\n",stderr);
+
+    config_c = libxl_domain_config_to_json(ctx, &d_config);
+    if (!config_c) {
+        fprintf(stderr, "unable to parse overridden config file\n");
         exit(2);
     }
+    *config_data_r = (uint8_t *)config_c;
+    *config_len_r = strlen(config_c) + 1; /* including trailing '\0' */
+
+    libxl_domain_config_dispose(&d_config);
 }
 
 static void save_domain_core_writeconfig(int fd, const char *source,
@@ -3339,6 +3364,8 @@ static void save_domain_core_writeconfig(int fd, const char *source,
     u32buf.u32 = config_len;
     ADD_OPTDATA(u32buf.b,    4);
     ADD_OPTDATA(config_data, config_len);
+    if (config_len)
+        hdr.mandatory_flags |= XL_MANDATORY_FLAG_JSON;
 
     /* that's the optional data */
 
@@ -4348,6 +4375,9 @@ int main_config_update(int argc, char **argv)
         exit(1);
     }
 
+    fprintf(stderr, "WARN: libxl now has better capability to manage domain configuration, "
+            "avoid using this command when possible\n");
+
     domid = find_domain(argv[1]);
     argc--; argv++;
 
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 4279b9f..9bc0bca 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -38,7 +38,9 @@ struct cmd_spec cmd_table[] = {
     { "config-update",
       &main_config_update, 1, 1,
       "Update a running domain's saved configuration, used when rebuilding "
-      "the domain after reboot",
+      "the domain after reboot."
+      "WARN: libxl now has better capability to manage domain configuration, "
+      "avoid using this command when possible",
       "<Domain> <ConfigFile> [options] [vars]",
       "-h                      Print this help.\n"
       "-f FILE, --defconfig=FILE\n                     Use the given configuration file.\n"
-- 
1.7.10.4