tools: Check for dangling --format parameters (RHBZ#1140894).

In most C tools, virt-sysprep and virt-customize, you have to put the
--format parameter before the corresponding -a parameter.  ie.  The
following is correct:

  guestfish --format qcow2 -a disk1 -a disk2

But the following is incorrect.  The --format parameter is dangling
and prior to this commit would have been silently ignored:

  guestfish -a disk1 -a disk2 --format qcow2

After this change, dangling --format parameters now lead to an error:

  guestfish: --format parameter must appear before -a parameter

In virt-customize, also check that --attach-format parameter appears
before --attach parameter.

Thanks: Lingfei Kong
This commit is contained in:
Richard W.M. Jones
2014-09-13 09:45:59 +01:00
parent 7845bcc20f
commit b7bdb63d89
17 changed files with 114 additions and 63 deletions

View File

@@ -122,6 +122,7 @@ main (int argc, char *argv[])
};
struct drv *drvs = NULL;
const char *format = NULL;
bool format_consumed = true;
int c;
int option_index;
int exit_code;
@@ -143,10 +144,7 @@ main (int argc, char *argv[])
if (STREQ (long_options[option_index].name, "long-options"))
display_long_options (long_options);
else if (STREQ (long_options[option_index].name, "format")) {
if (!optarg || STREQ (optarg, ""))
format = NULL;
else
format = optarg;
OPTION_format;
} else if (STREQ (long_options[option_index].name, "uuid")) {
uuid = 1;
} else {
@@ -211,6 +209,8 @@ main (int argc, char *argv[])
if (optind != argc)
usage (EXIT_FAILURE);
CHECK_OPTION_format_consumed;
/* virt-alignment-scan has two modes. If the user didn't specify
* any drives, then we do the scan on every libvirt guest. That's
* the if-clause below. If the user specified domains/drives, then

View File

@@ -111,6 +111,7 @@ main (int argc, char *argv[])
struct mp *mp;
char *p;
const char *format = NULL;
bool format_consumed = true;
int c;
int r;
int option_index;
@@ -134,10 +135,7 @@ main (int argc, char *argv[])
} else if (STREQ (long_options[option_index].name, "echo-keys")) {
echo_keys = 1;
} else if (STREQ (long_options[option_index].name, "format")) {
if (!optarg || STREQ (optarg, ""))
format = NULL;
else
format = optarg;
OPTION_format;
} else {
fprintf (stderr, _("%s: unknown long option: %s (%d)\n"),
program_name, long_options[option_index].name, option_index);
@@ -231,6 +229,8 @@ main (int argc, char *argv[])
if (optind >= argc || argc - optind < 1)
usage (EXIT_FAILURE);
CHECK_OPTION_format_consumed;
/* User must have specified some drives. */
if (drvs == NULL)
usage (EXIT_FAILURE);

View File

@@ -175,6 +175,7 @@ main (int argc, char *argv[])
};
struct drv *drvs = NULL;
const char *format = NULL;
bool format_consumed = true;
int c;
int option_index;
int no_title = 0; /* --no-title */
@@ -201,10 +202,7 @@ main (int argc, char *argv[])
} else if (STREQ (long_options[option_index].name, "echo-keys")) {
echo_keys = 1;
} else if (STREQ (long_options[option_index].name, "format")) {
if (!optarg || STREQ (optarg, ""))
format = NULL;
else
format = optarg;
OPTION_format;
} else if (STREQ (long_options[option_index].name, "all")) {
output = OUTPUT_ALL;
} else if (STREQ (long_options[option_index].name, "blkdevs") ||
@@ -296,6 +294,8 @@ main (int argc, char *argv[])
if (optind != argc)
usage (EXIT_FAILURE);
CHECK_OPTION_format_consumed;
/* -h and --csv doesn't make sense. Spreadsheets will corrupt these
* fields. (RHBZ#600977).
*/

View File

@@ -113,6 +113,7 @@ main (int argc, char *argv[])
};
struct drv *drvs = NULL;
const char *format = NULL;
bool format_consumed = true;
int c;
int r;
int option_index;
@@ -136,10 +137,7 @@ main (int argc, char *argv[])
} else if (STREQ (long_options[option_index].name, "echo-keys")) {
echo_keys = 1;
} else if (STREQ (long_options[option_index].name, "format")) {
if (!optarg || STREQ (optarg, ""))
format = NULL;
else
format = optarg;
OPTION_format;
} else {
fprintf (stderr, _("%s: unknown long option: %s (%d)\n"),
program_name, long_options[option_index].name, option_index);
@@ -191,6 +189,8 @@ main (int argc, char *argv[])
if (optind != argc)
usage (EXIT_FAILURE);
CHECK_OPTION_format_consumed;
/* User must have specified some drives. */
if (drvs == NULL)
usage (EXIT_FAILURE);

View File

@@ -168,6 +168,7 @@ main (int argc, char *argv[])
struct mp *mp;
char *p;
const char *format = NULL;
bool format_consumed = true;
int c;
int option_index;
#define MODE_LS_L 1
@@ -194,10 +195,7 @@ main (int argc, char *argv[])
} else if (STREQ (long_options[option_index].name, "echo-keys")) {
echo_keys = 1;
} else if (STREQ (long_options[option_index].name, "format")) {
if (!optarg || STREQ (optarg, ""))
format = NULL;
else
format = optarg;
OPTION_format;
} else if (STREQ (long_options[option_index].name, "checksum") ||
STREQ (long_options[option_index].name, "checksums")) {
if (!optarg || STREQ (optarg, ""))
@@ -327,6 +325,8 @@ main (int argc, char *argv[])
assert (inspector == 1 || mps != NULL);
assert (live == 0);
CHECK_OPTION_format_consumed;
/* Many flags only apply to -lR mode. */
if (mode != MODE_LS_LR &&
(csv || human || enable_uids || enable_times || enable_extra_stats ||

View File

@@ -32,7 +32,10 @@ let prog = Filename.basename Sys.executable_name
let main () =
let attach = ref [] in
let attach_format = ref None in
let set_attach_format = function
let attach_format_consumed = ref true in
let set_attach_format s =
attach_format_consumed := false;
match s with
| "auto" -> attach_format := None
| s -> attach_format := Some s
in
@@ -42,6 +45,11 @@ let main () =
let dryrun = ref false in
let files = ref [] in
let format = ref "auto" in
let format_consumed = ref true in
let set_format s =
format := s;
format_consumed := false
in
let libvirturi = ref "" in
let memsize = ref None in
let set_memsize arg = memsize := Some arg in
@@ -62,7 +70,8 @@ let main () =
eprintf "Error parsing URI '%s'. Look for error messages printed above.\n" arg;
exit 1 in
let format = match !format with "auto" -> None | fmt -> Some fmt in
files := (uri, format) :: !files
files := (uri, format) :: !files;
format_consumed := true
and set_domain dom =
if !domain <> None then (
eprintf (f_"%s: --domain option can only be given once\n") prog;
@@ -85,7 +94,7 @@ let main () =
"-n", Arg.Set dryrun, " " ^ s_"Perform a dry run";
"--dryrun", Arg.Set dryrun, " " ^ s_"Perform a dry run";
"--dry-run", Arg.Set dryrun, " " ^ s_"Perform a dry run";
"--format", Arg.Set_string format, s_"format" ^ " " ^ s_"Set format (default: auto)";
"--format", Arg.String set_format, s_"format" ^ " " ^ s_"Set format (default: auto)";
"--long-options", Arg.Unit display_long_options, " " ^ s_"List long options";
"-m", Arg.Int set_memsize, "mb" ^ " " ^ s_"Set memory size";
"--memsize", Arg.Int set_memsize, "mb" ^ " " ^ s_"Set memory size";
@@ -129,6 +138,12 @@ read the man page virt-customize(1).
prog in
Arg.parse argspec anon_fun usage_msg;
if not !format_consumed then
error ~prog (f_"--format parameter must appear before -a parameter");
if not !attach_format_consumed then
error ~prog (f_"--attach-format parameter must appear before --attach parameter");
(* Check -a and -d options. *)
let files = !files in
let domain = !domain in

View File

@@ -123,6 +123,7 @@ main (int argc, char *argv[])
struct drv *drvs = NULL;
struct drv *drv;
const char *format = NULL;
bool format_consumed = true;
int c;
int option_index;
size_t max_threads = 0;
@@ -143,10 +144,7 @@ main (int argc, char *argv[])
if (STREQ (long_options[option_index].name, "long-options"))
display_long_options (long_options);
else if (STREQ (long_options[option_index].name, "format")) {
if (!optarg || STREQ (optarg, ""))
format = NULL;
else
format = optarg;
OPTION_format;
} else if (STREQ (long_options[option_index].name, "csv")) {
csv = 1;
} else if (STREQ (long_options[option_index].name, "one-per-guest")) {
@@ -255,6 +253,8 @@ main (int argc, char *argv[])
if (optind != argc)
usage (EXIT_FAILURE);
CHECK_OPTION_format_consumed;
/* -h and --csv doesn't make sense. Spreadsheets will corrupt these
* fields. (RHBZ#600977).
*/

View File

@@ -182,6 +182,7 @@ main (int argc, char *argv[])
struct drv *drvs = NULL; /* First guest. */
struct drv *drvs2 = NULL; /* Second guest. */
const char *format = NULL;
bool format_consumed = true;
int c;
int option_index;
struct tree *tree1, *tree2;
@@ -211,10 +212,7 @@ main (int argc, char *argv[])
} else if (STREQ (long_options[option_index].name, "echo-keys")) {
echo_keys = 1;
} else if (STREQ (long_options[option_index].name, "format")) {
if (!optarg || STREQ (optarg, ""))
format = NULL;
else
format = optarg;
OPTION_format;
} else if (STREQ (long_options[option_index].name, "all")) {
enable_extra_stats = enable_times = enable_uids = enable_xattrs = 1;
} else if (STREQ (long_options[option_index].name, "atime")) {
@@ -344,6 +342,8 @@ main (int argc, char *argv[])
assert (inspector == 1);
assert (live == 0);
CHECK_OPTION_format_consumed;
unsigned errors = 0;
/* Mount up first guest. */

View File

@@ -128,6 +128,7 @@ main (int argc, char *argv[])
struct mp *mp;
char *p;
const char *format = NULL;
bool format_consumed = true;
int c;
int option_index;
@@ -150,10 +151,7 @@ main (int argc, char *argv[])
} else if (STREQ (long_options[option_index].name, "echo-keys")) {
echo_keys = 1;
} else if (STREQ (long_options[option_index].name, "format")) {
if (!optarg || STREQ (optarg, ""))
format = NULL;
else
format = optarg;
OPTION_format;
} else {
fprintf (stderr, _("%s: unknown long option: %s (%d)\n"),
program_name, long_options[option_index].name, option_index);
@@ -265,6 +263,8 @@ main (int argc, char *argv[])
if (optind >= argc || argc - optind < 1)
usage (EXIT_FAILURE);
CHECK_OPTION_format_consumed;
/* User must have specified some drives. */
if (drvs == NULL)
usage (EXIT_FAILURE);

View File

@@ -214,6 +214,7 @@ main (int argc, char *argv[])
struct mp *mp;
char *p, *file = NULL;
const char *format = NULL;
bool format_consumed = true;
int c;
int option_index;
struct sigaction sa;
@@ -274,10 +275,7 @@ main (int argc, char *argv[])
} else if (STREQ (long_options[option_index].name, "echo-keys")) {
echo_keys = 1;
} else if (STREQ (long_options[option_index].name, "format")) {
if (!optarg || STREQ (optarg, ""))
format = NULL;
else
format = optarg;
OPTION_format;
} else if (STREQ (long_options[option_index].name, "csh")) {
remote_control_csh = 1;
} else if (STREQ (long_options[option_index].name, "live")) {
@@ -475,6 +473,8 @@ main (int argc, char *argv[])
}
}
CHECK_OPTION_format_consumed;
/* If we've got drives to add, add them now. */
add_drives (drvs, 'a');

View File

@@ -19,6 +19,7 @@
#ifndef OPTIONS_H
#define OPTIONS_H
#include <stdbool.h>
#include <getopt.h>
#include "guestfs-internal-frontend.h"
@@ -140,10 +141,16 @@ extern void free_mps (struct mp *mp);
extern void display_long_options (const struct option *) __attribute__((noreturn));
#define OPTION_a \
option_a (optarg, format, &drvs)
do { \
option_a (optarg, format, &drvs); \
format_consumed = true; \
} while (0)
#define OPTION_A \
option_a (optarg, format, &drvs2)
do { \
option_a (optarg, format, &drvs2); \
format_consumed = true; \
} while (0)
#define OPTION_c \
libvirt_uri = optarg
@@ -154,6 +161,15 @@ extern void display_long_options (const struct option *) __attribute__((noreturn
#define OPTION_D \
option_d (optarg, &drvs2)
#define OPTION_format \
do { \
if (!optarg || STREQ (optarg, "")) \
format = NULL; \
else \
format = optarg; \
format_consumed = false; \
} while (0)
#define OPTION_i \
inspector = 1
@@ -217,4 +233,14 @@ extern void display_long_options (const struct option *) __attribute__((noreturn
#define OPTION_x \
guestfs_set_trace (g, 1)
#define CHECK_OPTION_format_consumed \
do { \
if (!format_consumed) { \
fprintf (stderr, \
_("%s: --format parameter must appear before -a parameter\n"), \
program_name); \
exit (EXIT_FAILURE); \
} \
} while (0)
#endif /* OPTIONS_H */

View File

@@ -44,9 +44,9 @@ $VG ./guestfish -x --format -a test-a.img </dev/null >test-a.out 2>&1
! grep -sq 'add_drive.*format' test-a.out
$VG ./guestfish -x -a test-a.img --format=qcow2 </dev/null >test-a.out 2>&1
$VG ./guestfish -x -a test-a.img --format=raw -a /dev/null </dev/null >test-a.out 2>&1
! grep -sq 'add_drive.*format' test-a.out
! grep -sq 'add_drive.*test-a.img.*format' test-a.out
rm test-a.out
rm test-a.img

View File

@@ -120,6 +120,7 @@ main (int argc, char *argv[])
};
struct drv *drvs = NULL;
const char *format = NULL;
bool format_consumed = true;
int c;
int option_index;
int retry, retries;
@@ -139,10 +140,7 @@ main (int argc, char *argv[])
if (STREQ (long_options[option_index].name, "long-options"))
display_long_options (long_options);
else if (STREQ (long_options[option_index].name, "format")) {
if (!optarg || STREQ (optarg, ""))
format = NULL;
else
format = optarg;
OPTION_format;
} else if (STREQ (long_options[option_index].name, "filesystem")) {
if (STREQ (optarg, "none"))
filesystem = NULL;
@@ -227,6 +225,8 @@ main (int argc, char *argv[])
if (optind != argc)
usage (EXIT_FAILURE);
CHECK_OPTION_format_consumed;
/* The user didn't specify any drives to format. */
if (drvs == NULL)
usage (EXIT_FAILURE);

View File

@@ -184,6 +184,7 @@ main (int argc, char *argv[])
struct mp *mp;
char *p;
const char *format = NULL;
bool format_consumed = true;
int c, r;
int option_index;
struct sigaction sa;
@@ -227,10 +228,7 @@ main (int argc, char *argv[])
if (guestfs_set_selinux (g, 1) == -1)
exit (EXIT_FAILURE);
} else if (STREQ (long_options[option_index].name, "format")) {
if (!optarg || STREQ (optarg, ""))
format = NULL;
else
format = optarg;
OPTION_format;
} else if (STREQ (long_options[option_index].name, "keys-from-stdin")) {
keys_from_stdin = 1;
} else if (STREQ (long_options[option_index].name, "echo-keys")) {
@@ -312,6 +310,8 @@ main (int argc, char *argv[])
}
}
CHECK_OPTION_format_consumed;
/* Check we have the right options. */
if (!live) {
if (!drvs || !(mps || inspector)) {

View File

@@ -119,6 +119,7 @@ main (int argc, char *argv[])
struct drv *drvs = NULL;
struct drv *drv;
const char *format = NULL;
bool format_consumed = true;
int c;
int option_index;
@@ -141,10 +142,7 @@ main (int argc, char *argv[])
} else if (STREQ (long_options[option_index].name, "echo-keys")) {
echo_keys = 1;
} else if (STREQ (long_options[option_index].name, "format")) {
if (!optarg || STREQ (optarg, ""))
format = NULL;
else
format = optarg;
OPTION_format;
} else if (STREQ (long_options[option_index].name, "xpath")) {
xpath = optarg;
} else {
@@ -234,6 +232,8 @@ main (int argc, char *argv[])
if (optind != argc)
usage (EXIT_FAILURE);
CHECK_OPTION_format_consumed;
/* XPath is modal: no drives should be specified. There must be
* one extra parameter on the command line.
*/

View File

@@ -122,6 +122,7 @@ main (int argc, char *argv[])
struct drv *drvs = NULL;
struct drv *drv;
const char *format = NULL;
bool format_consumed = true;
int c;
int option_index;
int network = 0;
@@ -152,10 +153,7 @@ main (int argc, char *argv[])
} else if (STREQ (long_options[option_index].name, "network")) {
network = 1;
} else if (STREQ (long_options[option_index].name, "format")) {
if (!optarg || STREQ (optarg, ""))
format = NULL;
else
format = optarg;
OPTION_format;
} else if (STREQ (long_options[option_index].name, "smp")) {
if (sscanf (optarg, "%d", &smp) != 1) {
fprintf (stderr, _("%s: could not parse --smp parameter '%s'\n"),
@@ -298,6 +296,8 @@ main (int argc, char *argv[])
if (optind != argc)
usage (EXIT_FAILURE);
CHECK_OPTION_format_consumed;
/* User must have specified some drives. */
if (drvs == NULL)
usage (EXIT_FAILURE);

View File

@@ -39,13 +39,19 @@ let main () =
let domain = ref None in
let dryrun = ref false in
let files = ref [] in
let format = ref "auto" in
let quiet = ref false in
let libvirturi = ref "" in
let mount_opts = ref "" in
let operations = ref None in
let trace = ref false in
let verbose = ref false in
let mount_opts = ref "" in
let format = ref "auto" in
let format_consumed = ref true in
let set_format s =
format := s;
format_consumed := false
in
let display_version () =
printf "virt-sysprep %s\n" Config.package_version;
@@ -56,7 +62,8 @@ let main () =
with Invalid_argument "URI.parse_uri" ->
error ~prog (f_"error parsing URI '%s'. Look for error messages printed above.") arg in
let format = match !format with "auto" -> None | fmt -> Some fmt in
files := (uri, format) :: !files
files := (uri, format) :: !files;
format_consumed := true
and set_domain dom =
if !domain <> None then
error ~prog (f_"--domain option can only be given once");
@@ -129,7 +136,7 @@ let main () =
"--dump-pod", Arg.Unit dump_pod, " " ^ s_"Dump POD (internal)";
"--dump-pod-options", Arg.Unit dump_pod_options, " " ^ s_"Dump POD for options (internal)";
"--enable", Arg.String set_enable, s_"operations" ^ " " ^ s_"Enable specific operations";
"--format", Arg.Set_string format, s_"format" ^ " " ^ s_"Set format (default: auto)";
"--format", Arg.String set_format, s_"format" ^ " " ^ s_"Set format (default: auto)";
"--list-operations", Arg.Unit list_operations, " " ^ s_"List supported operations";
"--long-options", Arg.Unit display_long_options, " " ^ s_"List long options";
"--mount-options", Arg.Set_string mount_opts, s_"opts" ^ " " ^ s_"Set mount options (eg /:noatime;/var:rw,noatime)";
@@ -163,6 +170,9 @@ read the man page virt-sysprep(1).
prog in
Arg.parse argspec anon_fun usage_msg;
if not !format_consumed then
error ~prog (f_"--format parameter must appear before -a parameter");
(* Check -a and -d options. *)
let files = !files in
let domain = !domain in