Richard W.M. Jones
2013-Sep-06 15:45 UTC
[Libguestfs] [PATCH supermin 0/2] helper: Implement device trees.
This two-part patch for supermin implements device trees (for ARM). The first patch introduces a more rational way to handle command line arguments in 'supermin-helper'. See the commit message for details. The old style is still supported for compatibility. The second patch adds an extra supermin-helper --dtb parameter specifying a wildcard. A device tree file which matches the selected kernel and this wildcard is linked to (or copied if --copy-kernel). The intended use is from libguestfs (on ARM) which can now do: supermin-helper -f ext2 --dtb 'vexpress-*a9.dtb' [etc] which will create a 'dtb' file alongside kernel, initrd, etc in the appliance directory. Qemu is invoked using: qemu-system-arm -kernel kernel -dtb dtb [...] (or the equivalent via libvirt). Rich.
Richard W.M. Jones
2013-Sep-06 15:45 UTC
[Libguestfs] [PATCH supermin 1/2] helper: Define new-style command line syntax.
From: "Richard W.M. Jones" <rjones@redhat.com> 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
Richard W.M. Jones
2013-Sep-06 15:45 UTC
[Libguestfs] [PATCH supermin 2/2] helper: Implement --dtb / device tree search.
From: "Richard W.M. Jones" <rjones@redhat.com> For architectures such as ARM which require a device tree, this implements a supermin-helper --dtb option which can be used to search for a device tree file matching a wildcard. Usage example: supermin-helper -f ext2 --dtb 'vexpress-*a9' supermin-d -o /tmp would find a suitable dtb and symlink it as /tmp/dtb (or exit with an error is none could be found). --- helper/helper.h | 2 +- helper/kernel.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++------- helper/main.c | 33 +++++++++++----- 3 files changed, 126 insertions(+), 24 deletions(-) diff --git a/helper/helper.h b/helper/helper.h index 281cec9..94ad9e1 100644 --- a/helper/helper.h +++ b/helper/helper.h @@ -65,7 +65,7 @@ extern struct writer cpio_writer; extern struct writer ext2_writer; /* kernel.c */ -extern const char *create_kernel (const char *hostcpu, const char *kernel); +extern const char *create_kernel (const char *hostcpu, const char *kernel, const char *dtb_wildcard, const char *dtb); /* utils.c */ extern void print_timestamped_message (const char *fs, ...); diff --git a/helper/kernel.c b/helper/kernel.c index 7957b25..4ff207b 100644 --- a/helper/kernel.c +++ b/helper/kernel.c @@ -26,6 +26,7 @@ #include <unistd.h> #include <errno.h> #include <sys/utsname.h> +#include <assert.h> #include "error.h" #include "xvasprintf.h" @@ -44,8 +45,8 @@ #define MODULESDIR "/lib/modules" static char* get_kernel_version (char* filename); -static const char *create_kernel_from_env (const char *hostcpu, const char *kernel, const char *kernel_env, const char *modpath_env); -static void copy_or_symlink_kernel (const char *from, const char *to); +static const char *create_kernel_from_env (const char *hostcpu, const char *kernel, const char *kernel_env, const char *modpath_env, const char *dtb_wildcard, const char *dtb); +static void copy_or_symlink_file (const char *what, const char *from, const char *to); static char * get_modpath (const char *kernel_name) @@ -109,8 +110,85 @@ has_modpath (const char *kernel_name) } } -/* Create the kernel. This chooses an appropriate kernel and makes a - * symlink to it (or copies it if --copy-kernel was passed). +/* Try to find a device tree file in the kernel's dtb directory which + * matches the 'dtb_wildcard' pattern. Copy the file (or symlink it) to + * 'dtb'. + * + * The dtb directory is formed by replacing "vmlinuz-" at the + * beginning of the 'kernel' filename with "dtb-". + * + * We can override the whole lot by setting $SUPERMIN_DTB. + */ +static void +get_dtb (const char *kernel, const char *dtb_wildcard, const char *dtb) +{ + char *dtb_env = getenv ("SUPERMIN_DTB"); + if (dtb_env) { + copy_or_symlink_file ("dtb", dtb_env, dtb); + return; + } + + if (!dtb_wildcard) + return; + + assert (dtb); /* command line arg parsing should ensure this */ + assert (kernel != NULL); + + /* Replace /vmlinuz- with /dtb- */ + if (strncmp (kernel, "vmlinuz-", 8) != 0) { + no_dtb_dir: + fprintf (stderr, + "supermin-helper: failed to find a dtb (device tree) directory.\n" + "I expected to take '%s'\n" + "and replace vmlinuz- with dtb- to form a directory.\n" + "You can set SUPERMIN_DTB to point to the dtb *file* that should\n" + "be used.\n", + kernel); + exit (EXIT_FAILURE); + } + + char *dtb_dir; + if (asprintf (&dtb_dir, KERNELDIR "/dtb-%s", &kernel[8]) == -1) { + perror ("asprintf"); + exit (EXIT_FAILURE); + } + + /* It should be a directory. */ + if (!isdir (dtb_dir)) + goto no_dtb_dir; + + if (verbose) + fprintf (stderr, "looking for dtb matching %s in %s\n", + dtb_wildcard, dtb_dir); + + /* Look for the wildcard match. + * XXX Should probably do some sorting here and/or worry if there + * are multiple matches. + */ + char **all_files = read_dir (dtb_dir); + char **candidates = filter_fnmatch (all_files, dtb_wildcard, FNM_NOESCAPE); + + if (candidates[0] == NULL) { + fprintf (stderr, + "supermin-helper: failed to find a matching device tree.\n" + "I looked for a file matching '%s' in directory '%s'\n" + "You can set SUPERMIN_DTB to point to the dtb file that should\n" + "be used.\n", + dtb_wildcard, dtb_dir); + exit (EXIT_FAILURE); + } + + if (verbose) + fprintf (stderr, "picked dtb %s\n", candidates[0]); + + char *tmp = xasprintf ("%s/%s", dtb_dir, candidates[0]); + copy_or_symlink_file ("dtb", tmp, dtb); + free (tmp); +} + +/* Create the kernel and device tree files. This chooses an + * appropriate kernel, and optionally a device tree, and makes a + * symlink to them (or copies them if --copy-kernel was passed). * * Look for the most recent kernel named vmlinuz-*.<arch>* which has a * corresponding directory in /lib/modules/. If the architecture is @@ -124,7 +202,8 @@ has_modpath (const char *kernel_name) * This function returns the module path (ie. /lib/modules/<version>). */ const char * -create_kernel (const char *hostcpu, const char *kernel) +create_kernel (const char *hostcpu, const char *kernel, + const char *dtb_wildcard, const char *dtb) { int is_x86; /* x86 but not x86-64 */ int is_arm; /* arm */ @@ -138,7 +217,8 @@ create_kernel (const char *hostcpu, const char *kernel) char *kernel_env = getenv ("SUPERMIN_KERNEL"); if (kernel_env) { char *modpath_env = getenv ("SUPERMIN_MODULES"); - return create_kernel_from_env (hostcpu, kernel, kernel_env, modpath_env); + return create_kernel_from_env (hostcpu, kernel, kernel_env, modpath_env, + dtb_wildcard, dtb); } /* In original: ls -1dvr /boot/vmlinuz-*.$arch* 2>/dev/null | grep -v xen */ @@ -176,15 +256,18 @@ create_kernel (const char *hostcpu, const char *kernel) sort (candidates, reverse_filevercmp); if (verbose) - fprintf (stderr, "picked %s\n", candidates[0]); + fprintf (stderr, "picked kernel %s\n", candidates[0]); if (kernel) { /* Choose the first candidate. */ char *tmp = xasprintf (KERNELDIR "/%s", candidates[0]); - copy_or_symlink_kernel (tmp, kernel); + copy_or_symlink_file ("kernel", tmp, kernel); free (tmp); } + /* Device tree. */ + get_dtb (candidates[0], dtb_wildcard, dtb); + return get_modpath (candidates[0]); /* Print more diagnostics here than the old script did. */ @@ -205,7 +288,8 @@ create_kernel (const char *hostcpu, const char *kernel) */ static const char * create_kernel_from_env (const char *hostcpu, const char *kernel, - const char *kernel_env, const char *modpath_env) + const char *kernel_env, const char *modpath_env, + const char *dtb_wildcard, const char *dtb) { if (verbose) { fprintf (stderr, @@ -250,25 +334,28 @@ create_kernel_from_env (const char *hostcpu, const char *kernel, /* Create the symlink. */ if (kernel) - copy_or_symlink_kernel (kernel_env, kernel); + copy_or_symlink_file ("kernel", kernel_env, kernel); + + /* Device tree. */ + get_dtb (kernel, dtb_wildcard, dtb); return modpath_env; } static void -copy_or_symlink_kernel (const char *from, const char *to) +copy_or_symlink_file (const char *what, const char *from, const char *to) { int fd1, fd2; char buf[BUFSIZ]; ssize_t r; if (verbose >= 2) - fprintf (stderr, "%s kernel %s -> %s\n", - !copy_kernel ? "symlink" : "copy", from, to); + fprintf (stderr, "%s %s %s -> %s\n", + !copy_kernel ? "symlink" : "copy", what, from, to); if (!copy_kernel) { if (symlink (from, to) == -1) - error (EXIT_FAILURE, errno, "creating kernel symlink %s %s", from, to); + error (EXIT_FAILURE, errno, "creating %s symlink %s %s", what, from, to); } else { fd1 = open (from, O_RDONLY | O_CLOEXEC); diff --git a/helper/main.c b/helper/main.c index 5632b9f..a4c33dc 100644 --- a/helper/main.c +++ b/helper/main.c @@ -187,9 +187,12 @@ main (int argc, char *argv[]) const char *hostcpu = host_cpu; /* Output files. */ - char *kernel = NULL, *initrd = NULL, *appliance = NULL; + char *kernel = NULL, *dtb = NULL, *initrd = NULL, *appliance = NULL; const char *output_dir = NULL; + /* Device tree wildcard (--dtb argument). */ + const char *dtb_wildcard = NULL; + uid_t euid = geteuid (); gid_t egid = getegid (); @@ -211,8 +214,8 @@ main (int argc, char *argv[]) copy_kernel = 1; } 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); + dtb_wildcard = optarg; + old_style = false; /* --dtb + old-style wouldn't work anyway */ } else if (strcmp (long_options[option_index].name, "host-cpu") == 0) { hostcpu = optarg; @@ -223,13 +226,8 @@ main (int argc, char *argv[]) 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; @@ -287,6 +285,8 @@ main (int argc, char *argv[]) bool needs_initrd; bool needs_appliance; + bool needs_dtb = dtb_wildcard != NULL; + if (strcmp (format, "cpio") == 0) { writer = &cpio_writer; needs_kernel = true; @@ -357,6 +357,15 @@ main (int argc, char *argv[]) } } + if (needs_dtb && !dtb) { + if (!output_dir) + goto no_output_dir; + if (asprintf (&dtb, "%s/dtb", output_dir) == -1) { + perror ("asprintf"); + exit (EXIT_FAILURE); + } + } + if (needs_initrd && !initrd) { if (!output_dir) goto no_output_dir; @@ -387,12 +396,16 @@ main (int argc, char *argv[]) if (verbose) { print_timestamped_message ("whitelist = %s, " "host_cpu = %s, " + "dtb_wildcard = %s, " "kernel = %s, " + "dtb = %s, " "initrd = %s, " "appliance = %s", whitelist ? : "(not specified)", hostcpu, + dtb_wildcard ? : "(not specified)", kernel ? : "(none)", + dtb ? : "(none)", initrd ? : "(none)", appliance ? : "(none)"); int i; @@ -441,13 +454,15 @@ main (int argc, char *argv[]) /* Remove the output files if they exist. */ if (kernel) unlink (kernel); + if (dtb) + unlink (dtb); if (initrd) unlink (initrd); if (appliance) unlink (appliance); /* Create kernel output file. */ - const char *modpath = create_kernel (hostcpu, kernel); + const char *modpath = create_kernel (hostcpu, kernel, dtb_wildcard, dtb); if (verbose) print_timestamped_message ("finished creating kernel"); -- 1.8.3.1