[PATCH 0 of 2] libxl cleanup: distinguish filenames from sources

[PATCH 0 of 2] libxl cleanup: distinguish filenames from sources

[George Dunlap]

libxl cleanup: distinguish filenames from sources

[PATCH 1 of 2] libxlu: Rename filename to config_source

[George Dunlap]

The "filename" is a bit of a misnomer, as it's only used during error
messages, and in most instances cases is actually set to "command
line".

Rename it to "config_source" to make it clear that it's not used to
actually open any files.

No functional changes.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

diff -r 34a8cded4b39 -r 750b4dbab4b5 tools/libxl/libxlu_cfg.c
--- a/tools/libxl/libxlu_cfg.c	Tue May 15 15:30:32 2012 +0100
+++ b/tools/libxl/libxlu_cfg.c	Tue May 15 15:45:55 2012 +0100
@@ -25,15 +25,15 @@
 #include "libxlu_cfg_l.h"
 #include "libxlu_cfg_i.h"
 
-XLU_Config *xlu_cfg_init(FILE *report, const char *report_filename) {
+XLU_Config *xlu_cfg_init(FILE *report, const char *report_source) {
     XLU_Config *cfg;
 
     cfg= malloc(sizeof(*cfg));
     if (!cfg) return 0;
 
     cfg->report= report;
-    cfg->filename= strdup(report_filename);
-    if (!cfg->filename) { free(cfg); return 0; }
+    cfg->config_source= strdup(report_source);
+    if (!cfg->config_source) { free(cfg); return 0; }
 
     cfg->settings= 0;
     return cfg;
@@ -51,7 +51,7 @@ static int ctx_prep(CfgParseContext *ctx
     e= xlu__cfg_yylex_init_extra(ctx, &ctx->scanner);
     if (e) {
         fprintf(cfg->report,"%s: unable to create scanner: %s\n",
-                cfg->filename, strerror(e));
+                cfg->config_source, strerror(e));
         return e;
     }
     return 0;
@@ -117,7 +117,7 @@ int xlu_cfg_readdata(XLU_Config *cfg, co
     buf = xlu__cfg_yy_scan_bytes(data, length, ctx.scanner);
     if (!buf) {
         fprintf(cfg->report,"%s: unable to allocate scanner buffer\n",
-                cfg->filename);
+                cfg->config_source);
         ctx.err= ENOMEM;
         goto xe;
     }
@@ -151,7 +151,7 @@ void xlu_cfg_destroy(XLU_Config *cfg) {
         set_next= set->next;
         xlu__cfg_set_free(set);
     }
-    free(cfg->filename);
+    free(cfg->config_source);
     free(cfg);
 }
 
@@ -178,7 +178,7 @@ static int find_atom(const XLU_Config *c
             fprintf(cfg->report,
                     "%s:%d: warning: parameter `%s' is"
                     " a list but should be a single value\n",
-                    cfg->filename, set->lineno, n);
+                    cfg->config_source, set->lineno, n);
         return EINVAL;
     }
     *set_r= set;
@@ -223,14 +223,14 @@ int xlu_cfg_get_long(const XLU_Config *c
             fprintf(cfg->report,
                     "%s:%d: warning: parameter `%s' could not be parsed"
                     " as a number: %s\n",
-                    cfg->filename, set->lineno, n, strerror(e));
+                    cfg->config_source, set->lineno, n, strerror(e));
         return e;
     }
     if (*ep || ep==set->values[0]) {
         if (!dont_warn)
             fprintf(cfg->report,
                     "%s:%d: warning: parameter `%s' is not a valid number\n",
-                    cfg->filename, set->lineno, n);
+                    cfg->config_source, set->lineno, n);
         return EINVAL;
     }
     *value_r= l;
@@ -258,7 +258,7 @@ int xlu_cfg_get_list(const XLU_Config *c
             fprintf(cfg->report,
                     "%s:%d: warning: parameter `%s' is a single value"
                     " but should be a list\n",
-                    cfg->filename, set->lineno, n);
+                    cfg->config_source, set->lineno, n);
         }
         return EINVAL;
     }
@@ -467,7 +467,7 @@ void xlu__cfg_yyerror(YYLTYPE *loc, CfgP
 
     fprintf(ctx->cfg->report,
             "%s:%d: config parsing error near %s%.*s%s%s: %s\n",
-            ctx->cfg->filename, lineno,
+            ctx->cfg->config_source, lineno,
             len?"`":"", len, text, len?"'":"", newline,
             msg);
     if (!ctx->err) ctx->err= EINVAL;
diff -r 34a8cded4b39 -r 750b4dbab4b5 tools/libxl/libxlu_disk.c
--- a/tools/libxl/libxlu_disk.c	Tue May 15 15:30:32 2012 +0100
+++ b/tools/libxl/libxlu_disk.c	Tue May 15 15:45:55 2012 +0100
@@ -10,7 +10,7 @@ void xlu__disk_err(DiskParseContext *dpc
             "%s: config parsing error in disk specification: %s"
             "%s%s%s"
             " in `%s'\n",
-            dpc->cfg->filename, message,
+            dpc->cfg->config_source, message,
             erroneous?": near `":"", erroneous?erroneous:"", erroneous?"'":"",
             dpc->spec);
     if (!dpc->err) dpc->err= EINVAL;
diff -r 34a8cded4b39 -r 750b4dbab4b5 tools/libxl/libxlu_internal.h
--- a/tools/libxl/libxlu_internal.h	Tue May 15 15:30:32 2012 +0100
+++ b/tools/libxl/libxlu_internal.h	Tue May 15 15:45:55 2012 +0100
@@ -38,7 +38,7 @@ struct XLU_ConfigSetting { /* transparen
 struct XLU_Config {
     XLU_ConfigSetting *settings;
     FILE *report;
-    char *filename;
+    char *config_source;
 };
 
 typedef struct {
diff -r 34a8cded4b39 -r 750b4dbab4b5 tools/libxl/libxlu_vif.c
--- a/tools/libxl/libxlu_vif.c	Tue May 15 15:30:32 2012 +0100
+++ b/tools/libxl/libxlu_vif.c	Tue May 15 15:45:55 2012 +0100
@@ -7,7 +7,7 @@ static const char *vif_internal_usec_re 
 static void xlu__vif_err(XLU_Config *cfg, const char *msg, const char *rate) {
     fprintf(cfg->report,
             "%s: config parsing error in vif: %s in `%s'\n",
-            cfg->filename, msg, rate);
+            cfg->config_source, msg, rate);
 }
 
 static int vif_parse_rate_bytes_per_sec(XLU_Config *cfg, const char *bytes,

[PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source"

[George Dunlap]

Many places in xl there's a variable or element named "filename" which
does not contain a filename, but the source of the data for reporting
purposes.  Worse, there are variables which are sometimes actually
used or interpreted as a filename depending on what other variables
are set.  This makes it difficult to tell when a string is purely
cosmetic, and when another bit of code may actually attempt to call
"open" with the string.

This patch makes a consistent distinction between "filename" (which
always refers to the name of an actual file, and may be interpreted as
such at some point) and "source" (which may be a filename, or may be
another data source such as a migration stream or saved data).

This does add some variables and reshuffle where assignments happen;
most notably, the "restore_filename" element of struct domain_create
is now only set when restoring from a file.

But at a high level, there should be no funcitonal changes.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

diff -r 750b4dbab4b5 -r d555dd6614af tools/libxl/libxl_utils.c
--- a/tools/libxl/libxl_utils.c	Tue May 15 15:45:55 2012 +0100
+++ b/tools/libxl/libxl_utils.c	Tue May 15 15:51:25 2012 +0100
@@ -334,7 +334,7 @@ int libxl_read_file_contents(libxl_ctx *
                                                                           \
   int libxl_##rw##_exactly(libxl_ctx *ctx, int fd,                 \
                            constdata void *data, ssize_t sz,              \
-                           const char *filename, const char *what) {      \
+                           const char *source, const char *what) {      \
       ssize_t got;                                                        \
                                                                           \
       while (sz > 0) {                                                    \
@@ -343,7 +343,7 @@ int libxl_read_file_contents(libxl_ctx *
               if (errno == EINTR) continue;                               \
               if (!ctx) return errno;                                     \
               LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to " #rw " %s%s%s", \
-                           what?what:"", what?" from ":"", filename);     \
+                           what?what:"", what?" from ":"", source);       \
               return errno;                                               \
           }                                                               \
           if (got == 0) {                                                 \
@@ -352,7 +352,7 @@ int libxl_read_file_contents(libxl_ctx *
                      zero_is_eof                                          \
                      ? "file/stream truncated reading %s%s%s"             \
                      : "file/stream write returned 0! writing %s%s%s",    \
-                     what?what:"", what?" from ":"", filename);           \
+                     what?what:"", what?" from ":"", source);             \
               return EPROTO;                                              \
           }                                                               \
           sz -= got;                                                      \
diff -r 750b4dbab4b5 -r d555dd6614af tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Tue May 15 15:45:55 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Tue May 15 15:51:25 2012 +0100
@@ -520,9 +520,9 @@ vcpp_out:
     return rc;
 }
 
-static void parse_config_data(const char *configfile_filename_report,
-                              const char *configfile_data,
-                              int configfile_len,
+static void parse_config_data(const char *config_source,
+                              const char *config_data,
+                              int config_len,
                               libxl_domain_config *d_config)
 {
     const char *buf;
@@ -537,15 +537,15 @@ static void parse_config_data(const char
     libxl_domain_create_info *c_info = &d_config->c_info;
     libxl_domain_build_info *b_info = &d_config->b_info;
 
-    config= xlu_cfg_init(stderr, configfile_filename_report);
+    config= xlu_cfg_init(stderr, config_source);
     if (!config) {
         fprintf(stderr, "Failed to allocate for configuration\n");
         exit(1);
     }
 
-    e= xlu_cfg_readdata(config, configfile_data, configfile_len);
+    e= xlu_cfg_readdata(config, config_data, config_len);
     if (e) {
-        fprintf(stderr, "Failed to parse config file: %s\n", strerror(e));
+        fprintf(stderr, "Failed to parse config: %s\n", strerror(e));
         exit(1);
     }
 
@@ -1524,6 +1524,8 @@ static int create_domain(struct domain_c
     const char *config_file = dom_info->config_file;
     const char *extra_config = dom_info->extra_config;
     const char *restore_file = dom_info->restore_file;
+    const char *config_source = NULL;
+    const char *restore_source = NULL;
     int migrate_fd = dom_info->migrate_fd;
 
     int i;
@@ -1539,24 +1541,28 @@ static int create_domain(struct domain_c
     pid_t child_console_pid = -1;
     struct save_file_header hdr;
 
+    int restoring = (restore_file || (migrate_fd >= 0));
+
     libxl_domain_config_init(&d_config);
 
-    if (restore_file) {
+    if (restoring) {
         uint8_t *optdata_begin = 0;
         const uint8_t *optdata_here = 0;
         union { uint32_t u32; char b[4]; } u32buf;
         uint32_t badflags;
 
         if (migrate_fd >= 0) {
+            restore_source = "<incoming migration stream>";
             restore_fd = migrate_fd;
         } else {
+            restore_source = restore_file;
             restore_fd = open(restore_file, O_RDONLY);
             rc = libxl_fd_set_cloexec(ctx, restore_fd, 1);
             if (rc) return rc;
         }
 
         CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, &hdr,
-                   sizeof(hdr), restore_file, "header") );
+                   sizeof(hdr), restore_source, "header") );
         if (memcmp(hdr.magic, savefileheader_magic, sizeof(hdr.magic))) {
             fprintf(stderr, "File has wrong magic number -"
                     " corrupt or for a different tool?\n");
@@ -1569,7 +1575,7 @@ static int create_domain(struct domain_c
         fprintf(stderr, "Loading new save file %s"
                 " (new xl fmt info"
                 " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n",
-                restore_file, hdr.mandatory_flags, hdr.optional_flags,
+                restore_source, hdr.mandatory_flags, hdr.optional_flags,
                 hdr.optional_data_len);
 
         badflags = hdr.mandatory_flags & ~( 0 /* none understood yet */ );
@@ -1582,7 +1588,7 @@ static int create_domain(struct domain_c
         if (hdr.optional_data_len) {
             optdata_begin = xmalloc(hdr.optional_data_len);
             CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, optdata_begin,
-                   hdr.optional_data_len, restore_file, "optdata") );
+                   hdr.optional_data_len, restore_source, "optdata") );
         }
 
 #define OPTDATA_LEFT  (hdr.optional_data_len - (optdata_here - optdata_begin))
@@ -1617,7 +1623,7 @@ static int create_domain(struct domain_c
                                        &config_data, &config_len);
         if (ret) { fprintf(stderr, "Failed to read config file: %s: %s\n",
                            config_file, strerror(errno)); return ERROR_FAIL; }
-        if (!restore_file && extra_config && strlen(extra_config)) {
+        if (!restoring && extra_config && strlen(extra_config)) {
             if (config_len > INT_MAX - (strlen(extra_config) + 2 + 1)) {
                 fprintf(stderr, "Failed to attach extra configration\n");
                 return ERROR_FAIL;
@@ -1632,19 +1638,20 @@ static int create_domain(struct domain_c
             config_len += sprintf(config_data + config_len, "\n%s\n",
                 extra_config);
         }
+        config_source=config_file;
     } else {
         if (!config_data) {
             fprintf(stderr, "Config file not specified and"
                     " none in save file\n");
             return ERROR_INVAL;
         }
-        config_file = "<saved>";
+        config_source = "<saved>";
     }
 
     if (!dom_info->quiet)
-        printf("Parsing config file %s\n", config_file);
-
-    parse_config_data(config_file, config_data, config_len, &d_config);
+        printf("Parsing config from %s\n", config_source);
+
+    parse_config_data(config_source, config_data, config_len, &d_config);
 
     if (migrate_fd >= 0) {
         if (d_config.c_info.name) {
@@ -1698,7 +1705,7 @@ start:
         autoconnect_console_how = 0;
     }
 
-    if ( restore_file ) {
+    if ( restoring ) {
         ret = libxl_domain_create_restore(ctx, &d_config,
                                           &domid, restore_fd,
                                           0, autoconnect_console_how);
@@ -2559,7 +2566,7 @@ static void list_domains_details(const l
 {
     libxl_domain_config d_config;
 
-    char *config_file;
+    char *config_source;
     uint8_t *data;
     int i, len, rc;
 
@@ -2570,13 +2577,13 @@ static void list_domains_details(const l
         rc = libxl_userdata_retrieve(ctx, info[i].domid, "xl", &data, &len);
         if (rc)
             continue;
-        CHK_ERRNO(asprintf(&config_file, "<domid %d data>", info[i].domid));
+        CHK_ERRNO(asprintf(&config_source, "<domid %d data>", info[i].domid));
         libxl_domain_config_init(&d_config);
-        parse_config_data(config_file, (char *)data, len, &d_config);
+        parse_config_data(config_source, (char *)data, len, &d_config);
         printf_info(default_output_format, info[i].domid, &d_config);
         libxl_domain_config_dispose(&d_config);
         free(data);
-        free(config_file);
+        free(config_source);
     }
 }
 
@@ -2679,7 +2686,7 @@ static void save_domain_core_begin(const
     }
 }
 
-static void save_domain_core_writeconfig(int fd, const char *filename,
+static void save_domain_core_writeconfig(int fd, const char *source,
                                   const uint8_t *config_data, int config_len)
 {
     struct save_file_header hdr;
@@ -2708,13 +2715,13 @@ static void save_domain_core_writeconfig
     /* that's the optional data */
 
     CHK_ERRNO( libxl_write_exactly(ctx, fd,
-        &hdr, sizeof(hdr), filename, "header") );
+        &hdr, sizeof(hdr), source, "header") );
     CHK_ERRNO( libxl_write_exactly(ctx, fd,
-        optdata_begin, hdr.optional_data_len, filename, "header") );
+        optdata_begin, hdr.optional_data_len, source, "header") );
 
     fprintf(stderr, "Saving to %s new xl format (info"
             " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n",
-            filename, hdr.mandatory_flags, hdr.optional_flags,
+            source, hdr.mandatory_flags, hdr.optional_flags,
             hdr.optional_data_len);
 }
 
@@ -3039,7 +3046,6 @@ static void migrate_receive(int debug, i
     dom_info.daemonize = daemonize;
     dom_info.monitor = monitor;
     dom_info.paused = 1;
-    dom_info.restore_file = "incoming migration stream";
     dom_info.migrate_fd = 0; /* stdin */
     dom_info.migration_domname_r = &migration_domname;
     dom_info.incr_generationid = 0;

Re: [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source"

[Ian Campbell]

On Tue, 2012-05-15 at 15:51 +0100, George Dunlap wrote:
> Many places in xl there's a variable or element named "filename" which
> does not contain a filename, but the source of the data for reporting
> purposes.  Worse, there are variables which are sometimes actually
> used or interpreted as a filename depending on what other variables
> are set.  This makes it difficult to tell when a string is purely
> cosmetic, and when another bit of code may actually attempt to call
> "open" with the string.
> 
> This patch makes a consistent distinction between "filename" (which
> always refers to the name of an actual file, and may be interpreted as
> such at some point) and "source" (which may be a filename, or may be
> another data source such as a migration stream or saved data).
> 
> This does add some variables and reshuffle where assignments happen;
> most notably, the "restore_filename" element of struct domain_create
> is now only set when restoring from a file.
> 
> But at a high level, there should be no funcitonal changes.
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

I've committed this (and the preceding patch), there were a few
conflicts with Goncalo's vncviewer patch (a new param to
parse_config_data). Please do check I've done the right thing.

> 
> diff -r 750b4dbab4b5 -r d555dd6614af tools/libxl/libxl_utils.c
> --- a/tools/libxl/libxl_utils.c Tue May 15 15:45:55 2012 +0100
> +++ b/tools/libxl/libxl_utils.c Tue May 15 15:51:25 2012 +0100
> @@ -334,7 +334,7 @@ int libxl_read_file_contents(libxl_ctx *
>                                                                            \
>    int libxl_##rw##_exactly(libxl_ctx *ctx, int fd,                 \
>                             constdata void *data, ssize_t sz,              \
> -                           const char *filename, const char *what) {      \
> +                           const char *source, const char *what) {      \
>        ssize_t got;                                                        \
>                                                                            \
>        while (sz > 0) {                                                    \
> @@ -343,7 +343,7 @@ int libxl_read_file_contents(libxl_ctx *
>                if (errno == EINTR) continue;                               \
>                if (!ctx) return errno;                                     \
>                LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to " #rw " %s%s%s", \
> -                           what?what:"", what?" from ":"", filename);     \
> +                           what?what:"", what?" from ":"", source);       \
>                return errno;                                               \
>            }                                                               \
>            if (got == 0) {                                                 \
> @@ -352,7 +352,7 @@ int libxl_read_file_contents(libxl_ctx *
>                       zero_is_eof                                          \
>                       ? "file/stream truncated reading %s%s%s"             \
>                       : "file/stream write returned 0! writing %s%s%s",    \
> -                     what?what:"", what?" from ":"", filename);           \
> +                     what?what:"", what?" from ":"", source);             \
>                return EPROTO;                                              \
>            }                                                               \
>            sz -= got;                                                      \
> diff -r 750b4dbab4b5 -r d555dd6614af tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c  Tue May 15 15:45:55 2012 +0100
> +++ b/tools/libxl/xl_cmdimpl.c  Tue May 15 15:51:25 2012 +0100
> @@ -520,9 +520,9 @@ vcpp_out:
>      return rc;
>  }
> 
> -static void parse_config_data(const char *configfile_filename_report,
> -                              const char *configfile_data,
> -                              int configfile_len,
> +static void parse_config_data(const char *config_source,
> +                              const char *config_data,
> +                              int config_len,
>                                libxl_domain_config *d_config)
>  {
>      const char *buf;
> @@ -537,15 +537,15 @@ static void parse_config_data(const char
>      libxl_domain_create_info *c_info = &d_config->c_info;
>      libxl_domain_build_info *b_info = &d_config->b_info;
> 
> -    config= xlu_cfg_init(stderr, configfile_filename_report);
> +    config= xlu_cfg_init(stderr, config_source);
>      if (!config) {
>          fprintf(stderr, "Failed to allocate for configuration\n");
>          exit(1);
>      }
> 
> -    e= xlu_cfg_readdata(config, configfile_data, configfile_len);
> +    e= xlu_cfg_readdata(config, config_data, config_len);
>      if (e) {
> -        fprintf(stderr, "Failed to parse config file: %s\n", strerror(e));
> +        fprintf(stderr, "Failed to parse config: %s\n", strerror(e));
>          exit(1);
>      }
> 
> @@ -1524,6 +1524,8 @@ static int create_domain(struct domain_c
>      const char *config_file = dom_info->config_file;
>      const char *extra_config = dom_info->extra_config;
>      const char *restore_file = dom_info->restore_file;
> +    const char *config_source = NULL;
> +    const char *restore_source = NULL;
>      int migrate_fd = dom_info->migrate_fd;
> 
>      int i;
> @@ -1539,24 +1541,28 @@ static int create_domain(struct domain_c
>      pid_t child_console_pid = -1;
>      struct save_file_header hdr;
> 
> +    int restoring = (restore_file || (migrate_fd >= 0));
> +
>      libxl_domain_config_init(&d_config);
> 
> -    if (restore_file) {
> +    if (restoring) {
>          uint8_t *optdata_begin = 0;
>          const uint8_t *optdata_here = 0;
>          union { uint32_t u32; char b[4]; } u32buf;
>          uint32_t badflags;
> 
>          if (migrate_fd >= 0) {
> +            restore_source = "<incoming migration stream>";
>              restore_fd = migrate_fd;
>          } else {
> +            restore_source = restore_file;
>              restore_fd = open(restore_file, O_RDONLY);
>              rc = libxl_fd_set_cloexec(ctx, restore_fd, 1);
>              if (rc) return rc;
>          }
> 
>          CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, &hdr,
> -                   sizeof(hdr), restore_file, "header") );
> +                   sizeof(hdr), restore_source, "header") );
>          if (memcmp(hdr.magic, savefileheader_magic, sizeof(hdr.magic))) {
>              fprintf(stderr, "File has wrong magic number -"
>                      " corrupt or for a different tool?\n");
> @@ -1569,7 +1575,7 @@ static int create_domain(struct domain_c
>          fprintf(stderr, "Loading new save file %s"
>                  " (new xl fmt info"
>                  " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n",
> -                restore_file, hdr.mandatory_flags, hdr.optional_flags,
> +                restore_source, hdr.mandatory_flags, hdr.optional_flags,
>                  hdr.optional_data_len);
> 
>          badflags = hdr.mandatory_flags & ~( 0 /* none understood yet */ );
> @@ -1582,7 +1588,7 @@ static int create_domain(struct domain_c
>          if (hdr.optional_data_len) {
>              optdata_begin = xmalloc(hdr.optional_data_len);
>              CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, optdata_begin,
> -                   hdr.optional_data_len, restore_file, "optdata") );
> +                   hdr.optional_data_len, restore_source, "optdata") );
>          }
> 
>  #define OPTDATA_LEFT  (hdr.optional_data_len - (optdata_here - optdata_begin))
> @@ -1617,7 +1623,7 @@ static int create_domain(struct domain_c
>                                         &config_data, &config_len);
>          if (ret) { fprintf(stderr, "Failed to read config file: %s: %s\n",
>                             config_file, strerror(errno)); return ERROR_FAIL; }
> -        if (!restore_file && extra_config && strlen(extra_config)) {
> +        if (!restoring && extra_config && strlen(extra_config)) {
>              if (config_len > INT_MAX - (strlen(extra_config) + 2 + 1)) {
>                  fprintf(stderr, "Failed to attach extra configration\n");
>                  return ERROR_FAIL;
> @@ -1632,19 +1638,20 @@ static int create_domain(struct domain_c
>              config_len += sprintf(config_data + config_len, "\n%s\n",
>                  extra_config);
>          }
> +        config_source=config_file;
>      } else {
>          if (!config_data) {
>              fprintf(stderr, "Config file not specified and"
>                      " none in save file\n");
>              return ERROR_INVAL;
>          }
> -        config_file = "<saved>";
> +        config_source = "<saved>";
>      }
> 
>      if (!dom_info->quiet)
> -        printf("Parsing config file %s\n", config_file);
> -
> -    parse_config_data(config_file, config_data, config_len, &d_config);
> +        printf("Parsing config from %s\n", config_source);
> +
> +    parse_config_data(config_source, config_data, config_len, &d_config);
> 
>      if (migrate_fd >= 0) {
>          if (d_config.c_info.name) {
> @@ -1698,7 +1705,7 @@ start:
>          autoconnect_console_how = 0;
>      }
> 
> -    if ( restore_file ) {
> +    if ( restoring ) {
>          ret = libxl_domain_create_restore(ctx, &d_config,
>                                            &domid, restore_fd,
>                                            0, autoconnect_console_how);
> @@ -2559,7 +2566,7 @@ static void list_domains_details(const l
>  {
>      libxl_domain_config d_config;
> 
> -    char *config_file;
> +    char *config_source;
>      uint8_t *data;
>      int i, len, rc;
> 
> @@ -2570,13 +2577,13 @@ static void list_domains_details(const l
>          rc = libxl_userdata_retrieve(ctx, info[i].domid, "xl", &data, &len);
>          if (rc)
>              continue;
> -        CHK_ERRNO(asprintf(&config_file, "<domid %d data>", info[i].domid));
> +        CHK_ERRNO(asprintf(&config_source, "<domid %d data>", info[i].domid));
>          libxl_domain_config_init(&d_config);
> -        parse_config_data(config_file, (char *)data, len, &d_config);
> +        parse_config_data(config_source, (char *)data, len, &d_config);
>          printf_info(default_output_format, info[i].domid, &d_config);
>          libxl_domain_config_dispose(&d_config);
>          free(data);
> -        free(config_file);
> +        free(config_source);
>      }
>  }
> 
> @@ -2679,7 +2686,7 @@ static void save_domain_core_begin(const
>      }
>  }
> 
> -static void save_domain_core_writeconfig(int fd, const char *filename,
> +static void save_domain_core_writeconfig(int fd, const char *source,
>                                    const uint8_t *config_data, int config_len)
>  {
>      struct save_file_header hdr;
> @@ -2708,13 +2715,13 @@ static void save_domain_core_writeconfig
>      /* that's the optional data */
> 
>      CHK_ERRNO( libxl_write_exactly(ctx, fd,
> -        &hdr, sizeof(hdr), filename, "header") );
> +        &hdr, sizeof(hdr), source, "header") );
>      CHK_ERRNO( libxl_write_exactly(ctx, fd,
> -        optdata_begin, hdr.optional_data_len, filename, "header") );
> +        optdata_begin, hdr.optional_data_len, source, "header") );
> 
>      fprintf(stderr, "Saving to %s new xl format (info"
>              " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n",
> -            filename, hdr.mandatory_flags, hdr.optional_flags,
> +            source, hdr.mandatory_flags, hdr.optional_flags,
>              hdr.optional_data_len);
>  }
> 
> @@ -3039,7 +3046,6 @@ static void migrate_receive(int debug, i
>      dom_info.daemonize = daemonize;
>      dom_info.monitor = monitor;
>      dom_info.paused = 1;
> -    dom_info.restore_file = "incoming migration stream";
>      dom_info.migrate_fd = 0; /* stdin */
>      dom_info.migration_domname_r = &migration_domname;
>      dom_info.incr_generationid = 0;
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source"

[George Dunlap]

On 15/05/12 16:29, Ian Campbell wrote:
> On Tue, 2012-05-15 at 15:51 +0100, George Dunlap wrote:
>> Many places in xl there's a variable or element named "filename" which
>> does not contain a filename, but the source of the data for reporting
>> purposes.  Worse, there are variables which are sometimes actually
>> used or interpreted as a filename depending on what other variables
>> are set.  This makes it difficult to tell when a string is purely
>> cosmetic, and when another bit of code may actually attempt to call
>> "open" with the string.
>>
>> This patch makes a consistent distinction between "filename" (which
>> always refers to the name of an actual file, and may be interpreted as
>> such at some point) and "source" (which may be a filename, or may be
>> another data source such as a migration stream or saved data).
>>
>> This does add some variables and reshuffle where assignments happen;
>> most notably, the "restore_filename" element of struct domain_create
>> is now only set when restoring from a file.
>>
>> But at a high level, there should be no funcitonal changes.
>>
>> Signed-off-by: George Dunlap<george.dunlap@eu.citrix.com>
> I've committed this (and the preceding patch), there were a few
> conflicts with Goncalo's vncviewer patch (a new param to
> parse_config_data). Please do check I've done the right thing.
Looks like you missed one s/config_file/config_source/ for 
parse_config_data; I'll follow-up with a patch.

  -George
>
>> diff -r 750b4dbab4b5 -r d555dd6614af tools/libxl/libxl_utils.c
>> --- a/tools/libxl/libxl_utils.c Tue May 15 15:45:55 2012 +0100
>> +++ b/tools/libxl/libxl_utils.c Tue May 15 15:51:25 2012 +0100
>> @@ -334,7 +334,7 @@ int libxl_read_file_contents(libxl_ctx *
>>                                                                             \
>>     int libxl_##rw##_exactly(libxl_ctx *ctx, int fd,                 \
>>                              constdata void *data, ssize_t sz,              \
>> -                           const char *filename, const char *what) {      \
>> +                           const char *source, const char *what) {      \
>>         ssize_t got;                                                        \
>>                                                                             \
>>         while (sz>  0) {                                                    \
>> @@ -343,7 +343,7 @@ int libxl_read_file_contents(libxl_ctx *
>>                 if (errno == EINTR) continue;                               \
>>                 if (!ctx) return errno;                                     \
>>                 LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to " #rw " %s%s%s", \
>> -                           what?what:"", what?" from ":"", filename);     \
>> +                           what?what:"", what?" from ":"", source);       \
>>                 return errno;                                               \
>>             }                                                               \
>>             if (got == 0) {                                                 \
>> @@ -352,7 +352,7 @@ int libxl_read_file_contents(libxl_ctx *
>>                        zero_is_eof                                          \
>>                        ? "file/stream truncated reading %s%s%s"             \
>>                        : "file/stream write returned 0! writing %s%s%s",    \
>> -                     what?what:"", what?" from ":"", filename);           \
>> +                     what?what:"", what?" from ":"", source);             \
>>                 return EPROTO;                                              \
>>             }                                                               \
>>             sz -= got;                                                      \
>> diff -r 750b4dbab4b5 -r d555dd6614af tools/libxl/xl_cmdimpl.c
>> --- a/tools/libxl/xl_cmdimpl.c  Tue May 15 15:45:55 2012 +0100
>> +++ b/tools/libxl/xl_cmdimpl.c  Tue May 15 15:51:25 2012 +0100
>> @@ -520,9 +520,9 @@ vcpp_out:
>>       return rc;
>>   }
>>
>> -static void parse_config_data(const char *configfile_filename_report,
>> -                              const char *configfile_data,
>> -                              int configfile_len,
>> +static void parse_config_data(const char *config_source,
>> +                              const char *config_data,
>> +                              int config_len,
>>                                 libxl_domain_config *d_config)
>>   {
>>       const char *buf;
>> @@ -537,15 +537,15 @@ static void parse_config_data(const char
>>       libxl_domain_create_info *c_info =&d_config->c_info;
>>       libxl_domain_build_info *b_info =&d_config->b_info;
>>
>> -    config= xlu_cfg_init(stderr, configfile_filename_report);
>> +    config= xlu_cfg_init(stderr, config_source);
>>       if (!config) {
>>           fprintf(stderr, "Failed to allocate for configuration\n");
>>           exit(1);
>>       }
>>
>> -    e= xlu_cfg_readdata(config, configfile_data, configfile_len);
>> +    e= xlu_cfg_readdata(config, config_data, config_len);
>>       if (e) {
>> -        fprintf(stderr, "Failed to parse config file: %s\n", strerror(e));
>> +        fprintf(stderr, "Failed to parse config: %s\n", strerror(e));
>>           exit(1);
>>       }
>>
>> @@ -1524,6 +1524,8 @@ static int create_domain(struct domain_c
>>       const char *config_file = dom_info->config_file;
>>       const char *extra_config = dom_info->extra_config;
>>       const char *restore_file = dom_info->restore_file;
>> +    const char *config_source = NULL;
>> +    const char *restore_source = NULL;
>>       int migrate_fd = dom_info->migrate_fd;
>>
>>       int i;
>> @@ -1539,24 +1541,28 @@ static int create_domain(struct domain_c
>>       pid_t child_console_pid = -1;
>>       struct save_file_header hdr;
>>
>> +    int restoring = (restore_file || (migrate_fd>= 0));
>> +
>>       libxl_domain_config_init(&d_config);
>>
>> -    if (restore_file) {
>> +    if (restoring) {
>>           uint8_t *optdata_begin = 0;
>>           const uint8_t *optdata_here = 0;
>>           union { uint32_t u32; char b[4]; } u32buf;
>>           uint32_t badflags;
>>
>>           if (migrate_fd>= 0) {
>> +            restore_source = "<incoming migration stream>";
>>               restore_fd = migrate_fd;
>>           } else {
>> +            restore_source = restore_file;
>>               restore_fd = open(restore_file, O_RDONLY);
>>               rc = libxl_fd_set_cloexec(ctx, restore_fd, 1);
>>               if (rc) return rc;
>>           }
>>
>>           CHK_ERRNO( libxl_read_exactly(ctx, restore_fd,&hdr,
>> -                   sizeof(hdr), restore_file, "header") );
>> +                   sizeof(hdr), restore_source, "header") );
>>           if (memcmp(hdr.magic, savefileheader_magic, sizeof(hdr.magic))) {
>>               fprintf(stderr, "File has wrong magic number -"
>>                       " corrupt or for a different tool?\n");
>> @@ -1569,7 +1575,7 @@ static int create_domain(struct domain_c
>>           fprintf(stderr, "Loading new save file %s"
>>                   " (new xl fmt info"
>>                   " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n",
>> -                restore_file, hdr.mandatory_flags, hdr.optional_flags,
>> +                restore_source, hdr.mandatory_flags, hdr.optional_flags,
>>                   hdr.optional_data_len);
>>
>>           badflags = hdr.mandatory_flags&  ~( 0 /* none understood yet */ );
>> @@ -1582,7 +1588,7 @@ static int create_domain(struct domain_c
>>           if (hdr.optional_data_len) {
>>               optdata_begin = xmalloc(hdr.optional_data_len);
>>               CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, optdata_begin,
>> -                   hdr.optional_data_len, restore_file, "optdata") );
>> +                   hdr.optional_data_len, restore_source, "optdata") );
>>           }
>>
>>   #define OPTDATA_LEFT  (hdr.optional_data_len - (optdata_here - optdata_begin))
>> @@ -1617,7 +1623,7 @@ static int create_domain(struct domain_c
>>                                          &config_data,&config_len);
>>           if (ret) { fprintf(stderr, "Failed to read config file: %s: %s\n",
>>                              config_file, strerror(errno)); return ERROR_FAIL; }
>> -        if (!restore_file&&  extra_config&&  strlen(extra_config)) {
>> +        if (!restoring&&  extra_config&&  strlen(extra_config)) {
>>               if (config_len>  INT_MAX - (strlen(extra_config) + 2 + 1)) {
>>                   fprintf(stderr, "Failed to attach extra configration\n");
>>                   return ERROR_FAIL;
>> @@ -1632,19 +1638,20 @@ static int create_domain(struct domain_c
>>               config_len += sprintf(config_data + config_len, "\n%s\n",
>>                   extra_config);
>>           }
>> +        config_source=config_file;
>>       } else {
>>           if (!config_data) {
>>               fprintf(stderr, "Config file not specified and"
>>                       " none in save file\n");
>>               return ERROR_INVAL;
>>           }
>> -        config_file = "<saved>";
>> +        config_source = "<saved>";
>>       }
>>
>>       if (!dom_info->quiet)
>> -        printf("Parsing config file %s\n", config_file);
>> -
>> -    parse_config_data(config_file, config_data, config_len,&d_config);
>> +        printf("Parsing config from %s\n", config_source);
>> +
>> +    parse_config_data(config_source, config_data, config_len,&d_config);
>>
>>       if (migrate_fd>= 0) {
>>           if (d_config.c_info.name) {
>> @@ -1698,7 +1705,7 @@ start:
>>           autoconnect_console_how = 0;
>>       }
>>
>> -    if ( restore_file ) {
>> +    if ( restoring ) {
>>           ret = libxl_domain_create_restore(ctx,&d_config,
>>                                             &domid, restore_fd,
>>                                             0, autoconnect_console_how);
>> @@ -2559,7 +2566,7 @@ static void list_domains_details(const l
>>   {
>>       libxl_domain_config d_config;
>>
>> -    char *config_file;
>> +    char *config_source;
>>       uint8_t *data;
>>       int i, len, rc;
>>
>> @@ -2570,13 +2577,13 @@ static void list_domains_details(const l
>>           rc = libxl_userdata_retrieve(ctx, info[i].domid, "xl",&data,&len);
>>           if (rc)
>>               continue;
>> -        CHK_ERRNO(asprintf(&config_file, "<domid %d data>", info[i].domid));
>> +        CHK_ERRNO(asprintf(&config_source, "<domid %d data>", info[i].domid));
>>           libxl_domain_config_init(&d_config);
>> -        parse_config_data(config_file, (char *)data, len,&d_config);
>> +        parse_config_data(config_source, (char *)data, len,&d_config);
>>           printf_info(default_output_format, info[i].domid,&d_config);
>>           libxl_domain_config_dispose(&d_config);
>>           free(data);
>> -        free(config_file);
>> +        free(config_source);
>>       }
>>   }
>>
>> @@ -2679,7 +2686,7 @@ static void save_domain_core_begin(const
>>       }
>>   }
>>
>> -static void save_domain_core_writeconfig(int fd, const char *filename,
>> +static void save_domain_core_writeconfig(int fd, const char *source,
>>                                     const uint8_t *config_data, int config_len)
>>   {
>>       struct save_file_header hdr;
>> @@ -2708,13 +2715,13 @@ static void save_domain_core_writeconfig
>>       /* that's the optional data */
>>
>>       CHK_ERRNO( libxl_write_exactly(ctx, fd,
>> -&hdr, sizeof(hdr), filename, "header") );
>> +&hdr, sizeof(hdr), source, "header") );
>>       CHK_ERRNO( libxl_write_exactly(ctx, fd,
>> -        optdata_begin, hdr.optional_data_len, filename, "header") );
>> +        optdata_begin, hdr.optional_data_len, source, "header") );
>>
>>       fprintf(stderr, "Saving to %s new xl format (info"
>>               " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n",
>> -            filename, hdr.mandatory_flags, hdr.optional_flags,
>> +            source, hdr.mandatory_flags, hdr.optional_flags,
>>               hdr.optional_data_len);
>>   }
>>
>> @@ -3039,7 +3046,6 @@ static void migrate_receive(int debug, i
>>       dom_info.daemonize = daemonize;
>>       dom_info.monitor = monitor;
>>       dom_info.paused = 1;
>> -    dom_info.restore_file = "incoming migration stream";
>>       dom_info.migrate_fd = 0; /* stdin */
>>       dom_info.migration_domname_r =&migration_domname;
>>       dom_info.incr_generationid = 0;
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>

Re: [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source"

[Ian Campbell]

On Tue, 2012-05-15 at 16:47 +0100, George Dunlap wrote:
> On 15/05/12 16:29, Ian Campbell wrote:
> > On Tue, 2012-05-15 at 15:51 +0100, George Dunlap wrote:
> >> Many places in xl there's a variable or element named "filename" which
> >> does not contain a filename, but the source of the data for reporting
> >> purposes.  Worse, there are variables which are sometimes actually
> >> used or interpreted as a filename depending on what other variables
> >> are set.  This makes it difficult to tell when a string is purely
> >> cosmetic, and when another bit of code may actually attempt to call
> >> "open" with the string.
> >>
> >> This patch makes a consistent distinction between "filename" (which
> >> always refers to the name of an actual file, and may be interpreted as
> >> such at some point) and "source" (which may be a filename, or may be
> >> another data source such as a migration stream or saved data).
> >>
> >> This does add some variables and reshuffle where assignments happen;
> >> most notably, the "restore_filename" element of struct domain_create
> >> is now only set when restoring from a file.
> >>
> >> But at a high level, there should be no funcitonal changes.
> >>
> >> Signed-off-by: George Dunlap<george.dunlap@eu.citrix.com>
> > I've committed this (and the preceding patch), there were a few
> > conflicts with Goncalo's vncviewer patch (a new param to
> > parse_config_data). Please do check I've done the right thing.
> Looks like you missed one s/config_file/config_source/ for 
> parse_config_data; I'll follow-up with a patch.

Thanks, I did build test but I guess this is a runtime issue?

> 
>   -George
> >
> >> diff -r 750b4dbab4b5 -r d555dd6614af tools/libxl/libxl_utils.c
> >> --- a/tools/libxl/libxl_utils.c Tue May 15 15:45:55 2012 +0100
> >> +++ b/tools/libxl/libxl_utils.c Tue May 15 15:51:25 2012 +0100
> >> @@ -334,7 +334,7 @@ int libxl_read_file_contents(libxl_ctx *
> >>                                                                             \
> >>     int libxl_##rw##_exactly(libxl_ctx *ctx, int fd,                 \
> >>                              constdata void *data, ssize_t sz,              \
> >> -                           const char *filename, const char *what) {      \
> >> +                           const char *source, const char *what) {      \
> >>         ssize_t got;                                                        \
> >>                                                                             \
> >>         while (sz>  0) {                                                    \
> >> @@ -343,7 +343,7 @@ int libxl_read_file_contents(libxl_ctx *
> >>                 if (errno == EINTR) continue;                               \
> >>                 if (!ctx) return errno;                                     \
> >>                 LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to " #rw " %s%s%s", \
> >> -                           what?what:"", what?" from ":"", filename);     \
> >> +                           what?what:"", what?" from ":"", source);       \
> >>                 return errno;                                               \
> >>             }                                                               \
> >>             if (got == 0) {                                                 \
> >> @@ -352,7 +352,7 @@ int libxl_read_file_contents(libxl_ctx *
> >>                        zero_is_eof                                          \
> >>                        ? "file/stream truncated reading %s%s%s"             \
> >>                        : "file/stream write returned 0! writing %s%s%s",    \
> >> -                     what?what:"", what?" from ":"", filename);           \
> >> +                     what?what:"", what?" from ":"", source);             \
> >>                 return EPROTO;                                              \
> >>             }                                                               \
> >>             sz -= got;                                                      \
> >> diff -r 750b4dbab4b5 -r d555dd6614af tools/libxl/xl_cmdimpl.c
> >> --- a/tools/libxl/xl_cmdimpl.c  Tue May 15 15:45:55 2012 +0100
> >> +++ b/tools/libxl/xl_cmdimpl.c  Tue May 15 15:51:25 2012 +0100
> >> @@ -520,9 +520,9 @@ vcpp_out:
> >>       return rc;
> >>   }
> >>
> >> -static void parse_config_data(const char *configfile_filename_report,
> >> -                              const char *configfile_data,
> >> -                              int configfile_len,
> >> +static void parse_config_data(const char *config_source,
> >> +                              const char *config_data,
> >> +                              int config_len,
> >>                                 libxl_domain_config *d_config)
> >>   {
> >>       const char *buf;
> >> @@ -537,15 +537,15 @@ static void parse_config_data(const char
> >>       libxl_domain_create_info *c_info =&d_config->c_info;
> >>       libxl_domain_build_info *b_info =&d_config->b_info;
> >>
> >> -    config= xlu_cfg_init(stderr, configfile_filename_report);
> >> +    config= xlu_cfg_init(stderr, config_source);
> >>       if (!config) {
> >>           fprintf(stderr, "Failed to allocate for configuration\n");
> >>           exit(1);
> >>       }
> >>
> >> -    e= xlu_cfg_readdata(config, configfile_data, configfile_len);
> >> +    e= xlu_cfg_readdata(config, config_data, config_len);
> >>       if (e) {
> >> -        fprintf(stderr, "Failed to parse config file: %s\n", strerror(e));
> >> +        fprintf(stderr, "Failed to parse config: %s\n", strerror(e));
> >>           exit(1);
> >>       }
> >>
> >> @@ -1524,6 +1524,8 @@ static int create_domain(struct domain_c
> >>       const char *config_file = dom_info->config_file;
> >>       const char *extra_config = dom_info->extra_config;
> >>       const char *restore_file = dom_info->restore_file;
> >> +    const char *config_source = NULL;
> >> +    const char *restore_source = NULL;
> >>       int migrate_fd = dom_info->migrate_fd;
> >>
> >>       int i;
> >> @@ -1539,24 +1541,28 @@ static int create_domain(struct domain_c
> >>       pid_t child_console_pid = -1;
> >>       struct save_file_header hdr;
> >>
> >> +    int restoring = (restore_file || (migrate_fd>= 0));
> >> +
> >>       libxl_domain_config_init(&d_config);
> >>
> >> -    if (restore_file) {
> >> +    if (restoring) {
> >>           uint8_t *optdata_begin = 0;
> >>           const uint8_t *optdata_here = 0;
> >>           union { uint32_t u32; char b[4]; } u32buf;
> >>           uint32_t badflags;
> >>
> >>           if (migrate_fd>= 0) {
> >> +            restore_source = "<incoming migration stream>";
> >>               restore_fd = migrate_fd;
> >>           } else {
> >> +            restore_source = restore_file;
> >>               restore_fd = open(restore_file, O_RDONLY);
> >>               rc = libxl_fd_set_cloexec(ctx, restore_fd, 1);
> >>               if (rc) return rc;
> >>           }
> >>
> >>           CHK_ERRNO( libxl_read_exactly(ctx, restore_fd,&hdr,
> >> -                   sizeof(hdr), restore_file, "header") );
> >> +                   sizeof(hdr), restore_source, "header") );
> >>           if (memcmp(hdr.magic, savefileheader_magic, sizeof(hdr.magic))) {
> >>               fprintf(stderr, "File has wrong magic number -"
> >>                       " corrupt or for a different tool?\n");
> >> @@ -1569,7 +1575,7 @@ static int create_domain(struct domain_c
> >>           fprintf(stderr, "Loading new save file %s"
> >>                   " (new xl fmt info"
> >>                   " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n",
> >> -                restore_file, hdr.mandatory_flags, hdr.optional_flags,
> >> +                restore_source, hdr.mandatory_flags, hdr.optional_flags,
> >>                   hdr.optional_data_len);
> >>
> >>           badflags = hdr.mandatory_flags&  ~( 0 /* none understood yet */ );
> >> @@ -1582,7 +1588,7 @@ static int create_domain(struct domain_c
> >>           if (hdr.optional_data_len) {
> >>               optdata_begin = xmalloc(hdr.optional_data_len);
> >>               CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, optdata_begin,
> >> -                   hdr.optional_data_len, restore_file, "optdata") );
> >> +                   hdr.optional_data_len, restore_source, "optdata") );
> >>           }
> >>
> >>   #define OPTDATA_LEFT  (hdr.optional_data_len - (optdata_here - optdata_begin))
> >> @@ -1617,7 +1623,7 @@ static int create_domain(struct domain_c
> >>                                          &config_data,&config_len);
> >>           if (ret) { fprintf(stderr, "Failed to read config file: %s: %s\n",
> >>                              config_file, strerror(errno)); return ERROR_FAIL; }
> >> -        if (!restore_file&&  extra_config&&  strlen(extra_config)) {
> >> +        if (!restoring&&  extra_config&&  strlen(extra_config)) {
> >>               if (config_len>  INT_MAX - (strlen(extra_config) + 2 + 1)) {
> >>                   fprintf(stderr, "Failed to attach extra configration\n");
> >>                   return ERROR_FAIL;
> >> @@ -1632,19 +1638,20 @@ static int create_domain(struct domain_c
> >>               config_len += sprintf(config_data + config_len, "\n%s\n",
> >>                   extra_config);
> >>           }
> >> +        config_source=config_file;
> >>       } else {
> >>           if (!config_data) {
> >>               fprintf(stderr, "Config file not specified and"
> >>                       " none in save file\n");
> >>               return ERROR_INVAL;
> >>           }
> >> -        config_file = "<saved>";
> >> +        config_source = "<saved>";
> >>       }
> >>
> >>       if (!dom_info->quiet)
> >> -        printf("Parsing config file %s\n", config_file);
> >> -
> >> -    parse_config_data(config_file, config_data, config_len,&d_config);
> >> +        printf("Parsing config from %s\n", config_source);
> >> +
> >> +    parse_config_data(config_source, config_data, config_len,&d_config);
> >>
> >>       if (migrate_fd>= 0) {
> >>           if (d_config.c_info.name) {
> >> @@ -1698,7 +1705,7 @@ start:
> >>           autoconnect_console_how = 0;
> >>       }
> >>
> >> -    if ( restore_file ) {
> >> +    if ( restoring ) {
> >>           ret = libxl_domain_create_restore(ctx,&d_config,
> >>                                             &domid, restore_fd,
> >>                                             0, autoconnect_console_how);
> >> @@ -2559,7 +2566,7 @@ static void list_domains_details(const l
> >>   {
> >>       libxl_domain_config d_config;
> >>
> >> -    char *config_file;
> >> +    char *config_source;
> >>       uint8_t *data;
> >>       int i, len, rc;
> >>
> >> @@ -2570,13 +2577,13 @@ static void list_domains_details(const l
> >>           rc = libxl_userdata_retrieve(ctx, info[i].domid, "xl",&data,&len);
> >>           if (rc)
> >>               continue;
> >> -        CHK_ERRNO(asprintf(&config_file, "<domid %d data>", info[i].domid));
> >> +        CHK_ERRNO(asprintf(&config_source, "<domid %d data>", info[i].domid));
> >>           libxl_domain_config_init(&d_config);
> >> -        parse_config_data(config_file, (char *)data, len,&d_config);
> >> +        parse_config_data(config_source, (char *)data, len,&d_config);
> >>           printf_info(default_output_format, info[i].domid,&d_config);
> >>           libxl_domain_config_dispose(&d_config);
> >>           free(data);
> >> -        free(config_file);
> >> +        free(config_source);
> >>       }
> >>   }
> >>
> >> @@ -2679,7 +2686,7 @@ static void save_domain_core_begin(const
> >>       }
> >>   }
> >>
> >> -static void save_domain_core_writeconfig(int fd, const char *filename,
> >> +static void save_domain_core_writeconfig(int fd, const char *source,
> >>                                     const uint8_t *config_data, int config_len)
> >>   {
> >>       struct save_file_header hdr;
> >> @@ -2708,13 +2715,13 @@ static void save_domain_core_writeconfig
> >>       /* that's the optional data */
> >>
> >>       CHK_ERRNO( libxl_write_exactly(ctx, fd,
> >> -&hdr, sizeof(hdr), filename, "header") );
> >> +&hdr, sizeof(hdr), source, "header") );
> >>       CHK_ERRNO( libxl_write_exactly(ctx, fd,
> >> -        optdata_begin, hdr.optional_data_len, filename, "header") );
> >> +        optdata_begin, hdr.optional_data_len, source, "header") );
> >>
> >>       fprintf(stderr, "Saving to %s new xl format (info"
> >>               " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n",
> >> -            filename, hdr.mandatory_flags, hdr.optional_flags,
> >> +            source, hdr.mandatory_flags, hdr.optional_flags,
> >>               hdr.optional_data_len);
> >>   }
> >>
> >> @@ -3039,7 +3046,6 @@ static void migrate_receive(int debug, i
> >>       dom_info.daemonize = daemonize;
> >>       dom_info.monitor = monitor;
> >>       dom_info.paused = 1;
> >> -    dom_info.restore_file = "incoming migration stream";
> >>       dom_info.migrate_fd = 0; /* stdin */
> >>       dom_info.migration_domname_r =&migration_domname;
> >>       dom_info.incr_generationid = 0;
> >>
> >> _______________________________________________
> >> Xen-devel mailing list
> >> Xen-devel@lists.xen.org
> >> http://lists.xen.org/xen-devel
> >
> 

[PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source"

[George Dunlap]

# HG changeset patch
# User George Dunlap <george.dunlap@eu.citrix.com>
# Date 1335971129 -3600
# Node ID 60013c9b8d62e39988db3c5b22c0510f5557756b
# Parent  c3eda4333f1562d68c8b56ae3ef9d73e6b68d873
xl: Make clear distinction between "filename" and "data source"

Many places in xl there's a variable or element named "filename" which
does not contain a filename, but the source of the data for reporting
purposes.  Worse, there are variables which are sometimes actually
used or interpreted as a filename depending on what other variables
are set.  This makes it difficult to tell when a string is purely
cosmetic, and when another bit of code may actually attempt to call
"open" with the string.

This patch makes a consistent distinction between "filename" (which
always refers to the name of an actual file, and may be interpreted as
such at some point) and "source" (which may be a filename, or may be
another data source such as a migration stream or saved data).

This does add some variables and reshuffle where assignments happen;
most notably, the "restore_filename" element of struct domain_create
is now only set when restoring from a file.

But at a high level, there should be no funcitonal changes.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

diff -r c3eda4333f15 -r 60013c9b8d62 tools/libxl/libxl_utils.c
--- a/tools/libxl/libxl_utils.c	Wed May 02 16:05:26 2012 +0100
+++ b/tools/libxl/libxl_utils.c	Wed May 02 16:05:29 2012 +0100
@@ -334,7 +334,7 @@ int libxl_read_file_contents(libxl_ctx *
                                                                           \
   int libxl_##rw##_exactly(libxl_ctx *ctx, int fd,                 \
                            constdata void *data, ssize_t sz,              \
-                           const char *filename, const char *what) {      \
+                           const char *source, const char *what) {      \
       ssize_t got;                                                        \
                                                                           \
       while (sz > 0) {                                                    \
@@ -343,7 +343,7 @@ int libxl_read_file_contents(libxl_ctx *
               if (errno == EINTR) continue;                               \
               if (!ctx) return errno;                                     \
               LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to " #rw " %s%s%s", \
-                           what?what:"", what?" from ":"", filename);     \
+                           what?what:"", what?" from ":"", source);       \
               return errno;                                               \
           }                                                               \
           if (got == 0) {                                                 \
@@ -352,7 +352,7 @@ int libxl_read_file_contents(libxl_ctx *
                      zero_is_eof                                          \
                      ? "file/stream truncated reading %s%s%s"             \
                      : "file/stream write returned 0! writing %s%s%s",    \
-                     what?what:"", what?" from ":"", filename);           \
+                     what?what:"", what?" from ":"", source);             \
               return EPROTO;                                              \
           }                                                               \
           sz -= got;                                                      \
diff -r c3eda4333f15 -r 60013c9b8d62 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Wed May 02 16:05:26 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Wed May 02 16:05:29 2012 +0100
@@ -520,9 +520,9 @@ vcpp_out:
     return rc;
 }
 
-static void parse_config_data(const char *configfile_filename_report,
-                              const char *configfile_data,
-                              int configfile_len,
+static void parse_config_data(const char *config_source,
+                              const char *config_data,
+                              int config_len,
                               libxl_domain_config *d_config)
 {
     const char *buf;
@@ -537,15 +537,15 @@ static void parse_config_data(const char
     libxl_domain_create_info *c_info = &d_config->c_info;
     libxl_domain_build_info *b_info = &d_config->b_info;
 
-    config= xlu_cfg_init(stderr, configfile_filename_report);
+    config= xlu_cfg_init(stderr, config_source);
     if (!config) {
         fprintf(stderr, "Failed to allocate for configuration\n");
         exit(1);
     }
 
-    e= xlu_cfg_readdata(config, configfile_data, configfile_len);
+    e= xlu_cfg_readdata(config, config_data, config_len);
     if (e) {
-        fprintf(stderr, "Failed to parse config file: %s\n", strerror(e));
+        fprintf(stderr, "Failed to parse config: %s\n", strerror(e));
         exit(1);
     }
 
@@ -1522,6 +1522,8 @@ static int create_domain(struct domain_c
     const char *config_file = dom_info->config_file;
     const char *extra_config = dom_info->extra_config;
     const char *restore_file = dom_info->restore_file;
+    const char *config_source = NULL;
+    const char *restore_source = NULL;
     int migrate_fd = dom_info->migrate_fd;
 
     int i;
@@ -1537,24 +1539,28 @@ static int create_domain(struct domain_c
     pid_t child_console_pid = -1;
     struct save_file_header hdr;
 
+    int restoring = (restore_file || (migrate_fd >= 0));
+
     libxl_domain_config_init(&d_config);
 
-    if (restore_file) {
+    if (restoring) {
         uint8_t *optdata_begin = 0;
         const uint8_t *optdata_here = 0;
         union { uint32_t u32; char b[4]; } u32buf;
         uint32_t badflags;
 
         if (migrate_fd >= 0) {
+            restore_source = "<incoming migration stream>"
             restore_fd = migrate_fd;
         } else {
+            restore_source = restore_file;
             restore_fd = open(restore_file, O_RDONLY);
             rc = libxl_fd_set_cloexec(ctx, restore_fd, 1);
             if (rc) return rc;
         }
 
         CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, &hdr,
-                   sizeof(hdr), restore_file, "header") );
+                   sizeof(hdr), restore_source, "header") );
         if (memcmp(hdr.magic, savefileheader_magic, sizeof(hdr.magic))) {
             fprintf(stderr, "File has wrong magic number -"
                     " corrupt or for a different tool?\n");
@@ -1567,7 +1573,7 @@ static int create_domain(struct domain_c
         fprintf(stderr, "Loading new save file %s"
                 " (new xl fmt info"
                 " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n",
-                restore_file, hdr.mandatory_flags, hdr.optional_flags,
+                restore_source, hdr.mandatory_flags, hdr.optional_flags,
                 hdr.optional_data_len);
 
         badflags = hdr.mandatory_flags & ~( 0 /* none understood yet */ );
@@ -1580,7 +1586,7 @@ static int create_domain(struct domain_c
         if (hdr.optional_data_len) {
             optdata_begin = xmalloc(hdr.optional_data_len);
             CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, optdata_begin,
-                   hdr.optional_data_len, restore_file, "optdata") );
+                   hdr.optional_data_len, restore_source, "optdata") );
         }
 
 #define OPTDATA_LEFT  (hdr.optional_data_len - (optdata_here - optdata_begin))
@@ -1615,7 +1621,7 @@ static int create_domain(struct domain_c
                                        &config_data, &config_len);
         if (ret) { fprintf(stderr, "Failed to read config file: %s: %s\n",
                            config_file, strerror(errno)); return ERROR_FAIL; }
-        if (!restore_file && extra_config && strlen(extra_config)) {
+        if (!restoring && extra_config && strlen(extra_config)) {
             if (config_len > INT_MAX - (strlen(extra_config) + 2 + 1)) {
                 fprintf(stderr, "Failed to attach extra configration\n");
                 return ERROR_FAIL;
@@ -1630,19 +1636,20 @@ static int create_domain(struct domain_c
             config_len += sprintf(config_data + config_len, "\n%s\n",
                 extra_config);
         }
+        config_source=config_file;
     } else {
         if (!config_data) {
             fprintf(stderr, "Config file not specified and"
                     " none in save file\n");
             return ERROR_INVAL;
         }
-        config_file = "<saved>";
+        config_source = "<saved>";
     }
 
     if (!dom_info->quiet)
-        printf("Parsing config file %s\n", config_file);
-
-    parse_config_data(config_file, config_data, config_len, &d_config);
+        printf("Parsing config from %s\n", config_source);
+
+    parse_config_data(config_source, config_data, config_len, &d_config);
 
     if (migrate_fd >= 0) {
         if (d_config.c_info.name) {
@@ -1693,7 +1700,7 @@ start:
         cb = NULL;
     }
 
-    if ( restore_file ) {
+    if ( restoring ) {
         ret = libxl_domain_create_restore(ctx, &d_config,
                                             cb, &child_console_pid,
                                             &domid, restore_fd);
@@ -2469,7 +2476,7 @@ static void list_domains_details(const l
 {
     libxl_domain_config d_config;
 
-    char *config_file;
+    char *config_source;
     uint8_t *data;
     int i, len, rc;
 
@@ -2480,13 +2487,13 @@ static void list_domains_details(const l
         rc = libxl_userdata_retrieve(ctx, info[i].domid, "xl", &data, &len);
         if (rc)
             continue;
-        CHK_ERRNO(asprintf(&config_file, "<domid %d data>", info[i].domid));
+        CHK_ERRNO(asprintf(&config_source, "<domid %d data>", info[i].domid));
         libxl_domain_config_init(&d_config);
-        parse_config_data(config_file, (char *)data, len, &d_config);
+        parse_config_data(config_source, (char *)data, len, &d_config);
         printf_info(default_output_format, info[i].domid, &d_config);
         libxl_domain_config_dispose(&d_config);
         free(data);
-        free(config_file);
+        free(config_source);
     }
 }
 
@@ -2589,7 +2596,7 @@ static void save_domain_core_begin(const
     }
 }
 
-static void save_domain_core_writeconfig(int fd, const char *filename,
+static void save_domain_core_writeconfig(int fd, const char *source,
                                   const uint8_t *config_data, int config_len)
 {
     struct save_file_header hdr;
@@ -2618,13 +2625,13 @@ static void save_domain_core_writeconfig
     /* that's the optional data */
 
     CHK_ERRNO( libxl_write_exactly(ctx, fd,
-        &hdr, sizeof(hdr), filename, "header") );
+        &hdr, sizeof(hdr), source, "header") );
     CHK_ERRNO( libxl_write_exactly(ctx, fd,
-        optdata_begin, hdr.optional_data_len, filename, "header") );
+        optdata_begin, hdr.optional_data_len, source, "header") );
 
     fprintf(stderr, "Saving to %s new xl format (info"
             " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n",
-            filename, hdr.mandatory_flags, hdr.optional_flags,
+            source, hdr.mandatory_flags, hdr.optional_flags,
             hdr.optional_data_len);
 }
 
@@ -2950,7 +2957,6 @@ static void migrate_receive(int debug, i
     dom_info.daemonize = daemonize;
     dom_info.monitor = monitor;
     dom_info.paused = 1;
-    dom_info.restore_file = "incoming migration stream";
     dom_info.migrate_fd = 0; /* stdin */
     dom_info.migration_domname_r = &migration_domname;
     dom_info.incr_generationid = 0;

Re: [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source"

[George Dunlap]

Hmm -- sorry, I seemed so have mucked some things up on the rebase...
please don't apply this one.

 -George

On Wed, May 2, 2012 at 4:10 PM, George Dunlap
<george.dunlap@eu.citrix.com> wrote:
> # HG changeset patch
> # User George Dunlap <george.dunlap@eu.citrix.com>
> # Date 1335971129 -3600
> # Node ID 60013c9b8d62e39988db3c5b22c0510f5557756b
> # Parent  c3eda4333f1562d68c8b56ae3ef9d73e6b68d873
> xl: Make clear distinction between "filename" and "data source"
>
> Many places in xl there's a variable or element named "filename" which
> does not contain a filename, but the source of the data for reporting
> purposes.  Worse, there are variables which are sometimes actually
> used or interpreted as a filename depending on what other variables
> are set.  This makes it difficult to tell when a string is purely
> cosmetic, and when another bit of code may actually attempt to call
> "open" with the string.
>
> This patch makes a consistent distinction between "filename" (which
> always refers to the name of an actual file, and may be interpreted as
> such at some point) and "source" (which may be a filename, or may be
> another data source such as a migration stream or saved data).
>
> This does add some variables and reshuffle where assignments happen;
> most notably, the "restore_filename" element of struct domain_create
> is now only set when restoring from a file.
>
> But at a high level, there should be no funcitonal changes.
>
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
>
> diff -r c3eda4333f15 -r 60013c9b8d62 tools/libxl/libxl_utils.c
> --- a/tools/libxl/libxl_utils.c Wed May 02 16:05:26 2012 +0100
> +++ b/tools/libxl/libxl_utils.c Wed May 02 16:05:29 2012 +0100
> @@ -334,7 +334,7 @@ int libxl_read_file_contents(libxl_ctx *
>                                                                           \
>   int libxl_##rw##_exactly(libxl_ctx *ctx, int fd,                 \
>                            constdata void *data, ssize_t sz,              \
> -                           const char *filename, const char *what) {      \
> +                           const char *source, const char *what) {      \
>       ssize_t got;                                                        \
>                                                                           \
>       while (sz > 0) {                                                    \
> @@ -343,7 +343,7 @@ int libxl_read_file_contents(libxl_ctx *
>               if (errno == EINTR) continue;                               \
>               if (!ctx) return errno;                                     \
>               LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to " #rw " %s%s%s", \
> -                           what?what:"", what?" from ":"", filename);     \
> +                           what?what:"", what?" from ":"", source);       \
>               return errno;                                               \
>           }                                                               \
>           if (got == 0) {                                                 \
> @@ -352,7 +352,7 @@ int libxl_read_file_contents(libxl_ctx *
>                      zero_is_eof                                          \
>                      ? "file/stream truncated reading %s%s%s"             \
>                      : "file/stream write returned 0! writing %s%s%s",    \
> -                     what?what:"", what?" from ":"", filename);           \
> +                     what?what:"", what?" from ":"", source);             \
>               return EPROTO;                                              \
>           }                                                               \
>           sz -= got;                                                      \
> diff -r c3eda4333f15 -r 60013c9b8d62 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c  Wed May 02 16:05:26 2012 +0100
> +++ b/tools/libxl/xl_cmdimpl.c  Wed May 02 16:05:29 2012 +0100
> @@ -520,9 +520,9 @@ vcpp_out:
>     return rc;
>  }
>
> -static void parse_config_data(const char *configfile_filename_report,
> -                              const char *configfile_data,
> -                              int configfile_len,
> +static void parse_config_data(const char *config_source,
> +                              const char *config_data,
> +                              int config_len,
>                               libxl_domain_config *d_config)
>  {
>     const char *buf;
> @@ -537,15 +537,15 @@ static void parse_config_data(const char
>     libxl_domain_create_info *c_info = &d_config->c_info;
>     libxl_domain_build_info *b_info = &d_config->b_info;
>
> -    config= xlu_cfg_init(stderr, configfile_filename_report);
> +    config= xlu_cfg_init(stderr, config_source);
>     if (!config) {
>         fprintf(stderr, "Failed to allocate for configuration\n");
>         exit(1);
>     }
>
> -    e= xlu_cfg_readdata(config, configfile_data, configfile_len);
> +    e= xlu_cfg_readdata(config, config_data, config_len);
>     if (e) {
> -        fprintf(stderr, "Failed to parse config file: %s\n", strerror(e));
> +        fprintf(stderr, "Failed to parse config: %s\n", strerror(e));
>         exit(1);
>     }
>
> @@ -1522,6 +1522,8 @@ static int create_domain(struct domain_c
>     const char *config_file = dom_info->config_file;
>     const char *extra_config = dom_info->extra_config;
>     const char *restore_file = dom_info->restore_file;
> +    const char *config_source = NULL;
> +    const char *restore_source = NULL;
>     int migrate_fd = dom_info->migrate_fd;
>
>     int i;
> @@ -1537,24 +1539,28 @@ static int create_domain(struct domain_c
>     pid_t child_console_pid = -1;
>     struct save_file_header hdr;
>
> +    int restoring = (restore_file || (migrate_fd >= 0));
> +
>     libxl_domain_config_init(&d_config);
>
> -    if (restore_file) {
> +    if (restoring) {
>         uint8_t *optdata_begin = 0;
>         const uint8_t *optdata_here = 0;
>         union { uint32_t u32; char b[4]; } u32buf;
>         uint32_t badflags;
>
>         if (migrate_fd >= 0) {
> +            restore_source = "<incoming migration stream>"
>             restore_fd = migrate_fd;
>         } else {
> +            restore_source = restore_file;
>             restore_fd = open(restore_file, O_RDONLY);
>             rc = libxl_fd_set_cloexec(ctx, restore_fd, 1);
>             if (rc) return rc;
>         }
>
>         CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, &hdr,
> -                   sizeof(hdr), restore_file, "header") );
> +                   sizeof(hdr), restore_source, "header") );
>         if (memcmp(hdr.magic, savefileheader_magic, sizeof(hdr.magic))) {
>             fprintf(stderr, "File has wrong magic number -"
>                     " corrupt or for a different tool?\n");
> @@ -1567,7 +1573,7 @@ static int create_domain(struct domain_c
>         fprintf(stderr, "Loading new save file %s"
>                 " (new xl fmt info"
>                 " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n",
> -                restore_file, hdr.mandatory_flags, hdr.optional_flags,
> +                restore_source, hdr.mandatory_flags, hdr.optional_flags,
>                 hdr.optional_data_len);
>
>         badflags = hdr.mandatory_flags & ~( 0 /* none understood yet */ );
> @@ -1580,7 +1586,7 @@ static int create_domain(struct domain_c
>         if (hdr.optional_data_len) {
>             optdata_begin = xmalloc(hdr.optional_data_len);
>             CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, optdata_begin,
> -                   hdr.optional_data_len, restore_file, "optdata") );
> +                   hdr.optional_data_len, restore_source, "optdata") );
>         }
>
>  #define OPTDATA_LEFT  (hdr.optional_data_len - (optdata_here - optdata_begin))
> @@ -1615,7 +1621,7 @@ static int create_domain(struct domain_c
>                                        &config_data, &config_len);
>         if (ret) { fprintf(stderr, "Failed to read config file: %s: %s\n",
>                            config_file, strerror(errno)); return ERROR_FAIL; }
> -        if (!restore_file && extra_config && strlen(extra_config)) {
> +        if (!restoring && extra_config && strlen(extra_config)) {
>             if (config_len > INT_MAX - (strlen(extra_config) + 2 + 1)) {
>                 fprintf(stderr, "Failed to attach extra configration\n");
>                 return ERROR_FAIL;
> @@ -1630,19 +1636,20 @@ static int create_domain(struct domain_c
>             config_len += sprintf(config_data + config_len, "\n%s\n",
>                 extra_config);
>         }
> +        config_source=config_file;
>     } else {
>         if (!config_data) {
>             fprintf(stderr, "Config file not specified and"
>                     " none in save file\n");
>             return ERROR_INVAL;
>         }
> -        config_file = "<saved>";
> +        config_source = "<saved>";
>     }
>
>     if (!dom_info->quiet)
> -        printf("Parsing config file %s\n", config_file);
> -
> -    parse_config_data(config_file, config_data, config_len, &d_config);
> +        printf("Parsing config from %s\n", config_source);
> +
> +    parse_config_data(config_source, config_data, config_len, &d_config);
>
>     if (migrate_fd >= 0) {
>         if (d_config.c_info.name) {
> @@ -1693,7 +1700,7 @@ start:
>         cb = NULL;
>     }
>
> -    if ( restore_file ) {
> +    if ( restoring ) {
>         ret = libxl_domain_create_restore(ctx, &d_config,
>                                             cb, &child_console_pid,
>                                             &domid, restore_fd);
> @@ -2469,7 +2476,7 @@ static void list_domains_details(const l
>  {
>     libxl_domain_config d_config;
>
> -    char *config_file;
> +    char *config_source;
>     uint8_t *data;
>     int i, len, rc;
>
> @@ -2480,13 +2487,13 @@ static void list_domains_details(const l
>         rc = libxl_userdata_retrieve(ctx, info[i].domid, "xl", &data, &len);
>         if (rc)
>             continue;
> -        CHK_ERRNO(asprintf(&config_file, "<domid %d data>", info[i].domid));
> +        CHK_ERRNO(asprintf(&config_source, "<domid %d data>", info[i].domid));
>         libxl_domain_config_init(&d_config);
> -        parse_config_data(config_file, (char *)data, len, &d_config);
> +        parse_config_data(config_source, (char *)data, len, &d_config);
>         printf_info(default_output_format, info[i].domid, &d_config);
>         libxl_domain_config_dispose(&d_config);
>         free(data);
> -        free(config_file);
> +        free(config_source);
>     }
>  }
>
> @@ -2589,7 +2596,7 @@ static void save_domain_core_begin(const
>     }
>  }
>
> -static void save_domain_core_writeconfig(int fd, const char *filename,
> +static void save_domain_core_writeconfig(int fd, const char *source,
>                                   const uint8_t *config_data, int config_len)
>  {
>     struct save_file_header hdr;
> @@ -2618,13 +2625,13 @@ static void save_domain_core_writeconfig
>     /* that's the optional data */
>
>     CHK_ERRNO( libxl_write_exactly(ctx, fd,
> -        &hdr, sizeof(hdr), filename, "header") );
> +        &hdr, sizeof(hdr), source, "header") );
>     CHK_ERRNO( libxl_write_exactly(ctx, fd,
> -        optdata_begin, hdr.optional_data_len, filename, "header") );
> +        optdata_begin, hdr.optional_data_len, source, "header") );
>
>     fprintf(stderr, "Saving to %s new xl format (info"
>             " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n",
> -            filename, hdr.mandatory_flags, hdr.optional_flags,
> +            source, hdr.mandatory_flags, hdr.optional_flags,
>             hdr.optional_data_len);
>  }
>
> @@ -2950,7 +2957,6 @@ static void migrate_receive(int debug, i
>     dom_info.daemonize = daemonize;
>     dom_info.monitor = monitor;
>     dom_info.paused = 1;
> -    dom_info.restore_file = "incoming migration stream";
>     dom_info.migrate_fd = 0; /* stdin */
>     dom_info.migration_domname_r = &migration_domname;
>     dom_info.incr_generationid = 0;
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

[PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source"

[George Dunlap]

Many places in xl there's a variable or element named "filename" which
does not contain a filename, but the source of the data for reporting
purposes.  Worse, there are variables which are sometimes actually
used or interpreted as a filename depending on what other variables
are set.  This makes it difficult to tell when a string is purely
cosmetic, and when another bit of code may actually attempt to call
"open" with the string.

This patch makes a consistent distinction between "filename" (which
always refers to the name of an actual file, and may be interpreted as
such at some point) and "source" (which may be a filename, or may be
another data source such as a migration stream or saved data).

This does add some variables and reshuffle where assignments happen;
most notably, the "restore_filename" element of struct domain_create
is now only set when restoring from a file.

But at a high level, there should be no funcitonal changes.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

diff -r 5ca90c805046 -r 30cc13e25e01 tools/libxl/libxl_utils.c
--- a/tools/libxl/libxl_utils.c	Tue Apr 03 18:00:24 2012 +0100
+++ b/tools/libxl/libxl_utils.c	Tue Apr 03 19:02:19 2012 +0100
@@ -334,7 +334,7 @@ int libxl_read_file_contents(libxl_ctx *
                                                                           \
   int libxl_##rw##_exactly(libxl_ctx *ctx, int fd,                 \
                            constdata void *data, ssize_t sz,              \
-                           const char *filename, const char *what) {      \
+                           const char *source, const char *what) {      \
       ssize_t got;                                                        \
                                                                           \
       while (sz > 0) {                                                    \
@@ -343,7 +343,7 @@ int libxl_read_file_contents(libxl_ctx *
               if (errno == EINTR) continue;                               \
               if (!ctx) return errno;                                     \
               LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to " #rw " %s%s%s", \
-                           what?what:"", what?" from ":"", filename);     \
+                           what?what:"", what?" from ":"", source);       \
               return errno;                                               \
           }                                                               \
           if (got == 0) {                                                 \
@@ -352,7 +352,7 @@ int libxl_read_file_contents(libxl_ctx *
                      zero_is_eof                                          \
                      ? "file/stream truncated reading %s%s%s"             \
                      : "file/stream write returned 0! writing %s%s%s",    \
-                     what?what:"", what?" from ":"", filename);           \
+                     what?what:"", what?" from ":"", source);             \
               return EPROTO;                                              \
           }                                                               \
           sz -= got;                                                      \
diff -r 5ca90c805046 -r 30cc13e25e01 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Tue Apr 03 18:00:24 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Tue Apr 03 19:02:19 2012 +0100
@@ -507,9 +507,9 @@ vcpp_out:
     return rc;
 }
 
-static void parse_config_data(const char *configfile_filename_report,
-                              const char *configfile_data,
-                              int configfile_len,
+static void parse_config_data(const char *config_source,
+                              const char *config_data,
+                              int config_len,
                               libxl_domain_config *d_config)
 {
     const char *buf;
@@ -524,15 +524,15 @@ static void parse_config_data(const char
     libxl_domain_create_info *c_info = &d_config->c_info;
     libxl_domain_build_info *b_info = &d_config->b_info;
 
-    config= xlu_cfg_init(stderr, configfile_filename_report);
+    config= xlu_cfg_init(stderr, config_source);
     if (!config) {
         fprintf(stderr, "Failed to allocate for configuration\n");
         exit(1);
     }
 
-    e= xlu_cfg_readdata(config, configfile_data, configfile_len);
+    e= xlu_cfg_readdata(config, config_data, config_len);
     if (e) {
-        fprintf(stderr, "Failed to parse config file: %s\n", strerror(e));
+        fprintf(stderr, "Failed to parse config: %s\n", strerror(e));
         exit(1);
     }
 
@@ -1467,6 +1467,8 @@ static int create_domain(struct domain_c
     const char *config_file = dom_info->config_file;
     const char *extra_config = dom_info->extra_config;
     const char *restore_file = dom_info->restore_file;
+    const char *config_source = NULL;
+    const char *restore_source = NULL;
     int migrate_fd = dom_info->migrate_fd;
 
     int i;
@@ -1482,24 +1484,28 @@ static int create_domain(struct domain_c
     pid_t child_console_pid = -1;
     struct save_file_header hdr;
 
+    int restoring = (restore_file || (migrate_fd >= 0));
+
     memset(&d_config, 0x00, sizeof(d_config));
 
-    if (restore_file) {
+    if (restoring) {
         uint8_t *optdata_begin = 0;
         const uint8_t *optdata_here = 0;
         union { uint32_t u32; char b[4]; } u32buf;
         uint32_t badflags;
 
         if (migrate_fd >= 0) {
+            restore_source = "incoming migration stream";
             restore_fd = migrate_fd;
         } else {
+            restore_source = restore_file;
             restore_fd = open(restore_file, O_RDONLY);
             rc = libxl_fd_set_cloexec(ctx, restore_fd, 1);
             if (rc) return rc;
         }
 
         CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, &hdr,
-                   sizeof(hdr), restore_file, "header") );
+                   sizeof(hdr), restore_source, "header") );
         if (memcmp(hdr.magic, savefileheader_magic, sizeof(hdr.magic))) {
             fprintf(stderr, "File has wrong magic number -"
                     " corrupt or for a different tool?\n");
@@ -1512,7 +1518,7 @@ static int create_domain(struct domain_c
         fprintf(stderr, "Loading new save file %s"
                 " (new xl fmt info"
                 " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n",
-                restore_file, hdr.mandatory_flags, hdr.optional_flags,
+                restore_source, hdr.mandatory_flags, hdr.optional_flags,
                 hdr.optional_data_len);
 
         badflags = hdr.mandatory_flags & ~( 0 /* none understood yet */ );
@@ -1525,7 +1531,7 @@ static int create_domain(struct domain_c
         if (hdr.optional_data_len) {
             optdata_begin = xmalloc(hdr.optional_data_len);
             CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, optdata_begin,
-                   hdr.optional_data_len, restore_file, "optdata") );
+                   hdr.optional_data_len, restore_source, "optdata") );
         }
 
 #define OPTDATA_LEFT  (hdr.optional_data_len - (optdata_here - optdata_begin))
@@ -1560,7 +1566,7 @@ static int create_domain(struct domain_c
                                        &config_data, &config_len);
         if (ret) { fprintf(stderr, "Failed to read config file: %s: %s\n",
                            config_file, strerror(errno)); return ERROR_FAIL; }
-        if (!restore_file && extra_config && strlen(extra_config)) {
+        if (!restoring && extra_config && strlen(extra_config)) {
             if (config_len > INT_MAX - (strlen(extra_config) + 2 + 1)) {
                 fprintf(stderr, "Failed to attach extra configration\n");
                 return ERROR_FAIL;
@@ -1575,19 +1581,20 @@ static int create_domain(struct domain_c
             config_len += sprintf(config_data + config_len, "\n%s\n",
                 extra_config);
         }
+        config_source=config_file;
     } else {
         if (!config_data) {
             fprintf(stderr, "Config file not specified and"
                     " none in save file\n");
             return ERROR_INVAL;
         }
-        config_file = "<saved>";
+        config_source = "<saved>";
     }
 
     if (!dom_info->quiet)
-        printf("Parsing config file %s\n", config_file);
-
-    parse_config_data(config_file, config_data, config_len, &d_config);
+        printf("Parsing config from %s\n", config_source);
+
+    parse_config_data(config_source, config_data, config_len, &d_config);
 
     if (migrate_fd >= 0) {
         if (d_config.c_info.name) {
@@ -1638,7 +1645,7 @@ start:
         cb = NULL;
     }
 
-    if ( restore_file ) {
+    if ( restoring ) {
         ret = libxl_domain_create_restore(ctx, &d_config,
                                             cb, &child_console_pid,
                                             &domid, restore_fd);
@@ -2410,7 +2417,7 @@ static void list_domains_details(const l
 {
     libxl_domain_config d_config;
 
-    char *config_file;
+    char *config_source;
     uint8_t *data;
     int i, len, rc;
 
@@ -2421,13 +2428,13 @@ static void list_domains_details(const l
         rc = libxl_userdata_retrieve(ctx, info[i].domid, "xl", &data, &len);
         if (rc)
             continue;
-        CHK_ERRNO(asprintf(&config_file, "<domid %d data>", info[i].domid));
+        CHK_ERRNO(asprintf(&config_source, "<domid %d data>", info[i].domid));
         memset(&d_config, 0x00, sizeof(d_config));
-        parse_config_data(config_file, (char *)data, len, &d_config);
+        parse_config_data(config_source, (char *)data, len, &d_config);
         printf_info(default_output_format, info[i].domid, &d_config);
         libxl_domain_config_dispose(&d_config);
         free(data);
-        free(config_file);
+        free(config_source);
     }
 }
 
@@ -2530,7 +2537,7 @@ static void save_domain_core_begin(const
     }
 }
 
-static void save_domain_core_writeconfig(int fd, const char *filename,
+static void save_domain_core_writeconfig(int fd, const char *source,
                                   const uint8_t *config_data, int config_len)
 {
     struct save_file_header hdr;
@@ -2559,13 +2566,13 @@ static void save_domain_core_writeconfig
     /* that's the optional data */
 
     CHK_ERRNO( libxl_write_exactly(ctx, fd,
-        &hdr, sizeof(hdr), filename, "header") );
+        &hdr, sizeof(hdr), source, "header") );
     CHK_ERRNO( libxl_write_exactly(ctx, fd,
-        optdata_begin, hdr.optional_data_len, filename, "header") );
+        optdata_begin, hdr.optional_data_len, source, "header") );
 
     fprintf(stderr, "Saving to %s new xl format (info"
             " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n",
-            filename, hdr.mandatory_flags, hdr.optional_flags,
+            source, hdr.mandatory_flags, hdr.optional_flags,
             hdr.optional_data_len);
 }
 
@@ -2891,7 +2898,6 @@ static void migrate_receive(int debug, i
     dom_info.daemonize = daemonize;
     dom_info.monitor = monitor;
     dom_info.paused = 1;
-    dom_info.restore_file = "incoming migration stream";
     dom_info.migrate_fd = 0; /* stdin */
     dom_info.migration_domname_r = &migration_domname;
     dom_info.incr_generationid = 0;

Re: [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source"

[George Dunlap]

On 03/04/12 19:28, George Dunlap wrote:
> Many places in xl there's a variable or element named "filename" which
> does not contain a filename, but the source of the data for reporting
> purposes.  Worse, there are variables which are sometimes actually
> used or interpreted as a filename depending on what other variables
> are set.  This makes it difficult to tell when a string is purely
> cosmetic, and when another bit of code may actually attempt to call
> "open" with the string.
>
> This patch makes a consistent distinction between "filename" (which
> always refers to the name of an actual file, and may be interpreted as
> such at some point) and "source" (which may be a filename, or may be
> another data source such as a migration stream or saved data).
>
> This does add some variables and reshuffle where assignments happen;
> most notably, the "restore_filename" element of struct domain_create
> is now only set when restoring from a file.
>
> But at a high level, there should be no funcitonal changes.
>
> Signed-off-by: George Dunlap<george.dunlap@eu.citrix.com>
Sorry, this was meant to be an RFC patch before I did more thorough 
testing to make sure there aren't actually any regressions. Please give 
your opinions but do not apply yet.
>
> diff -r 5ca90c805046 -r 30cc13e25e01 tools/libxl/libxl_utils.c
> --- a/tools/libxl/libxl_utils.c	Tue Apr 03 18:00:24 2012 +0100
> +++ b/tools/libxl/libxl_utils.c	Tue Apr 03 19:02:19 2012 +0100
> @@ -334,7 +334,7 @@ int libxl_read_file_contents(libxl_ctx *
>                                                                             \
>     int libxl_##rw##_exactly(libxl_ctx *ctx, int fd,                 \
>                              constdata void *data, ssize_t sz,              \
> -                           const char *filename, const char *what) {      \
> +                           const char *source, const char *what) {      \
>         ssize_t got;                                                        \
>                                                                             \
>         while (sz>  0) {                                                    \
> @@ -343,7 +343,7 @@ int libxl_read_file_contents(libxl_ctx *
>                 if (errno == EINTR) continue;                               \
>                 if (!ctx) return errno;                                     \
>                 LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to " #rw " %s%s%s", \
> -                           what?what:"", what?" from ":"", filename);     \
> +                           what?what:"", what?" from ":"", source);       \
>                 return errno;                                               \
>             }                                                               \
>             if (got == 0) {                                                 \
> @@ -352,7 +352,7 @@ int libxl_read_file_contents(libxl_ctx *
>                        zero_is_eof                                          \
>                        ? "file/stream truncated reading %s%s%s"             \
>                        : "file/stream write returned 0! writing %s%s%s",    \
> -                     what?what:"", what?" from ":"", filename);           \
> +                     what?what:"", what?" from ":"", source);             \
>                 return EPROTO;                                              \
>             }                                                               \
>             sz -= got;                                                      \
> diff -r 5ca90c805046 -r 30cc13e25e01 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c	Tue Apr 03 18:00:24 2012 +0100
> +++ b/tools/libxl/xl_cmdimpl.c	Tue Apr 03 19:02:19 2012 +0100
> @@ -507,9 +507,9 @@ vcpp_out:
>       return rc;
>   }
>
> -static void parse_config_data(const char *configfile_filename_report,
> -                              const char *configfile_data,
> -                              int configfile_len,
> +static void parse_config_data(const char *config_source,
> +                              const char *config_data,
> +                              int config_len,
>                                 libxl_domain_config *d_config)
>   {
>       const char *buf;
> @@ -524,15 +524,15 @@ static void parse_config_data(const char
>       libxl_domain_create_info *c_info =&d_config->c_info;
>       libxl_domain_build_info *b_info =&d_config->b_info;
>
> -    config= xlu_cfg_init(stderr, configfile_filename_report);
> +    config= xlu_cfg_init(stderr, config_source);
>       if (!config) {
>           fprintf(stderr, "Failed to allocate for configuration\n");
>           exit(1);
>       }
>
> -    e= xlu_cfg_readdata(config, configfile_data, configfile_len);
> +    e= xlu_cfg_readdata(config, config_data, config_len);
>       if (e) {
> -        fprintf(stderr, "Failed to parse config file: %s\n", strerror(e));
> +        fprintf(stderr, "Failed to parse config: %s\n", strerror(e));
>           exit(1);
>       }
>
> @@ -1467,6 +1467,8 @@ static int create_domain(struct domain_c
>       const char *config_file = dom_info->config_file;
>       const char *extra_config = dom_info->extra_config;
>       const char *restore_file = dom_info->restore_file;
> +    const char *config_source = NULL;
> +    const char *restore_source = NULL;
>       int migrate_fd = dom_info->migrate_fd;
>
>       int i;
> @@ -1482,24 +1484,28 @@ static int create_domain(struct domain_c
>       pid_t child_console_pid = -1;
>       struct save_file_header hdr;
>
> +    int restoring = (restore_file || (migrate_fd>= 0));
> +
>       memset(&d_config, 0x00, sizeof(d_config));
>
> -    if (restore_file) {
> +    if (restoring) {
>           uint8_t *optdata_begin = 0;
>           const uint8_t *optdata_here = 0;
>           union { uint32_t u32; char b[4]; } u32buf;
>           uint32_t badflags;
>
>           if (migrate_fd>= 0) {
> +            restore_source = "incoming migration stream";
>               restore_fd = migrate_fd;
>           } else {
> +            restore_source = restore_file;
>               restore_fd = open(restore_file, O_RDONLY);
>               rc = libxl_fd_set_cloexec(ctx, restore_fd, 1);
>               if (rc) return rc;
>           }
>
>           CHK_ERRNO( libxl_read_exactly(ctx, restore_fd,&hdr,
> -                   sizeof(hdr), restore_file, "header") );
> +                   sizeof(hdr), restore_source, "header") );
>           if (memcmp(hdr.magic, savefileheader_magic, sizeof(hdr.magic))) {
>               fprintf(stderr, "File has wrong magic number -"
>                       " corrupt or for a different tool?\n");
> @@ -1512,7 +1518,7 @@ static int create_domain(struct domain_c
>           fprintf(stderr, "Loading new save file %s"
>                   " (new xl fmt info"
>                   " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n",
> -                restore_file, hdr.mandatory_flags, hdr.optional_flags,
> +                restore_source, hdr.mandatory_flags, hdr.optional_flags,
>                   hdr.optional_data_len);
>
>           badflags = hdr.mandatory_flags&  ~( 0 /* none understood yet */ );
> @@ -1525,7 +1531,7 @@ static int create_domain(struct domain_c
>           if (hdr.optional_data_len) {
>               optdata_begin = xmalloc(hdr.optional_data_len);
>               CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, optdata_begin,
> -                   hdr.optional_data_len, restore_file, "optdata") );
> +                   hdr.optional_data_len, restore_source, "optdata") );
>           }
>
>   #define OPTDATA_LEFT  (hdr.optional_data_len - (optdata_here - optdata_begin))
> @@ -1560,7 +1566,7 @@ static int create_domain(struct domain_c
>                                          &config_data,&config_len);
>           if (ret) { fprintf(stderr, "Failed to read config file: %s: %s\n",
>                              config_file, strerror(errno)); return ERROR_FAIL; }
> -        if (!restore_file&&  extra_config&&  strlen(extra_config)) {
> +        if (!restoring&&  extra_config&&  strlen(extra_config)) {
>               if (config_len>  INT_MAX - (strlen(extra_config) + 2 + 1)) {
>                   fprintf(stderr, "Failed to attach extra configration\n");
>                   return ERROR_FAIL;
> @@ -1575,19 +1581,20 @@ static int create_domain(struct domain_c
>               config_len += sprintf(config_data + config_len, "\n%s\n",
>                   extra_config);
>           }
> +        config_source=config_file;
>       } else {
>           if (!config_data) {
>               fprintf(stderr, "Config file not specified and"
>                       " none in save file\n");
>               return ERROR_INVAL;
>           }
> -        config_file = "<saved>";
> +        config_source = "<saved>";
>       }
>
>       if (!dom_info->quiet)
> -        printf("Parsing config file %s\n", config_file);
> -
> -    parse_config_data(config_file, config_data, config_len,&d_config);
> +        printf("Parsing config from %s\n", config_source);
> +
> +    parse_config_data(config_source, config_data, config_len,&d_config);
>
>       if (migrate_fd>= 0) {
>           if (d_config.c_info.name) {
> @@ -1638,7 +1645,7 @@ start:
>           cb = NULL;
>       }
>
> -    if ( restore_file ) {
> +    if ( restoring ) {
>           ret = libxl_domain_create_restore(ctx,&d_config,
>                                               cb,&child_console_pid,
>                                               &domid, restore_fd);
> @@ -2410,7 +2417,7 @@ static void list_domains_details(const l
>   {
>       libxl_domain_config d_config;
>
> -    char *config_file;
> +    char *config_source;
>       uint8_t *data;
>       int i, len, rc;
>
> @@ -2421,13 +2428,13 @@ static void list_domains_details(const l
>           rc = libxl_userdata_retrieve(ctx, info[i].domid, "xl",&data,&len);
>           if (rc)
>               continue;
> -        CHK_ERRNO(asprintf(&config_file, "<domid %d data>", info[i].domid));
> +        CHK_ERRNO(asprintf(&config_source, "<domid %d data>", info[i].domid));
>           memset(&d_config, 0x00, sizeof(d_config));
> -        parse_config_data(config_file, (char *)data, len,&d_config);
> +        parse_config_data(config_source, (char *)data, len,&d_config);
>           printf_info(default_output_format, info[i].domid,&d_config);
>           libxl_domain_config_dispose(&d_config);
>           free(data);
> -        free(config_file);
> +        free(config_source);
>       }
>   }
>
> @@ -2530,7 +2537,7 @@ static void save_domain_core_begin(const
>       }
>   }
>
> -static void save_domain_core_writeconfig(int fd, const char *filename,
> +static void save_domain_core_writeconfig(int fd, const char *source,
>                                     const uint8_t *config_data, int config_len)
>   {
>       struct save_file_header hdr;
> @@ -2559,13 +2566,13 @@ static void save_domain_core_writeconfig
>       /* that's the optional data */
>
>       CHK_ERRNO( libxl_write_exactly(ctx, fd,
> -&hdr, sizeof(hdr), filename, "header") );
> +&hdr, sizeof(hdr), source, "header") );
>       CHK_ERRNO( libxl_write_exactly(ctx, fd,
> -        optdata_begin, hdr.optional_data_len, filename, "header") );
> +        optdata_begin, hdr.optional_data_len, source, "header") );
>
>       fprintf(stderr, "Saving to %s new xl format (info"
>               " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n",
> -            filename, hdr.mandatory_flags, hdr.optional_flags,
> +            source, hdr.mandatory_flags, hdr.optional_flags,
>               hdr.optional_data_len);
>   }
>
> @@ -2891,7 +2898,6 @@ static void migrate_receive(int debug, i
>       dom_info.daemonize = daemonize;
>       dom_info.monitor = monitor;
>       dom_info.paused = 1;
> -    dom_info.restore_file = "incoming migration stream";
>       dom_info.migrate_fd = 0; /* stdin */
>       dom_info.migration_domname_r =&migration_domname;
>       dom_info.incr_generationid = 0;

Re: [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source"

[Ian Jackson]

George Dunlap writes ("[Xen-devel] [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source""):
> Many places in xl there's a variable or element named "filename" which
> does not contain a filename, but the source of the data for reporting
> purposes.  Worse, there are variables which are sometimes actually
> used or interpreted as a filename depending on what other variables
> are set.  This makes it difficult to tell when a string is purely
> cosmetic, and when another bit of code may actually attempt to call
> "open" with the string.
> 
> This patch makes a consistent distinction between "filename" (which
> always refers to the name of an actual file, and may be interpreted as
> such at some point) and "source" (which may be a filename, or may be
> another data source such as a migration stream or saved data).

I think this is a worthy goal.

Ian.