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

2022-05-20 Thread Anthony PERARD
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 

Reviewed-by: Anthony PERARD 

Thanks for the clean up.

-- 
Anthony PERARD



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

2022-04-29 Thread Elliott Mitchell
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 
---
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(-)

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, _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(_info, 0, sizeof(dom_info));
-
 for (; optind < argc; optind++) {
 if (strchr(argv[optind], '=') != NULL) {
 string_realloc_append(_info.extra_config, argv[optind]);
 string_realloc_append(_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 =