[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Implement the sync libnetfs stubs.
From: |
Sergiu Ivanov |
Subject: |
Re: [PATCH] Implement the sync libnetfs stubs. |
Date: |
Sun, 12 Jul 2009 22:50:07 +0300 |
User-agent: |
Mutt/1.5.18 (2008-05-17) |
Hello,
On Sat, Jul 11, 2009 at 03:56:02AM +0200, olafBuddenhagen@gmx.net wrote:
> On Wed, Jul 08, 2009 at 10:30:41PM +0300, Sergiu Ivanov wrote:
>
> > diff --git a/netfs.c b/netfs.c
> > index 89d1bf6..d8211e0 100644
> > --- a/netfs.c
> > +++ b/netfs.c
> > @@ -1,7 +1,10 @@
> > /* 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 <moritz@duesseldorf.ccc.de>.
> >
> > + Modified by Sergiu Ivanov <unlimitedscolobb@gmail.com>.
> > +
>
> You can do this if it's important to you -- but so far it's not common
> in the Hurd to include author information for every change made to a
> file... In fact, many files do not even have the original author
> mentioned.
No, it's not that important to me :-) I just tried to be consistent.
Do I understand it right that only the copyright years update is
compulsory?
> > 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
> > @@ -282,7 +285,36 @@ error_t
> > netfs_attempt_sync (struct iouser *cred, struct node *np,
> > int wait)
> > {
> > - return EOPNOTSUPP;
> > + error_t 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 directory associated with `np`. */
> > + node_ulfs_iterate_unlocked (np)
> > + {
> > + /* Get the information about the current filesystem. */
> > + err = ulfs_get_num (i, &ulfs);
>
> I don't think you really got the idea of the iterator... No need for
> "i".
Well :-) I'd say that the idea of an iterator is one of my favourites,
so I'm rather inclined to think that I understand it pretty okay ;-)
The problem is that unionfs has a separate list of information about
the underlying filesystems and then, each node has a list of *ports*
to the underlying filesystems, too. This means that I need to operate
on *two* separate lists to do the syncing: from the first list I get
the information about whether the filesystem is writable and from the
second one I get the port. node_ulfs_iterate_unlocked takes me
through the list of filesystems in which this node is present and I
have to maintain an index into the (parallel) list of underlying
filesystems to extract the information about the current filesystem.
> > + if (err)
> > + break;
>
> I wonder whether it wouldn't perhaps be better to continue in spite of
> errors?...
Yes, it's definitely better to do it like that. And also, I think I
forgot to add the check for the validity of the port corresponding to
the current filesystem :-(
> > +
> > + if (ulfs->flags & FLAG_ULFS_WRITABLE)
> > + {
> > + err = file_sync (node_ulfs->port, wait, 0);
> > + if (err)
> > + break;
> > + }
> > +
> > + ++i;
> > + }
> > +
> > + mutex_unlock (&ulfs_lock);
> > + return err;
> > }
> >
> > /* This should sync the entire remote filesystem. If WAIT is set,
> > @@ -290,7 +322,10 @@ netfs_attempt_sync (struct iouser *cred, struct node
> > *np,
> > error_t
> > netfs_attempt_syncfs (struct iouser *cred, int wait)
> > {
> > - return 0;
> > + /* The complete list of ports to the merged filesystems is
> > + maintained in the root node of unionfs, so if we sync it, we sync
> > + every single merged directory. */
> > + return netfs_attempt_sync (cred, netfs_root_node, wait);
>
> I'm don't think this is really the right approach. Why not forward the
> syncfs to all unioned filesystems?
Fredrik told me in another mail that it might happen that I won't have
the right to get the control port of the filesystem I'm working with.
AIUI, it's like this: I have access to my home directory, but I don't
have access to the control port of the / filesystem under which my ~/
is situated, because if I had it, I could do many bad things to it.
OTOH, I'm not sure whether syncing the whole filesystem is that very
good, because in this way we lose the specificity: it's very probable
that such syncing will employ a larger number of directories than just
those merged by unionfs. Consider the following:
$ settrans -a foo unionfs ~/bar/ ~/baz/
If unionfs syncs the *whole* filesystem to which ~/bar/ and ~/baz/
belong, it will sync not only these two directories, but the whole
/home/ directory (at least).
(I hope I understood you correctly.)
Regards,
scolobb