Richard W.M. Jones
2010-Aug-23 15:06 UTC
[Libguestfs] [PATCH] Change to using ext2-based, cached supermin appliance.
This patch changes libguestfs to use the ext2-based appliance, cached if possible. I've tested this with the alpha version of febootstrap from here: http://koji.fedoraproject.org/koji/buildinfo?buildID=191338 and it at least passes the tests. There are some hacky bits right now: (1) We add the root -drive option last on the command line, then (in the initrd) reverse-search through the block devices, mounting the first one we find. Dan pointed out that we could name the drive using the qemu '-drive file=..,serial=LIBGUESTFSROOT' option, which would allow unambiguous identification. However there are other issues with this. (2) We take steps to ignore the new device from the point of view of the user, but I haven't rigorously gone through the whole API to check that the device is truly hidden everywhere. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v -------------- next part -------------->From 5c31f6126ba4ea3e9056c34c300f6f5e332ab997 Mon Sep 17 00:00:00 2001From: Richard Jones <rjones at redhat.com> Date: Mon, 23 Aug 2010 11:12:29 +0100 Subject: [PATCH] Change to using ext2-based, cached supermin appliance. This changes the method used to build the supermin appliance to use the new ext2-based appliance supported by latest febootstrap. The appliance can also be cached, so we avoid rebuilding it each time it is used. Mailing list discussion goes into the rationale and details: https://www.redhat.com/archives/libguestfs/2010-August/msg00028.html Requires febootstrap >= 2.8. --- README | 2 +- daemon/daemon.h | 4 + daemon/devsparts.c | 4 + daemon/guestfsd.c | 26 +++ po/POTFILES.in | 1 + src/Makefile.am | 1 + src/appliance.c | 465 ++++++++++++++++++++++++++++++++++++++++++++++++ src/guestfs-internal.h | 1 + src/guestfs.c | 6 - src/launch.c | 210 +++------------------- 10 files changed, 529 insertions(+), 191 deletions(-) create mode 100644 src/appliance.c diff --git a/README b/README index 15e6581..b31f9a1 100644 --- a/README +++ b/README @@ -40,7 +40,7 @@ Requirements - recent QEMU >= 0.10 with vmchannel support http://lists.gnu.org/archive/html/qemu-devel/2009-02/msg01042.html -- febootstrap >= 2.7 +- febootstrap >= 2.8 - fakeroot diff --git a/daemon/daemon.h b/daemon/daemon.h index 55f7b08..4c1b9b0 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -37,6 +37,8 @@ extern int sysroot_len; extern char *sysroot_path (const char *path); +extern int is_root_device (const char *device); + extern int xwrite (int sock, const void *buf, size_t len) __attribute__((__warn_unused_result__)); extern int xread (int sock, void *buf, size_t len) @@ -198,6 +200,8 @@ extern void reply (xdrproc_t xdrp, char *ret); reply_with_error ("%s: %s: expecting a device name", __func__, (path)); \ fail_stmt; \ } \ + if (is_root_device (path)) \ + reply_with_error ("%s: %s: device not found", __func__, path); \ if (device_name_translation ((path)) == -1) { \ int err = errno; \ int r = cancel_stmt; \ diff --git a/daemon/devsparts.c b/daemon/devsparts.c index 60e7aa8..95e4a68 100644 --- a/daemon/devsparts.c +++ b/daemon/devsparts.c @@ -60,6 +60,10 @@ foreach_block_device (block_dev_func_t func) char dev_path[256]; snprintf (dev_path, sizeof dev_path, "/dev/%s", d->d_name); + /* Ignore the root device. */ + if (is_root_device (dev_path)) + continue; + /* RHBZ#514505: Some versions of qemu <= 0.10 add a * CD-ROM device even though we didn't request it. Try to * detect this by seeing if the device contains media. diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c index 49aca08..4e29338 100644 --- a/daemon/guestfsd.c +++ b/daemon/guestfsd.c @@ -74,6 +74,12 @@ static char *read_cmdline (void); # define MAX(a,b) ((a)>(b)?(a):(b)) #endif +/* If root device is an ext2 filesystem, this is the major and minor. + * This is so we can ignore this device from the point of view of the + * user, eg. in guestfs_list_devices and many other places. + */ +static dev_t root_device = 0; + int verbose = 0; static int print_shell_quote (FILE *stream, const struct printf_info *info, const void *const *args); @@ -163,6 +169,10 @@ main (int argc, char *argv[]) #endif #endif + struct stat statbuf; + if (stat ("/", &statbuf) == 0) + root_device = statbuf.st_dev; + for (;;) { c = getopt_long (argc, argv, options, long_options, NULL); if (c == -1) break; @@ -449,6 +459,22 @@ read_cmdline (void) return r; } +/* Return true iff device is the root device (and therefore should be + * ignored from the point of view of user calls). + */ +int +is_root_device (const char *device) +{ + struct stat statbuf; + if (stat (device, &statbuf) == -1) { + perror (device); + return 0; + } + if (statbuf.st_rdev == root_device) + return 1; + return 0; +} + /* Turn "/path" into "/sysroot/path". * * Caller must check for NULL and call reply_with_perror ("malloc") diff --git a/po/POTFILES.in b/po/POTFILES.in index 843e8e0..e5fb857 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -103,6 +103,7 @@ regressions/test-lvm-mapping.pl regressions/test-noexec-stack.pl ruby/ext/guestfs/_guestfs.c src/actions.c +src/appliance.c src/bindtests.c src/guestfs.c src/inspect.c diff --git a/src/Makefile.am b/src/Makefile.am index cc01459..39fa230 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -125,6 +125,7 @@ libguestfs_la_SOURCES = \ guestfs_protocol.h \ gettext.h \ actions.c \ + appliance.c \ bindtests.c \ inspect.c \ launch.c \ diff --git a/src/appliance.c b/src/appliance.c new file mode 100644 index 0000000..3c3279b --- /dev/null +++ b/src/appliance.c @@ -0,0 +1,465 @@ +/* libguestfs + * Copyright (C) 2010 Red Hat Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <stdarg.h> +#include <unistd.h> +#include <string.h> +#include <fcntl.h> +#include <time.h> +#include <sys/stat.h> +#include <sys/select.h> +#include <utime.h> + +#ifdef HAVE_SYS_TYPES_H +#include <sys/types.h> +#endif + +#include "guestfs.h" +#include "guestfs-internal.h" +#include "guestfs-internal-actions.h" +#include "guestfs_protocol.h" + +static const char *kernel_name = "vmlinuz." REPO "." host_cpu; +static const char *initrd_name = "initramfs." REPO "." host_cpu ".img"; + +static int find_path (guestfs_h *g, int (*pred) (guestfs_h *g, const char *pelem, void *data), void *data, char **pelem); +static int dir_contains_file (const char *dir, const char *file); +static int dir_contains_files (const char *dir, ...); +static int contains_supermin_appliance (guestfs_h *g, const char *path, void *data); +static int contains_ordinary_appliance (guestfs_h *g, const char *path, void *data); +static char *calculate_supermin_checksum (guestfs_h *g, const char *supermin_path); +static int check_for_cached_appliance (guestfs_h *g, const char *supermin_path, const char *checksum, char **kernel, char **initrd, char **appliance); +static int build_supermin_appliance (guestfs_h *g, const char *supermin_path, const char *checksum, char **kernel, char **initrd, char **appliance); + +/* Locate or build the appliance. + * + * This function locates or builds the appliance as necessary, + * handling the supermin appliance, caching of supermin-built + * appliances, or using an ordinary appliance. + * + * The return value is 0 = good, -1 = error. Returned in '*kernel' + * will be the name of the kernel to use, '*initrd' the name of the + * initrd, '*appliance' the name of the ext2 root filesystem. + * '*appliance' can be NULL, meaning that we are using an ordinary + * (non-ext2) appliance. All three strings must be freed by the + * caller. However the referenced files themselves must not be + * deleted. + * + * The process is as follows: + * + * (1) Look for the first element of g->path which contains a + * supermin appliance skeleton. If no element has this, skip + * straight to step (5). + * (2) Calculate the checksum of this supermin appliance. + * (3) Check whether $TMPDIR/$checksum/ directory exists, contains + * a cached appliance, and passes basic security checks. If so, + * return this appliance. + * (4) Try to build the supermin appliance into $TMPDIR/$checksum/. + * If this is successful, return it. + * (5) Check each element of g->path, looking for an ordinary appliance. + * If one is found, return it. + */ +int +guestfs___build_appliance (guestfs_h *g, + char **kernel, char **initrd, char **appliance) +{ + int r; + + /* Step (1). */ + char *supermin_path; + r = find_path (g, contains_supermin_appliance, NULL, &supermin_path); + if (r == -1) + return -1; + + if (r == 1) { + /* Step (2): calculate checksum. */ + char *checksum = calculate_supermin_checksum (g, supermin_path); + if (checksum) { + /* Step (3): cached appliance exists? */ + r = check_for_cached_appliance (g, supermin_path, checksum, + kernel, initrd, appliance); + if (r != 0) { + free (supermin_path); + free (checksum); + return r == 1 ? 0 : -1; + } + + /* Step (4): build supermin appliance. */ + r = build_supermin_appliance (g, supermin_path, checksum, + kernel, initrd, appliance); + free (supermin_path); + free (checksum); + return r; + } + free (supermin_path); + } + + /* Step (5). */ + char *path; + r = find_path (g, contains_ordinary_appliance, NULL, &path); + if (r == -1) + return -1; + + if (r == 1) { + size_t len = strlen (path); + *kernel = safe_malloc (g, len + strlen (kernel_name) + 2); + *initrd = safe_malloc (g, len + strlen (initrd_name) + 2); + sprintf (*kernel, "%s/%s", path, kernel_name); + sprintf (*initrd, "%s/%s", path, initrd_name); + *appliance = NULL; + + free (path); + return 0; + } + + error (g, _("cannot find any suitable libguestfs supermin or ordinary appliance on LIBGUESTFS_PATH (search path: %s)"), + g->path); + return -1; +} + +static int +contains_supermin_appliance (guestfs_h *g, const char *path, void *data) +{ + return dir_contains_files (path, "supermin.d", "kmod.whitelist", NULL); +} + +static int +contains_ordinary_appliance (guestfs_h *g, const char *path, void *data) +{ + return dir_contains_files (path, kernel_name, initrd_name, NULL); +} + +/* supermin_path is a path which is known to contain a supermin + * appliance. Using febootstrap-supermin-helper -f checksum calculate + * the checksum so we can see if it is cached. + */ +static char * +calculate_supermin_checksum (guestfs_h *g, const char *supermin_path) +{ + size_t len = 2 * strlen (supermin_path) + 256; + char cmd[len]; + snprintf (cmd, len, + "febootstrap-supermin-helper%s " + "-f checksum " + "-k '%s/kmod.whitelist' " + "'%s/supermin.d' " + host_cpu, + g->verbose ? " --verbose" : "", + supermin_path, + supermin_path); + + if (g->verbose) + guestfs___print_timestamped_message (g, "%s", cmd); + + /* Errors here are non-fatal, so we don't need to call error(). */ + FILE *pp = popen (cmd, "r"); + if (pp == NULL) + return NULL; + + char checksum[256]; + if (fgets (checksum, sizeof checksum, pp) == NULL) { + pclose (pp); + return NULL; + } + + if (pclose (pp) == -1) { + perror ("pclose"); + return NULL; + } + + len = strlen (checksum); + + if (len < 16) { /* sanity check */ + fprintf (stderr, "libguestfs: internal error: febootstrap-supermin-helper -f checksum returned a short string\n"); + return NULL; + } + + if (len > 0 && checksum[len-1] == '\n') + checksum[--len] = '\0'; + + return safe_strndup (g, checksum, len); +} + +/* Check for cached appliance in $TMPDIR/$checksum. Check it exists + * and passes some basic security checks. + * + * Returns: + * 1 = exists, and passes + * 0 = does not exist + * -1 = error which should abort the whole launch process + */ +static int +security_check_cache_file (guestfs_h *g, const char *filename, + const struct stat *statbuf) +{ + uid_t uid = geteuid (); + + if (statbuf->st_uid != uid) { + error (g, ("libguestfs cached appliance %s is not owned by UID %d\n"), + filename, uid); + return -1; + } + + if ((statbuf->st_mode & 0022) != 0) { + error (g, ("libguestfs cached appliance %s is writable by group or other (mode %o)\n"), + filename, statbuf->st_mode); + return -1; + } + + return 0; +} + +static int +check_for_cached_appliance (guestfs_h *g, + const char *supermin_path, const char *checksum, + char **kernel, char **initrd, char **appliance) +{ + const char *tmpdir = guestfs___tmpdir (); + + size_t len = strlen (tmpdir) + strlen (checksum) + 2; + char cachedir[len]; + snprintf (cachedir, len, "%s/%s", tmpdir, checksum); + + /* Touch the directory to prevent it being deleting in a rare race + * between us doing the checks and a tmp cleaner running. Note this + * doesn't create the directory, and we ignore any error. + */ + (void) utime (cachedir, NULL); + + /* See if the cache directory exists and passes some simple checks + * to make sure it has not been tampered with. Note that geteuid() + * forms a part of the checksum. + */ + struct stat statbuf; + if (lstat (cachedir, &statbuf) == -1) + return 0; + + if (security_check_cache_file (g, cachedir, &statbuf) == -1) + return -1; + + int ret; + + *kernel = safe_malloc (g, len + 8 /* / + "kernel" + \0 */); + *initrd = safe_malloc (g, len + 8 /* / + "initrd" + \0 */); + *appliance = safe_malloc (g, len + 6 /* / + "root" + \0 */); + sprintf (*kernel, "%s/kernel", cachedir); + sprintf (*initrd, "%s/initrd", cachedir); + sprintf (*appliance, "%s/root", cachedir); + + /* Touch the files to prevent them being deleted, and to bring the + * cache up to date. Note this doesn't create the files. + */ + (void) utime (*kernel, NULL); + + /* NB. *kernel is a symlink, so we want to check the kernel, not the + * link (stat, not lstat). We don't do a security check on the + * kernel since it's always under /boot. + */ + if (stat (*kernel, &statbuf) == -1) { + ret = 0; + goto error; + } + + (void) utime (*initrd, NULL); + + if (lstat (*initrd, &statbuf) == -1) { + ret = 0; + goto error; + } + + if (security_check_cache_file (g, *initrd, &statbuf) == -1) { + ret = -1; + goto error; + } + + (void) utime (*appliance, NULL); + + if (lstat (*appliance, &statbuf) == -1) { + ret = 0; + goto error; + } + + if (security_check_cache_file (g, *appliance, &statbuf) == -1) { + ret = -1; + goto error; + } + + /* Exists! */ + return 1; + + error: + free (*kernel); + free (*initrd); + free (*appliance); + return ret; +} + +/* Build supermin appliance from supermin_path to $TMPDIR/$checksum. + * + * Returns: + * 0 = built + * -1 = error (aborts launch) + */ +static int +build_supermin_appliance (guestfs_h *g, + const char *supermin_path, const char *checksum, + char **kernel, char **initrd, char **appliance) +{ + if (g->verbose) + guestfs___print_timestamped_message (g, "begin building supermin appliance"); + + const char *tmpdir = guestfs___tmpdir (); + size_t cdlen = strlen (tmpdir) + strlen (checksum) + 2; + char cachedir[cdlen]; + snprintf (cachedir, cdlen, "%s/%s", tmpdir, checksum); + + /* Don't worry about this failing, because the command below will + * fail if the directory doesn't exist. Note the directory might + * already exist, eg. if a tmp cleaner has removed the existing + * appliance but not the directory itself. + */ + (void) mkdir (cachedir, 0755); + + /* Set a sensible umask in the subprocess, so kernel and initrd + * output files are world-readable (RHBZ#610880). + */ + size_t cmdlen = 2 * strlen (supermin_path) + 3 * (cdlen + 16) + 256; + char cmd[cmdlen]; + snprintf (cmd, cmdlen, + "umask 0022; " + "febootstrap-supermin-helper%s " + "-f ext2 " + "-k '%s/kmod.whitelist' " + "'%s/supermin.d' " + host_cpu " " + "%s/kernel %s/initrd %s/root", + g->verbose ? " --verbose" : "", + supermin_path, supermin_path, + cachedir, cachedir, cachedir); + if (g->verbose) + guestfs___print_timestamped_message (g, "%s", cmd); + int r = system (cmd); + if (r == -1 || WEXITSTATUS (r) != 0) { + error (g, _("external command failed: %s"), cmd); + return -1; + } + + if (g->verbose) + guestfs___print_timestamped_message (g, "finished building supermin appliance"); + + *kernel = safe_malloc (g, cdlen + 8 /* / + "kernel" + \0 */); + *initrd = safe_malloc (g, cdlen + 8 /* / + "initrd" + \0 */); + *appliance = safe_malloc (g, cdlen + 6 /* / + "root" + \0 */); + sprintf (*kernel, "%s/kernel", cachedir); + sprintf (*initrd, "%s/initrd", cachedir); + sprintf (*appliance, "%s/root", cachedir); + + return 0; +} + +/* Search elements of g->path, returning the first path element which + * matches the predicate function 'pred'. + * + * Function 'pred' must return a true or false value. If it returns + * -1 then the entire search is aborted. + * + * Return values: + * 1 = a path element matched, it is returned in *pelem_ret and must be + * freed by the caller, + * 0 = no path element matched, *pelem_ret is set to NULL, or + * -1 = error which aborts the launch process + */ +static int +find_path (guestfs_h *g, + int (*pred) (guestfs_h *g, const char *pelem, void *data), + void *data, + char **pelem_ret) +{ + size_t len; + int r; + const char *pelem = g->path; + + /* Note that if g->path is an empty string, we want to check the + * current directory (for backwards compatibility with + * libguestfs < 1.5.4). + */ + do { + len = strcspn (pelem, ":"); + + /* Empty element or "." means current directory. */ + if (len == 0) + *pelem_ret = safe_strdup (g, "."); + else + *pelem_ret = safe_strndup (g, pelem, len); + + r = pred (g, *pelem_ret, data); + if (r == -1) { + free (*pelem_ret); + return -1; + } + + if (r != 0) /* predicate matched */ + return 1; + + free (*pelem_ret); + + if (pelem[len] == ':') + pelem += len + 1; + else + pelem += len; + } while (*pelem); + + /* Predicate didn't match on any path element. */ + *pelem_ret = NULL; + return 0; +} + +/* Returns true iff file is contained in dir. */ +static int +dir_contains_file (const char *dir, const char *file) +{ + size_t dirlen = strlen (dir); + size_t filelen = strlen (file); + size_t len = dirlen + filelen + 2; + char path[len]; + + snprintf (path, len, "%s/%s", dir, file); + return access (path, F_OK) == 0; +} + +/* Returns true iff every listed file is contained in 'dir'. */ +static int +dir_contains_files (const char *dir, ...) +{ + va_list args; + const char *file; + + va_start (args, dir); + while ((file = va_arg (args, const char *)) != NULL) { + if (!dir_contains_file (dir, file)) { + va_end (args); + return 0; + } + } + va_end (args); + return 1; +} diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index 73a14ab..f2abeba 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -209,6 +209,7 @@ extern int guestfs___recv_file (guestfs_h *g, const char *filename); extern int guestfs___send_to_daemon (guestfs_h *g, const void *v_buf, size_t n); extern int guestfs___recv_from_daemon (guestfs_h *g, uint32_t *size_rtn, void **buf_rtn); extern int guestfs___accept_from_daemon (guestfs_h *g); +extern int guestfs___build_appliance (guestfs_h *g, char **kernel, char **initrd, char **appliance); #define error guestfs_error #define perrorf guestfs_perrorf diff --git a/src/guestfs.c b/src/guestfs.c index cef80db..74de38c 100644 --- a/src/guestfs.c +++ b/src/guestfs.c @@ -220,12 +220,6 @@ guestfs_close (guestfs_h *g) snprintf (filename, sizeof filename, "%s/sock", g->tmpdir); unlink (filename); - snprintf (filename, sizeof filename, "%s/initrd", g->tmpdir); - unlink (filename); - - snprintf (filename, sizeof filename, "%s/kernel", g->tmpdir); - unlink (filename); - rmdir (g->tmpdir); free (g->tmpdir); diff --git a/src/launch.c b/src/launch.c index 9deb0cf..f84a128 100644 --- a/src/launch.c +++ b/src/launch.c @@ -224,53 +224,15 @@ guestfs__add_cdrom (guestfs_h *g, const char *filename) return guestfs__config (g, "-cdrom", filename); } -/* Returns true iff file is contained in dir. */ -static int -dir_contains_file (const char *dir, const char *file) -{ - int dirlen = strlen (dir); - int filelen = strlen (file); - int len = dirlen+filelen+2; - char path[len]; - - snprintf (path, len, "%s/%s", dir, file); - return access (path, F_OK) == 0; -} - -/* Returns true iff every listed file is contained in 'dir'. */ -static int -dir_contains_files (const char *dir, ...) -{ - va_list args; - const char *file; - - va_start (args, dir); - while ((file = va_arg (args, const char *)) != NULL) { - if (!dir_contains_file (dir, file)) { - va_end (args); - return 0; - } - } - va_end (args); - return 1; -} - -static int build_supermin_appliance (guestfs_h *g, const char *path, char **kernel, char **initrd); static int is_openable (guestfs_h *g, const char *path, int flags); static void print_cmdline (guestfs_h *g); -static const char *kernel_name = "vmlinuz." REPO "." host_cpu; -static const char *initrd_name = "initramfs." REPO "." host_cpu ".img"; - int guestfs__launch (guestfs_h *g) { - int r, pmore; - size_t len; + int r; int wfd[2], rfd[2]; int tries; - char *path, *pelem, *pend; - char *kernel = NULL, *initrd = NULL; int null_vmchannel_sock; char unixsock[256]; struct sockaddr_un addr; @@ -302,101 +264,17 @@ guestfs__launch (guestfs_h *g) } } - /* Allow anyone to read the temporary directory. There are no - * secrets in the kernel or initrd files. The socket in this + /* Allow anyone to read the temporary directory. The socket in this * directory won't be readable but anyone can see it exists if they * want. (RHBZ#610880). */ if (chmod (g->tmpdir, 0755) == -1) fprintf (stderr, "chmod: %s: %m (ignored)\n", g->tmpdir); - /* First search g->path for the supermin appliance, and try to - * synthesize a kernel and initrd from that. If it fails, we - * try the path search again looking for a backup ordinary - * appliance. - */ - pelem = path = safe_strdup (g, g->path); - do { - pend = strchrnul (pelem, ':'); - pmore = *pend == ':'; - *pend = '\0'; - len = pend - pelem; - - /* Empty element of "." means cwd. */ - if (len == 0 || (len == 1 && *pelem == '.')) { - if (g->verbose) - fprintf (stderr, - "looking for supermin appliance in current directory\n"); - if (dir_contains_files (".", - "supermin.d", "kmod.whitelist", NULL)) { - if (build_supermin_appliance (g, ".", &kernel, &initrd) == -1) - return -1; - break; - } - } - /* Look at <path>/supermin* etc. */ - else { - if (g->verbose) - fprintf (stderr, "looking for supermin appliance in %s\n", pelem); - - if (dir_contains_files (pelem, - "supermin.d", "kmod.whitelist", NULL)) { - if (build_supermin_appliance (g, pelem, &kernel, &initrd) == -1) - return -1; - break; - } - } - - pelem = pend + 1; - } while (pmore); - - free (path); - - if (kernel == NULL || initrd == NULL) { - /* Search g->path for the kernel and initrd. */ - pelem = path = safe_strdup (g, g->path); - do { - pend = strchrnul (pelem, ':'); - pmore = *pend == ':'; - *pend = '\0'; - len = pend - pelem; - - /* Empty element or "." means cwd. */ - if (len == 0 || (len == 1 && *pelem == '.')) { - if (g->verbose) - fprintf (stderr, - "looking for appliance in current directory\n"); - if (dir_contains_files (".", kernel_name, initrd_name, NULL)) { - kernel = safe_strdup (g, kernel_name); - initrd = safe_strdup (g, initrd_name); - break; - } - } - /* Look at <path>/kernel etc. */ - else { - if (g->verbose) - fprintf (stderr, "looking for appliance in %s\n", pelem); - - if (dir_contains_files (pelem, kernel_name, initrd_name, NULL)) { - kernel = safe_malloc (g, len + strlen (kernel_name) + 2); - initrd = safe_malloc (g, len + strlen (initrd_name) + 2); - sprintf (kernel, "%s/%s", pelem, kernel_name); - sprintf (initrd, "%s/%s", pelem, initrd_name); - break; - } - } - - pelem = pend + 1; - } while (pmore); - - free (path); - } - - if (kernel == NULL || initrd == NULL) { - error (g, _("cannot find %s or %s on LIBGUESTFS_PATH (current path = %s)"), - kernel_name, initrd_name, g->path); - goto cleanup0; - } + /* Locate and/or build the appliance. */ + char *kernel = NULL, *initrd = NULL, *appliance = NULL; + if (guestfs___build_appliance (g, &kernel, &initrd, &appliance) == -1) + return -1; if (g->verbose) guestfs___print_timestamped_message (g, "begin testing qemu features"); @@ -614,12 +492,29 @@ guestfs__launch (guestfs_h *g) g->append ? g->append : ""); add_cmdline (g, "-kernel"); - add_cmdline (g, (char *) kernel); + add_cmdline (g, kernel); add_cmdline (g, "-initrd"); - add_cmdline (g, (char *) initrd); + add_cmdline (g, initrd); add_cmdline (g, "-append"); add_cmdline (g, buf); + /* Add the ext2 appliance drive (last of all). */ + if (appliance) { + const char *cachemode = ""; + if (qemu_supports (g, "cache=")) { + if (qemu_supports (g, "unsafe")) + cachemode = ",cache=unsafe"; + else if (qemu_supports (g, "writeback")) + cachemode = ",cache=writeback"; + } + + char buf2[PATH_MAX + 64]; + add_cmdline (g, "-drive"); + snprintf (buf2, sizeof buf2, "file=%s,snapshot=on,if=" DRIVE_IF "%s", + appliance, cachemode); + add_cmdline (g, buf2); + } + /* Finish off the command line. */ incr_cmdline_size (g); g->cmdline[g->cmdline_size-1] = NULL; @@ -866,6 +761,7 @@ guestfs__launch (guestfs_h *g) g->state = CONFIG; free (kernel); free (initrd); + free (appliance); return -1; } @@ -913,60 +809,6 @@ print_cmdline (guestfs_h *g) fputc ('\n', stderr); } -/* This function does the hard work of building the supermin appliance - * on the fly. 'path' is the directory containing the control files. - * 'kernel' and 'initrd' are where we will return the names of the - * kernel and initrd (only initrd is built). The work is done by - * an external script. We just tell it where to put the result. - */ -static int -build_supermin_appliance (guestfs_h *g, const char *path, - char **kernel, char **initrd) -{ - char cmd[4096]; - int r, len; - - if (g->verbose) - print_timestamped_message (g, "begin building supermin appliance"); - - len = strlen (g->tmpdir); - *kernel = safe_malloc (g, len + 8); - snprintf (*kernel, len+8, "%s/kernel", g->tmpdir); - *initrd = safe_malloc (g, len + 8); - snprintf (*initrd, len+8, "%s/initrd", g->tmpdir); - - /* Set a sensible umask in the subprocess, so kernel and initrd - * output files are world-readable (RHBZ#610880). - */ - snprintf (cmd, sizeof cmd, - "umask 0002; " - "febootstrap-supermin-helper%s " - "-k '%s/kmod.whitelist' " - "'%s/supermin.d' " - host_cpu " " - "%s %s", - g->verbose ? " --verbose" : "", - path, - path, - *kernel, *initrd); - if (g->verbose) - print_timestamped_message (g, "%s", cmd); - - r = system (cmd); - if (r == -1 || WEXITSTATUS(r) != 0) { - error (g, _("external command failed: %s"), cmd); - free (*kernel); - free (*initrd); - *kernel = *initrd = NULL; - return -1; - } - - if (g->verbose) - print_timestamped_message (g, "finished building supermin appliance"); - - return 0; -} - /* Compute Y - X and return the result in milliseconds. * Approximately the same as this code: * http://www.mpp.mpg.de/~huber/util/timevaldiff.c -- 1.7.1
Richard W.M. Jones
2010-Aug-23 17:04 UTC
[Libguestfs] [PATCH] Change to using ext2-based, cached supermin appliance.
I can report the good news that on Fedora 13 (ie. non-broken kernel) this patch and the updated febootstrap reduces the boot time below the magic number: real 0m4.954s user 0m2.117s sys 0m1.739s (Unfortunately the times are not as good with the broken kernel 2.6.35, because even with the much smaller -kernel and -initrd qemu-kvm takes many seconds to start up). Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
Possibly Parallel Threads
- Help with testing libguestfs 1.3.12 + new febootstrap
- OCaml 4.09.0 rebuild complete in Rawhide (was: Re: OCaml 4.09.0 will be added to Fedora 32 via a side tag)
- libvirtd for el7
- xen4centos kernel version / debuginfo
- "aug_init: Augeas initialization failed" on Fedora