[Libguestfs] [PATCH supermin 1/2] helper: Define new-style command line syntax.

From: "Richard W.M. Jones" <rjones(a)redhat.com&gt; The old syntax, inherited from when this used to be a shell script, was a little bit crazy, eg: supermin-helper -f ext2 supermin.d x86_64 kernel initrd root where 'supermin.d' is an input, and 'kernel', 'initrd' and 'root' are outputs (files overwritten if present) and 'x86_64' should really be a flag. This commit defines a new-style syntax. In the new-style, the same command would be written: supermin-helper -f ext2 --host-cpu x86_64 supermin.d \ --output-kernel kernel --output-initrd initrd --output-appliance root The new-style syntax also allows you to define an output directory and lets you omit the host-cpu (if it's the same as the compile architecture, which is almost always true), so: supermin-helper -f ext2 supermin.d -o /tmp (which would write /tmp/kernel etc.) The old-style syntax is still supported for backwards compatibility. This also adds a currently non-functional --dtb parameter / SUPERMIN_DTB environment variable for specifying how to search for device trees, which is the motivation for this change. --- configure.ac | 3 + helper/cpio.c | 4 +- helper/main.c | 324 +++++++++++++++++++++++++++++++-------------- helper/supermin-helper.pod | 162 ++++++++++++++++++++--- tests/test-build-bash.sh | 13 +- 5 files changed, 381 insertions(+), 125 deletions(-) diff --git a/configure.ac b/configure.ac index bd3d573..207c19e 100644 --- a/configure.ac +++ b/configure.ac @@ -37,6 +37,9 @@ AC_SYS_LARGEFILE gl_INIT +dnl Define a C symbol for the host CPU architecture. +AC_DEFINE_UNQUOTED([host_cpu],["$host_cpu"],[Host architecture.]) + # Define $(SED). m4_ifdef([AC_PROG_SED],[ AC_PROG_SED diff --git a/helper/cpio.c b/helper/cpio.c index 3189560..55db96a 100644 --- a/helper/cpio.c +++ b/helper/cpio.c @@ -253,9 +253,9 @@ static void cpio_start (const char *hostcpu, const char *appliance, const char *modpath, const char *initrd) { - out_fd = open (appliance, O_WRONLY | O_CREAT | O_TRUNC | O_NOCTTY, 0644); + out_fd = open (initrd, O_WRONLY | O_CREAT | O_TRUNC | O_NOCTTY, 0644); if (out_fd == -1) - error (EXIT_FAILURE, errno, "open: %s", appliance); + error (EXIT_FAILURE, errno, "open: %s", initrd); out_offset = 0; } diff --git a/helper/main.c b/helper/main.c index 4123ae9..5632b9f 100644 --- a/helper/main.c +++ b/helper/main.c @@ -20,6 +20,7 @@ #include <stdio.h> #include <stdlib.h> +#include <stdbool.h> #include <string.h> #include <errno.h> #include <unistd.h> @@ -42,13 +43,19 @@ int copy_kernel = 0; enum { HELP_OPTION = CHAR_MAX + 1 }; -static const char *options = "f:g:k:u:vV"; +static const char *options = "f:g:k:o:u:vV"; static const struct option long_options[] = { { "help", 0, 0, HELP_OPTION }, { "copy-kernel", 0, 0, 0 }, + { "dtb", required_argument, 0, 0 }, { "format", required_argument, 0, 'f' }, { "group", required_argument, 0, 'g' }, + { "host-cpu", required_argument, 0, 0 }, { "kmods", required_argument, 0, 'k' }, + { "output-appliance", required_argument, 0, 0 }, + { "output-dtb", required_argument, 0, 0 }, + { "output-initrd", required_argument, 0, 0 }, + { "output-kernel", required_argument, 0, 0 }, { "user", required_argument, 0, 'u' }, { "verbose", 0, 0, 'v' }, { "version", 0, 0, 'V' }, @@ -59,43 +66,54 @@ static void usage (FILE *f, const char *progname) { fprintf (f, - "%s: build the supermin appliance on the fly\n" - "\n" - "Usage:\n" - " %s [-options] inputs [...] host_cpu kernel initrd\n" - " %s -f ext2 inputs [...] host_cpu kernel initrd appliance\n" - " %s -f checksum inputs [...] host_cpu\n" - " %s --help\n" - " %s --version\n" - "\n" - "This program is used by supermin to build the supermin appliance\n" - "(kernel and initrd output files). You should NOT need to run this\n" - "program directly except if you are debugging tricky supermin\n" - "appliance problems.\n" - "\n" - "NB: The kernel and initrd parameters are OUTPUT parameters. If\n" - "those files exist, they are overwritten by the output.\n" - "\n" - "Options:\n" - " --help\n" - " Display this help text and exit.\n" - " -f cpio|ext2|checksum | --format cpio|ext2|checksum\n" - " Specify output format (default: cpio).\n" - " --copy-kernel\n" - " Copy the kernel instead of symlinking to it.\n" - " -u user | --user user\n" - " The user name or uid the appliance will run as. Use of this\n" - " option requires root privileges.\n" - " -g group | --group group\n" - " The group name or gid the appliance will run as. Use of\n" - " this option requires root privileges.\n" - " -k file | --kmods file\n" - " Specify kernel module whitelist.\n" - " --verbose | -v\n" - " Enable verbose messages (give multiple times for more verbosity).\n" - " --version | -V\n" - " Display version number and exit.\n", - progname, progname, progname, progname, progname, progname); + "%s: build the supermin appliance on the fly\n" + "\n" + "Usage:\n" + " %s [-f cpio|ext2] -o outputdir input [input...]\n" + "or:\n" + " %s [-f cpio|ext2] --output-kernel kernel \\\n" + " [--output-dtb dtb] --output-initrd initrd \\\n" + " [--output-appliance appliance] input [input...]\n" + "or:\n" + " %s -f checksum input [input ...]\n" + "or:\n" + " %s --help\n" + " %s --version\n" + "\n" + "This program is used to build the full appliance from the supermin appliance.\n" + "\n" + "Options:\n" + " --help\n" + " Display this help text and exit.\n" + " --copy-kernel\n" + " Copy the kernel & device tree instead of symlinking to it.\n" + " --dtb wildcard\n" + " Search for a device tree matching wildcard.\n" + " -f cpio|ext2|checksum | --format cpio|ext2|checksum\n" + " Specify output format (default: cpio).\n" + " --host-cpu cpu\n" + " Host CPU type (default: " host_cpu ").\n" + " -k file | --kmods file\n" + " Specify kernel module whitelist.\n" + " -o outputdir\n" + " Write output to outputdir/kernel etc.\n" + " --output-appliance path\n" + " Write appliance to path (overrides -o).\n" + " --output-dtb path\n" + " Write device tree to path (overrides -o).\n" + " --output-initrd path\n" + " Write initrd to path (overrides -o).\n" + " --output-kernel path\n" + " Write kernel to path (overrides -o).\n" + " -u user | --user user\n" + " -g group | --group group\n" + " The user name or uid, and group name or gid the appliance will\n" + " run as. Use of these options requires root privileges.\n" + " --verbose | -v\n" + " Enable verbose messages (give multiple times for more verbosity).\n" + " --version | -V\n" + " Display version number and exit.\n", + progname, progname, progname, progname, progname, progname); } static uid_t @@ -163,9 +181,20 @@ main (int argc, char *argv[]) const char *format = "cpio"; const char *whitelist = NULL; + /* For the reason this was originally added, see + * https://bugzilla.redhat.com/show_bug.cgi?id=558593 + */ + const char *hostcpu = host_cpu; + + /* Output files. */ + char *kernel = NULL, *initrd = NULL, *appliance = NULL; + const char *output_dir = NULL; + uid_t euid = geteuid (); gid_t egid = getegid (); + bool old_style = true; + /* Command line arguments. */ for (;;) { int option_index; @@ -180,7 +209,37 @@ main (int argc, char *argv[]) case 0: /* options which are long only */ if (strcmp (long_options[option_index].name, "copy-kernel") == 0) { copy_kernel = 1; - } else { + } + else if (strcmp (long_options[option_index].name, "dtb") == 0) { + fprintf (stderr, "%s: error: --dtb is not implemented yet\n", argv[0]); + exit (EXIT_FAILURE); + } + else if (strcmp (long_options[option_index].name, "host-cpu") == 0) { + hostcpu = optarg; + old_style = false; + } + else if (strcmp (long_options[option_index].name, "output-kernel") == 0) { + kernel = optarg; + old_style = false; + } + else if (strcmp (long_options[option_index].name, "output-dtb") == 0) { + /* + dtb = optarg; + old_style = false; + */ + fprintf (stderr, "%s: error: --output-dtb is not implemented yet\n", + argv[0]); + exit (EXIT_FAILURE); + } + else if (strcmp (long_options[option_index].name, "output-initrd") == 0) { + initrd = optarg; + old_style = false; + } + else if (strcmp (long_options[option_index].name, "output-appliance") == 0) { + appliance = optarg; + old_style = false; + } + else { fprintf (stderr, "%s: unknown long option: %s (%d)\n", argv[0], long_options[option_index].name, option_index); exit (EXIT_FAILURE); @@ -203,6 +262,11 @@ main (int argc, char *argv[]) whitelist = optarg; break; + case 'o': + output_dir = optarg; + old_style = false; + break; + case 'v': verbose++; break; @@ -217,59 +281,29 @@ main (int argc, char *argv[]) } } - /* We need to set the real, not effective, uid here to work round a - * misfeature in bash. bash will automatically reset euid to uid when - * invoked. As shell is used in places by supermin-helper, this - * results in code running with varying privilege. */ - uid_t uid = getuid (); - gid_t gid = getgid (); - - if (uid != euid || gid != egid) { - if (uid != 0) { - fprintf (stderr, "The -u and -g options require root privileges.\n"); - usage (stderr, argv[0]); - exit (EXIT_FAILURE); - } - - /* Need to become root first because setgid and setuid require it */ - if (seteuid (0) == -1) { - perror ("seteuid"); - exit (EXIT_FAILURE); - } - - /* Set gid and uid to command-line parameters */ - if (setgid (egid) == -1) { - perror ("setgid"); - exit (EXIT_FAILURE); - } - - /* Kill supplemental groups from parent process (RHBZ#902476). */ - if (setgroups (1, &egid) == -1) { - perror ("setgroups"); - exit (EXIT_FAILURE); - } - - if (setuid (euid) == -1) { - perror ("setuid"); - exit (EXIT_FAILURE); - } - } - /* Select the correct writer module. */ struct writer *writer; - int nr_outputs; + bool needs_kernel; + bool needs_initrd; + bool needs_appliance; if (strcmp (format, "cpio") == 0) { writer = &cpio_writer; - nr_outputs = 2; /* kernel and appliance (== initrd) */ + needs_kernel = true; + needs_initrd = true; + needs_appliance = false; } else if (strcmp (format, "ext2") == 0) { writer = &ext2_writer; - nr_outputs = 3; /* kernel, initrd, appliance */ + needs_kernel = true; + needs_initrd = true; + needs_appliance = true; } else if (strcmp (format, "checksum") == 0) { writer = &checksum_writer; - nr_outputs = 0; /* (none) */ + needs_kernel = false; + needs_initrd = false; + needs_appliance = false; } else { fprintf (stderr, @@ -278,14 +312,71 @@ main (int argc, char *argv[]) exit (EXIT_FAILURE); } - /* [optind .. optind+nr_inputs-1] hostcpu [argc-nr_outputs-1 .. argc-1] - * <---- nr_inputs ----> 1 <---- nr_outputs ----> - */ char **inputs = &argv[optind]; - int nr_inputs = argc - nr_outputs - 1 - optind; - char **outputs = &argv[optind+nr_inputs+1]; - /*assert (outputs [nr_outputs] == NULL); - assert (inputs [nr_inputs + 1 + nr_outputs] == NULL);*/ + int nr_inputs; + + /* Old-style arguments? */ + if (old_style) { + int nr_outputs; + + if (strcmp (format, "cpio") == 0) + nr_outputs = 2; /* kernel and appliance (== initrd) */ + else if (strcmp (format, "ext2") == 0) + nr_outputs = 3; /* kernel, initrd, appliance */ + else if (strcmp (format, "checksum") == 0) + nr_outputs = 0; /* (none) */ + else + abort (); + + /* [optind .. optind+nr_inputs-1] hostcpu [argc-nr_outputs-1 .. argc-1] + * <---- nr_inputs ----> 1 <---- nr_outputs ----> + */ + nr_inputs = argc - nr_outputs - 1 - optind; + char **outputs = &argv[optind+nr_inputs+1]; + /*assert (outputs [nr_outputs] == NULL); + assert (inputs [nr_inputs + 1 + nr_outputs] == NULL);*/ + + if (nr_outputs > 0) + kernel = outputs[0]; + if (nr_outputs > 1) + initrd = outputs[1]; + if (nr_outputs > 2) + appliance = outputs[2]; + } + /* New-style? Check all outputs were defined. */ + else { + if (needs_kernel && !kernel) { + if (!output_dir) { + no_output_dir: + fprintf (stderr, "%s: use -o to specify output directory or --output-[kernel|dtb|initrd|appliance]\n", argv[0]); + exit (EXIT_FAILURE); + } + if (asprintf (&kernel, "%s/kernel", output_dir) == -1) { + perror ("asprintf"); + exit (EXIT_FAILURE); + } + } + + if (needs_initrd && !initrd) { + if (!output_dir) + goto no_output_dir; + if (asprintf (&initrd, "%s/initrd", output_dir) == -1) { + perror ("asprintf"); + exit (EXIT_FAILURE); + } + } + + if (needs_appliance && !appliance) { + if (!output_dir) + goto no_output_dir; + if (asprintf (&appliance, "%s/appliance", output_dir) == -1) { + perror ("asprintf"); + exit (EXIT_FAILURE); + } + } + + nr_inputs = argc - optind; + } if (nr_inputs < 1) { fprintf (stderr, "%s: not enough files specified on the command line\n", @@ -293,18 +384,6 @@ main (int argc, char *argv[]) exit (EXIT_FAILURE); } - /* See: https://bugzilla.redhat.com/show_bug.cgi?id=558593 */ - const char *hostcpu = outputs[-1]; - - /* Output files. */ - const char *kernel = NULL, *initrd = NULL, *appliance = NULL; - if (nr_outputs > 0) - kernel = outputs[0]; - if (nr_outputs > 1) - initrd = appliance = outputs[1]; - if (nr_outputs > 2) - appliance = outputs[2]; - if (verbose) { print_timestamped_message ("whitelist = %s, " "host_cpu = %s, " @@ -312,18 +391,59 @@ main (int argc, char *argv[]) "initrd = %s, " "appliance = %s", whitelist ? : "(not specified)", - hostcpu, kernel, initrd, appliance); + hostcpu, + kernel ? : "(none)", + initrd ? : "(none)", + appliance ? : "(none)"); int i; for (i = 0; i < nr_inputs; ++i) print_timestamped_message ("inputs[%d] = %s", i, inputs[i]); } + /* We need to set the real, not effective, uid here to work round a + * misfeature in bash. bash will automatically reset euid to uid when + * invoked. As shell is used in places by supermin-helper, this + * results in code running with varying privilege. */ + uid_t uid = getuid (); + gid_t gid = getgid (); + + if (uid != euid || gid != egid) { + if (uid != 0) { + fprintf (stderr, "The -u and -g options require root privileges.\n"); + usage (stderr, argv[0]); + exit (EXIT_FAILURE); + } + + /* Need to become root first because setgid and setuid require it */ + if (seteuid (0) == -1) { + perror ("seteuid"); + exit (EXIT_FAILURE); + } + + /* Set gid and uid to command-line parameters */ + if (setgid (egid) == -1) { + perror ("setgid"); + exit (EXIT_FAILURE); + } + + /* Kill supplemental groups from parent process (RHBZ#902476). */ + if (setgroups (1, &egid) == -1) { + perror ("setgroups"); + exit (EXIT_FAILURE); + } + + if (setuid (euid) == -1) { + perror ("setuid"); + exit (EXIT_FAILURE); + } + } + /* Remove the output files if they exist. */ if (kernel) unlink (kernel); if (initrd) unlink (initrd); - if (appliance && initrd != appliance) + if (appliance) unlink (appliance); /* Create kernel output file. */ diff --git a/helper/supermin-helper.pod b/helper/supermin-helper.pod index 6b2fd2c..ee2ec22 100644 --- a/helper/supermin-helper.pod +++ b/helper/supermin-helper.pod @@ -4,8 +4,24 @@ supermin-helper - Reconstruct initramfs from supermin appliance. =head1 SYNOPSIS - supermin-helper supermin.img hostfiles.txt host_cpu kernel initrd - supermin-helper input [...] host_cpu kernel initrd +New style (since supermin 4.1.5): + + supermin-helper [-f cpio|ext2] -o outputdir input [input...] + +or: + + supermin-helper [-f cpio|ext2] --output-kernel kernel \ + [--output-dtb dtb] --output-initrd initrd \ + [--output-appliance appliance] input [input...] + +or: + + supermin-helper -f checksum input [input ...] + +Old style (still supported in this version but deprecated): + + supermin-helper [-f cpio] supermin.img hostfiles.txt host_cpu kernel initrd + supermin-helper [-f cpio] input [...] host_cpu kernel initrd supermin-helper -f ext2 input [...] host_cpu kernel initrd appliance @@ -18,21 +34,24 @@ supermin appliance. First you should be familiar with L<supermin(8)>. =head1 PARAMETERS -Of the required parameters, the first few are I<input> files, and the -last two or three are I<output> files. +Specify the I<input> file(s), and I<-o> or I<--output-*> flags +indicating where you want the appliance to be written. + +Use the I<-f> option to select what type of appliance you want. C<supermin.img> and C<hostfiles.txt> are the input files which describe the supermin appliance. (You can also use a directory name here which is searched for files). -C<host_cpu> should be the host CPU, eg. C<x86_64> or C<i686>. +To write the appliance to a directory, use I<-o outputdir>. The +directory should already exist. Files called C<outputdir/kernel>, +C<outputdir/dtb>, C<outputdir/initrd> and/or C<outputdir/appliance> +will be written. (Not all files are written, it depends on what kind +of appliance you asked for and what architecture you are running on) -C<kernel>, C<initrd> and C<appliance> are the temporary output files -that this script produces. These output files are meant to be used -just for booting the appliance, and should be deleted straight -afterwards. The extra C<appliance> parameter is only required when -the format is C<ext2>. None of these parameters are needed for -the checksum output C<-f checksum>. +To write files with specific names instead, use the +I<--output-kernel>, I<--output-dtb>, I<--output-initrd> and/or +I<--output-appliance> options. =head1 OPTIONS @@ -42,6 +61,46 @@ the checksum output C<-f checksum>. Display brief command line usage, and exit. +=item B<--copy-kernel> + +Copy the kernel (and device tree, if created) instead of symlinking to +the kernel in C</boot>. + +This is fractionally slower, but is necessary if you want to change +the permissions or SELinux label on the kernel or device tree. + +=item B<--dtb wildcard> + +If specified, search for a device tree which is compatible with the +selected kernel and the name of which matches the given wildcard. You +can use a wildcard such as C<vexpress-*a9*.dtb> which would match +C<vexpress-v2p-ca9.dtb>. + +Notes: + +=over 4 + +=item * + +You may need to quote the wildcard to prevent it from being expanded +by your shell. + +=item * + +If no I<--dtb> option is given, no device tree will be looked for. + +=item * + +You only need a device tree on architectures such as ARM and PowerPC +which use them. On other architectures, don't use this option. + +=item * + +If you use this option and no compatible device tree can be found, +supermin-helper will exit with an error. + +=back + =item B<-f fmt> =item B<--format fmt> @@ -54,8 +113,8 @@ Select the output format for the appliance. Possible formats are: A Linux initramfs. This is the default. -In this case you have to supply names for the C<kernel> -and C<initrd>, where the C<initrd> is the appliance. +In this case you have to supply output names for the C<kernel> and +C<initrd>. The C<initrd> is the appliance. Note that L<cpio(1)> might not be able to extract this file fully. The format used by the Linux kernel is not quite a true cpio file. @@ -64,8 +123,8 @@ The format used by the Linux kernel is not quite a true cpio file. An ext2 filesystem. -In this case you have to supply names for the C<kernel>, a small -C<initrd> which is used just to locate the appliance, and the +In this case you have to supply output names for the C<kernel>, a +small C<initrd> which is used just to locate the appliance, and the C<appliance> (the ext2 filesystem). =item checksum @@ -83,12 +142,12 @@ host_cpu and the UID of the current user are included in the checksum. =back -=item B<--copy-kernel> +=item B<--host-cpu cpu> -Copy the kernel instead of symlinking to the kernel in C</boot>. - -This is fractionally slower, but is necessary if you want to change -the permissions or SELinux label on the kernel. +Specify the host CPU (eg. C<i686>, C<x86_64>). This is used as a +substring match when searching for compatible kernels. If not +specified, it defaults to the host CPU that supermin-helper was +compiled on. =item B<-k file> @@ -110,6 +169,65 @@ If this option is not specified, then every kernel module from the host will be included. This is safer, but can produce rather large appliances which need a lot more memory to boot. +=item B<-o outputdir> + +Write the appliance to the named directory. Two or more of the +following files will be created (the exact files created depends on +the type of appliance you asked for and the architecture): + +=over 4 + +=item C<outputdir/kernel> + +(ie. A file literally called C<kernel> in the directory I<outputdir> +that you specified). This is usually a symlink to the kernel, unless +you gave the I<--copy-kernel> option. + +=item C<outputdir/dtb> + +The device tree. See also the I<--dtb> option. + +This is only created on architectures that use device trees, eg. ARM. + +This is usually a symlink to the device tree binary file, unless you +gave the I<--copy-kernel> option. + +=item C<outputdir/initrd> + +The initrd. For I<-f cpio> this also contains the full appliance. +For I<-f ext2> this is just a small initrd which is sufficient to find +and mount the appliance disk. + +=item C<outputdir/appliance> + +The appliance disk (only for I<-f ext2>). + +=back + +=item B<--output-kernel kernel> + +Instead of using the literal hard-coded name C<kernel>, write the +kernel to the named path. +This overrides the I<-o outputdir> option (if present). + +=item B<--output-dtb dtb> + +Instead of using the literal hard-coded name C<dtb>, write the +device tree to the named path. +This overrides the I<-o outputdir> option (if present). + +=item B<--output-initrd initrd> + +Instead of using the literal hard-coded name C<initrd>, write the +initrd to the named path. +This overrides the I<-o outputdir> option (if present). + +=item B<--output-initrd appliance> + +Instead of using the literal hard-coded name C<appliance>, write the +initrd to the named path. +This overrides the I<-o outputdir> option (if present). + =item B<-u user> =item B<--user user> @@ -199,6 +317,10 @@ eg. C</lib/modules/3.0.x86_64/> This has no effect if C<SUPERMIN_KERNEL> is not set. +=item SUPERMIN_DTB + +Force the given device tree file to be used. + =back =head1 SEE ALSO diff --git a/tests/test-build-bash.sh b/tests/test-build-bash.sh index a7cd57e..c9316fa 100755 --- a/tests/test-build-bash.sh +++ b/tests/test-build-bash.sh @@ -24,12 +24,23 @@ d=test-build-bash.d rm -rf $d mkdir -p $d +set -x + # We assume 'bash' is a package everywhere. ../src/supermin -v --names bash -o $d arch="$(uname -m)" -# Check all supermin-helper formats work. +# Check all supermin-helper formats work (new-style). +../helper/supermin-helper -v -f checksum --host-cpu $arch $d +../helper/supermin-helper -v -f cpio --host-cpu $arch $d \ + --output-kernel test-build-bash.kernel --output-initrd test-build-bash.initrd +../helper/supermin-helper -v -f ext2 --host-cpu $arch $d \ + --output-kernel test-build-bash.kernel \ + --output-initrd test-build-bash.initrd \ + --output-appliance test-build-bash.root + +# Check all supermin-helper formats work (old-style). ../helper/supermin-helper -v -f checksum $d $arch ../helper/supermin-helper -v -f cpio $d $arch \ test-build-bash.kernel test-build-bash.initrd -- 1.8.3.1