[PATCH 2/2] tools/xl: Allow specifying JSON for domain configuration file format

[PATCH 2/2] tools/xl: Allow specifying JSON for domain configuration file format

[Elliott Mitchell]

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.

Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
---
 tools/xl/xl.h           |  5 +++++
 tools/xl/xl_cmdtable.c  |  2 ++
 tools/xl/xl_vmcontrol.c | 13 +++++++++++--
 3 files changed, 18 insertions(+), 2 deletions(-)

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;
 };
 
 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"
       "-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;
     } 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;
+        break;
     case 'n':
         dryrun_only = 1;
         break;
-- 
2.30.2

[PATCH 1/2] tools/xl: Sort create command options

[Elliott Mitchell]

Hopefully simplify future changes by sorting options lists for
`xl create`.

Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
---
 tools/xl/xl_cmdtable.c  | 10 +++++-----
 tools/xl/xl_vmcontrol.c | 40 ++++++++++++++++++++--------------------
 2 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 661323d488..f546beaceb 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -24,16 +24,16 @@ const struct cmd_spec cmd_table[] = {
       &main_create, 1, 1,
       "Create a domain from config file <filename>",
       "<ConfigFile> [options] [vars]",
+      "-c                      Connect to the console after the domain is created.\n"
+      "-d                      Enable debug messages.\n"
+      "-e                      Do not wait in the background for the death of the domain.\n"
+      "-F                      Run in foreground until death of the domain.\n"
       "-h                      Print this help.\n"
       "-p                      Leave the domain paused after it is created.\n"
-      "-c                      Connect to the console after the domain is created.\n"
       "-f FILE, --defconfig=FILE\n                     Use the given configuration file.\n"
-      "-q, --quiet             Quiet.\n"
       "-n, --dryrun            Dry run - prints the resulting configuration\n"
       "                         (deprecated in favour of global -N option).\n"
-      "-d                      Enable debug messages.\n"
-      "-F                      Run in foreground until death of the domain.\n"
-      "-e                      Do not wait in the background for the death of the domain.\n"
+      "-q, --quiet             Quiet.\n"
       "-V, --vncviewer         Connect to the VNC display after the domain is created.\n"
       "-A, --vncviewer-autopass\n"
       "                        Pass VNC password to viewer via stdin.\n"
diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
index 435155a033..2ec4140258 100644
--- a/tools/xl/xl_vmcontrol.c
+++ b/tools/xl/xl_vmcontrol.c
@@ -1169,13 +1169,13 @@ int main_create(int argc, char **argv)
     int paused = 0, debug = 0, daemonize = 1, console_autoconnect = 0,
         quiet = 0, monitor = 1, vnc = 0, vncautopass = 0, ignore_masks = 0;
     int opt, rc;
-    static struct option opts[] = {
+    static const struct option opts[] = {
+        {"defconfig", 1, 0, 'f'},
         {"dryrun", 0, 0, 'n'},
+        {"ignore-global-affinity-masks", 0, 0, 'i'},
         {"quiet", 0, 0, 'q'},
-        {"defconfig", 1, 0, 'f'},
         {"vncviewer", 0, 0, 'V'},
         {"vncviewer-autopass", 0, 0, 'A'},
-        {"ignore-global-affinity-masks", 0, 0, 'i'},
         COMMON_LONG_OPTS
     };
 
@@ -1186,12 +1186,15 @@ int main_create(int argc, char **argv)
         argc--; argv++;
     }
 
-    SWITCH_FOREACH_OPT(opt, "Fnqf:pcdeVAi", opts, "create", 0) {
-    case 'f':
-        filename = optarg;
+    SWITCH_FOREACH_OPT(opt, "Ffnq:AVcdeip", opts, "create", 0) {
+    case 'A':
+        vnc = vncautopass = 1;
         break;
-    case 'p':
-        paused = 1;
+    case 'F':
+        daemonize = 0;
+        break;
+    case 'V':
+        vnc = 1;
         break;
     case 'c':
         console_autoconnect = 1;
@@ -1199,28 +1202,25 @@ int main_create(int argc, char **argv)
     case 'd':
         debug = 1;
         break;
-    case 'F':
-        daemonize = 0;
-        break;
     case 'e':
         daemonize = 0;
         monitor = 0;
         break;
+    case 'f':
+        filename = optarg;
+        break;
+    case 'i':
+        ignore_masks = 1;
+        break;
     case 'n':
         dryrun_only = 1;
         break;
+    case 'p':
+        paused = 1;
+        break;
     case 'q':
         quiet = 1;
         break;
-    case 'V':
-        vnc = 1;
-        break;
-    case 'A':
-        vnc = vncautopass = 1;
-        break;
-    case 'i':
-        ignore_masks = 1;
-        break;
     }
 
     memset(&dom_info, 0, sizeof(dom_info));
-- 
2.30.2

[PATCH 0/2] Allow use of JSON in domain configuration files

[Elliott Mitchell]

While the traditional domain configuration file format works acceptably,
I can see uses for having full JSON support.  As such add "-j" and "-J"
to `xl create` to specify format.  The traditional format is the current
default.

While attempting this, it came up that options for `xl create` aren't in
a consistent order.  I'm concerned about moving the VNC options apart,
but the others have been sorted.

Elliott Mitchell (2):
  tools/xl: Sort create command options
  tools/xl: Allow specifying JSON for domain configuration file format

 tools/xl/xl.h           |  5 ++++
 tools/xl/xl_cmdtable.c  | 12 ++++++----
 tools/xl/xl_vmcontrol.c | 51 ++++++++++++++++++++++++-----------------
 3 files changed, 42 insertions(+), 26 deletions(-)

-- 
2.30.2

Re: [PATCH 0/2] Allow use of JSON in domain configuration files

[Elliott Mitchell]

On Tue, Apr 19, 2022 at 07:41:57PM -0700, Elliott Mitchell wrote:
> While the traditional domain configuration file format works acceptably,
> I can see uses for having full JSON support.  As such add "-j" and "-J"
> to `xl create` to specify format.  The traditional format is the current
> default.

Crucial note here, this hasn't been properly tested yet.  Hopefully I
got this right on the first try, but since that never works I need to
advise actually testing.   %-/


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445

Re: [PATCH 1/2] tools/xl: Sort create command options

[Anthony PERARD]

On Tue, Apr 19, 2022 at 06:56:03PM -0700, Elliott Mitchell wrote:
> Hopefully simplify future changes by sorting options lists for
> `xl create`.

I'm not sure that sorting options make changes easier, as it would mean
one has to make sure the new option is sorted as well ;-). But sorting
the options in the help message is probably useful; I've looked at a few
linux utilities and they tend to have them sorted so doing this for xl
create sound fine.

> Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
> ---
> diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
> index 435155a033..2ec4140258 100644
> --- a/tools/xl/xl_vmcontrol.c
> +++ b/tools/xl/xl_vmcontrol.c
> @@ -1169,13 +1169,13 @@ int main_create(int argc, char **argv)
>      int paused = 0, debug = 0, daemonize = 1, console_autoconnect = 0,
>          quiet = 0, monitor = 1, vnc = 0, vncautopass = 0, ignore_masks = 0;
>      int opt, rc;
> -    static struct option opts[] = {
> +    static const struct option opts[] = {

Could you add a note in the commit message that "opts" is now
const?

> +        {"defconfig", 1, 0, 'f'},
>          {"dryrun", 0, 0, 'n'},
> +        {"ignore-global-affinity-masks", 0, 0, 'i'},
>          {"quiet", 0, 0, 'q'},
> -        {"defconfig", 1, 0, 'f'},
>          {"vncviewer", 0, 0, 'V'},
>          {"vncviewer-autopass", 0, 0, 'A'},
> -        {"ignore-global-affinity-masks", 0, 0, 'i'},
>          COMMON_LONG_OPTS
>      };
>  
> @@ -1186,12 +1186,15 @@ int main_create(int argc, char **argv)
>          argc--; argv++;
>      }
>  
> -    SWITCH_FOREACH_OPT(opt, "Fnqf:pcdeVAi", opts, "create", 0) {
> -    case 'f':
> -        filename = optarg;
> +    SWITCH_FOREACH_OPT(opt, "Ffnq:AVcdeip", opts, "create", 0) {

The list of short options aren't really sorted here. Also -q doesn't
take an argument, but -f should keep requiring one.

Thanks,


-- 
Anthony PERARD

Re: [PATCH 2/2] tools/xl: Allow specifying JSON for domain configuration file format

[Anthony PERARD]

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@m5p.com>
> ---
> 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

Re: [PATCH 1/2] tools/xl: Sort create command options

[Elliott Mitchell]

On Fri, Apr 29, 2022 at 10:39:27AM +0100, Anthony PERARD wrote:
> On Tue, Apr 19, 2022 at 06:56:03PM -0700, Elliott Mitchell wrote:
> > Hopefully simplify future changes by sorting options lists for
> > `xl create`.
> 
> I'm not sure that sorting options make changes easier, as it would mean
> one has to make sure the new option is sorted as well ;-). But sorting
> the options in the help message is probably useful; I've looked at a few
> linux utilities and they tend to have them sorted so doing this for xl
> create sound fine.

This ends up revolving around the question, is the work involved in
keeping things sorted more or less than the annoyance caused by having
them unsorted?  I tend towards keep them sorted since I find trying to
identify available option letters when they're in random order is rather
troublesome.

> > Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
> > ---
> > diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
> > index 435155a033..2ec4140258 100644
> > --- a/tools/xl/xl_vmcontrol.c
> > +++ b/tools/xl/xl_vmcontrol.c
> > @@ -1169,13 +1169,13 @@ int main_create(int argc, char **argv)
> >      int paused = 0, debug = 0, daemonize = 1, console_autoconnect = 0,
> >          quiet = 0, monitor = 1, vnc = 0, vncautopass = 0, ignore_masks = 0;
> >      int opt, rc;
> > -    static struct option opts[] = {
> > +    static const struct option opts[] = {
> 
> Could you add a note in the commit message that "opts" is now
> const?

Okay.

> > +        {"defconfig", 1, 0, 'f'},
> >          {"dryrun", 0, 0, 'n'},
> > +        {"ignore-global-affinity-masks", 0, 0, 'i'},
> >          {"quiet", 0, 0, 'q'},
> > -        {"defconfig", 1, 0, 'f'},
> >          {"vncviewer", 0, 0, 'V'},
> >          {"vncviewer-autopass", 0, 0, 'A'},
> > -        {"ignore-global-affinity-masks", 0, 0, 'i'},
> >          COMMON_LONG_OPTS
> >      };
> >  
> > @@ -1186,12 +1186,15 @@ int main_create(int argc, char **argv)
> >          argc--; argv++;
> >      }
> >  
> > -    SWITCH_FOREACH_OPT(opt, "Fnqf:pcdeVAi", opts, "create", 0) {
> > -    case 'f':
> > -        filename = optarg;
> > +    SWITCH_FOREACH_OPT(opt, "Ffnq:AVcdeip", opts, "create", 0) {
> 
> The list of short options aren't really sorted here. Also -q doesn't
> take an argument, but -f should keep requiring one.

Needed to reread the documentation on getopt() behavior.  I remembered
it being group before the colon didn't take options, ones after colon
did take options.  Instead it is colon for every option with argument.

Other issue is dictionary sort versus ASCII sort.  ie "AaBbCcDdEeFf..."
versus "A-Za-z".


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445

Re: [PATCH 2/2] tools/xl: Allow specifying JSON for domain configuration file format

[Elliott Mitchell]

On Fri, Apr 29, 2022 at 01:34:40PM +0100, Anthony PERARD wrote:
> 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.

What I wonder is why the behind the scenes format is flexible enough to
fully handle domain configuration, yet wasn't reused for domain
configuration.  Then I look at the parser for domain configuration files
and it isn't really that great.

Add those two together and moving towards domain configuration files
being JSON seems natural.  Look at the "vif" and "disk" sections.  Those
could be very naturally handled as JSON, while the current parser isn't
rather limited.

There may be need to modify libxl_domain_config_from_json() to ensure it
gives good error messages.

> > Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
> > ---
> > 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.

What about "config_format" to match below?

> 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.

Okay.

Over time the default can be changed.  Document plans to move to JSON
exclusively.  Then after a few versions switch the default.  Then after
more versions remove the old format.

> >  };
> >  
> >  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.

I was thinking of this as a transition mechanism.  Have "-J" for when the
default has been changed, but the code to handle the original format
hasn't been removed.

> >        "-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.

Uh, yeah.  Too many different coding standards.  Plus things being
passed around.  Erk.  %-/

> >      } 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.

Uh huh.  Indeed.  I saw the "dom_info.extra_config = NULL;" and figured
dom_info was valid at that point, but the memset() is later.  Certainly
need to remedy that.

Having looked, that has gotten pretty awful.  That really needs some
rework to avoid confusion.  Next version...


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445