Libguestfs - Jul 2010 - [PATCH 0/3] RFC: Allow use of external QEMU process with libguestfs

This attempts to implement the idea proposed in

  https://www.redhat.com/archives/libguestfs/2010-April/msg00087.html

The idea is that an externally managed QEMU (manual, or via libvirt)
can boot the appliance kernel/initrd. libguestfs can then be just told
of the UNIX domain socket associated with the guest daemon.

An example based on guestfish.

 1. Step one, find the appliance kernel/initrd (building the supermin
    appliance image if neccessary)

   ><fs> find-appliance 
   ><fs> get-kernel 
   /tmp/libguestfsqJB1iN/kernel
   ><fs> get-initrd 
   /tmp/libguestfsqJB1iN/initrd

 2. Boot a QEMU instance with this info

    /usr/libexec/qemu-kvm 
      -drive file=/dev/HostVG/f11i386,cache=off,if=virtio 
      -enable-kvm -nodefaults -nographic 
      -serial file:/tmp/guest/boot.log -monitor stdio 
      -m 500 -no-reboot 
      -chardev socket,id=guestfsvmc,path=/tmp/guest/sock,server,nowait 
      -net
user,vlan=0,net=169.254.0.0/16,guestfwd=tcp:169.254.2.4:6666-chardev:guestfsvmc
      -net nic,model=virtio,vlan=0 
      -append 'panic=1 console=ttyS0 udevtimeout=300 noapic acpi=off
printk.time=1 cgroup_disable=memory selinux=0
guestfs_vmchannel=tcp:169.254.2.4:6666  TERM=xterm'
      -kernel /tmp/libguestfsqJB1iN/kernel
      -initrd /tmp/libguestfsqJB1iN/initrd

 3. Tell guestfish to connect to this instance

   ><fs> launch-method attach
   ><fs> sockpath /tmp/guest/sock
   ><fs> launch


The temporary kernel/initrd from 'find-appliance' will be
automatically deleted when the guestfs handle is closed.

Instead of using 'find-appliance' an app can manually invoke
the febootstrap-supermin-helper to build the initrd/kernel
but is a fragile coupling to libguestfs internals. Thus it is
preferable to let libguestfs locate & inform you of the kernl
and initrd

The guestfs__launch code does many different things at once.
To enable code reuse in later patches, pull out some reusable
bits of functionality into static help methods

 - guestfs__connect_tcp() wait for a connection from a
   QEMU slirp client over a TCP socket

 - guestfs__connect_unix() connect to a UNIX domain socket
   server running in QEMU

 - guestfs__connect_handshake() perform the initial
   handshake with the guest daemon.
---
 src/guestfs.c |  240 +++++++++++++++++++++++++++++++++------------------------
 1 files changed, 138 insertions(+), 102 deletions(-)

diff --git a/src/guestfs.c b/src/guestfs.c
index 1439361..6e9947f 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -935,6 +935,141 @@ 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";
 
+/* Null vmchannel implementation: We listen on g->sock for a
+ * connection.  The connection could come from any local process
+ * so we must check it comes from the appliance (or at least
+ * from our UID) for security reasons.
+ */
+int
+guestfs__connect_tcp (guestfs_h *g, int null_vmchannel_sock)
+{
+  int sock = -1;
+  uid_t uid;
+
+  while (sock == -1) {
+    sock = accept_from_daemon (g);
+    if (sock == -1)
+      goto cleanup1;
+
+    if (check_peer_euid (g, sock, &uid) == -1)
+      goto cleanup1;
+    if (uid != geteuid ()) {
+      fprintf (stderr,
+	       "libguestfs: warning: unexpected connection from UID %d to port
%d\n",
+	       uid, null_vmchannel_sock);
+      close (sock);
+      sock = -1;
+      continue;
+    }
+  }
+
+  if (fcntl (sock, F_SETFL, O_NONBLOCK) == -1) {
+    perrorf (g, "fcntl");
+    goto cleanup1;
+  }
+
+  close (g->sock);
+  g->sock = sock;
+
+  g->state = LAUNCHING;
+  return 0;
+
+ cleanup1:
+  return -1;
+}
+
+
+/* Other vmchannel.  Open the Unix socket.
+ *
+ * The vmchannel implementation that got merged with qemu sucks in
+ * a number of ways.  Both ends do connect(2), which means that no
+ * one knows what, if anything, is connected to the other end, or
+ * if it becomes disconnected.  Even worse, we have to wait some
+ * indeterminate time for qemu to create the socket and connect to
+ * it (which happens very early in qemu's start-up), so any code
+ * that uses vmchannel is inherently racy.  Hence this silly loop.
+ */
+int
+guestfs__connect_unix(guestfs_h *g, const char *unixsock)
+{
+  struct sockaddr_un addr;
+
+  g->sock = socket (AF_UNIX, SOCK_STREAM, 0);
+  if (g->sock == -1) {
+    perrorf (g, "socket");
+    goto cleanup1;
+  }
+
+  if (fcntl (g->sock, F_SETFL, O_NONBLOCK) == -1) {
+    perrorf (g, "fcntl");
+    goto cleanup1;
+  }
+
+  memset(&addr, 0, sizeof(addr));
+  addr.sun_family = AF_UNIX;
+  strncpy (addr.sun_path, unixsock, UNIX_PATH_MAX);
+  addr.sun_path[UNIX_PATH_MAX-1] = '\0';
+
+  int tries = 100;
+  /* Always sleep at least once to give qemu a small chance to start up. */
+  usleep (10000);
+  while (tries > 0) {
+    int r = connect (g->sock, (struct sockaddr *) &addr, sizeof addr);
+    if ((r == -1 && errno == EINPROGRESS) || r == 0)
+      goto connected;
+
+    if (errno != ENOENT)
+      perrorf (g, "connect");
+    tries--;
+    usleep (100000);
+  }
+
+  error (g, _("failed to connect to vmchannel socket"));
+  goto cleanup1;
+
+ connected: ;
+  g->state = LAUNCHING;
+  return 0;
+
+ cleanup1:
+  return -1;
+}
+
+
+/* Wait for qemu to start and to connect back to us via vmchannel and
+ * send the GUESTFS_LAUNCH_FLAG message.
+ */
+int
+guestfs__connect_handshake(guestfs_h *g)
+{
+  uint32_t size;
+  void *buf = NULL;
+  int r = recv_from_daemon (g, &size, &buf);
+  free (buf);
+
+  if (r == -1) return -1;
+
+  if (size != GUESTFS_LAUNCH_FLAG) {
+    error (g, _("guestfs_launch failed, see earlier error
messages"));
+    return -1;
+  }
+
+  if (g->verbose)
+    print_timestamped_message (g, "appliance is up");
+
+  /* This is possible in some really strange situations, such as
+   * guestfsd starts up OK but then qemu immediately exits.  Check for
+   * it because the caller is probably expecting to be able to send
+   * commands after this function returns.
+   */
+  if (g->state != READY) {
+    error (g, _("qemu launched and contacted daemon, but state !=
READY"));
+    return -1;
+  }
+
+  return 0;
+}
+
 int
 guestfs__launch (guestfs_h *g)
 {
@@ -948,7 +1083,6 @@ guestfs__launch (guestfs_h *g)
   char *kernel = NULL, *initrd = NULL;
   int null_vmchannel_sock;
   char unixsock[256];
-  struct sockaddr_un addr;
 
   /* Configured? */
   if (!g->cmdline) {
@@ -1405,113 +1539,15 @@ guestfs__launch (guestfs_h *g)
   }
 
   if (null_vmchannel_sock) {
-    int sock = -1;
-    uid_t uid;
-
-    /* Null vmchannel implementation: We listen on g->sock for a
-     * connection.  The connection could come from any local process
-     * so we must check it comes from the appliance (or at least
-     * from our UID) for security reasons.
-     */
-    while (sock == -1) {
-      sock = accept_from_daemon (g);
-      if (sock == -1)
-        goto cleanup1;
-
-      if (check_peer_euid (g, sock, &uid) == -1)
-        goto cleanup1;
-      if (uid != geteuid ()) {
-        fprintf (stderr,
-                 "libguestfs: warning: unexpected connection from UID %d
to port %d\n",
-                 uid, null_vmchannel_sock);
-        close (sock);
-        sock = -1;
-        continue;
-      }
-    }
-
-    if (fcntl (sock, F_SETFL, O_NONBLOCK) == -1) {
-      perrorf (g, "fcntl");
+    if (guestfs__connect_tcp (g, null_vmchannel_sock) < 0)
       goto cleanup1;
-    }
-
-    close (g->sock);
-    g->sock = sock;
   } else {
-    /* Other vmchannel.  Open the Unix socket.
-     *
-     * The vmchannel implementation that got merged with qemu sucks in
-     * a number of ways.  Both ends do connect(2), which means that no
-     * one knows what, if anything, is connected to the other end, or
-     * if it becomes disconnected.  Even worse, we have to wait some
-     * indeterminate time for qemu to create the socket and connect to
-     * it (which happens very early in qemu's start-up), so any code
-     * that uses vmchannel is inherently racy.  Hence this silly loop.
-     */
-    g->sock = socket (AF_UNIX, SOCK_STREAM, 0);
-    if (g->sock == -1) {
-      perrorf (g, "socket");
+    if (guestfs__connect_unix (g, unixsock) < 0)
       goto cleanup1;
-    }
-
-    if (fcntl (g->sock, F_SETFL, O_NONBLOCK) == -1) {
-      perrorf (g, "fcntl");
-      goto cleanup1;
-    }
-
-    addr.sun_family = AF_UNIX;
-    strncpy (addr.sun_path, unixsock, UNIX_PATH_MAX);
-    addr.sun_path[UNIX_PATH_MAX-1] = '\0';
-
-    tries = 100;
-    /* Always sleep at least once to give qemu a small chance to start up. */
-    usleep (10000);
-    while (tries > 0) {
-      r = connect (g->sock, (struct sockaddr *) &addr, sizeof addr);
-      if ((r == -1 && errno == EINPROGRESS) || r == 0)
-        goto connected;
-
-      if (errno != ENOENT)
-        perrorf (g, "connect");
-      tries--;
-      usleep (100000);
-    }
-
-    error (g, _("failed to connect to vmchannel socket"));
-    goto cleanup1;
-
-  connected: ;
   }
 
-  g->state = LAUNCHING;
-
-  /* Wait for qemu to start and to connect back to us via vmchannel and
-   * send the GUESTFS_LAUNCH_FLAG message.
-   */
-  uint32_t size;
-  void *buf = NULL;
-  r = recv_from_daemon (g, &size, &buf);
-  free (buf);
-
-  if (r == -1) return -1;
-
-  if (size != GUESTFS_LAUNCH_FLAG) {
-    error (g, _("guestfs_launch failed, see earlier error
messages"));
-    goto cleanup1;
-  }
-
-  if (g->verbose)
-    print_timestamped_message (g, "appliance is up");
-
-  /* This is possible in some really strange situations, such as
-   * guestfsd starts up OK but then qemu immediately exits.  Check for
-   * it because the caller is probably expecting to be able to send
-   * commands after this function returns.
-   */
-  if (g->state != READY) {
-    error (g, _("qemu launched and contacted daemon, but state !=
READY"));
+  if (guestfs__connect_handshake (g) < 0)
     goto cleanup1;
-  }
 
   return 0;
 
-- 
1.6.6.1

This introduces 2 new API calls to allow connecting to an
externally spawned QEMU + appliance. The default behaviour
remains unchanged, and can be altered by running

   guestfs_set_launch_method(g, "attach");
   guestfs_set_sockpath(g, "/path/to/unix/socket");

Or in guestfish

   launch-method attach
   sockpath /path/to/unix/socket

The unix socket must point to a QEMU chardev connected to
the guest daemon.  The 'sockpath' also has effect when using
the default 'spawn' method, preventing the use of the tmpdir
for the socket.
---
 src/generator.ml |   50 ++++++++++++++++++++++++
 src/guestfs.c    |  110 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 153 insertions(+), 7 deletions(-)

diff --git a/src/generator.ml b/src/generator.ml
index d640343..641973a 100755
--- a/src/generator.ml
+++ b/src/generator.ml
@@ -610,6 +610,56 @@ Return the current qemu binary.
 This is always non-NULL.  If it wasn't set already, then this will
 return the default qemu binary name.");
 
+  ("set_launch_method", (RErr, [OptString "methodvalue"]),
-1, [FishAlias "launch-method"],
+   [],
+   "set the appliance launch method",
+   "\
+Set the appliance launch method that we will use.
+
+Valid methods are
+
+ * spawn:  spawn a QEMU process for the appliance (default)
+ * attach: attach to existing QEMU over a UNIX socket
+
+You can also override this by setting the C<LIBGUESTFS_METHOD>
+environment variable.
+
+Setting C<method> to C<NULL> restores the default launch
method.");
+
+  ("get_launch_method", (RConstString "methodvalue", []),
-1, [],
+   [InitNone, Always, TestRun (
+      [["get_launch_method"]])],
+   "get the appliance launch method",
+   "\
+Return the appliance launch method.
+
+This is always non-NULL. If it wasn't set already, then this will
+return the default method.");
+
+  ("set_sockpath", (RErr, [OptString "sockpath"]), -1,
[FishAlias "sockpath"],
+   [],
+   "set the UNIX domain socket path",
+   "\
+Set the UNIX domain socket path that we will use.
+
+The default is a file in a randomly named temporary directory.
+
+You can also override this by setting the C<LIBGUESTFS_SOCKPATH>
+environment variable.
+
+Setting C<sockpath> to C<NULL> restores the default UNIX domain
+socket path.");
+
+  ("get_sockpath", (RConstString "sockpath", []), -1, [],
+   [InitNone, Always, TestRun (
+      [["get_sockpath"]])],
+   "get the UNIX domain socket path",
+   "\
+Return the current UNIX domain socket path.
+
+This is NULL if a random temporary directory is to be used,
+otherwise it returns the configured socket path.");
+
   ("set_path", (RErr, [OptString "searchpath"]), -1,
[FishAlias "path"],
    [],
    "set the search path",
diff --git a/src/guestfs.c b/src/guestfs.c
index 6e9947f..29636cc 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -133,6 +133,11 @@ static int qemu_supports (guestfs_h *g, const char
*option);
 #define GUESTFWD_ADDR "169.254.2.4"
 #define GUESTFWD_PORT "6666"
 
+
+#define GUESTFS_LAUNCH_METHOD_SPAWN "spawn"
+#define GUESTFS_LAUNCH_METHOD_ATTACH "attach"
+
+
 /* GuestFS handle and connection. */
 enum state { CONFIG, LAUNCHING, READY, BUSY, NO_HANDLE };
 
@@ -163,6 +168,8 @@ struct guestfs_h
   int direct;
   int recovery_proc;
 
+  char *method;                 /* Appliance launch method */
+  char *sockpath;               /* Path to UNIX socket */
   char *path;			/* Path to kernel, initrd. */
   char *qemu;			/* Qemu binary. */
   char *append;			/* Append to kernel command line. */
@@ -226,6 +233,14 @@ guestfs_create (void)
   g->path = str != NULL ? strdup (str) : strdup (GUESTFS_DEFAULT_PATH);
   if (!g->path) goto error;
 
+  str = getenv ("LIBGUESTFS_METHOD");
+  g->method = str != NULL ? strdup (str) : strdup
(GUESTFS_LAUNCH_METHOD_SPAWN);
+  if (!g->method) goto error;
+
+  str = getenv ("LIBGUESTFS_SOCKPATH");
+  g->sockpath = str != NULL ? strdup (str) : NULL;
+  if (str && !g->sockpath) goto error;
+
   str = getenv ("LIBGUESTFS_QEMU");
   g->qemu = str != NULL ? strdup (str) : strdup (QEMU);
   if (!g->qemu) goto error;
@@ -365,6 +380,8 @@ guestfs_close (guestfs_h *g)
   gl_lock_unlock (handles_lock);
 
   free (g->last_error);
+  free (g->method);
+  free (g->sockpath);
   free (g->path);
   free (g->qemu);
   free (g->append);
@@ -611,6 +628,38 @@ guestfs__get_path (guestfs_h *g)
 }
 
 int
+guestfs__set_launch_method (guestfs_h *g, const char *value)
+{
+  free (g->method);
+
+  g->method = value == NULL ?
+    safe_strdup (g, GUESTFS_LAUNCH_METHOD_SPAWN) :
+    safe_strdup (g, value);
+  return 0;
+}
+
+const char *
+guestfs__get_launch_method (guestfs_h *g)
+{
+  return g->method;
+}
+
+int
+guestfs__set_sockpath (guestfs_h *g, const char *sockpath)
+{
+  free (g->sockpath);
+
+  g->sockpath = sockpath == NULL ? NULL : safe_strdup (g, sockpath);
+  return 0;
+}
+
+const char *
+guestfs__get_sockpath (guestfs_h *g)
+{
+  return g->sockpath;
+}
+
+int
 guestfs__set_qemu (guestfs_h *g, const char *qemu)
 {
   free (g->qemu);
@@ -1071,7 +1120,7 @@ guestfs__connect_handshake(guestfs_h *g)
 }
 
 int
-guestfs__launch (guestfs_h *g)
+guestfs__launch_spawn (guestfs_h *g)
 {
   const char *tmpdir;
   char dir_template[PATH_MAX];
@@ -1090,11 +1139,6 @@ guestfs__launch (guestfs_h *g)
     return -1;
   }
 
-  if (g->state != CONFIG) {
-    error (g, _("the libguestfs handle has already been launched"));
-    return -1;
-  }
-
   /* Start the clock ... */
   gettimeofday (&g->launch_t, NULL);
 
@@ -1255,7 +1299,10 @@ guestfs__launch (guestfs_h *g)
     /* Using some vmchannel impl.  We need to create a local Unix
      * domain socket for qemu to use.
      */
-    snprintf (unixsock, sizeof unixsock, "%s/sock", g->tmpdir);
+    if (g->sockpath)
+      snprintf (unixsock, sizeof unixsock, "%s", g->sockpath);
+    else
+      snprintf (unixsock, sizeof unixsock, "%s/sock", g->tmpdir);
     unlink (unixsock);
     null_vmchannel_sock = 0;
   }
@@ -1577,6 +1624,50 @@ guestfs__launch (guestfs_h *g)
   return -1;
 }
 
+
+int
+guestfs__launch_attach (guestfs_h *g)
+{
+  if (!g->sockpath) {
+    error (g, _("no socket path specified with launch method
'%s'"), g->method);
+    return -1;
+  }
+
+  if (guestfs__connect_unix (g, g->sockpath) < 0)
+    goto cleanup0;
+
+  if (guestfs__connect_handshake (g) < 0)
+    goto cleanup0;
+
+  return 0;
+
+ cleanup0:
+  if (g->sock >= 0) {
+    close (g->sock);
+    g->sock = -1;
+  }
+  g->state = CONFIG;
+  return -1;
+}
+
+
+int
+guestfs__launch (guestfs_h *g)
+{
+  if (g->state != CONFIG) {
+    error (g, _("the libguestfs handle has already been launched"));
+    return -1;
+  }
+
+  if (strcmp(g->method, GUESTFS_LAUNCH_METHOD_ATTACH) == 0)
+    return guestfs__launch_attach (g);
+  else if (strcmp(g->method, GUESTFS_LAUNCH_METHOD_SPAWN) == 0)
+    return guestfs__launch_spawn (g);
+
+  error (g, "unknown launch method '%s'", g->method);
+  return -1;
+}
+
 /* This function is used to print the qemu command line before it gets
  * executed, when in verbose mode.
  */
@@ -1905,6 +1996,11 @@ guestfs__kill_subprocess (guestfs_h *g)
     return -1;
   }
 
+  if (strcmp(g->method, GUESTFS_LAUNCH_METHOD_SPAWN) != 0) {
+    error (g, _("cannot kill subprocess with launch method
'%s'"), g->method);
+    return -1;
+  }
+
   if (g->verbose)
     fprintf (stderr, "sending SIGTERM to process %d\n", g->pid);
 
-- 
1.6.6.1

To facilitate apps which want to run QEMU themselves, provide
new APIs for locating the appliance and querying the associated
kernel/initrd paths.

  ><fs> find-appliance
  ><fs> get-kernel
  /tmp/libguestfsJYEam9/kernel
  ><fs> get-initrd
  /tmp/libguestfsJYEam9/initrd

Now boot a QEMU pointing at these initrd/kernel images
and attach to it

  ><fs> launch-method attach
  ><fs> sockpath /tmp/guest/sock
  ><fs> launch

The method for locating the appliance may create temporary
files. These will be deleted when the guestfs handle is
closed.
---
 src/generator.ml |   29 +++++++++++
 src/guestfs.c    |  139 ++++++++++++++++++++++++++++++++++--------------------
 2 files changed, 117 insertions(+), 51 deletions(-)

diff --git a/src/generator.ml b/src/generator.ml
index 641973a..41f8014 100755
--- a/src/generator.ml
+++ b/src/generator.ml
@@ -458,6 +458,15 @@ using L<qemu(1)>.
 You should call this after configuring the handle
 (eg. adding drives) but before performing any actions.");
 
+  ("find_appliance", (RErr, []), -1, [FishAlias
"find_appliance"],
+   [],
+   "find the appliance kernel/initrd",
+   "\
+Find the guest daemon appliance kernel and initrd images.
+
+This prefers to build a super-min appliance, but falls
+back to a traditional appliance if the former was not found.");
+
   ("wait_ready", (RErr, []), -1, [NotInFish],
    [],
    "wait until the qemu subprocess launches (no op)",
@@ -660,6 +669,26 @@ Return the current UNIX domain socket path.
 This is NULL if a random temporary directory is to be used,
 otherwise it returns the configured socket path.");
 
+  ("get_kernel", (RConstString "kernel", []), -1, [],
+   [InitNone, Always, TestRun (
+      [["get_kernel"]])],
+   "get the appliance kernel path",
+   "\
+Return the appliance kernel path.
+
+This is NULL if the kernel has not yet been located,
+otherwise it returns the kernel path.");
+
+  ("get_initrd", (RConstString "initrd", []), -1, [],
+   [InitNone, Always, TestRun (
+      [["get_initrd"]])],
+   "get the appliance initrd path",
+   "\
+Return the current appliance initrd path.
+
+This is NULL if the initrd has not yet been located,
+otherwise it returns the initrd path.");
+
   ("set_path", (RErr, [OptString "searchpath"]), -1,
[FishAlias "path"],
    [],
    "set the search path",
diff --git a/src/guestfs.c b/src/guestfs.c
index 29636cc..850dc9a 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -170,7 +170,9 @@ struct guestfs_h
 
   char *method;                 /* Appliance launch method */
   char *sockpath;               /* Path to UNIX socket */
-  char *path;			/* Path to kernel, initrd. */
+  char *kernel;                 /* Path to kernel */
+  char *initrd;                 /* Path to initrd */
+  char *path;			/* Search path to locate kernel, initrd. */
   char *qemu;			/* Qemu binary. */
   char *append;			/* Append to kernel command line. */
 
@@ -383,6 +385,8 @@ guestfs_close (guestfs_h *g)
   free (g->method);
   free (g->sockpath);
   free (g->path);
+  free (g->initrd);
+  free (g->kernel);
   free (g->qemu);
   free (g->append);
   free (g->qemu_help);
@@ -659,6 +663,18 @@ guestfs__get_sockpath (guestfs_h *g)
   return g->sockpath;
 }
 
+const char *
+guestfs__get_kernel (guestfs_h *g)
+{
+  return g->kernel;
+}
+
+const char *
+guestfs__get_initrd (guestfs_h *g)
+{
+  return g->initrd;
+}
+
 int
 guestfs__set_qemu (guestfs_h *g, const char *qemu)
 {
@@ -977,7 +993,7 @@ dir_contains_files (const char *dir, ...)
 }
 
 static void print_timestamped_message (guestfs_h *g, const char *fs, ...);
-static int build_supermin_appliance (guestfs_h *g, const char *path, char
**kernel, char **initrd);
+static int build_supermin_appliance (guestfs_h *g, const char *path);
 static int is_openable (guestfs_h *g, const char *path, int flags);
 static void print_cmdline (guestfs_h *g);
 
@@ -1119,28 +1135,15 @@ guestfs__connect_handshake(guestfs_h *g)
   return 0;
 }
 
+
 int
-guestfs__launch_spawn (guestfs_h *g)
+guestfs__find_appliance (guestfs_h *g)
 {
-  const char *tmpdir;
-  char dir_template[PATH_MAX];
-  int r, pmore;
   size_t len;
-  int wfd[2], rfd[2];
-  int tries;
+  int r, pmore;
   char *path, *pelem, *pend;
-  char *kernel = NULL, *initrd = NULL;
-  int null_vmchannel_sock;
-  char unixsock[256];
-
-  /* Configured? */
-  if (!g->cmdline) {
-    error (g, _("you must call guestfs_add_drive before
guestfs_launch"));
-    return -1;
-  }
-
-  /* Start the clock ... */
-  gettimeofday (&g->launch_t, NULL);
+  const char *tmpdir;
+  char dir_template[PATH_MAX];
 
   /* Make the temporary directory. */
 #ifdef P_tmpdir
@@ -1149,6 +1152,11 @@ guestfs__launch_spawn (guestfs_h *g)
   tmpdir = "/tmp";
 #endif
 
+  if (g->tmpdir ||
+      g->kernel ||
+      g->initrd)
+    return 0;
+
   tmpdir = getenv ("TMPDIR") ? : tmpdir;
   snprintf (dir_template, sizeof dir_template, "%s/libguestfsXXXXXX",
tmpdir);
 
@@ -1156,7 +1164,7 @@ guestfs__launch_spawn (guestfs_h *g)
     g->tmpdir = safe_strdup (g, dir_template);
     if (mkdtemp (g->tmpdir) == NULL) {
       perrorf (g, _("%s: cannot create temporary directory"),
dir_template);
-      goto cleanup0;
+      return -1;
     }
   }
 
@@ -1179,7 +1187,7 @@ guestfs__launch_spawn (guestfs_h *g)
                  "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)
+        if (build_supermin_appliance (g, ".") == -1)
           return -1;
         break;
       }
@@ -1191,7 +1199,7 @@ guestfs__launch_spawn (guestfs_h *g)
 
       if (dir_contains_files (pelem,
                               "supermin.d",
"kmod.whitelist", NULL)) {
-        if (build_supermin_appliance (g, pelem, &kernel, &initrd) ==
-1)
+        if (build_supermin_appliance (g, pelem) == -1)
           return -1;
         break;
       }
@@ -1202,7 +1210,7 @@ guestfs__launch_spawn (guestfs_h *g)
 
   free (path);
 
-  if (kernel == NULL || initrd == NULL) {
+  if (g->kernel == NULL || g->initrd == NULL) {
     /* Search g->path for the kernel and initrd. */
     pelem = path = safe_strdup (g, g->path);
     do {
@@ -1217,8 +1225,8 @@ guestfs__launch_spawn (guestfs_h *g)
           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);
+          g->kernel = safe_strdup (g, kernel_name);
+          g->initrd = safe_strdup (g, initrd_name);
           break;
         }
       }
@@ -1228,10 +1236,10 @@ guestfs__launch_spawn (guestfs_h *g)
           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);
+          g->kernel = safe_malloc (g, len + strlen (kernel_name) + 2);
+          g->initrd = safe_malloc (g, len + strlen (initrd_name) + 2);
+          sprintf (g->kernel, "%s/%s", pelem, kernel_name);
+          sprintf (g->initrd, "%s/%s", pelem, initrd_name);
           break;
         }
       }
@@ -1242,12 +1250,36 @@ guestfs__launch_spawn (guestfs_h *g)
     free (path);
   }
 
-  if (kernel == NULL || initrd == NULL) {
+  if (g->kernel == NULL || g->initrd == NULL) {
     error (g, _("cannot find %s or %s on LIBGUESTFS_PATH (current path =
%s)"),
            kernel_name, initrd_name, g->path);
-    goto cleanup0;
+    return -1;
   }
 
+  return 0;
+}
+
+int
+guestfs__launch_spawn (guestfs_h *g)
+{
+  int r;
+  int wfd[2], rfd[2];
+  int tries;
+  int null_vmchannel_sock;
+  char unixsock[256];
+
+  /* Configured? */
+  if (!g->cmdline) {
+    error (g, _("you must call guestfs_add_drive before
guestfs_launch"));
+    return -1;
+  }
+
+  /* Start the clock ... */
+  gettimeofday (&g->launch_t, NULL);
+
+  if (guestfs__find_appliance (g) < 0)
+    return -1;
+
   if (g->verbose)
     print_timestamped_message (g, "begin testing qemu features");
 
@@ -1467,9 +1499,9 @@ guestfs__launch_spawn (guestfs_h *g)
               g->append ? g->append : "");
 
     add_cmdline (g, "-kernel");
-    add_cmdline (g, (char *) kernel);
+    add_cmdline (g, g->kernel);
     add_cmdline (g, "-initrd");
-    add_cmdline (g, (char *) initrd);
+    add_cmdline (g, g->initrd);
     add_cmdline (g, "-append");
     add_cmdline (g, buf);
 
@@ -1516,11 +1548,6 @@ guestfs__launch_spawn (guestfs_h *g)
   /* Parent (library). */
   g->pid = r;
 
-  free (kernel);
-  kernel = NULL;
-  free (initrd);
-  initrd = NULL;
-
   /* Fork the recovery process off which will kill qemu if the parent
    * process fails to do so (eg. if the parent segfaults).
    */
@@ -1619,8 +1646,6 @@ guestfs__launch_spawn (guestfs_h *g)
     g->sock = -1;
   }
   g->state = CONFIG;
-  free (kernel);
-  free (initrd);
   return -1;
 }
 
@@ -1702,20 +1727,27 @@ print_cmdline (guestfs_h *g)
  * 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)
+build_supermin_appliance (guestfs_h *g, const char *path)
 {
   char cmd[4096];
   int r, len;
+  bool tmpkernel = false;
+  bool tmpinitrd = false;
 
   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);
+  if (!g->kernel) {
+    g->kernel = safe_malloc (g, len + 8);
+    snprintf (g->kernel, len+8, "%s/kernel", g->tmpdir);
+    tmpkernel = true;
+  }
+  if (!g->initrd) {
+    g->initrd = safe_malloc (g, len + 8);
+    snprintf (g->initrd, len+8, "%s/initrd", g->tmpdir);
+    tmpinitrd = true;
+  }
 
   snprintf (cmd, sizeof cmd,
             "febootstrap-supermin-helper%s "
@@ -1726,16 +1758,21 @@ build_supermin_appliance (guestfs_h *g, const char
*path,
             g->verbose ? " --verbose" : "",
             path,
             path,
-            *kernel, *initrd);
+            g->kernel, g->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;
+    if (tmpkernel) {
+      free (g->kernel);
+      g->kernel = NULL;
+    }
+    if (tmpinitrd) {
+      free (g->initrd);
+      g->initrd = NULL;
+    }
     return -1;
   }
 
-- 
1.6.6.1

Add an API which allows querying of the core kernel arguments
required for the appliance kernel/initrd.

  ><fs> get-kernel-args
  panic=1 console=ttyS0 udevtimeout=300 noapic acpi=off printk.time=1
cgroup_disable=memory

This does not include the vmchannel configuration parameter,
nor optional args such as selinux, or verbosity setting.
---
 src/generator.ml |   10 ++++++++++
 src/guestfs.c    |   27 +++++++++++++++++----------
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/src/generator.ml b/src/generator.ml
index 41f8014..79619d7 100755
--- a/src/generator.ml
+++ b/src/generator.ml
@@ -689,6 +689,16 @@ Return the current appliance initrd path.
 This is NULL if the initrd has not yet been located,
 otherwise it returns the initrd path.");
 
+  ("get_kernel_args", (RConstString "append", []), -1, [],
+   [InitNone, Always, TestRun (
+      [["get_kernel_args"]])],
+   "get the appliance kernel arguments",
+   "\
+Return the basic appliance kernel arguments
+
+This returns the basic appliance kernel arguments
+without the guestfwd configuration parameter");
+
   ("set_path", (RErr, [OptString "searchpath"]), -1,
[FishAlias "path"],
    [],
    "set the search path",
diff --git a/src/guestfs.c b/src/guestfs.c
index 850dc9a..1463e1a 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -675,6 +675,21 @@ guestfs__get_initrd (guestfs_h *g)
   return g->initrd;
 }
 
+const char *
+guestfs__get_kernel_args (guestfs_h *g)
+{
+#define LINUX_CMDLINE							\
+    "panic=1 "         /* force kernel to panic if daemon exits */	\
+    "console=ttyS0 "   /* serial console */				\
+    "udevtimeout=300 " /* good for very slow systems (RHBZ#480319) */
\
+    "noapic "          /* workaround for RHBZ#502058 - ok if not SMP
*/ \
+    "acpi=off "        /* we don't need ACPI, turn it off */		\
+    "printk.time=1 "   /* display timestamp before kernel messages */
\
+    "cgroup_disable=memory " /* saves us about 5 MB of RAM */
+
+    return LINUX_CMDLINE;
+}
+
 int
 guestfs__set_qemu (guestfs_h *g, const char *qemu)
 {
@@ -1475,23 +1490,15 @@ guestfs__launch_spawn (guestfs_h *g)
     add_cmdline (g, "-net");
     add_cmdline (g, "nic,model=" NET_IF ",vlan=0");
 
-#define LINUX_CMDLINE							\
-    "panic=1 "         /* force kernel to panic if daemon exits */	\
-    "console=ttyS0 "   /* serial console */				\
-    "udevtimeout=300 " /* good for very slow systems (RHBZ#480319) */
\
-    "noapic "          /* workaround for RHBZ#502058 - ok if not SMP
*/ \
-    "acpi=off "        /* we don't need ACPI, turn it off */		\
-    "printk.time=1 "   /* display timestamp before kernel messages */
\
-    "cgroup_disable=memory " /* saves us about 5 MB of RAM */
-
     /* Linux kernel command line. */
     snprintf (buf, sizeof buf,
-              LINUX_CMDLINE
+	      "%s "             /* Basic command line */
               "%s "             /* (selinux) */
               "%s "             /* (vmchannel) */
               "%s "             /* (verbose) */
               "TERM=%s "        /* (TERM environment variable) */
               "%s",             /* (append) */
+	      guestfs__get_kernel_args (g),
               g->selinux ? "selinux=1 enforcing=0" :
"selinux=0",
               vmchannel ? vmchannel : "",
               g->verbose ? "guestfs_verbose=1" : "",
-- 
1.6.6.1

On Mon, Jul 05, 2010 at 12:25:58PM -0400, Daniel P. Berrange wrote:
[...]

My only quibble about the API would be:
>    ><fs> sockpath /tmp/guest/sock
Can we reuse the existing path variable for this?
>    ><fs> launch
> 
> The temporary kernel/initrd from 'find-appliance' will be
> automatically deleted when the guestfs handle is closed.
Internally I think launch should delete the kernel/initrd (this is a
change from current behaviour too).  Nothing needs these after launch,
not even external qemus, because qemu loads them into the appliance
RAM.  By deleting them early there is more chance that we don't leave
these tmp files lying around.
> Instead of using 'find-appliance' an app can manually invoke
> the febootstrap-supermin-helper to build the initrd/kernel
> but is a fragile coupling to libguestfs internals. Thus it is
> preferable to let libguestfs locate & inform you of the kernel
> and initrd
Agreed.

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