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_transla... [2] http://git.annexia.org/?p=libguestfs.git;a=blob;f=daemon/mount.c;h=be289dade... [3] http://git.annexia.org/?p=libguestfs.git;a=blob;f=daemon/mount.c;h=be289dade...

-- 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@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@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 :|