diff mbox series

[v2,2/3] tools/xl: Use sparse init for dom_info, remove duplicate vars

Message ID a444edf57dbb1ea45ce4af471bf2c5f9b362bbde.1651285313.git.ehem+xen@m5p.com (mailing list archive)
State New, archived
Headers show
Series Allow use of JSON in domain configuration files | expand

Commit Message

Elliott Mitchell April 29, 2022, 10:45 p.m. UTC
Rather than having shadow variables for every element of dom_info, it is
better to properly initialize dom_info at the start.  This also removes
the misleading memset() in the middle of main_create().

Remove the dryrun element of domain_create as that has been displaced
by the global "dryrun_only" variable.

Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
---
v2:
This was added due to the confusing situation with dom_info.
---
 tools/xl/xl.h           |  1 -
 tools/xl/xl_vmcontrol.c | 76 ++++++++++++++++++++---------------------
 2 files changed, 37 insertions(+), 40 deletions(-)

Comments

Anthony PERARD May 20, 2022, 1:48 p.m. UTC | #1
On Fri, Apr 29, 2022 at 03:45:25PM -0700, Elliott Mitchell wrote:
> Rather than having shadow variables for every element of dom_info, it is
> better to properly initialize dom_info at the start.  This also removes
> the misleading memset() in the middle of main_create().
> 
> Remove the dryrun element of domain_create as that has been displaced
> by the global "dryrun_only" variable.
> 
> Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>

Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks for the clean up.
diff mbox series

Patch

diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index c5c4bedbdd..72538d6a81 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -34,7 +34,6 @@  struct domain_create {
     int daemonize;
     int monitor; /* handle guest reboots etc */
     int paused;
-    int dryrun;
     int quiet;
     int vnc;
     int vncautopass;
diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
index d081c6c290..4bf041fb01 100644
--- a/tools/xl/xl_vmcontrol.c
+++ b/tools/xl/xl_vmcontrol.c
@@ -854,8 +854,8 @@  int create_domain(struct domain_create *dom_info)
         }
     }
 
-    if (debug || dom_info->dryrun) {
-        FILE *cfg_print_fh = (debug && !dom_info->dryrun) ? stderr : stdout;
+    if (debug || dryrun_only) {
+        FILE *cfg_print_fh = (debug && !dryrun_only) ? stderr : stdout;
         if (default_output_format == OUTPUT_FORMAT_SXP) {
             printf_info_sexp(-1, &d_config, cfg_print_fh);
         } else {
@@ -873,7 +873,7 @@  int create_domain(struct domain_create *dom_info)
 
 
     ret = 0;
-    if (dom_info->dryrun)
+    if (dryrun_only)
         goto out;
 
 start:
@@ -1164,10 +1164,26 @@  out:
 
 int main_create(int argc, char **argv)
 {
-    const char *filename = NULL;
-    struct domain_create dom_info;
-    int paused = 0, debug = 0, daemonize = 1, console_autoconnect = 0,
-        quiet = 0, monitor = 1, vnc = 0, vncautopass = 0, ignore_masks = 0;
+    struct domain_create dom_info = {
+        /* Command-line options */
+        .config_file = NULL,
+        .console_autoconnect = 0,
+        .debug = 0,
+        .daemonize = 1,
+        .ignore_global_affinity_masks = 0,
+        .monitor = 1,
+        .paused = 0,
+        .quiet = 0,
+        .vnc = 0,
+        .vncautopass = 0,
+
+        /* Extra configuration file settings */
+        .extra_config = NULL,
+
+        /* FDs, initialize to invalid */
+        .migrate_fd = -1,
+        .send_back_fd = -1,
+    };
     int opt, rc;
     static const struct option opts[] = {
         {"defconfig", 1, 0, 'f'},
@@ -1179,58 +1195,54 @@  int main_create(int argc, char **argv)
         COMMON_LONG_OPTS
     };
 
-    dom_info.extra_config = NULL;
-
     if (argv[1] && argv[1][0] != '-' && !strchr(argv[1], '=')) {
-        filename = argv[1];
+        dom_info.config_file = argv[1];
         argc--; argv++;
     }
 
     SWITCH_FOREACH_OPT(opt, "AFVcdef:inpq", opts, "create", 0) {
     case 'A':
-        vnc = vncautopass = 1;
+        dom_info.vnc = dom_info.vncautopass = 1;
         break;
     case 'F':
-        daemonize = 0;
+        dom_info.daemonize = 0;
         break;
     case 'V':
-        vnc = 1;
+        dom_info.vnc = 1;
         break;
     case 'c':
-        console_autoconnect = 1;
+        dom_info.console_autoconnect = 1;
         break;
     case 'd':
-        debug = 1;
+        dom_info.debug = 1;
         break;
     case 'e':
-        daemonize = 0;
-        monitor = 0;
+        dom_info.daemonize = 0;
+        dom_info.monitor = 0;
         break;
     case 'f':
-        filename = optarg;
+        dom_info.config_file = optarg;
         break;
     case 'i':
-        ignore_masks = 1;
+        dom_info.ignore_global_affinity_masks = 1;
         break;
     case 'n':
         dryrun_only = 1;
         break;
     case 'p':
-        paused = 1;
+        dom_info.paused = 1;
         break;
     case 'q':
-        quiet = 1;
+        dom_info.quiet = 1;
         break;
     }
 
-    memset(&dom_info, 0, sizeof(dom_info));
-
     for (; optind < argc; optind++) {
         if (strchr(argv[optind], '=') != NULL) {
             string_realloc_append(&dom_info.extra_config, argv[optind]);
             string_realloc_append(&dom_info.extra_config, "\n");
-        } else if (!filename) {
-            filename = argv[optind];
+        } else if (!dom_info.config_file) {
+            dom_info.config_file = argv[optind];
         } else {
             help("create");
             free(dom_info.extra_config);
@@ -1238,20 +1250,6 @@  int main_create(int argc, char **argv)
         }
     }
 
-    dom_info.debug = debug;
-    dom_info.daemonize = daemonize;
-    dom_info.monitor = monitor;
-    dom_info.paused = paused;
-    dom_info.dryrun = dryrun_only;
-    dom_info.quiet = quiet;
-    dom_info.config_file = filename;
-    dom_info.migrate_fd = -1;
-    dom_info.send_back_fd = -1;
-    dom_info.vnc = vnc;
-    dom_info.vncautopass = vncautopass;
-    dom_info.console_autoconnect = console_autoconnect;
-    dom_info.ignore_global_affinity_masks = ignore_masks;
-
     rc = create_domain(&dom_info);
     if (rc < 0) {
         free(dom_info.extra_config);