Re: [PATCH] Implement the sync libnetfs stubs.

[Thomas Schwinge]

Hello!

On Sun, Aug 16, 2009 at 08:57:46PM +0200, olafBuddenhagen@gmx.net wrote:
> On Tue, Aug 04, 2009 at 12:30:30AM +0200, Thomas Schwinge wrote:
> > Please, please, please, let's try to finally get some of these patches
> > installed before discussing matters to death.
> 
> The real problem is not patches being discussed to death, but rather
> patches that have been reviewed still not getting comitted... Like some
> of Zheng Da's patches for example.

I know about this problem.  Unfortunately, my time is limited.


> The ironic thing here is that once you actually started looking at this
> patch, you did the same kind of "discussing to death" :-)

Touché.

> > So.  Sergiu, if you need specific review of some parts of this patch
> > (or any other patches), then please say so, otherwise please get it
> > installed.
> 
> I'd be very grateful for you not to tell people to ignore my remarks and
> commit broken patches.

Sorry, that never was my intention.  I thought that all issues had been
addressed.  Unfortunately, as we know now, this wasn't the case.

> The origianl patch is wrong and needs to be reverted, and a new one
> comitted once all concerns actually have been addressed.

A follow-up patch has been published (but not yet installed) that makes
netfs_attempt_sync behave as expected, and I think all of us three agree
that this is correct.  I think that this follow-up patch should simply be
installed on top of the one that is in the repository.  No need for
reverting anything; for example, even though the patch in the repository
is not totally correct, it already is an improvement as compared to the
situation before.

Then, what indeed needs discussion is netfs_attempt_syncfs.  Instead of
continuing to speculate, I began documenting the Hurd's RPCs.  Some of
them are documented in the manual, some are in definition or header
files, some can be second-guessed by looking though library sources that
implement them, etc.  Look here (and contribute!):
<http://www.bddebian.com/~hurd-web/hurd/interface/>.  Especially, I added
what I found for file_sync, file_syncfs, and fsys_syncfs.  And the thing
is that I couldn't find a really clear example for the syncfs ones, about
their exact modus operandi.  What one does find is that for some
implementations they indeed foreward syncfs to all child servers, but I'm
missing a rationale why this is better than syncing the root directory.
But, as the forwarding is the technique that is already being applied, I
have no objections for changing unionfs' netfs_attempt_syncfs in this way
-- Sergiu also already posted a patch to do that.  So, then I think all
of us agree, correct?  I suggest: (1.) fix for netfs_attempt_sync is to
be installed on top of the current master; (2.) change for
netfs_attempt_syncfs is to be installed on top of that.


> > I assume that you can confirm in some way (testing, staring at the
> > code, ...) that it does the correct thing.  And should there be any
> > breakage, and we discover it later on, we'll repair it later on.
> 
> The correct functioning of filesystem syncing is very hard to test; and
> any errors will be extremely hard to track down later on. Which makes it
> all the more important that we be able to say with some confidence that
> the code is really doing the right thing.

Actually it is not that hard to test once we have a proper testing
framework.  Unfortunately nobody is working on that so far.  My idea is
to have a remote-controllable (pseudo) translator, which the testing
framework can query, and with which it can interact: create unsynced data
in /foo; file_sync ("/bar"); query /foo -- should still be unsynched;
file_syncfs ("/bar"); query /foo -- should be synced now (as per our
above expectations).


Regards,
 Thomas

signature.asc
Description: Digital signature