Libguestfs - Nov 2009 - [PATCH 0/5] Four fixes for FUSE support and a test script

This set of patches implements a test script for FUSE, and fixes
some of the bugs that I found by doing that.

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

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
-------------- next part --------------
>From 93514b1fdb56a0cb6b85b79aff9f08fbd71a9560 Mon Sep 17 00:00:00 2001
From: Richard Jones <rjones at redhat.com>
Date: Tue, 17 Nov 2009 16:59:52 +0000
Subject: [PATCH 1/5] fuse: Fix symlink creation (RHBZ#538069).

The parameters were swapped, preventing symlinks from being created.
Furthermore, we need to invalidate the cache for both parameters.
---
 fuse/guestmount.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fuse/guestmount.c b/fuse/guestmount.c
index 739d8cb..8e081a6 100644
--- a/fuse/guestmount.c
+++ b/fuse/guestmount.c
@@ -425,9 +425,10 @@ fg_symlink (const char *from, const char *to)
 
   if (read_only) return -EROFS;
 
+  dir_cache_invalidate (from);
   dir_cache_invalidate (to);
 
-  r = guestfs_ln_s (g, to, from);
+  r = guestfs_ln_s (g, from, to);
   if (r == -1)
     return error ();
 
-- 
1.6.5.2

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
-------------- next part --------------
>From f5eec5299fc97de6bb094f23e36dace83fb46363 Mon Sep 17 00:00:00 2001
From: Richard Jones <rjones at redhat.com>
Date: Tue, 17 Nov 2009 17:02:15 +0000
Subject: [PATCH 2/5] fuse: Fix hard link creation.

The parameters were swapped.  We also need to invalidate the
cache for both parameters.
---
 fuse/guestmount.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fuse/guestmount.c b/fuse/guestmount.c
index 8e081a6..fe53822 100644
--- a/fuse/guestmount.c
+++ b/fuse/guestmount.c
@@ -462,9 +462,10 @@ fg_link (const char *from, const char *to)
 
   if (read_only) return -EROFS;
 
+  dir_cache_invalidate (from);
   dir_cache_invalidate (to);
 
-  r = guestfs_ln (g, to, from);
+  r = guestfs_ln (g, from, to);
   if (r == -1)
     return error ();
 
-- 
1.6.5.2

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/
-------------- next part --------------
>From e94b78db7739e693460e7acfe7c76d295ee17dce Mon Sep 17 00:00:00 2001
From: Richard Jones <rjones at redhat.com>
Date: Tue, 17 Nov 2009 17:01:19 +0000
Subject: [PATCH 3/5] fuse: Fix cache invalidation in rename operation.

We need to invalidate both parameters, otherwise the old (moved)
file can appear that it still exists after the move.
---
 fuse/guestmount.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fuse/guestmount.c b/fuse/guestmount.c
index fe53822..9d16cef 100644
--- a/fuse/guestmount.c
+++ b/fuse/guestmount.c
@@ -442,6 +442,7 @@ fg_rename (const char *from, const char *to)
 
   if (read_only) return -EROFS;
 
+  dir_cache_invalidate (from);
   dir_cache_invalidate (to);
 
   /* XXX It's not clear how close the 'mv' command is to the
-- 
1.6.5.2

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
-------------- next part --------------
>From 3c5d63be17d281634f036f53aa32103ad4ab1c41 Mon Sep 17 00:00:00 2001
From: Richard Jones <rjones at redhat.com>
Date: Tue, 17 Nov 2009 17:02:45 +0000
Subject: [PATCH 4/5] fuse: Fix read for empty files.

Error handling for the guestfs_pread call was incorrect, which
meant that empty files could produce spurious error messages.
---
 fuse/guestmount.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/fuse/guestmount.c b/fuse/guestmount.c
index 9d16cef..669bf80 100644
--- a/fuse/guestmount.c
+++ b/fuse/guestmount.c
@@ -621,8 +621,14 @@ fg_read (const char *path, char *buf, size_t size, off_t
offset,
   if (size > limit)
     size = limit;
 
+  /* Note the correct error handling here is tricky, because in the
+   * case where the call returns a zero-length buffer, it might return
+   * NULL.  However it won't adjust rsize along the error path, so we
+   * can set rsize to something beforehand and use that as a flag.
+   */
+  rsize = 1;
   r = guestfs_pread (g, path, size, offset, &rsize);
-  if (r == NULL)
+  if (rsize == 1 && r == NULL)
     return error ();
 
   /* This should never happen, but at least it stops us overflowing
-- 
1.6.5.2

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://et.redhat.com/~rjones/libguestfs/
See what it can do: http://et.redhat.com/~rjones/libguestfs/recipes.html
-------------- next part --------------
>From 8612ad24476eee32c811e889a319559c6b04a4d7 Mon Sep 17 00:00:00 2001
From: Richard Jones <rjones at redhat.com>
Date: Tue, 17 Nov 2009 17:03:31 +0000
Subject: [PATCH 5/5] fuse: Add tests for guestmount.

This script contains non-exhaustive tests for the system calls
implemented by guestmount.
---
 fuse/Makefile.am  |    8 +++
 fuse/test-fuse.sh |  165 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 173 insertions(+), 0 deletions(-)
 create mode 100755 fuse/test-fuse.sh

diff --git a/fuse/Makefile.am b/fuse/Makefile.am
index c041058..ebf948a 100644
--- a/fuse/Makefile.am
+++ b/fuse/Makefile.am
@@ -39,6 +39,8 @@ guestmount_LDADD = \
 	$(top_builddir)/src/libguestfs.la \
 	../gnulib/lib/libgnu.la
 
+# Documentation.
+
 man_MANS = guestmount.1
 
 guestmount.1: guestmount.pod
@@ -60,4 +62,10 @@ $(top_builddir)/html/guestmount.1.html: guestmount.pod
 	  --outfile html/guestmount.1.html \
 	  fuse/guestmount.pod
 
+# Tests.
+
+TESTS = test-fuse.sh
+TESTS_ENVIRONMENT = \
+	top_builddir=..
+
 endif
diff --git a/fuse/test-fuse.sh b/fuse/test-fuse.sh
new file mode 100755
index 0000000..57ee310
--- /dev/null
+++ b/fuse/test-fuse.sh
@@ -0,0 +1,165 @@
+#!/bin/bash -
+# libguestfs
+# Copyright (C) 2009 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.
+
+unset CDPATH
+set -e
+#set -v
+
+if [ -z "$top_builddir" ]; then
+    echo "$0: environment variable \$top_builddir must be set"
+    exit 1
+fi
+
+nr_stages=$(grep "^stage " $0 | wc -l)
+
+# Allow top_builddir to be a relative path, but also make it absolute,
+# and move to that directory for the initial phase of the script.
+top_builddir=$(cd "$top_builddir" > /dev/null; pwd)
+
+# Paths to the other programs and files.  NB: Must be absolute paths.
+guestfish="$top_builddir/fish/guestfish"
+guestmount="$top_builddir/fuse/guestmount"
+image="$top_builddir/fuse/test.img"
+mp="$top_builddir/fuse/test-mp"
+
+# Ensure everything is cleaned up on exit.
+rm -f "$image"
+mkdir -p "$mp"
+fusermount -u "$mp" >/dev/null 2>&1 ||:
+function cleanup ()
+{
+    status=$?
+    set +e
+    [ $status = 0 ] || echo "*** FAILED ***"
+    echo "Unmounting filesystem and cleaning up."
+    cd "$top_builddir"
+    # By the way, if you run this manually from within gnome, then
+    # randomly /usr/libexec/gvfs-gdu-volume-monitor will decide to do
+    # whatever it does in the mountpoint directory, preventing you
+    # from unmounting it!
+    if [ -x /sbin/fuser ]; then /sbin/fuser "$mp"; fi
+    fusermount -u "$mp"
+    rm -f "$image"
+    rm -rf "$mp"
+    exit $status
+}
+trap cleanup INT TERM QUIT EXIT
+
+s=1
+function stage ()
+{
+    echo "test-fuse: $s/$nr_stages:" "$@" "..."
+    ((s++))
+}
+
+stage Create filesystem with some inital content
+$guestfish <<EOF
+  sparse "$image" 10M
+  run
+  part-disk /dev/sda mbr
+  mkfs ext2 /dev/sda1
+  mount /dev/sda1 /
+  write-file /hello.txt hello 0
+  write-file /world.txt "hello world" 0
+  touch /empty
+EOF
+
+stage Mounting the filesystem
+$guestmount -a "$image" -m /dev/sda1 "$mp"
+
+stage Changing into mounted directory
+cd "$mp"
+
+stage Checking initial files exist
+[ -n "$(echo *)" ]
+[ "$(ls empty hello.txt world.txt)" = "empty
+hello.txt
+world.txt" ]
+
+stage Checking initial files contain expected content
+[ "$(cat hello.txt)" = "hello" ]
+[ "$(cat world.txt)" = "hello world" ]
+cat empty ;# should print nothing
+[ -z "$(cat empty)" ]
+
+stage Checking file modes of initial content
+[ "$(stat -c %a empty)" = "644" ]
+[ "$(stat -c %a hello.txt)" = "644" ]
+[ "$(stat -c %a world.txt)" = "644" ]
+
+stage Checking unlink
+touch new
+rm -f new ;# force because file is "owned" by root
+
+stage Checking symbolic link
+ln -s hello.txt symlink
+[ -L symlink ]
+
+stage Checking readlink
+[ "$(readlink symlink)" = "hello.txt" ]
+
+stage Checking hard link
+[ "$(stat -c %h hello.txt)" -eq 1 ]
+ln hello.txt link
+[ "$(stat -c %h link)" -eq 2 ]
+[ "$(stat -c %h hello.txt)" -eq 2 ]
+rm -f link
+[ ! -e link ]
+
+# This fails because of caching.  The problem is that the linked file
+# ("hello.txt") is cached with a link count of 2. 
unlink("link")
+# invalidates the cache for "link", but _not_ for
"hello.txt" which
+# still has the now-incorrect cached value.  However there's not much
+# we can do about this since searching for all linked inodes of a file
+# is an O(n) operation.
+#[ "$(stat -c %h hello.txt)" -eq 1 ]
+
+stage Checking mkdir
+mkdir newdir
+[ -d newdir ]
+
+stage Checking rmdir
+rmdir newdir
+[ ! -e newdir ]
+
+stage Checking rename
+touch old
+mv old new
+[ -f new ]
+[ ! -e old ]
+rm -f new
+
+stage Checking chmod
+touch new
+chmod a+x new
+[ -x new ]
+chmod a-x new
+[ ! -x new ]
+chmod a-w new
+[ ! -w new ]
+chmod a+w new
+[ -w new ]
+rm -f new
+
+#stage XXX truncate
+#stage XXX utimens
+#stage XXX chown
+#stage XXX statfs/statvfs
+#stage XXX fsync
+#stage XXX xattr operations
+#stage XXX mknod
-- 
1.6.5.2