Hello,
On Tue, Aug 11, 2009 at 11:42:41AM +0200, Thomas Schwinge wrote:
> 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.)
Is it my duty to look after my commits showing up on commit-hurd? If
so, I'm sorry and please tell me how to do that, because I don't even
know what ``commit-hurd'' is :-(
> On Sun, Jul 12, 2009 at 10:50:07PM +0300, Sergiu Ivanov wrote:
> > On Sat, Jul 11, 2009 at 03:56:02AM +0200, olafbuddenha...@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 :-(
I had completely forgotten about this :-( My new patch includes the
check, too.
> > > > +
> > > > + 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;
> }
I agree with your statement about always syncing all nodes and, IIRC,
antrik has already mentioned that, and I have forgotten about that :-(
I'm rather inclined to implement the ``last one wins'' approach, since
it really is simpler and more common. Please, take a look at my new
patch and tell me if it's acceptable.
Regards,
scolobb
