Re: nilfs_cleanerd on root partition

[Jiro SEKIBA <jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>]

Hi,

At Mon, 22 Jun 2009 03:00:37 +0900 (JST),
Ryusuke Konishi wrote:
> 
> On Mon, 22 Jun 2009 01:05:10 +0900 (JST), Ryusuke Konishi wrote:
> > Hi,
> > On Sun, 21 Jun 2009 17:37:08 +0300, Sami Liedes wrote:
> > > On Sun, Jun 21, 2009 at 05:02:34PM +0300, Sami Liedes wrote:
> > > > You say I should start the cleanerd by running
> > > > 
> > > >  # nilfs_cleanerd /dev/sdb1 /
> > > > 
> > > > However nothing in the output of `nilfs_cleanerd -h' indicates
> > > > anything of the second parameter (mount point). Looking at the source,
> > > > that's how mount.nilfs2 runs it too, and that's exactly the case where
> > > > it fails.
> > > 
> > > OK, I debugged a bit and figured out what the real problem is. It's
> > > the fact that nilfs_find_fs() is too strict about the mount points
> > > being the same. Namely, it considers /media/lacie and /media/lacie/ to
> > > be different mount points (it merely does a strcmp()).
> > >
> > > /proc/mounts always uses the former syntax, so if the mount command is
> > > something like `mount /dev/mapper/lacie /media/lacie/ -t nilfs2', the
> > > cleanerd is not started because mount passes "/media/lacie/" as the
> > > mount point, and nilfs_find_fs() expects it to not have the last
> > > slash.
> > > 
> > > 	Sami
> > 
> > Grr, that's an elementary and influential bug.  Okay, I'll fix it.
> > 
> > Sami, thank you for finding this!
> > 
> > Cheers,
> > Ryusuke Konishi
> 
> The following patch will fix this.  I've pushed it to the git repo,
> too.
> 
> Regards,
> Ryusuke Konishi
> ---
>  lib/nilfs.c |   29 +++++++++++++++++++++++++----
>  1 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/nilfs.c b/lib/nilfs.c
> index 15ee98f..9c80538 100644
> --- a/lib/nilfs.c
> +++ b/lib/nilfs.c
> @@ -147,14 +147,27 @@ static int nilfs_find_fs(struct nilfs *nilfs, const char *dev, const char *dir,
>  	size_t len;
>  	int ret, n;
>  	char canonical[PATH_MAX + 2];
> +	char *cdev = NULL, *cdir = NULL;
>  
> -	if (dev && myrealpath(dev, canonical, sizeof(canonical)))
> -		dev = canonical;
> +	ret = -1;
> +	if (dev && myrealpath(dev, canonical, sizeof(canonical))) {
> +		cdev = strdup(canonical);
> +		if (!cdev)
> +			goto failed;
> +		dev = cdev;
> +	}
> +
> +	if (dir && myrealpath(dir, canonical, sizeof(canonical))) {
> +		cdir = strdup(canonical);
> +		if (!cdir)
> +			goto failed_dev;
> +		dir = cdir;
> +	}
>  
>  	fp = fopen(PROCMOUNTS, "r");
>  	if (fp == NULL)
> -		return -1;
> -	ret = -1;
> +		goto failed_dir;
> +
>  	while (fgets(line, sizeof(line), fp) != NULL) {
>  		n = tokenize(line, mntent, NMNTFLDS);
>  		assert(n == NMNTFLDS);
> @@ -181,6 +194,14 @@ static int nilfs_find_fs(struct nilfs *nilfs, const char *dev, const char *dir,
>  		}
>  	}
>  	fclose(fp);
> +
> + failed_dir:
> +	free(cdir);
> +
> + failed_dev:
> +	free(cdev);
> +
> + failed:
>  	return ret;
>  }
>  
> -- 
> 1.6.2

I've just pull the git repository and it just works fine for me!

thanks!

regards,
-- 
Jiro SEKIBA <jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>