* [PATCH 1 of 2] libxlu: Rename filename to config_source
[not found] <patchbomb.1333477720@kodo2>
@ 2012-04-03 18:28 ` George Dunlap
2012-04-03 18:28 ` [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source" George Dunlap
1 sibling, 0 replies; 10+ messages in thread
From: George Dunlap @ 2012-04-03 18:28 UTC (permalink / raw)
To: xen-devel; +Cc: 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 0625fe50a2ee -r 5ca90c805046 tools/libxl/libxlu_cfg.c
--- a/tools/libxl/libxlu_cfg.c Tue Apr 03 14:46:27 2012 +0100
+++ b/tools/libxl/libxlu_cfg.c Tue Apr 03 18:00:24 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 0625fe50a2ee -r 5ca90c805046 tools/libxl/libxlu_disk.c
--- a/tools/libxl/libxlu_disk.c Tue Apr 03 14:46:27 2012 +0100
+++ b/tools/libxl/libxlu_disk.c Tue Apr 03 18:00:24 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 0625fe50a2ee -r 5ca90c805046 tools/libxl/libxlu_internal.h
--- a/tools/libxl/libxlu_internal.h Tue Apr 03 14:46:27 2012 +0100
+++ b/tools/libxl/libxlu_internal.h Tue Apr 03 18:00:24 2012 +0100
@@ -36,7 +36,7 @@ struct XLU_ConfigSetting { /* transparen
struct XLU_Config {
XLU_ConfigSetting *settings;
FILE *report;
- char *filename;
+ char *config_source;
};
typedef struct {
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source"
[not found] <patchbomb.1333477720@kodo2>
2012-04-03 18:28 ` [PATCH 1 of 2] libxlu: Rename filename to config_source George Dunlap
@ 2012-04-03 18:28 ` George Dunlap
2012-04-03 18:52 ` George Dunlap
2012-04-04 15:18 ` Ian Jackson
1 sibling, 2 replies; 10+ messages in thread
From: George Dunlap @ 2012-04-03 18:28 UTC (permalink / raw)
To: xen-devel; +Cc: 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;
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source"
2012-04-03 18:28 ` [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source" George Dunlap
@ 2012-04-03 18:52 ` George Dunlap
2012-04-04 15:18 ` Ian Jackson
1 sibling, 0 replies; 10+ messages in thread
From: George Dunlap @ 2012-04-03 18:52 UTC (permalink / raw)
To: xen-devel@lists.xensource.com
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;
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source"
2012-04-03 18:28 ` [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source" George Dunlap
2012-04-03 18:52 ` George Dunlap
@ 2012-04-04 15:18 ` Ian Jackson
1 sibling, 0 replies; 10+ messages in thread
From: Ian Jackson @ 2012-04-04 15:18 UTC (permalink / raw)
To: George Dunlap; +Cc: xen-devel
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source"
[not found] <patchbomb.1335971421@kodo2>
@ 2012-05-02 15:10 ` George Dunlap
2012-05-04 9:49 ` George Dunlap
0 siblings, 1 reply; 10+ messages in thread
From: George Dunlap @ 2012-05-02 15:10 UTC (permalink / raw)
To: xen-devel; +Cc: 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;
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source"
2012-05-02 15:10 ` George Dunlap
@ 2012-05-04 9:49 ` George Dunlap
0 siblings, 0 replies; 10+ messages in thread
From: George Dunlap @ 2012-05-04 9:49 UTC (permalink / raw)
To: xen-devel; +Cc: 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source"
2012-05-15 14:51 [PATCH 0 of 2] libxl cleanup: distinguish filenames from sources George Dunlap
@ 2012-05-15 14:51 ` George Dunlap
2012-05-15 15:29 ` Ian Campbell
0 siblings, 1 reply; 10+ messages in thread
From: George Dunlap @ 2012-05-15 14:51 UTC (permalink / raw)
To: xen-devel; +Cc: 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;
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source"
2012-05-15 14:51 ` [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source" George Dunlap
@ 2012-05-15 15:29 ` Ian Campbell
2012-05-15 15:47 ` George Dunlap
0 siblings, 1 reply; 10+ messages in thread
From: Ian Campbell @ 2012-05-15 15:29 UTC (permalink / raw)
To: George Dunlap; +Cc: xen-devel@lists.xensource.com
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source"
2012-05-15 15:29 ` Ian Campbell
@ 2012-05-15 15:47 ` George Dunlap
2012-05-15 15:52 ` Ian Campbell
0 siblings, 1 reply; 10+ messages in thread
From: George Dunlap @ 2012-05-15 15:47 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel@lists.xensource.com
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
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source"
2012-05-15 15:47 ` George Dunlap
@ 2012-05-15 15:52 ` Ian Campbell
0 siblings, 0 replies; 10+ messages in thread
From: Ian Campbell @ 2012-05-15 15:52 UTC (permalink / raw)
To: George Dunlap; +Cc: xen-devel@lists.xensource.com
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
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-05-15 15:52 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <patchbomb.1333477720@kodo2>
2012-04-03 18:28 ` [PATCH 1 of 2] libxlu: Rename filename to config_source George Dunlap
2012-04-03 18:28 ` [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source" George Dunlap
2012-04-03 18:52 ` George Dunlap
2012-04-04 15:18 ` Ian Jackson
[not found] <patchbomb.1335971421@kodo2>
2012-05-02 15:10 ` George Dunlap
2012-05-04 9:49 ` George Dunlap
2012-05-15 14:51 [PATCH 0 of 2] libxl cleanup: distinguish filenames from sources George Dunlap
2012-05-15 14:51 ` [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source" George Dunlap
2012-05-15 15:29 ` Ian Campbell
2012-05-15 15:47 ` George Dunlap
2012-05-15 15:52 ` Ian Campbell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).