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

[Stefan Seyfried]

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;
> +		}
-- 
Stefan Seyfried                  \ "I didn't want to write for pay. I
QA / R&D Team Mobile Devices      \ wanted to be paid for what I write."
SUSE LINUX Products GmbH, Nürnberg \                    -- Leonard Cohen