Re: [PATCH] Implement the sync libnetfs stubs.

[olafBuddenhagen]

Hi,

On Wed, Nov 04, 2009 at 06:56:41PM +0200, Sergiu Ivanov wrote:
> On Mon, Oct 26, 2009 at 01:03:29AM +0100, olafBuddenhagen@gmx.net
> wrote:

> > 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).

Yes, I also think definitions should go before statements; I said so
myself in some earlier mail...

However, I didn't mean moving the *definition*, but the *initialization*
-- quite a different thing :-)

Not sure though whether it makes sense to move it inside the mutex
lock... I see advantages to both variants. However, it might make sense
to move the comment for the loop above the initialization and/or the
mutex. (Again, I'm not sure... Just an idea.)

> > 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.

Well, one could argue that if unionfs doesn't touch it, it's also not
responsible for syncing... But I guess that's not what clients expect;
and as I said earlier, more syncing can never hurt...

> That's why I think I agree with you and I made unionfs sync every
> unioned directory.

Well, did you actually test how it behaves with really readonly
filesystems? (Most notably that it doesn't return an error status?)

-antrik-