[PATCH] do not stat autofs mounts(was Re: hall and autofs)

[David Zeuthen]

I've committed this. Thanks.

On Wed, 2006-05-17 at 18:55 -0400, John (J5) Palmieri wrote:
> Thanks for the eyes.  I also noticed I used sizeof instead of strlen in
> strncmp.  Patch take 3.
> 
> On Thu, 2006-05-18 at 00:35 +0200, Stefan Seyfried wrote:
> > On Wed, May 17, 2006 at 05:52:55PM -0400, John (J5) Palmieri wrote:
> > > Take 2.  I had an error iterating the autofs_mount list. 
> > > s/autofs_node = autofs_mount->next/autofs_node = autofs_node->next
> > 
> > first, i am not familiar with HAL internals nor glib, so i cannot really
> > ACK or NAK the patch :-)
> > 
> > But it is important that you check for the trailing slash; Imagine autofs
> > configured on "/m", and HAL mounts on "/media". If you now strncmp on "/m",
> > you never will descend on /media => you need to check on "/m/".
> > 
> > 
> > > > > To fix this, you need to collect a list of all autofs mountpoints and skip
> > > > > everything below them.
> > > > > 
> > > > > > +		 * If this is an autofs mount (fstype == 'autofs') ignore the mount. Reason:
> > > > > > +		 *  1. stats on mounts managed by autofs will cause the mount
> > > > > > +		 *     point to be remounted during an unmount
> > > > > 
> > > > > well, but you do not catch the mount point with this check...
> > > > > 
> > > > > > -		if (strcmp(mnt.mnt_type, "nfs") == 0)
> > > > > > +		if (strcmp(mnt.mnt_type, "nfs") == 0 ||
> > > > > > +		    strcmp(mnt.mnt_type, "autofs") == 0)
> > > > > >  			continue;
> > > > > 
> > > > > does not hurt, but i'd bet that it also does not help. ;-)
> > 
> > BTW: this does actually help, since it is totally useless to stat the autofs
> > "parent" dir, so this saves us from useless operations.
> > 
> > > Index: blockdev.c
> > > ===================================================================
> > > RCS file: /cvs/hal/hal/hald/linux2/blockdev.c,v
> > > retrieving revision 1.41
> > > diff -u -r1.41 blockdev.c
> > > --- blockdev.c	9 May 2006 20:13:42 -0000	1.41
> > > +++ blockdev.c	17 May 2006 21:50:15 -0000
> > > @@ -185,6 +185,7 @@
> > >  	dev_t devt = makedev(0, 0);
> > >  	GSList *volumes = NULL;
> > >  	GSList *volume;
> > > +        GSList *autofs_mounts = NULL;
> > >  
> > >  	/* open /proc/mounts */
> > >  	g_snprintf (buf, sizeof (buf), "%s/mounts", get_hal_proc_path ());
> > > @@ -213,6 +214,19 @@
> > >  		if (strcmp(mnt.mnt_type, "nfs") == 0)
> > >  			continue;
> > >  
> > > +		/* If this is an autofs mount (fstype == 'autofs') 
> > > +		 * store the mount in a list for later use. 
> > > +		 * On mounts managed by autofs accessing files below the mount
> > > +		 * point cause the mount point to be remounted after an 
> > > +		 * unmount.  We keep the list so we do not check for
> > > +		 * the .created-by-hal file on mounts under autofs mount points
> > > +		 */
> > > +		if (strcmp(mnt.mnt_type, "autofs") == 0) {
> > > +			autofs_mounts = g_slist_append (autofs_mounts,
> > > +							g_strdup (mnt.mnt_dir));
> > 
> > if mnt.mnt_dir does not alrady contain a trailing slash, then append it here...
> > 
> > > +			continue;
> > > +		}
> > > +
> > >  		/* check the underlying device of the mount point */
> > >  		if (stat (mnt.mnt_dir, &statbuf) != 0)
> > >  			continue;
> > > @@ -254,6 +268,7 @@
> > >  		HalDevice *dev;
> > >  		char *mount_point;
> > >  		char *mount_point_hal_file;
> > > +		GSList *autofs_node;
> > >  
> > >  		dev = HAL_DEVICE (volume->data);
> > >  		mount_point = g_strdup (hal_device_property_get_string (dev, "volume.mount_point"));
> > > @@ -264,8 +279,20 @@
> > >  		device_property_atomic_update_end ();
> > >  		HAL_INFO (("set %s to unmounted", hal_device_get_udi (dev)));
> > >  
> > > +		/* check to see if mount point falls under autofs */
> > > +		autofs_node = autofs_mounts;
> > > +		while (autofs_node != NULL) {
> > > +			char *am = (char *)autofs_node->data;
> > 
> > ... or here, this might save one byte per mountpoint ;-)
> > 
> > > +
> > > +			if (strncmp (am, mount_point, sizeof(am)) == 0);
> > > +				break;
> > > +
> > > +			autofs_node = autofs_node->next;
> > > +		}
> _______________________________________________
> hal mailing list
> hal at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/hal