[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH 1 of 2] xl: correct argument parsing for some sub-commands



# HG changeset patch
# User Ian Campbell <ian.campbell@xxxxxxxxxx>
# 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@xxxxxxxxxx>

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,

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.