|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 2 of 2] xl: Make clear distinction between "filename" and "data source"
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@xxxxxxxxxxxxx>
> > 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@xxxxxxxxxxxxx
> >> http://lists.xen.org/xen-devel
> >
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |