On Wed, Jun 22, 2011 at 02:06:27PM +0100, Richard W.M. Jones wrote:
 Second version.  I have tested these and they work.
 
 The earlier version of the patch which modified the mount-vfs call so
 that the Device parameter became a String would not have worked
 properly.  It would mean that device name translation[1] would not
 have been done on that parameter any longer.
 
 I also considered changing all the mount calls so that they take a
 string instead of a device name, and then doing device name
 translation manually (ie. calling RESOLVE_DEVICE conditionally, as is
 done in do_umount[2]).  This would also allow bind mounts, but would
 also lead to a very complex do_mount_vfs[3] implementation.
 
 So this implements a separate API: guestfs_mount_9p, specifically for
 this purpose of mounting 9p filesystems discovered through
 guestfs_list_9p.  The advantage of having an API is it's simple and
 specialized, and we can deprecate it later if we find a better way to
 do it.
 
 Please test it, and tell me if it works for you.
 
 Rich.
 
 [1] 
http://libguestfs.org/guestfs.3.html#algorithm_for_block_device_name_tran...
 [2]
http://git.annexia.org/?p=libguestfs.git;a=blob;f=daemon/mount.c;h=be289d...
 [3]
http://git.annexia.org/?p=libguestfs.git;a=blob;f=daemon/mount.c;h=be289d...
 
 -- 
 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  
 From 5f10c3350338bbca735a74db26f98da968957bd9 Mon Sep 17 00:00:00
2001
 From: "Richard W.M. Jones" <rjones(a)redhat.com>
 Date: Wed, 22 Jun 2011 10:13:23 +0100
 Subject: [PATCH 1/2] New API: list-9p lists 9p filesystem mount tags
  (RHBZ#714981).
 
 ---
  daemon/9p.c                    |  170 ++++++++++++++++++++++++++++++++++++++++
  daemon/Makefile.am             |    1 +
  generator/generator_actions.ml |    7 ++
  po/POTFILES.in                 |    1 +
  src/MAX_PROC_NR                |    2 +-
  5 files changed, 180 insertions(+), 1 deletions(-)
  create mode 100644 daemon/9p.c
 
 diff --git a/daemon/9p.c b/daemon/9p.c
 new file mode 100644
 index 0000000..bc95803
 --- /dev/null
 +++ b/daemon/9p.c
 @@ -0,0 +1,170 @@
 +/* libguestfs - the guestfsd daemon
 + * Copyright (C) 2011 Red Hat Inc.
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + *
 + * This program 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 General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program; if not, write to the Free Software
 + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 + */
 +
 +#include <config.h>
 +
 +#include <stdio.h>
 +#include <stdlib.h>
 +#include <string.h>
 +#include <unistd.h>
 +#include <limits.h>
 +#include <errno.h>
 +#include <sys/types.h>
 +#include <sys/stat.h>
 +#include <dirent.h>
 +#include <fcntl.h>
 +
 +#include "daemon.h"
 +#include "actions.h"
 +
 +#define BUS_PATH "/sys/bus/virtio/drivers/9pnet_virtio"
 +
 +static char *read_whole_file (const char *filename);
 +
 +/* 
https://bugzilla.redhat.com/show_bug.cgi?id=714981#c1 */
 +char **
 +do_list_9p (void)
 +{
 +  char **r = NULL;
 +  int size = 0, alloc = 0;
 +
 +  DIR *dir;
 +  int err = 0;
 +
 +  dir = opendir (BUS_PATH);
 +  if (!dir) {
 +    perror ("opendir: " BUS_PATH);
 +    if (errno != ENOENT)
 +      return NULL;
 +
 +    /* If this directory doesn't exist, it probably means that
 +     * the virtio driver isn't loaded.  Don't return an error
 +     * in this case, but return an empty list.
 +     */
 +    if (add_string (&r, &size, &alloc, NULL) == -1)
 +      return NULL;
 +
 +    return r;
 +  }
 +
 +  while (1) {
 +    errno = 0;
 +    struct dirent *d = readdir (dir);
 +    if (d == NULL) break;
 +
 +    if (STRPREFIX (d->d_name, "virtio")) {
 +      char mount_tag_path[256];
 +      snprintf (mount_tag_path, sizeof mount_tag_path,
 +                BUS_PATH "/%s/mount_tag", d->d_name);
 +
 +      /* A bit unclear, but it looks like the virtio transport allows
 +       * the mount tag length to be unlimited (or up to 65536 bytes).
 +       * See: linux/include/linux/virtio_9p.h
 +       */
 +      char *mount_tag = read_whole_file (mount_tag_path);
 +      if (mount_tag == 0)
 +        continue;
 +
 +      if (add_string (&r, &size, &alloc, mount_tag) == -1) {
 +        free (mount_tag);
 +        free_stringslen (r, size);
 +        closedir (dir);
 +        return NULL;
 +      }
 +
 +      free (mount_tag);
 +    }
 +  }
 +
 +  /* Check readdir didn't fail */
 +  if (errno != 0) {
 +    reply_with_perror ("readdir: /sys/block");
 +    free_stringslen (r, size);
 +    closedir (dir);
 +    return NULL;
 +  }
 +
 +  /* Close the directory handle */
 +  if (closedir (dir) == -1) {
 +    reply_with_perror ("closedir: /sys/block");
 +    free_stringslen (r, size);
 +    return NULL;
 +  }
 +
 +  /* Sort the tags.  Note that r might be NULL if there are no tags. */
 +  if (r != NULL)
 +    sort_strings (r, size);
 +
 +  /* NULL terminate the list */
 +  if (add_string (&r, &size, &alloc, NULL) == -1)
 +    return NULL;
 +
 +  return r;
 +}
 +
 +/* Read whole file into dynamically allocated array.  If there is an
 + * error, DON'T call reply_with_perror, just return NULL.  Returns a
 + * \0-terminated string.
 + */
 +static char *
 +read_whole_file (const char *filename)
 +{
 +  char *r = NULL;
 +  size_t alloc = 0, size = 0;
 +  int fd;
 +
 +  fd = open (filename, O_RDONLY);
 +  if (fd == -1) {
 +    perror (filename);
 +    return NULL;
 +  }
 +
 +  while (1) {
 +    alloc += 256;
 +    char *r2 = realloc (r, alloc);
 +    if (r2 == NULL) {
 +      perror ("realloc");
 +      free (r);
 +      return NULL;
 +    }
 +    r = r2;
 +
 +    /* The '- 1' in the size calculation ensures there is space below
 +     * to add \0 to the end of the input.
 +     */
 +    ssize_t n = read (fd, r + size, alloc - size - 1);
 +    if (n == -1) {
 +      perror (filename);
 +      free (r);
 +      return NULL;
 +    }
 +    if (n == 0)
 +      break;
 +    size += n;
 +  }
 +
 +  if (close (fd) == -1) {
 +    perror (filename);
 +    free (r);
 +    return NULL;
 +  }
 +
 +  r[size] = '\0';
 +
 +  return r;
 +}
 diff --git a/daemon/Makefile.am b/daemon/Makefile.am
 index 8fb070f..78049c9 100644
 --- a/daemon/Makefile.am
 +++ b/daemon/Makefile.am
 @@ -88,6 +88,7 @@ errnostring.h: $(libsrcdir)/errnostring.h
  
  noinst_PROGRAMS = guestfsd
  guestfsd_SOURCES = \
 +	9p.c \
  	actions.h \
  	available.c \
  	augeas.c \
 diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml
 index 74b6515..634ceeb 100644
 --- a/generator/generator_actions.ml
 +++ b/generator/generator_actions.ml
 @@ -5960,6 +5960,13 @@ This returns true iff the device exists and contains all zero
bytes.
  
  Note that for large devices this can take a long time to run.");
  
 +  ("list_9p", (RStringList "mounttags", [], []), 285, [],
 +   [],
 +   "list 9p filesystems",
 +   "\
 +List all 9p filesystems attached to the guest.  A list of
 +mount tags is returned.");
 +
  ]
  
  let all_functions = non_daemon_functions @ daemon_functions
 diff --git a/po/POTFILES.in b/po/POTFILES.in
 index 7c0df52..da47c91 100644
 --- a/po/POTFILES.in
 +++ b/po/POTFILES.in
 @@ -1,6 +1,7 @@
  cat/virt-cat.c
  cat/virt-filesystems.c
  cat/virt-ls.c
 +daemon/9p.c
  daemon/augeas.c
  daemon/available.c
  daemon/base64.c
 diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
 index c9716b7..6cf4452 100644
 --- a/src/MAX_PROC_NR
 +++ b/src/MAX_PROC_NR
 @@ -1 +1 @@
 -284
 +285
 -- 
 1.7.5.2
  
 
 From dd422b9797b2b73764bb04ec46aebbf7b08cb9d8 Mon Sep 17 00:00:00
2001
 From: "Richard W.M. Jones" <rjones(a)redhat.com>
 Date: Wed, 22 Jun 2011 13:55:14 +0100
 Subject: [PATCH 2/2] New API: mount-9p lets you mount 9p filesystems
  (RHBZ#714981).
 
 ---
  daemon/9p.c                    |   55 ++++++++++++++++++++++++++++++++++++++++
  generator/generator_actions.ml |   11 ++++++++
  src/MAX_PROC_NR                |    2 +-
  3 files changed, 67 insertions(+), 1 deletions(-)
 
 diff --git a/daemon/9p.c b/daemon/9p.c
 index bc95803..27b2230 100644
 --- a/daemon/9p.c
 +++ b/daemon/9p.c
 @@ -168,3 +168,58 @@ read_whole_file (const char *filename)
  
    return r;
  }
 +
 +int
 +do_mount_9p (const char *options, const char *mount_tag, const char *mountpoint)
 +{
 +  char *mp = NULL, *opts = NULL, *err = NULL;
 +  struct stat statbuf;
 +  int r = -1;
 +
 +  ABS_PATH (mountpoint, , return -1);
 +
 +  mp = sysroot_path (mountpoint);
 +  if (!mp) {
 +    reply_with_perror ("malloc");
 +    goto out;
 +  }
 +
 +  /* Check the mountpoint exists and is a directory. */
 +  if (stat (mp, &statbuf) == -1) {
 +    reply_with_perror ("%s", mountpoint);
 +    goto out;
 +  }
 +  if (!S_ISDIR (statbuf.st_mode)) {
 +    reply_with_perror ("%s: mount point is not a directory", mountpoint);
 +    goto out;
 +  }
 +
 +  /* Add trans=virtio to the options. */
 +  if (STREQ (options, "")) {
 +    opts = strdup ("trans=virtio");
 +    if (opts == NULL) {
 +      reply_with_perror ("strdup");
 +      goto out;
 +    }
 +  }
 +  else {
 +    if (asprintf (&opts, "trans=virtio,%s", options) == -1) {
 +      reply_with_perror ("asprintf");
 +      goto out;
 +    }
 +  }
 +
 +  r = command (NULL, &err,
 +               "mount", "-o", opts, "-t", "9p",
mount_tag, mp, NULL);
 +  if (r == -1) {
 +    reply_with_error ("%s on %s: %s", mount_tag, mountpoint, err);
 +    goto out;
 +  }
 +
 +  r = 0;
 + out:
 +  free (err);
 +  free (opts);
 +  free (mp);
 +  return r;
 +}
 diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml
 index 634ceeb..27a02d0 100644
 --- a/generator/generator_actions.ml
 +++ b/generator/generator_actions.ml
 @@ -5967,6 +5967,17 @@ Note that for large devices this can take a long time to
run.");
  List all 9p filesystems attached to the guest.  A list of
  mount tags is returned.");
  
 +  ("mount_9p", (RErr, [String "options"; String
"mounttag"; String "mountpoint"], []), 286, [],
 +   [],
 +   "mount 9p filesystem",
 +   "\
 +Mount the virtio-9p filesystem with the tag C<mounttag> on the
 +directory C<mountpoint>.
 +
 +If required, C<trans=virtio> will be automatically added to the options.
 +Any other options required can be passed in the C<options> parameter,
 +or this can be left as an empty string."); 
Is it possible to make 'options' the 3rd parameter, and optional,
so you don't need todo
<fs> mount-9p "" org.kernel.other /tmp/ 
Which is the common case.
Aside from that, the patch works fine
Daniel
-- 
|: 
http://berrange.com      -o-    
http://www.flickr.com/photos/dberrange/ :|
|: 
http://libvirt.org              -o-             
http://virt-manager.org :|
|: 
http://autobuild.org       -o-         
http://search.cpan.org/~danberr/ :|
|: 
http://entangle-photo.org       -o-       
http://live.gnome.org/gtk-vnc :|