Re: [PATCH] Implement the sync libnetfs stubs.

[Thomas Schwinge]

Hello!

This comment is based on the version of the patch that you installed into
master.  (By the way: this commit didn't show up on commit-hurd; I'll
have a look at that.)


On Sun, Jul 12, 2009 at 10:50:07PM +0300, Sergiu Ivanov wrote:
> 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:
> > > +    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;
> > > +  }

I agree that syncing the other nodes should continue even if any of the
nodes fail to do so.  Or is there a reason not to do it like that?  My
idea is that all nodes are tought to receive the sync command in
parallel, and then all their return values are collected and combined in
a passimistic manner (that is, failure wins).  If you agree, then please
change this code accordingly.  Like this perhaps (totally untested)?

    node_ulfs_iterate_unlocked (np)
    {
      error_t err_l = ulfs_get_num (i, &ulfs);
      if (! err_l
          && (ulfs->flags & FLAG_ULFS_WRITABLE))
        {
          err_l = file_sync (node_ulfs->port, wait, 0);
          /* We can only report back a single error value; the first one
             wins.  */
          if (! err)
            err = err_l;
        }
      ++i;
    }


Regards,
 Thomas

signature.asc
Description: Digital signature