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

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

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@xxxxxxxxxxxxx> wrote:
> # HG changeset patch
> # User George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> # 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@xxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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