bug-hurd
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 3/3] Add the mountee to the list of merged filesystems.


From: Sergiu Ivanov
Subject: [PATCH 3/3] Add the mountee to the list of merged filesystems.
Date: Wed, 4 Nov 2009 19:08:45 +0200
User-agent: Mutt/1.5.20 (2009-06-14)

* mount.c (start_mountee): Add the mountee's filesystem to the
list of merged filesystems.
* node.c (node_init_root): Take into consideration the fact that
an empty string refers to the mountee root.
* ulfs.c (ulfs_check): Likewise.
(ulfs_register): Don't check whether "" is a valid directory.
---
Hello,

On Fri, Oct 30, 2009 at 10:13:09AM +0100, olafBuddenhagen@gmx.net wrote:
> On Mon, Aug 17, 2009 at 08:55:37PM +0300, Sergiu Ivanov wrote:
> > On Sun, Aug 16, 2009 at 07:56:03PM +0200, olafBuddenhagen@gmx.net wrote:
> > > On Mon, Aug 03, 2009 at 08:42:27PM +0300, Sergiu Ivanov wrote:
> 
> > > > +  /* A path equal to "" will mean that the current ULFS entry is the
> > > > +     mountee port.  */
> > > > +  ulfs_register ("", 0, 0);
> > > 
> > > This comment would actually be more appropriate near the definition of
> > > the actual data structure and/or the function filling it in...
> > > 
> > > Of course, it doesn't hurt to mention it here *in addition* to that :-)
> > 
> > I've added the corresponding comment to ulfs_register, but I didn't
> > add anything to variable or structure declarations, because I'm not
> > sure whether it would be suitable to describe the convention in the
> > comment to the declaration of struct ulfs or in the comment to the
> > declaration of ulfs_chain.
> 
> The latter I'd say -- it's not really a property of the ulfs structure
> itself, but rather a special entry in the list...

I've added the corresponding comment to both declaration and
definition of ulfs_chain_start (in ulfs.[ch]).
 
> > Also, in ulfs.h, both are near the declaration of ulfs_register, so it
> > seems to me that it's sufficient to describe the convention in the
> > comment to ulfs_register only.
> 
> Perhaps. Though generally, properly documenting data structures is more
> important than documenting functions... So I'd rather do it the other
> way round :-)

Hm, I didn't know this convention; I'll keep it in mind.  I have
eventually commented both the data structures and the functions,
which, I hope, is not a problem :-)

> > > I actually wonder whether the patches are really split up in the
> > > most useful manner... But I'd rather leave it as is now.
> > 
> > I'm asking out of pure interest: what different way of splitting the
> > functionality across patches do you envision?
> 
> Quite frankly, I don't know :-)

I see :-)
 
> > diff --git a/mount.c b/mount.c
> [...]
> > @@ -535,8 +539,13 @@ node_init_root (node_t *node)
> >       break;
> >  
> >        if (ulfs->path)
> > -   node_ulfs->port = file_name_lookup (ulfs->path,
> > -                                       O_READ | O_DIRECTORY, 0);
> > +   {
> > +   if (!ulfs->path[0])
> 
> You forgot to indent the contents of the block I think?...

Yeah, sure :-( Corrected.
 
> > diff --git a/ulfs.c b/ulfs.c
> [...]
> > @@ -212,14 +216,16 @@ ulfs_for_each_under_priv (char *path_under,
> >    return err;
> >  }
> >  
> > -/* Register a new underlying filesystem.  */
> > +/* Register a new underlying filesystem.  A null path refers to the
> > +   underlying filesystem; a path equal to an empty string refers to
> > +   the filesystem of the mountee.  */
> 
> This comment is very confusing, as "underlying filesystem" is used in
> two different meanings side by side... Please try to reword it.
> 
> It's very unfortunate that the original unionfs often refers to the
> constituents of the union as "underlying filesystems" -- perhaps it
> would be useful to change this globally...

If I understood you correctly, we decided to drop the word combination
``underlying filesystem'' in the meaning of ``unioned directory'' in
the whole unionfs.  I'll do this in a separate patch soon (I hope).
 
> Aside from these formalities, this patch looks fine to me :-)

Great :-)

Regards,
scolobb
 
---
 mount.c |   14 +++++++++++++-
 mount.h |    4 +++-
 node.c  |   15 ++++++++++++---
 ulfs.c  |   23 ++++++++++++++++++-----
 ulfs.h  |    8 ++++++--
 5 files changed, 52 insertions(+), 12 deletions(-)

diff --git a/mount.c b/mount.c
index e325d3d..64dc63b 100644
--- a/mount.c
+++ b/mount.c
@@ -27,6 +27,7 @@
 
 #include "mount.h"
 #include "lib.h"
+#include "ulfs.h"
 
 /* The command line for starting the mountee.  */
 char * mountee_argz;
@@ -138,7 +139,9 @@ start_mountee (node_t * np, char * argz, size_t argz_len, 
int flags,
   return err;
 }                              /* start_mountee */
 
-/* Sets up a proxy node and sets the translator on it.  */
+/* Sets up a proxy node, sets the translator on it, and registers the
+   filesystem published by the translator in the list of merged
+   filesystems.  */
 error_t
 setup_unionmount (void)
 {
@@ -165,6 +168,15 @@ setup_unionmount (void)
   if (err)
     return err;
 
+  /* A path equal to "" will mean that the current ULFS entry is the
+     mountee port.  */
+  ulfs_register ("", 0, 0);
+
+  /* Reinitialize the list of merged filesystems to take into account
+     the newly added mountee's filesystem.  */
+  ulfs_check ();
+  node_init_root (netfs_root_node);
+
   mountee_started = 1;
 
   return 0;
diff --git a/mount.h b/mount.h
index ce860e6..53679ad 100644
--- a/mount.h
+++ b/mount.h
@@ -44,7 +44,9 @@ error_t
 start_mountee (node_t * np, char * argz, size_t argz_len, int flags,
               mach_port_t * port);
 
-/* Sets up a proxy node and sets the translator on it.  */
+/* Sets up a proxy node, sets the translator on it, and registers the
+   filesystem published by the translator in the list of merged
+   filesystems.  */
 error_t
 setup_unionmount (void);
 
diff --git a/node.c b/node.c
index cf9a8b4..f8ad886 100644
--- a/node.c
+++ b/node.c
@@ -1,7 +1,10 @@
 /* Hurd unionfs
-   Copyright (C) 2001, 2002, 2005 Free Software Foundation, Inc.
+   Copyright (C) 2001, 2002, 2005, 2009 Free Software Foundation, Inc.
+
    Written by Moritz Schulte <moritz@duesseldorf.ccc.de>.
 
+   Adapted for unionmount by Sergiu Ivanov <unlimitedscolobb@gmail.com>.
+
    This program is free software; you can redistribute it and/or
    modify it under the terms of the GNU General Public License as
    published by the Free Software Foundation; either version 2 of the
@@ -33,6 +36,7 @@
 #include "node.h"
 #include "ulfs.h"
 #include "lib.h"
+#include "mount.h"
 
 /* Declarations for functions only used in this file.  */
 
@@ -535,8 +539,13 @@ node_init_root (node_t *node)
          break;
 
       if (ulfs->path)
-       node_ulfs->port = file_name_lookup (ulfs->path,
-                                           O_READ | O_DIRECTORY, 0);
+       {
+         if (!ulfs->path[0])
+           node_ulfs->port = mountee_root;
+         else
+           node_ulfs->port = file_name_lookup (ulfs->path,
+                                               O_READ | O_DIRECTORY, 0);
+       }
       else
        node_ulfs->port = underlying_node;
          
diff --git a/ulfs.c b/ulfs.c
index 3c565a5..7d0043a 100644
--- a/ulfs.c
+++ b/ulfs.c
@@ -1,7 +1,10 @@
 /* Hurd unionfs
-   Copyright (C) 2001, 2002, 2005 Free Software Foundation, Inc.
+   Copyright (C) 2001, 2002, 2005, 2009 Free Software Foundation, Inc.
+
    Written by Moritz Schulte <moritz@duesseldorf.ccc.de>.
 
+   Adapted for unionmount by Sergiu Ivanov <unlimitedscolobb@gmail.com>.
+
    This program is free software; you can redistribute it and/or
    modify it under the terms of the GNU General Public License as
    published by the Free Software Foundation; either version 2 of the
@@ -31,8 +34,11 @@
 
 #include "lib.h"
 #include "ulfs.h"
+#include "mount.h"
 
-/* The start of the ulfs chain.  */
+/* The start of the ulfs chain.  A null path refers to the underlying
+   filesystem; a path equal to an empty string refers to the
+   filesystem of the mountee.  */
 ulfs_t *ulfs_chain_start;
 
 /* The end of the ulfs chain, we need this, to go through the chain in
@@ -212,14 +218,16 @@ ulfs_for_each_under_priv (char *path_under,
   return err;
 }
 
-/* Register a new underlying filesystem.  */
+/* Register a new underlying filesystem.  A null path refers to the
+   underlying filesystem; a path equal to an empty string refers to
+   the filesystem of the mountee.  */
 error_t
 ulfs_register (char *path, int flags, int priority)
 {
   ulfs_t *ulfs;
   error_t err;
 
-  if (path)
+  if (path && path[0])
     {
       err = check_dir (path);
       if (err)
@@ -261,7 +269,12 @@ ulfs_check ()
     {
       
       if (u->path)
-       p = file_name_lookup (u->path, O_READ | O_DIRECTORY, 0);
+       {
+         if (!u->path[0])
+           p = mountee_root;
+         else
+           p = file_name_lookup (u->path, O_READ | O_DIRECTORY, 0);
+       }
       else
        p = underlying_node;
          
diff --git a/ulfs.h b/ulfs.h
index 532e3c7..e79e1ee 100644
--- a/ulfs.h
+++ b/ulfs.h
@@ -36,7 +36,9 @@ typedef struct ulfs
 /* The according ulfs is marked writable.  */
 #define FLAG_ULFS_WRITABLE 0x00000001
 
-/* The start of the ulfs chain.  */
+/* The start of the ulfs chain.  A null path refers to the underlying
+   filesystem; a path equal to an empty string refers to the
+   filesystem of the mountee.  */
 extern ulfs_t *ulfs_chain_start;
 
 /* The end of the ulfs chain, we need this, to go through the chain in
@@ -49,7 +51,9 @@ extern unsigned int ulfs_num;
 /* The lock protecting the ulfs data structures.  */
 extern struct mutex ulfs_lock;
 
-/* Register a new underlying filesystem.  */
+/* Register a new underlying filesystem.  A null path refers to the
+   underlying filesystem; a path equal to an empty string refers to
+   the filesystem of the mountee.  */
 error_t ulfs_register (char *path, int flags, int priority);
 
 /* Unregister an underlying filesystem.  */
-- 
1.6.4.3





reply via email to

[Prev in Thread] Current Thread [Next in Thread]