Hello!
On Fri, Aug 14, 2009 at 10:26:10PM +0300, Sergiu Ivanov wrote:
> On Fri, Aug 14, 2009 at 08:26:18PM +0200, Thomas Schwinge wrote:
> > On Fri, Aug 14, 2009 at 07:41:08PM +0300, Sergiu Ivanov wrote:
> > > On Fri, Aug 14, 2009 at 05:18:59PM +0200, Thomas Schwinge wrote:
> > > node_ulfs_iterate_unlocked (np)
> > > {
> > > + error_t err;
> > > +
> > > /* Get the information about the current filesystem. */
> > > err = ulfs_get_num (i, &ulfs);
> > > if (err)
> > > - break;
> > > + {
> > > + final_err = err;
> > > + continue;
> > > + }By the way: this continue statement would have been erroneous nevertheless, as we'd miss the increment of i in this case. > > Looking at ulfs_get_num's implementation I wonder whether we should > > actually report its failing back to the invoker of netfs_attempt_sync? > > Wouldn't ulfs_get_num failing be a sign of corrupted internal state? So, > > would either a silent continue or even a assert (err == 0) be more > > appropriate here? > > You are right -- if a node contains more node_ulfs entries than there > are registered in ulfs_chain, then something has gone seriously > corrupted. However, I have a question, which is related to > consistency (again *sigh*): ulfs_get_num is invoked in two places > only: in netfs_attempt_sync and in node_init_root (node.c:533 in my > code version). In node_init_root the return value of ulfs_get_num is > checked in an if statement. Is it alright to check this value via an > assert in netfs_attempt_sync? Or should I change the handling of the > return value in node_init_root instead? I don't really know the unionfs code, but if these two structures are always meant to be kept synchronized (which I don't really know, so you'd have to verify that -- or unify these structures, which was the long-term plan, isn't it?), then a fatal error (assert) is OK. But then, node_init_root raises other issues: what is the err check at the beginning of node_ulfs_iterate_unlocked good for? > --- a/netfs.c > +++ b/netfs.c OK for master, I'd say. Regards, Thomas
signature.asc
Description: Digital signature
