On Wed, 2006-07-05 at 14:40 -0400, Erez Zadok wrote: > In message <[EMAIL PROTECTED]>, Josef Sipek writes: > > On Wed, Jul 05, 2006 at 02:03:34PM -0400, Erez Zadok wrote: > > > Which means that we would have to buffer the whole new file until the file > > gets closed. And _then_ proceed with parsing. AFAIK, the write syscall will > > translate into one ->write op if the size is <= PAGE_SIZE. > > Yes, but as long as we stick to <=PAGE_SIZE, which I hope we'll never go > above, then there's a much smaller chance that pdflush will kick off a > ->write page event on a partial set. > > But, we don't have to buffer anything: we can simply mark that the config > file had been modified, and on ->close of the file, do all the processing. > And we can just ignore ->writepage for the config file (lest we actually > want a persistent _backup_ of the union configuration, for restarting > purposes?) > > > > We have seen users of Unionfs who do a lot of branch reconfigurations in > > > rapid succession. So we need to be more careful than what people have > > > done > > > in the past in Linux. > > > > As long as the branch management code is properly locked (to prevent races) > > it shouldn't be a problem. > > No, while proper locking is needed, it's not enough. Don't think about > per-branch correctness. Think about higher-level SEMANTIC correctness from > the sysadmin's perspective. You have to effectively grab a lock before a > series of changes, which you'd release after the series is done; but you > can't do it w/ a real kernel lock b/c of deadlock risks -- you need to find > another way to convey that atomicity. > > > > The more I think of it, the more I believe we should keep the first > > > implementation really really simple: when users want to make any changes, > > > they read in the current config, change it all they want, and then cat it > > > back in. That way we defer all complex add/del/insert/change ops to a > > > simpler userland wrapper tool, which'd be much easier to develop and > > > debug. > > > > I'd even defer much of the complex parsing to userland. We could have: > > > > /unionfs/ > > | > > |------ union1/ > > | > > |------ config (read-only file with /proc/mounts -like syntax) > > |------ add (add branch special file) > > |------ delete (delete mode) > > |------ branch1 > > | > > |------ dir (path to lower directory) > > |------ mode (mode for the branch) > > |------ remove (remove this branch > > |------ branch2 > > | > > .... > > > > I wouldn't allow users to change the config file directly, only through the > > add/remove branch special files, etc. > > Again, you don't allow me to add+del in one shot. You'll need to give me a > *writeable* /unionfs/union1/config file. >
I think that all of the modifications need to be done through this config file and we should keep the pseudofs view to present the information to people in a nice manner. If we do decide to do all the processing on the close of the file which seems reasonable don't we still run the risk of a race condition here? What if two people write to the file at the same time and then close? We won't be sure that the file is even valid at that point. -- David Quigley <[EMAIL PROTECTED]> _______________________________________________ unionfs mailing list [email protected] http://www.fsl.cs.sunysb.edu/mailman/listinfo/unionfs
