11:05 a.m.
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
11:06 a.m.
New subject: [PATCH 1/5] fuse: Fix symlink creation (RHBZ#538069).
--
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
11:56 a.m.
New subject: [PATCH 1/5] fuse: Fix symlink creation (RHBZ#538069).
On 17/11/09 17:06, Richard W.M. Jones wrote:
> From 93514b1fdb56a0cb6b85b79aff9f08fbd71a9560 Mon Sep 17 00:00:00
2001
From: Richard Jones<rjones(a)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);
As discussed on IRC, this isn't necessary (and could be confusing).
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
Apart from that, ACK.
Matt
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team
M: +44 (0)7977 267231
GPG ID: D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
11:57 a.m.
New subject: [PATCH 1/5] fuse: Fix symlink creation (RHBZ#538069).
On Tue, Nov 17, 2009 at 05:56:53PM +0000, Matthew Booth wrote:
On 17/11/09 17:06, Richard W.M. Jones wrote:
>> From 93514b1fdb56a0cb6b85b79aff9f08fbd71a9560 Mon Sep 17 00:00:00 2001
> From: Richard Jones<rjones(a)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);
As discussed on IRC, this isn't necessary (and could be confusing).
> 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
Apart from that, ACK.
Thanks - pushed without the extra cache invalidation, and also
tested this it's not necessary.
Rich.
--
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
11:06 a.m.
New subject: [PATCH 2/5] fuse: Fix hard link creation.
--
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
12:04 p.m.
New subject: [PATCH 2/5] fuse: Fix hard link creation.
On 17/11/09 17:06, Richard W.M. Jones wrote:
> From f5eec5299fc97de6bb094f23e36dace83fb46363 Mon Sep 17 00:00:00
2001
From: Richard Jones<rjones(a)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
ACK.
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team
M: +44 (0)7977 267231
GPG ID: D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
11:06 a.m.
New subject: [PATCH 3/5] fuse: Fix cache invalidation in rename operation.
--
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/
12:06 p.m.
New subject: [PATCH 3/5] fuse: Fix cache invalidation in rename operation.
On 17/11/09 17:06, Richard W.M. Jones wrote:
> From e94b78db7739e693460e7acfe7c76d295ee17dce Mon Sep 17 00:00:00
2001
From: Richard Jones<rjones(a)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
ACK.
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team
M: +44 (0)7977 267231
GPG ID: D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
11:07 a.m.
New subject: [PATCH 4/5] fuse: Fix read for empty files.
--
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
1:29 a.m.
New subject: [PATCH 4/5] fuse: Fix read for empty files.
Richard W.M. Jones wrote:
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 ();
That change looks fine.
However, that you (and every guestfs_pread caller) would have to
endure such contortions suggests that this API is too hard to use.
Or at least too easy to misuse.
Maybe it will be used so rarely that it doesn't matter...
But it's still early in the life of libguestfs, so if you
think you might want to change it, now is better/easier than later.
3:44 a.m.
New subject: [PATCH 4/5] fuse: Fix read for empty files.
On Wed, Nov 18, 2009 at 08:29:18AM +0100, Jim Meyering wrote:
Richard W.M. Jones wrote:
> 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 ();
That change looks fine.
However, that you (and every guestfs_pread caller) would have to
endure such contortions suggests that this API is too hard to use.
Or at least too easy to misuse.
Maybe it will be used so rarely that it doesn't matter...
But it's still early in the life of libguestfs, so if you
think you might want to change it, now is better/easier than later.
It certainly needs to be documented ...
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
3:03 a.m.
New subject: [PATCH 4/5] fuse: Fix read for empty files.
On 17/11/09 17:07, Richard W.M. Jones wrote:
> From 3c5d63be17d281634f036f53aa32103ad4ab1c41 Mon Sep 17 00:00:00
2001
From: Richard Jones<rjones(a)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
I've stared at this til my head hurts, and I don't get it. Why is 1 not
a valid returned size from guestfs_pread? Why is NULL not an indicator
of failure?
The documentation says:
This function returns a buffer, or NULL on error. The size of the
returned buffer is written to *size_r. The caller must free the
returned buffer after use.
If NULL isn't sufficient to indicate error, guestfs_pread needs to be
fixed, not this funtion.
Matt
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team
M: +44 (0)7977 267231
GPG ID: D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
3:11 a.m.
New subject: [PATCH 4/5] fuse: Fix read for empty files.
Matthew Booth wrote:
On 17/11/09 17:07, Richard W.M. Jones wrote:
>> From 3c5d63be17d281634f036f53aa32103ad4ab1c41 Mon Sep 17 00:00:00 2001
> From: Richard Jones<rjones(a)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
I've stared at this til my head hurts, and I don't get it. Why is 1
not a valid returned size from guestfs_pread? Why is NULL not an
indicator of failure?
1 is a valid result size, but the only way to get
rsize of 1 (assuming the prior initialization) and
*also* r == NULL is in this failure case.
> + if (rsize == 1&& r == NULL)
(oops, spacing:
should be
if (rsize == 1 && r == NULL)
The documentation says:
This function returns a buffer, or NULL on error. The size of the
returned buffer is written to *size_r. The caller must free the
returned buffer after use.
If NULL isn't sufficient to indicate error, guestfs_pread needs to be
fixed, not this funtion.
Right. The API is too hard to use properly.
An alternative is to make it so guestfs_pread returns a non-NULL
pointer to one malloc'd (and unused) byte for an empty read,
and also sets *rsize to 0. *THEN* a NULL return value could
suffice to indicate failure.
4:18 a.m.
New subject: [PATCH 4/5] fuse: Fix read for empty files.
On Wed, Nov 18, 2009 at 10:11:41AM +0100, Jim Meyering wrote:
An alternative is to make it so guestfs_pread returns a non-NULL
pointer to one malloc'd (and unused) byte for an empty read,
and also sets *rsize to 0. *THEN* a NULL return value could
suffice to indicate failure.
This is what I've done ... patch coming up shortly.
Rich.
--
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
11:08 a.m.
New subject: [PATCH 5/5] fuse: Add tests for guestmount.
--
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
11:16 a.m.
New subject: [PATCH 5/5 v2] fuse: Add tests for guestmount.
Sorry, previous patch didn't actually work when run from 'make check'.
This fixes the issue.
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
1:22 p.m.
New subject: [PATCH 5/5 v3] fuse: Add tests for guestmount.
This adds tests for a few more system calls.
Rich.
--
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
3:44 a.m.
New subject: [PATCH 5/5 v3] fuse: Add tests for guestmount.
On 17/11/09 19:22, Richard W.M. Jones wrote:
+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" ]
You need to explicitly set the umask to 022 at the beginning of the
script for this to be reliable.
+stage Checking sizes of initial content
+[ "$(stat -c %s empty)" -eq 0 ]
+[ "$(stat -c %s hello.txt)" -eq 5 ]
+[ "$(stat -c %s world.txt)" -eq 11 ]
While you're at it, you could add tests for every other stat field.
+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 ]
Eurgh. What's the impact of not caching?
Alternatively, inotify can spot an updated link count. What's the
potential for adding asynchronous cache invalidation from the daemon?
+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 ]
+chmod a-r new
+[ ! -r new ]
+chmod a+r new
+[ -r new ]
+rm -f new
+
+stage Checking truncate
+truncate -s 10000 truncated
+[ "$(stat -c %s truncated)" -eq 10000 ]
+truncate -c -s 1000 truncated
+[ "$(stat -c %s truncated)" -eq 1000 ]
+truncate -c -s 10 truncated
+[ "$(stat -c %s truncated)" -eq 10 ]
+truncate -c -s 0 truncated
+[ "$(stat -c %s truncated)" -eq 0 ]
+rm -f truncated
+
+stage Checking utimens and timestamps
+for ts in 12345 1234567 987654321; do
+ # NB: It's not possible to set the ctime with touch.
+ touch -a -d @$ts timestamp
+ [ "$(stat -c %X timestamp)" -eq $ts ]
+ touch -m -d @$ts timestamp
+ [ "$(stat -c %Y timestamp)" -eq $ts ]
+ touch -d @$ts timestamp
+ [ "$(stat -c %X timestamp)" -eq $ts ]
+ [ "$(stat -c %Y timestamp)" -eq $ts ]
+done
+
+# These ones are not yet tested by the current script:
+#stage XXX statfs/statvfs
+#stage XXX xattr operations
+
+# These ones cannot easily be tested by the current script, eg because
+# this script doesn't run as root:
+#stage XXX fsync
+#stage XXX chown
+#stage XXX mknod
-- 1.6.5.2
Looks good.
Matt
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team
M: +44 (0)7977 267231
GPG ID: D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
4:23 a.m.
New subject: [PATCH 5/5 v3] fuse: Add tests for guestmount.
On Wed, Nov 18, 2009 at 09:44:10AM +0000, Matthew Booth wrote:
> +stage Checking file modes of initial content
> +[ "$(stat -c %a empty)" = "644" ]
> +[ "$(stat -c %a hello.txt)" = "644" ]
> +[ "$(stat -c %a world.txt)" = "644" ]
You need to explicitly set the umask to 022 at the beginning of the
script for this to be reliable.
libguestfs guarantees the initial umask already:
http://libguestfs.org/guestfs.3.html#guestfs_umask
http://git.annexia.org/?p=libguestfs.git;a=blob;f=daemon/guestfsd.c;h=db0...
> +stage Checking sizes of initial content
> +[ "$(stat -c %s empty)" -eq 0 ]
> +[ "$(stat -c %s hello.txt)" -eq 5 ]
> +[ "$(stat -c %s world.txt)" -eq 11 ]
While you're at it, you could add tests for every other stat field.
Yup, I've checked most of them, but more would be better.
> +# 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 ]
Eurgh. What's the impact of not caching?
It's really really slow.
Note that the problem here is fairly minor. You'll see a link count
of 2 instead of 1 for just a few seconds.
Alternatively, inotify can spot an updated link count. What's the
potential for adding asynchronous cache invalidation from the daemon?
I don't think inotify helps here ... The linked file could be
anywhere in the whole filesystem, so you'd have to have inotify
watches in every single directory in the filesystem.
I really don't think there's a good way to solve this ...
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
7:17 a.m.
New subject: [PATCH 5/5 v3] fuse: Add tests for guestmount.
On 11/18/2009 10:23 AM, Richard W.M. Jones wrote:
On Wed, Nov 18, 2009 at 09:44:10AM +0000, Matthew Booth wrote:
>> +stage Checking file modes of initial content
>> +[ "$(stat -c %a empty)" = "644" ]
>> +[ "$(stat -c %a hello.txt)" = "644" ]
>> +[ "$(stat -c %a world.txt)" = "644" ]
>
> You need to explicitly set the umask to 022 at the beginning of the
> script for this to be reliable.
libguestfs guarantees the initial umask already:
http://libguestfs.org/guestfs.3.html#guestfs_umask
http://git.annexia.org/?p=libguestfs.git;a=blob;f=daemon/guestfsd.c;h=db0...
The relevant umask is surely the umask of the user running the test.
However, given that the tests pass and my umask is 002, I guess it's not
passing this umask through to the daemon. That's a bug.
>> +stage Checking sizes of initial content
>> +[ "$(stat -c %s empty)" -eq 0 ]
>> +[ "$(stat -c %s hello.txt)" -eq 5 ]
>> +[ "$(stat -c %s world.txt)" -eq 11 ]
>
> While you're at it, you could add tests for every other stat field.
Yup, I've checked most of them, but more would be better.
>> +# 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 ]
>
> Eurgh. What's the impact of not caching?
It's really really slow.
Note that the problem here is fairly minor. You'll see a link count
of 2 instead of 1 for just a few seconds.
> Alternatively, inotify can spot an updated link count. What's the
> potential for adding asynchronous cache invalidation from the daemon?
I don't think inotify helps here ... The linked file could be
anywhere in the whole filesystem, so you'd have to have inotify
watches in every single directory in the filesystem.
Actually I was thinking inotify in the daemon when a file is cached,
dropped when dropped by the client, with an explicit notification to the
client when invalidated on the daemon side.
However, that's a rather large hammer for a small nut. I'll let it pass ;)
Matt
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team
M: +44 (0)7977 267231
GPG ID: D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
7:33 a.m.
New subject: [PATCH 5/5 v3] fuse: Add tests for guestmount.
On Wed, Nov 18, 2009 at 01:17:20PM +0000, Matthew Booth wrote:
On 11/18/2009 10:23 AM, Richard W.M. Jones wrote:
> On Wed, Nov 18, 2009 at 09:44:10AM +0000, Matthew Booth wrote:
>>> +stage Checking file modes of initial content
>>> +[ "$(stat -c %a empty)" = "644" ]
>>> +[ "$(stat -c %a hello.txt)" = "644" ]
>>> +[ "$(stat -c %a world.txt)" = "644" ]
>>
>> You need to explicitly set the umask to 022 at the beginning of the
>> script for this to be reliable.
>
> libguestfs guarantees the initial umask already:
>
> http://libguestfs.org/guestfs.3.html#guestfs_umask
>
http://git.annexia.org/?p=libguestfs.git;a=blob;f=daemon/guestfsd.c;h=db0...
The relevant umask is surely the umask of the user running the test.
However, given that the tests pass and my umask is 002, I guess it's not
passing this umask through to the daemon. That's a bug.
Note this has nothing to do with FUSE: the files are created in that
script using guestfish.
The daemon is documented to set the umask to 022, which is why the
tests pass. Passing the user's umask to the daemon by default would
break the existing guarantee and be one more thing for every script to
have to think about, but in case you want to pass your local umask you
could do this in shell scripts:
guestfish <<EOF
[...]
umask $(($(umask)))
[...]
EOF
The rather convoluted shell invocation is because of a real bug in
guestfish which is that it doesn't appear to parse octal and
hexadecimal numbers properly.
Rich.
--
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
5407
days inactive
5408
days old
20 comments
3 participants
participants (3)
-
Jim Meyering -
Matthew Booth -
Richard W.M. Jones