* [PATCH 1 of 2] xl: correct argument parsing for some sub-commands
2010-08-23 14:50 [PATCH 0 of 2] xl: fixes and improvements to xl command line parsing Ian Campbell
@ 2010-08-23 14:50 ` Ian Campbell
2010-08-23 14:50 ` [PATCH 2 of 2] xl: treat sub-command main function like a regular C main() function Ian Campbell
1 sibling, 0 replies; 3+ messages in thread
From: Ian Campbell @ 2010-08-23 14:50 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell
# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1282574054 -3600
# Node ID 2210bb76868ff58c1c97738f43c52b3e893dd178
# Parent aa344916733f86fcde905953bad8b1cbc3020cd0
xl: correct argument parsing for some sub-commands.
XL sub-commands are expected to parse their arguments relative to the
global variable "optind" rather than treating argc+argv as zero
based. This is because the argc+argv passed to sub-commands include
the entire original command line, not just the sub command specific bits.
Not all commands do this and they are therefore broken if the user
uses "xl -v command", correct such problems
dump-core:
- did not handle "-h" option.
{network,network2,block}-{attach,list,detach} :
- handled arguments without reference to optind
- checked number of arguments before processing getopt loop,
breaking "-h" option handling
An example of the breakage:
# xl -v block-list d32-2
Vdev BE handle state evt-ch ring-ref BE-path
block-list is an invalid domain identifier
51712 0 1 4 13 8 /local/domain/0/backend/vbd/1/
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
diff -r aa344916733f -r 2210bb76868f tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Mon Aug 23 09:46:03 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c Mon Aug 23 15:34:14 2010 +0100
@@ -2870,6 +2870,17 @@ int main_migrate(int argc, char **argv)
int main_dump_core(int argc, char **argv)
{
+ int opt;
+ while ((opt = getopt(argc, argv, "h")) != -1) {
+ switch (opt) {
+ case 'h':
+ help("dump-core");
+ return 0;
+ default:
+ fprintf(stderr, "option `%c' not supported.\n", opt);
+ break;
+ }
+ }
if ( argc-optind < 2 ) {
help("dump-core");
return 2;
@@ -4032,27 +4043,27 @@ int main_networkattach(int argc, char **
int i;
unsigned int val;
- if ((argc < 3) || (argc > 11)) {
+ while ((opt = getopt(argc, argv, "h")) != -1) {
+ switch (opt) {
+ case 'h':
+ help("network-attach");
+ return 0;
+ default:
+ fprintf(stderr, "option `%c' not supported.\n", opt);
+ break;
+ }
+ }
+ if ((argc-optind < 2) || (argc-optind > 11)) {
help("network-attach");
return 0;
}
- while ((opt = getopt(argc, argv, "h")) != -1) {
- switch (opt) {
- case 'h':
- help("network-attach");
- return 0;
- default:
- fprintf(stderr, "option `%c' not supported.\n", opt);
- break;
- }
- }
-
- if (domain_qualifier_to_domid(argv[2], &domid, 0) < 0) {
- fprintf(stderr, "%s is an invalid domain identifier\n", argv[2]);
+
+ if (domain_qualifier_to_domid(argv[optind], &domid, 0) < 0) {
+ fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]);
return 1;
}
init_nic_info(&nic, -1);
- for (argv += 3, argc -= 3; argc > 0; ++argv, --argc) {
+ for (argv += optind+1, argc -= optind+1; argc > 0; ++argv, --argc) {
if (!strncmp("type=", *argv, 5)) {
if (!strncmp("vif", (*argv) + 5, 4)) {
nic.nictype = NICTYPE_VIF;
@@ -4113,10 +4124,6 @@ int main_networklist(int argc, char **ar
libxl_nicinfo *nics;
unsigned int nb, i;
- if (argc < 3) {
- help("network-list");
- return 1;
- }
while ((opt = getopt(argc, argv, "h")) != -1) {
switch (opt) {
case 'h':
@@ -4127,11 +4134,15 @@ int main_networklist(int argc, char **ar
break;
}
}
+ if (argc-optind < 1) {
+ help("network-list");
+ return 1;
+ }
/* Idx BE MAC Hdl Sta evch txr/rxr BE-path */
printf("%-3s %-2s %-17s %-6s %-5s %-6s %5s/%-5s %-30s\n",
"Idx", "BE", "Mac Addr.", "handle", "state", "evt-ch", "tx-", "rx-ring-ref", "BE-path");
- for (argv += 2, argc -= 2; argc > 0; --argc, ++argv) {
+ for (argv += optind, argc -= optind; argc > 0; --argc, ++argv) {
if (domain_qualifier_to_domid(*argv, &domid, 0) < 0) {
fprintf(stderr, "%s is an invalid domain identifier\n", *argv);
continue;
@@ -4162,34 +4173,34 @@ int main_networkdetach(int argc, char **
int opt;
libxl_device_nic nic;
- if (argc != 4) {
+ while ((opt = getopt(argc, argv, "h")) != -1) {
+ switch (opt) {
+ case 'h':
+ help("network-detach");
+ return 0;
+ default:
+ fprintf(stderr, "option `%c' not supported.\n", opt);
+ break;
+ }
+ }
+ if (argc-optind != 2) {
help("network-detach");
return 0;
}
- while ((opt = getopt(argc, argv, "h")) != -1) {
- switch (opt) {
- case 'h':
- help("network-detach");
- return 0;
- default:
- fprintf(stderr, "option `%c' not supported.\n", opt);
- break;
- }
- }
-
- if (domain_qualifier_to_domid(argv[2], &domid, 0) < 0) {
- fprintf(stderr, "%s is an invalid domain identifier\n", argv[2]);
- return 1;
- }
-
- if (!strchr(argv[3], ':')) {
- if (libxl_devid_to_device_nic(&ctx, domid, argv[3], &nic)) {
- fprintf(stderr, "Unknown device %s.\n", argv[3]);
- return 1;
- }
- } else {
- if (libxl_mac_to_device_nic(&ctx, domid, argv[3], &nic)) {
- fprintf(stderr, "Unknown device %s.\n", argv[3]);
+
+ if (domain_qualifier_to_domid(argv[optind], &domid, 0) < 0) {
+ fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]);
+ return 1;
+ }
+
+ if (!strchr(argv[optind+1], ':')) {
+ if (libxl_devid_to_device_nic(&ctx, domid, argv[optind+1], &nic)) {
+ fprintf(stderr, "Unknown device %s.\n", argv[optind+1]);
+ return 1;
+ }
+ } else {
+ if (libxl_mac_to_device_nic(&ctx, domid, argv[optind+1], &nic)) {
+ fprintf(stderr, "Unknown device %s.\n", argv[optind+1]);
return 1;
}
}
@@ -4208,22 +4219,22 @@ int main_blockattach(int argc, char **ar
uint32_t fe_domid, be_domid = 0;
libxl_device_disk disk = { 0 };
- if ((argc < 5) || (argc > 7)) {
+ while ((opt = getopt(argc, argv, "h")) != -1) {
+ switch (opt) {
+ case 'h':
+ help("block-attach");
+ return 0;
+ default:
+ fprintf(stderr, "option `%c' not supported.\n", opt);
+ break;
+ }
+ }
+ if ((argc-optind < 3) || (argc-optind > 5)) {
help("block-attach");
return 0;
}
- while ((opt = getopt(argc, argv, "h")) != -1) {
- switch (opt) {
- case 'h':
- help("block-attach");
- return 0;
- default:
- fprintf(stderr, "option `%c' not supported.\n", opt);
- break;
- }
- }
-
- tok = strtok(argv[3], ":");
+
+ tok = strtok(argv[optind+1], ":");
if (!strcmp(tok, "phy")) {
disk.phystype = PHYSTYPE_PHY;
} else if (!strcmp(tok, "file")) {
@@ -4251,22 +4262,23 @@ int main_blockattach(int argc, char **ar
fprintf(stderr, "Error: missing path to disk image.\n");
return 1;
}
- disk.virtpath = argv[4];
+ disk.virtpath = argv[optind+2];
disk.unpluggable = 1;
- disk.readwrite = ((argc <= 4) || (argv[5][0] == 'w'));
-
- if (domain_qualifier_to_domid(argv[2], &fe_domid, 0) < 0) {
- fprintf(stderr, "%s is an invalid domain identifier\n", argv[2]);
- return 1;
- }
- if (argc == 7) {
- if (domain_qualifier_to_domid(argv[6], &be_domid, 0) < 0) {
- fprintf(stderr, "%s is an invalid domain identifier\n", argv[6]);
+ disk.readwrite = ((argc-optind <= 2) || (argv[optind+3][0] == 'w'));
+
+ if (domain_qualifier_to_domid(argv[optind], &fe_domid, 0) < 0) {
+ fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]);
+ return 1;
+ }
+ if (argc-optind == 5) {
+ if (domain_qualifier_to_domid(argv[optind+4], &be_domid, 0) < 0) {
+ fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind+4]);
return 1;
}
}
disk.domid = fe_domid;
disk.backend_domid = be_domid;
+
if (libxl_device_disk_add(&ctx, fe_domid, &disk)) {
fprintf(stderr, "libxl_device_disk_add failed.\n");
}
@@ -4280,24 +4292,24 @@ int main_blocklist(int argc, char **argv
libxl_device_disk *disks;
libxl_diskinfo diskinfo;
- if (argc < 3) {
+ while ((opt = getopt(argc, argv, "h")) != -1) {
+ switch (opt) {
+ case 'h':
+ help("block-list");
+ return 0;
+ default:
+ fprintf(stderr, "option `%c' not supported.\n", opt);
+ break;
+ }
+ }
+ if (argc-optind < 1) {
help("block-list");
return 0;
- }
- while ((opt = getopt(argc, argv, "h")) != -1) {
- switch (opt) {
- case 'h':
- help("block-list");
- return 0;
- default:
- fprintf(stderr, "option `%c' not supported.\n", opt);
- break;
- }
}
printf("%-5s %-3s %-6s %-5s %-6s %-8s %-30s\n",
"Vdev", "BE", "handle", "state", "evt-ch", "ring-ref", "BE-path");
- for (argv += 2, argc -= 2; argc > 0; --argc, ++argv) {
+ for (argv += optind, argc -= optind; argc > 0; --argc, ++argv) {
if (domain_qualifier_to_domid(*argv, &domid, 0) < 0) {
fprintf(stderr, "%s is an invalid domain identifier\n", *argv);
continue;
@@ -4326,27 +4338,27 @@ int main_blockdetach(int argc, char **ar
int opt;
libxl_device_disk disk;
- if (argc != 4) {
+ while ((opt = getopt(argc, argv, "h")) != -1) {
+ switch (opt) {
+ case 'h':
+ help("block-detach");
+ return 0;
+ default:
+ fprintf(stderr, "option `%c' not supported.\n", opt);
+ break;
+ }
+ }
+ if (argc-optind != 2) {
help("block-detach");
return 0;
}
- while ((opt = getopt(argc, argv, "h")) != -1) {
- switch (opt) {
- case 'h':
- help("block-detach");
- return 0;
- default:
- fprintf(stderr, "option `%c' not supported.\n", opt);
- break;
- }
- }
-
- if (domain_qualifier_to_domid(argv[2], &domid, 0) < 0) {
- fprintf(stderr, "%s is an invalid domain identifier\n", argv[2]);
- return 1;
- }
- if (libxl_devid_to_device_disk(&ctx, domid, argv[3], &disk)) {
- fprintf(stderr, "Error: Device %s not connected.\n", argv[3]);
+
+ if (domain_qualifier_to_domid(argv[optind], &domid, 0) < 0) {
+ fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]);
+ return 1;
+ }
+ if (libxl_devid_to_device_disk(&ctx, domid, argv[optind+1], &disk)) {
+ fprintf(stderr, "Error: Device %s not connected.\n", argv[optind+1]);
return 1;
}
if (libxl_device_disk_del(&ctx, &disk, 1)) {
@@ -4364,27 +4376,27 @@ int main_network2attach(int argc, char *
unsigned int val, i;
libxl_device_net2 net2;
- if ((argc < 3) || (argc > 12)) {
+ while ((opt = getopt(argc, argv, "h")) != -1) {
+ switch (opt) {
+ case 'h':
+ help("network2-attach");
+ return 0;
+ default:
+ fprintf(stderr, "option `%c' not supported.\n", opt);
+ break;
+ }
+ }
+ if ((argc-optind < 1) || (argc-optind > 10)) {
help("network2-attach");
return 0;
}
- while ((opt = getopt(argc, argv, "h")) != -1) {
- switch (opt) {
- case 'h':
- help("network2-attach");
- return 0;
- default:
- fprintf(stderr, "option `%c' not supported.\n", opt);
- break;
- }
- }
-
- if (domain_qualifier_to_domid(argv[2], &domid, 0) < 0) {
- fprintf(stderr, "%s is an invalid domain identifier\n", argv[1]);
+
+ if (domain_qualifier_to_domid(argv[optind], &domid, 0) < 0) {
+ fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]);
return 1;
}
init_net2_info(&net2, -1);
- for (argv += 3, argc -= 3; argc > 0; --argc, ++argv) {
+ for (argv += optind+1, argc -= optind+1; argc > 0; --argc, ++argv) {
if (!strncmp("front_mac=", *argv, 10)) {
tok = strtok((*argv) + 10, ":");
for (i = 0; tok && i < 6; tok = strtok(NULL, ":"), ++i) {
@@ -4457,25 +4469,25 @@ int main_network2list(int argc, char **a
unsigned int nb;
libxl_net2info *net2s;
- if (argc < 3) {
+ while ((opt = getopt(argc, argv, "h")) != -1) {
+ switch (opt) {
+ case 'h':
+ help("network2-list");
+ return 0;
+ default:
+ fprintf(stderr, "option `%c' not supported.\n", opt);
+ break;
+ }
+ }
+ if (argc - optind < 1) {
help("network2-list");
return 0;
- }
- while ((opt = getopt(argc, argv, "h")) != -1) {
- switch (opt) {
- case 'h':
- help("network2-list");
- return 0;
- default:
- fprintf(stderr, "option `%c' not supported.\n", opt);
- break;
- }
}
printf("%-3s %-2s %-5s %-17s %-17s %-7s %-6s %-30s\n",
"Idx", "BE", "state", "Mac Addr.", "Remote Mac Addr.",
"trusted", "filter", "backend");
- for (argv += 2, argc -=2; argc > 0; --argc, ++argv) {
+ for (argv += optind, argc -=optind; argc > 0; --argc, ++argv) {
if (domain_qualifier_to_domid(*argv, &domid, 0) < 0) {
fprintf(stderr, "%s is an invalid domain identifier\n", *argv);
continue;
@@ -4501,27 +4513,27 @@ int main_network2detach(int argc, char *
int opt;
libxl_device_net2 net2;
- if (argc != 4) {
+ while ((opt = getopt(argc, argv, "h")) != -1) {
+ switch (opt) {
+ case 'h':
+ help("network2-detach");
+ return 0;
+ default:
+ fprintf(stderr, "option `%c' not supported.\n", opt);
+ break;
+ }
+ }
+ if (argc-optind != 2) {
help("network2-detach");
return 0;
}
- while ((opt = getopt(argc, argv, "h")) != -1) {
- switch (opt) {
- case 'h':
- help("network2-detach");
- return 0;
- default:
- fprintf(stderr, "option `%c' not supported.\n", opt);
- break;
- }
- }
-
- if (domain_qualifier_to_domid(argv[2], &domid, 0) < 0) {
- fprintf(stderr, "%s is an invalid domain identifier\n", argv[2]);
- return 1;
- }
- if (libxl_devid_to_device_net2(&ctx, domid, argv[3], &net2)) {
- fprintf(stderr, "Error: Device %s not connected.\n", argv[3]);
+
+ if (domain_qualifier_to_domid(argv[optind], &domid, 0) < 0) {
+ fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]);
+ return 1;
+ }
+ if (libxl_devid_to_device_net2(&ctx, domid, argv[optind+1], &net2)) {
+ fprintf(stderr, "Error: Device %s not connected.\n", argv[optind+1]);
return 1;
}
if (libxl_device_net2_del(&ctx, &net2, 1)) {
diff -r aa344916733f -r 2210bb76868f tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c Mon Aug 23 09:46:03 2010 +0100
+++ b/tools/libxl/xl_cmdtable.c Mon Aug 23 15:34:14 2010 +0100
@@ -241,7 +241,7 @@ struct cmd_spec cmd_table[] = {
"Create a new virtual network device",
"<Domain> [type=<type>] [mac=<mac>] [bridge=<bridge>] "
"[ip=<ip>] [script=<script>] [backend=<BackDomain>] [vifname=<name>] "
- "[rate=<rate>] [model=<model>][accel=<accel>]",
+ "[rate=<rate>] [model=<model>] [accel=<accel>]",
},
{ "network-list",
&main_networklist,
^ permalink raw reply [flat|nested] 3+ messages in thread