On Thu, 25 Mar 2010, Ian Jackson wrote:
> We introduce a new format for saved domains. The new format, in
> contrast to the old:
> * Has a magic number which can distinguish it from other kinds of file
> * Is extensible
> * Can contains the domain configuration file
>
> On domain creation we remember the actual config file used (using the
> toolstack data feature of libxl, just introduced), and by default save
> it to the save file.
>
> However, options are provided for the following:
> * Reading and writing old-style xl saved domain files. (These will
> become deprecated at some point in the future.)
> * When saving a domain, supplying an alternative config file to
> store in the savefile.
> * When restoring a domain, supplying an alternative config file.
>
> If a domain is restored with a different config file, it is the
> responsibility of the xl user to ensure that the two configs are
> "compatible". Changing the targets of virtual devices is supported;
> changing other features of the domain is not recommended. Bad changes
> may lead to undefined behaviour in the domain, and are in practice
> likely to cause resume failures or crashes.
>
I think that providing compatibility with the old-style xl saved domain
files is overkill and just adds complexity for no good reason.
We should stick to the new format and forget about the old one
altogether.
For the same reason I would remove the option to save\restore from an
alternative config file.
Please keep it simple.
> +
> +static const char savefileheader_magic[32]=
> + "Xen saved domain, xl format\n \0 \r";
> +
> +typedef struct {
> + char magic[32]; /* savefileheader_magic */
> + /* All uint32_ts are in domain's byte order. */
> + uint32_t byteorder; /* SAVEFILE_BYTEORDER_VALUE */
> + uint32_t mandatory_flags; /* unknown flags => reject restore */
> + uint32_t optional_flags; /* unknown flags => reject restore */
> + uint32_t optional_data_len; /* skip, or skip tail, if not understood */
> +} SaveFileHeader;
> +
I don't think SaveFileHeader is consistent with the naming convention
used so far; could you please call it save_file_header instead?
> -static void parse_config_file(const char *filename,
> +static void parse_config_data(const char *configfile_filename_report,
> + const char *configfile_data,
> + int configfile_len,
> libxl_domain_create_info *c_info,
> libxl_domain_build_info *b_info,
> libxl_device_disk **disks,
I think at this point we should remove const 'char *filename' altogether
> @@ -693,27 +746,124 @@ static void create_domain(int debug, int daemonize,
> const char *config_file, con
> int ret;
> libxl_device_model_starting *dm_starting = 0;
> libxl_waiter *w1 = NULL, *w2 = NULL;
> + void *config_data = 0;
> + int config_len = 0;
> + int restore_fd = -1;
> + SaveFileHeader hdr;
> +
> memset(&dm_info, 0x00, sizeof(dm_info));
>
> + if (libxl_ctx_init(&ctx, LIBXL_VERSION)) {
> + fprintf(stderr, "cannot init xl context\n");
> + exit(1);
> + }
> +
> + if (restore_file) {
> + uint8_t *optdata_begin = 0;
> + const uint8_t *optdata_here = 0;
> + union { uint32_t u32; char b[4]; } u32buf;
> +
> + restore_fd = open(restore_file, O_RDONLY);
> +
> + if (suspend_old_xl_format) {
> + memset(&hdr,0,sizeof(hdr));
just remove this case
> + } else {
> + uint32_t badflags;
> +
> + CHK_ERRNO( libxl_read_exactly(&ctx, restore_fd, &hdr,
> + sizeof(hdr), restore_file, "header") );
> + if (memcmp(hdr.magic, savefileheader_magic, sizeof(hdr.magic))) {
> + fprintf(stderr, "File has wrong magic number -"
> + " corrupt or needs -O?\n");
> + exit(2);
> + }
> + if (hdr.byteorder != SAVEFILE_BYTEORDER_VALUE) {
> + fprintf(stderr, "File has wrong byte order\n");
> + exit(2);
> + }
> + 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,
> + hdr.optional_data_len);
> +
> + badflags = hdr.mandatory_flags & ~( 0 /* none understood yet */
> );
> + if (badflags) {
> + fprintf(stderr, "Savefile has mandatory flag(s) 0x%"PRIx32" "
> + "which are not supported; need newer xl\n",
> + badflags);
> + exit(2);
> + }
> + }
> + 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") );
> + }
> +
> +#define OPTDATA_LEFT (hdr.optional_data_len - (optdata_here -
> optdata_begin))
> +#define WITH_OPTDATA(amt, body) \
> + if (OPTDATA_LEFT < (amt)) { \
> + fprintf(stderr, "Savefile truncated.\n"); exit(2); \
> + } else { \
> + body; \
> + optdata_here += (amt); \
> + }
> +
> + optdata_here = optdata_begin;
> +
> + if (OPTDATA_LEFT) {
> + fprintf(stderr, " Savefile contains xl domain config\n");
> + WITH_OPTDATA(4, {
> + memcpy(u32buf.b, optdata_here, 4);
> + config_len = u32buf.u32;
> + });
> + WITH_OPTDATA(config_len, {
> + config_data = xmalloc(config_len);
> + memcpy(config_data, optdata_here, config_len);
> + });
> + }
> +
> + }
> +
> + if (config_file) {
> + free(config_data); config_data = 0;
> + ret = libxl_read_file_contents(&ctx, config_file,
> + &config_data, &config_len);
> + if (ret) { fprintf(stderr, "Failed to read config file: %s: %s\n",
> + config_file, strerror(errno)); exit(1); }
> + } else {
> + if (!config_data) {
> + fprintf(stderr, "Config file not specified and"
> + " none in save file\n");
> + exit(1);
> + }
> + config_file = "<saved>";
> + }
> +
> printf("Parsing config file %s\n", config_file);
> - parse_config_file(config_file, &info1, &info2, &disks, &num_disks,
> &vifs, &num_vifs, &pcidevs, &num_pcidevs, &vfbs, &num_vfbs, &vkbs, &num_vkbs,
> &dm_info);
> +
> + parse_config_data(config_file, config_data, config_len, &info1, &info2,
> &disks, &num_disks, &vifs, &num_vifs, &pcidevs, &num_pcidevs, &vfbs,
> &num_vfbs, &vkbs, &num_vkbs, &dm_info);
> +
just remove config_file
> @@ -900,16 +1048,18 @@ static void help(char *command)
> printf("Usage: xl unpause <Domain>\n\n");
> printf("Unpause a paused domain.\n\n");
> } else if(!strcmp(command, "save")) {
> - printf("Usage: xl save [options] <Domain> <CheckpointFile>\n\n");
> + printf("Usage: xl save [options] <Domain> <CheckpointFile>
> [<ConfigFile>]\n\n");
> printf("Save a domain state to restore later.\n\n");
> printf("Options:\n\n");
> printf("-h Print this help.\n");
> + printf("-O Old (configless) xl save format.\n");
> printf("-c Leave domain running after creating
> the snapshot.\n");
> } else if(!strcmp(command, "restore")) {
> - printf("Usage: xl restore [options] <ConfigFile>
> <CheckpointFile>\n\n");
> + printf("Usage: xl restore [options] [<ConfigFile>]
> <CheckpointFile>\n\n");
> printf("Restore a domain from a saved state.\n\n");
> printf("Options:\n\n");
> printf("-h Print this help.\n");
> + printf("-O Old (configless) xl save format.\n");
> printf("-p Do not unpause domain after restoring
> it.\n");
> printf("-e Do not wait in the background for the
> death of the domain.\n");
do not add [<ConfigFile>], if you really want to provide this feature,
use an option instead, so that we keep compatibility with xm:
xl save [options] <Domain> <CheckpointFile>
xl restore [options] <CheckpointFile>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home