[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/2] tools/xl: Allow specifying JSON for domain configuration file format
On Tue, Apr 19, 2022 at 06:23:41PM -0700, Elliott Mitchell wrote: > JSON is currently used when saving domains to mass storage. Being able > to use JSON as the normal input to `xl create` has potential to be > valuable. Add the functionality. "potential", right, but it isn't hasn't been really tested. When implemented, I think the intend of the json format was for libxl to communicate with itself while migrating a guest (or save/restore). It would be nice to know if it actually can work. > Signed-off-by: Elliott Mitchell <ehem+xen@xxxxxxx> > --- > diff --git a/tools/xl/xl.h b/tools/xl/xl.h > index c5c4bedbdd..a0c03f96df 100644 > --- a/tools/xl/xl.h > +++ b/tools/xl/xl.h > @@ -49,6 +49,11 @@ struct domain_create { > int migrate_fd; /* -1 means none */ > int send_back_fd; /* -1 means none */ > char **migration_domname_r; /* from malloc */ > + enum { > + FORMAT_DEFAULT, > + FORMAT_JSON, > + FORMAT_LEGACY, > + } format; I think the name "format" here is too generic, we are in the struct "domain_create" and this new format field isn't intended to specify the format of the domain. I think "config_file_format" would be better, as it is only used for the format of the config_file. Also I don't think having "_DEFAULT" would be useful, we need to know what the format is intended to be before starting to parse the file, and I don't think changing the default is going to work. So for the enum, we could have "_LEGACY = 0". The prefix "FORMAT_" feels a bit generic, maybe "CONFIG_FORMAT_" would be better, or something else. > }; > > int create_domain(struct domain_create *dom_info); > diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c > index f546beaceb..04d579a596 100644 > --- a/tools/xl/xl_cmdtable.c > +++ b/tools/xl/xl_cmdtable.c > @@ -31,6 +31,8 @@ const struct cmd_spec cmd_table[] = { > "-h Print this help.\n" > "-p Leave the domain paused after it is > created.\n" > "-f FILE, --defconfig=FILE\n Use the given > configuration file.\n" > + "-j, --json Interpret configuration file as JSON format\n" > + "-J Use traditional configuration file format > (current default)\n" I don't think this new "-J" option would be useful, just the "-j" should be enough. > "-n, --dryrun Dry run - prints the resulting > configuration\n" > " (deprecated in favour of global -N > option).\n" > "-q, --quiet Quiet.\n" > diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c > index 2ec4140258..41bd919d1d 100644 > --- a/tools/xl/xl_vmcontrol.c > +++ b/tools/xl/xl_vmcontrol.c > @@ -789,7 +789,7 @@ int create_domain(struct domain_create *dom_info) > extra_config); > } > config_source=config_file; > - config_in_json = false; > + config_in_json = dom_info.format == FORMAT_JSON ? true : false; This doesn't build, "dom_info" is a pointer. Also, "? true : false;" is weird in C. > } else { > if (!config_data) { > fprintf(stderr, "Config file not specified and" > @@ -1173,6 +1173,7 @@ int main_create(int argc, char **argv) > {"defconfig", 1, 0, 'f'}, > {"dryrun", 0, 0, 'n'}, > {"ignore-global-affinity-masks", 0, 0, 'i'}, > + {"json", 0, 0, 'j'}, > {"quiet", 0, 0, 'q'}, > {"vncviewer", 0, 0, 'V'}, > {"vncviewer-autopass", 0, 0, 'A'}, > @@ -1181,18 +1182,23 @@ int main_create(int argc, char **argv) > > dom_info.extra_config = NULL; > > + dom_info.format = FORMAT_DEFAULT; > + > if (argv[1] && argv[1][0] != '-' && !strchr(argv[1], '=')) { > filename = argv[1]; > argc--; argv++; > } > > - SWITCH_FOREACH_OPT(opt, "Ffnq:AVcdeip", opts, "create", 0) { > + SWITCH_FOREACH_OPT(opt, "FJfjnq:AVcdeip", opts, "create", 0) { > case 'A': > vnc = vncautopass = 1; > break; > case 'F': > daemonize = 0; > break; > + case 'J': > + dom_info.format = FORMAT_LEGACY; > + break; > case 'V': > vnc = 1; > break; > @@ -1212,6 +1218,9 @@ int main_create(int argc, char **argv) > case 'i': > ignore_masks = 1; > break; > + case 'j': > + dom_info.format = FORMAT_JSON; This setting is ignored, as "dom_info" is reset later. > + break; > case 'n': > dryrun_only = 1; > break; Thanks, -- Anthony PERARD
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |