* netfs.c (netfs_attempt_sync): Sync every writable directory
associated with the supplied node.
(netfs_attempt_syncfs): Send file_syncfs to every writable
directory maintained by unionfs.
---
Hello,
On Mon, Oct 26, 2009 at 01:03:29AM +0100, olafbuddenha...@gmx.net wrote:
> On Mon, Aug 17, 2009 at 11:44:59PM +0300, Sergiu Ivanov wrote:
>
> > @@ -282,7 +283,45 @@ error_t
> > netfs_attempt_sync (struct iouser *cred, struct node *np,
> > int wait)
> > {
> > - return EOPNOTSUPP;
> > + /* The error we are going to report back (last failure wins). */
> > + error_t final_err = 0;
> > +
> > + /* The index of the currently analyzed filesystem. */
> > + int i = 0;
>
> I think the initialization of "i" should be as close to the loop as
> possible -- after all, it's a loop counter...
I moved it closer to the loop itself, but I didn't move it further
than locking the mutex, because locking the mutex is also a part of
initialization, and I am somehow inclined to keep variable
definitions before operations (but this is subjective).
> > + /* Get the information about the current filesystem. */
> > + err = ulfs_get_num (i, &ulfs);
> > + assert (err == 0);
>
> Minor nitpick: it's more common to do such checks with "!err".
Fixed.
> > +
> > + /* Since `np` may not necessarily be present in every underlying
> > + directory, having a null port is perfectly valid. */
> > + if ((node_ulfs->port != MACH_PORT_NULL)
> > + && (ulfs->flags & FLAG_ULFS_WRITABLE))
>
> Not sure whether I asked this before: is there actually any reason not
> to attempt syncing filesystems without FLAG_ULFS_WRITABLE as well?...
>
> (I don't know how file_sync() or file_syncfs() bahave on filesystems or
> nodes that really are not writable -- but IIRC that's not what
> FLAG_ULFS_WRITABLE conveys anyways?...)
A quick search didn't reveal any indications about whether these RPCs
should fail on a really read-only filesystem, so, technically, syncing
such filesystems should not be a problem. At first, I could not see
*conceptual* reasons for syncing directories not marked with
FLAG_ULFS_WRITABLE flag, but I can see one now. Since this
unionfs-specific flag only influences the work of unionfs, and unionfs
does not control *regular* files in unioned directories, a user may
modify files in directories not marked with FLAG_ULFS_WRITABLE. On
invocation of file_sync{,fs} on such a directory, these changes should
be expected to be synced, too. That's why I think I agree with you
and I made unionfs sync every unioned directory.
> > + /* Sync every writable filesystem maintained by unionfs.
> > +
> > + TODO: Rewrite this after having modified ulfs.c and node.c to
> > + store the paths and ports to the underlying directories in one
> > + place, because now iterating over both lists looks ugly. */
> > + node_ulfs_iterate_unlocked (netfs_root_node)
> > + {
> > + error_t err;
> > +
> > + /* Get the information about the current filesystem. */
> > + err = ulfs_get_num (i, &ulfs);
> > + assert (err == 0);
> > +
> > + /* Note that, unlike the situation in netfs_attempt_sync, having a
> > + null port here is abnormal. */
>
> Perhaps it would be helpful to state more explicitely that having a NULL
> port *on the unionfs root node* is abnormal -- I didn't realize this
> point at first.
>
> (Maybe you should actually assert() this.)
Done.
Regards,
scolobb
---
netfs.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 80 insertions(+), 3 deletions(-)
diff --git a/netfs.c b/netfs.c
index 89d1bf6..84bc779 100644
--- a/netfs.c
+++ b/netfs.c
@@ -1,5 +1,6 @@
/* Hurd unionfs
- Copyright (C) 2001, 2002, 2003, 2005 Free Software Foundation, Inc.
+ Copyright (C) 2001, 2002, 2003, 2005, 2009 Free Software Foundation, Inc.
+
Written by Moritz Schulte <mor...@duesseldorf.ccc.de>.
This program is free software; you can redistribute it and/or
@@ -282,7 +283,45 @@ error_t
netfs_attempt_sync (struct iouser *cred, struct node *np,
int wait)
{
- return EOPNOTSUPP;
+ /* The error we are going to report back (last failure wins). */
+ error_t final_err = 0;
+
+ /* The information about the currently analyzed filesystem. */
+ ulfs_t * ulfs;
+
+ /* The index of the currently analyzed filesystem. */
+ int i = 0;
+
+ mutex_lock (&ulfs_lock);
+
+ /* Sync every writable directory associated with `np`.
+
+ TODO: Rewrite this after having modified ulfs.c and node.c to
+ store the paths and ports to the underlying directories in one
+ place, because now iterating over both lists looks ugly. */
+ node_ulfs_iterate_unlocked (np)
+ {
+ error_t err;
+
+ /* Get the information about the current filesystem. */
+ err = ulfs_get_num (i, &ulfs);
+ assert (!err);
+
+ /* Since `np` may not necessarily be present in every underlying
+ directory, having a null port is perfectly valid. */
+ if ((node_ulfs->port != MACH_PORT_NULL)
+ && (ulfs->flags & FLAG_ULFS_WRITABLE))
+ {
+ err = file_sync (node_ulfs->port, wait, 0);
+ if (err)
+ final_err = err;
+ }
+
+ ++i;
+ }
+
+ mutex_unlock (&ulfs_lock);
+ return final_err;
}
/* This should sync the entire remote filesystem. If WAIT is set,
@@ -290,7 +329,45 @@ netfs_attempt_sync (struct iouser *cred, struct node *np,
error_t
netfs_attempt_syncfs (struct iouser *cred, int wait)
{
- return 0;
+ /* The error we are going to report back (last failure wins). */
+ error_t final_err = 0;
+
+ /* The index of the currently analyzed filesystem. */
+ int i = 0;
+
+ /* The information about the currently analyzed filesystem. */
+ ulfs_t * ulfs;
+
+ mutex_lock (&ulfs_lock);
+
+ /* Sync every writable filesystem maintained by unionfs.
+
+ TODO: Rewrite this after having modified ulfs.c and node.c to
+ store the paths and ports to the underlying directories in one
+ place, because now iterating over both lists looks ugly. */
+ node_ulfs_iterate_unlocked (netfs_root_node)
+ {
+ error_t err;
+
+ /* Get the information about the current filesystem. */
+ err = ulfs_get_num (i, &ulfs);
+ assert (err == 0);
+
+ /* Note that, unlike the situation in netfs_attempt_sync, having a
+ null port on the unionfs root node is abnormal. */
+ assert (node_ulfs->port != MACH_PORT_NULL);
+ if (ulfs->flags & FLAG_ULFS_WRITABLE)
+ {
+ err = file_syncfs (node_ulfs->port, wait, 0);
+ if (err)
+ final_err = err;
+ }
+
+ ++i;
+ }
+
+ mutex_unlock (&ulfs_lock);
+ return final_err;
}
/* lookup */
--
1.6.4.3
