Patchwork e4defrag: Add option -m mtab to e4defrag

login
register
mail settings
Submitter Ashish Sangwan
Date Oct. 17, 2012, 4:52 p.m.
Message ID <1350492760-2106-1-git-send-email-ashish.sangwan2@gmail.com>
Download mbox | patch
Permalink /patch/192106/
State New
Headers show

Comments

Ashish Sangwan - Oct. 17, 2012, 4:52 p.m.
Currently, e4defrag will not work on machines where /etc/mtab is not present
OR is empty.
This patch does 3 things:
a) Add option "-m mtab" to e4defrag so that user can specify any file to
be scanned for otbtaining mounted filesystems info.
b) If mtab option is not specified, first we try to scan /proc/mounts, if failed
to access this file, we try with /etc/mtab.
c) In function is_ext4, check if the varibale "mnt_type" is null before
calling strcmp, otherwise segfault would occur if mtab is empty. 

Signed-off-by: Ashish Sangwan <ashish.sangwan2@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
---
 misc/e4defrag.8.in |   10 ++++++++++
 misc/e4defrag.c    |   34 ++++++++++++++++++++++------------
 2 files changed, 32 insertions(+), 12 deletions(-)
Eric Sandeen - Oct. 17, 2012, 5:06 p.m.
On 10/17/12 11:52 AM, Ashish Sangwan wrote:
> Currently, e4defrag will not work on machines where /etc/mtab is not present
> OR is empty.
> This patch does 3 things:
> a) Add option "-m mtab" to e4defrag so that user can specify any file to
> be scanned for otbtaining mounted filesystems info.

Under what conditions would this be useful; i.e. when is mtab information
ever in a file other than /etc/mtab?

Thanks,
-Eric

> b) If mtab option is not specified, first we try to scan /proc/mounts, if failed
> to access this file, we try with /etc/mtab.
> c) In function is_ext4, check if the varibale "mnt_type" is null before
> calling strcmp, otherwise segfault would occur if mtab is empty. 
> 
> Signed-off-by: Ashish Sangwan <ashish.sangwan2@gmail.com>
> Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
> ---
>  misc/e4defrag.8.in |   10 ++++++++++
>  misc/e4defrag.c    |   34 ++++++++++++++++++++++------------
>  2 files changed, 32 insertions(+), 12 deletions(-)
> 
> diff --git a/misc/e4defrag.8.in b/misc/e4defrag.8.in
> index 75e1bc9..7090e51 100644
> --- a/misc/e4defrag.8.in
> +++ b/misc/e4defrag.8.in
> @@ -9,6 +9,9 @@ e4defrag \- online defragmenter for ext4 filesystem
>  [
>  .B \-v
>  ]
> +[
> +.B \-m mtab
> +]
>  .I target
>  \&...
>  .SH DESCRIPTION
> @@ -57,6 +60,13 @@ is never defragmented.
>  .B \-v
>  Print error messages and the fragmentation count before and after defrag for
>  each file.
> +.TP
> +.B \-m mtab
> +This option will specify the file to be scanned for obtaining mounted filesystems
> +information. If this option is not specified, the default is to use
> +.B /proc/mounts .
> +If this file is not accessible, e4defrag will try to get required information from
> +.B /etc/mtab
>  .SH NOTES
>  .B e4defrag
>  does not support swap file, files in lost+found directory, and files allocated
> diff --git a/misc/e4defrag.c b/misc/e4defrag.c
> index 4b31d03..3567e9f 100644
> --- a/misc/e4defrag.c
> +++ b/misc/e4defrag.c
> @@ -123,6 +123,7 @@
>  #define NGMSG_FILE_OPEN		"Failed to open"
>  #define NGMSG_FILE_UNREG	"File is not regular file"
>  #define NGMSG_LOST_FOUND	"Can not process \"lost+found\""
> +#define _PATH_PROC_MOUNTS       "/proc/mounts"
>  
>  /* Data type for filesystem-wide blocks number */
>  typedef unsigned long long ext4_fsblk_t;
> @@ -262,10 +263,8 @@ static int fallocate64(int fd, int mode, loff_t offset, loff_t len)
>   * @dir_path_len:	the length of directory.
>   */
>  static int get_mount_point(const char *devname, char *mount_point,
> -							int dir_path_len)
> +			   int dir_path_len, const char *mtab)
>  {
> -	/* Refer to /etc/mtab */
> -	const char	*mtab = MOUNTED;
>  	FILE		*fp = NULL;
>  	struct mntent	*mnt = NULL;
>  	struct stat64	sb;
> @@ -278,7 +277,7 @@ static int get_mount_point(const char *devname, char *mount_point,
>  
>  	fp = setmntent(mtab, "r");
>  	if (fp == NULL) {
> -		perror("Couldn't access /etc/mtab");
> +		printf("Couldn't access %s\n", mtab);
>  		return -1;
>  	}
>  
> @@ -313,14 +312,12 @@ static int get_mount_point(const char *devname, char *mount_point,
>   *
>   * @file:		the file's name.
>   */
> -static int is_ext4(const char *file, char *devname)
> +static int is_ext4(const char *file, char *devname, const char *mtab)
>  {
>  	int 	maxlen = 0;
>  	int	len, ret;
>  	FILE	*fp = NULL;
>  	char	*mnt_type = NULL;
> -	/* Refer to /etc/mtab */
> -	const char	*mtab = MOUNTED;
>  	char	file_path[PATH_MAX + 1];
>  	struct mntent	*mnt = NULL;
>  	struct statfs64	fsbuf;
> @@ -345,7 +342,7 @@ static int is_ext4(const char *file, char *devname)
>  
>  	fp = setmntent(mtab, "r");
>  	if (fp == NULL) {
> -		perror("Couldn't access /etc/mtab");
> +		printf("Couldn't access %s", mtab);
>  		return -1;
>  	}
>  
> @@ -374,6 +371,10 @@ static int is_ext4(const char *file, char *devname)
>  	}
>  
>  	endmntent(fp);
> +	if (mnt_type == NULL) {
> +		printf("Could not get mount information from %s\n", mtab);
> +		return -1;
> +	}
>  	if (strcmp(mnt_type, FS_EXT4) == 0) {
>  		FREE(mnt_type);
>  		return 0;
> @@ -1724,6 +1725,7 @@ int main(int argc, char *argv[])
>  	int	success_flag = 0;
>  	char	dir_name[PATH_MAX + 1];
>  	char	dev_name[PATH_MAX + 1];
> +	char *mtab = NULL;
>  	struct stat64	buf;
>  	ext2_filsys fs = NULL;
>  
> @@ -1731,7 +1733,7 @@ int main(int argc, char *argv[])
>  	if (argc == 1)
>  		goto out;
>  
> -	while ((opt = getopt(argc, argv, "vc")) != EOF) {
> +	while ((opt = getopt(argc, argv, "vcm:")) != EOF) {
>  		switch (opt) {
>  		case 'v':
>  			mode_flag |= DETAIL;
> @@ -1739,11 +1741,19 @@ int main(int argc, char *argv[])
>  		case 'c':
>  			mode_flag |= STATISTIC;
>  			break;
> +		case 'm':
> +			mtab = optarg;
> +			break;
>  		default:
>  			goto out;
>  		}
>  	}
> -
> +	if (!mtab) {
> +		if (access(_PATH_PROC_MOUNTS, R_OK) == 0)
> +			mtab = _PATH_PROC_MOUNTS;
> +		else
> +			mtab = _PATH_MOUNTED;
> +	} 
>  	if (argc == optind)
>  		goto out;
>  
> @@ -1797,7 +1807,7 @@ int main(int argc, char *argv[])
>  		if (S_ISBLK(buf.st_mode)) {
>  			/* Block device */
>  			strncpy(dev_name, argv[i], strnlen(argv[i], PATH_MAX));
> -			if (get_mount_point(argv[i], dir_name, PATH_MAX) < 0)
> +			if (get_mount_point(argv[i], dir_name, PATH_MAX, mtab) < 0)
>  				continue;
>  			if (lstat64(dir_name, &buf) < 0) {
>  				perror(NGMSG_FILE_INFO);
> @@ -1833,7 +1843,7 @@ int main(int argc, char *argv[])
>  		 * filesystem type checked in get_mount_point()
>  		 */
>  		if (arg_type == FILENAME || arg_type == DIRNAME) {
> -			if (is_ext4(argv[i], dev_name) < 0)
> +			if (is_ext4(argv[i], dev_name, mtab) < 0)
>  				continue;
>  			if (realpath(argv[i], dir_name) == NULL) {
>  				perror("Couldn't get full path");
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ashish Sangwan - Oct. 18, 2012, 2:57 a.m.
On Wed, Oct 17, 2012 at 10:36 PM, Eric Sandeen <sandeen@redhat.com> wrote:
> On 10/17/12 11:52 AM, Ashish Sangwan wrote:
>> Currently, e4defrag will not work on machines where /etc/mtab is not present
>> OR is empty.
>> This patch does 3 things:
>> a) Add option "-m mtab" to e4defrag so that user can specify any file to
>> be scanned for otbtaining mounted filesystems info.
>
> Under what conditions would this be useful; i.e. when is mtab information
> ever in a file other than /etc/mtab?

On embedded systems usually /etc is read-only. In that case /etc/mtab,
though  present, is empty.
When we tried to run e4defrag on our system, it gave segfault due to
the reasons mentioned in patch.

Then we look into xfs_fsr and found that it has this -m mtab option
due to which it worked without any problem
on our system. Also, xfs_fsr scans /proc/mounts first. So we changed
e4defrag accordingly.
>
> Thanks,
> -Eric
>
>> b) If mtab option is not specified, first we try to scan /proc/mounts, if failed
>> to access this file, we try with /etc/mtab.
>> c) In function is_ext4, check if the varibale "mnt_type" is null before
>> calling strcmp, otherwise segfault would occur if mtab is empty.
>>
>> Signed-off-by: Ashish Sangwan <ashish.sangwan2@gmail.com>
>> Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
>> ---
>>  misc/e4defrag.8.in |   10 ++++++++++
>>  misc/e4defrag.c    |   34 ++++++++++++++++++++++------------
>>  2 files changed, 32 insertions(+), 12 deletions(-)
>>
>> diff --git a/misc/e4defrag.8.in b/misc/e4defrag.8.in
>> index 75e1bc9..7090e51 100644
>> --- a/misc/e4defrag.8.in
>> +++ b/misc/e4defrag.8.in
>> @@ -9,6 +9,9 @@ e4defrag \- online defragmenter for ext4 filesystem
>>  [
>>  .B \-v
>>  ]
>> +[
>> +.B \-m mtab
>> +]
>>  .I target
>>  \&...
>>  .SH DESCRIPTION
>> @@ -57,6 +60,13 @@ is never defragmented.
>>  .B \-v
>>  Print error messages and the fragmentation count before and after defrag for
>>  each file.
>> +.TP
>> +.B \-m mtab
>> +This option will specify the file to be scanned for obtaining mounted filesystems
>> +information. If this option is not specified, the default is to use
>> +.B /proc/mounts .
>> +If this file is not accessible, e4defrag will try to get required information from
>> +.B /etc/mtab
>>  .SH NOTES
>>  .B e4defrag
>>  does not support swap file, files in lost+found directory, and files allocated
>> diff --git a/misc/e4defrag.c b/misc/e4defrag.c
>> index 4b31d03..3567e9f 100644
>> --- a/misc/e4defrag.c
>> +++ b/misc/e4defrag.c
>> @@ -123,6 +123,7 @@
>>  #define NGMSG_FILE_OPEN              "Failed to open"
>>  #define NGMSG_FILE_UNREG     "File is not regular file"
>>  #define NGMSG_LOST_FOUND     "Can not process \"lost+found\""
>> +#define _PATH_PROC_MOUNTS       "/proc/mounts"
>>
>>  /* Data type for filesystem-wide blocks number */
>>  typedef unsigned long long ext4_fsblk_t;
>> @@ -262,10 +263,8 @@ static int fallocate64(int fd, int mode, loff_t offset, loff_t len)
>>   * @dir_path_len:    the length of directory.
>>   */
>>  static int get_mount_point(const char *devname, char *mount_point,
>> -                                                     int dir_path_len)
>> +                        int dir_path_len, const char *mtab)
>>  {
>> -     /* Refer to /etc/mtab */
>> -     const char      *mtab = MOUNTED;
>>       FILE            *fp = NULL;
>>       struct mntent   *mnt = NULL;
>>       struct stat64   sb;
>> @@ -278,7 +277,7 @@ static int get_mount_point(const char *devname, char *mount_point,
>>
>>       fp = setmntent(mtab, "r");
>>       if (fp == NULL) {
>> -             perror("Couldn't access /etc/mtab");
>> +             printf("Couldn't access %s\n", mtab);
>>               return -1;
>>       }
>>
>> @@ -313,14 +312,12 @@ static int get_mount_point(const char *devname, char *mount_point,
>>   *
>>   * @file:            the file's name.
>>   */
>> -static int is_ext4(const char *file, char *devname)
>> +static int is_ext4(const char *file, char *devname, const char *mtab)
>>  {
>>       int     maxlen = 0;
>>       int     len, ret;
>>       FILE    *fp = NULL;
>>       char    *mnt_type = NULL;
>> -     /* Refer to /etc/mtab */
>> -     const char      *mtab = MOUNTED;
>>       char    file_path[PATH_MAX + 1];
>>       struct mntent   *mnt = NULL;
>>       struct statfs64 fsbuf;
>> @@ -345,7 +342,7 @@ static int is_ext4(const char *file, char *devname)
>>
>>       fp = setmntent(mtab, "r");
>>       if (fp == NULL) {
>> -             perror("Couldn't access /etc/mtab");
>> +             printf("Couldn't access %s", mtab);
>>               return -1;
>>       }
>>
>> @@ -374,6 +371,10 @@ static int is_ext4(const char *file, char *devname)
>>       }
>>
>>       endmntent(fp);
>> +     if (mnt_type == NULL) {
>> +             printf("Could not get mount information from %s\n", mtab);
>> +             return -1;
>> +     }
>>       if (strcmp(mnt_type, FS_EXT4) == 0) {
>>               FREE(mnt_type);
>>               return 0;
>> @@ -1724,6 +1725,7 @@ int main(int argc, char *argv[])
>>       int     success_flag = 0;
>>       char    dir_name[PATH_MAX + 1];
>>       char    dev_name[PATH_MAX + 1];
>> +     char *mtab = NULL;
>>       struct stat64   buf;
>>       ext2_filsys fs = NULL;
>>
>> @@ -1731,7 +1733,7 @@ int main(int argc, char *argv[])
>>       if (argc == 1)
>>               goto out;
>>
>> -     while ((opt = getopt(argc, argv, "vc")) != EOF) {
>> +     while ((opt = getopt(argc, argv, "vcm:")) != EOF) {
>>               switch (opt) {
>>               case 'v':
>>                       mode_flag |= DETAIL;
>> @@ -1739,11 +1741,19 @@ int main(int argc, char *argv[])
>>               case 'c':
>>                       mode_flag |= STATISTIC;
>>                       break;
>> +             case 'm':
>> +                     mtab = optarg;
>> +                     break;
>>               default:
>>                       goto out;
>>               }
>>       }
>> -
>> +     if (!mtab) {
>> +             if (access(_PATH_PROC_MOUNTS, R_OK) == 0)
>> +                     mtab = _PATH_PROC_MOUNTS;
>> +             else
>> +                     mtab = _PATH_MOUNTED;
>> +     }
>>       if (argc == optind)
>>               goto out;
>>
>> @@ -1797,7 +1807,7 @@ int main(int argc, char *argv[])
>>               if (S_ISBLK(buf.st_mode)) {
>>                       /* Block device */
>>                       strncpy(dev_name, argv[i], strnlen(argv[i], PATH_MAX));
>> -                     if (get_mount_point(argv[i], dir_name, PATH_MAX) < 0)
>> +                     if (get_mount_point(argv[i], dir_name, PATH_MAX, mtab) < 0)
>>                               continue;
>>                       if (lstat64(dir_name, &buf) < 0) {
>>                               perror(NGMSG_FILE_INFO);
>> @@ -1833,7 +1843,7 @@ int main(int argc, char *argv[])
>>                * filesystem type checked in get_mount_point()
>>                */
>>               if (arg_type == FILENAME || arg_type == DIRNAME) {
>> -                     if (is_ext4(argv[i], dev_name) < 0)
>> +                     if (is_ext4(argv[i], dev_name, mtab) < 0)
>>                               continue;
>>                       if (realpath(argv[i], dir_name) == NULL) {
>>                               perror("Couldn't get full path");
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Sandeen - Oct. 18, 2012, 3 a.m.
On 10/17/12 9:57 PM, Ashish Sangwan wrote:
> On Wed, Oct 17, 2012 at 10:36 PM, Eric Sandeen <sandeen@redhat.com> wrote:
>> On 10/17/12 11:52 AM, Ashish Sangwan wrote:
>>> Currently, e4defrag will not work on machines where /etc/mtab is not present
>>> OR is empty.
>>> This patch does 3 things:
>>> a) Add option "-m mtab" to e4defrag so that user can specify any file to
>>> be scanned for otbtaining mounted filesystems info.
>>
>> Under what conditions would this be useful; i.e. when is mtab information
>> ever in a file other than /etc/mtab?
> 
> On embedded systems usually /etc is read-only. In that case /etc/mtab,
> though  present, is empty.
> When we tried to run e4defrag on our system, it gave segfault due to
> the reasons mentioned in patch.

Clearly the segfault needs to be fixed, and looking at /proc/mounts makes sense.
But I just wonder about the usefulness of being able to specify an arbitrary
file in lieu of /etc/mtab.  Unless there's a clear usecase I'm not sure I'd
add it.

> Then we look into xfs_fsr and found that it has this -m mtab option
> due to which it worked without any problem
> on our system. Also, xfs_fsr scans /proc/mounts first. So we changed
> e4defrag accordingly.

Compatibility with xfs_fsr is the only reason I can think of to do it :)

I just can't imagine why mount information would be anywhere other than
/etc/mtab or /proc/mounts.

Just pushing back on the complexity a little.

Thanks,
-Eric

>> Thanks,
>> -Eric
>>
>>> b) If mtab option is not specified, first we try to scan /proc/mounts, if failed
>>> to access this file, we try with /etc/mtab.
>>> c) In function is_ext4, check if the varibale "mnt_type" is null before
>>> calling strcmp, otherwise segfault would occur if mtab is empty.
>>>
>>> Signed-off-by: Ashish Sangwan <ashish.sangwan2@gmail.com>
>>> Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
>>> ---
>>>  misc/e4defrag.8.in |   10 ++++++++++
>>>  misc/e4defrag.c    |   34 ++++++++++++++++++++++------------
>>>  2 files changed, 32 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/misc/e4defrag.8.in b/misc/e4defrag.8.in
>>> index 75e1bc9..7090e51 100644
>>> --- a/misc/e4defrag.8.in
>>> +++ b/misc/e4defrag.8.in
>>> @@ -9,6 +9,9 @@ e4defrag \- online defragmenter for ext4 filesystem
>>>  [
>>>  .B \-v
>>>  ]
>>> +[
>>> +.B \-m mtab
>>> +]
>>>  .I target
>>>  \&...
>>>  .SH DESCRIPTION
>>> @@ -57,6 +60,13 @@ is never defragmented.
>>>  .B \-v
>>>  Print error messages and the fragmentation count before and after defrag for
>>>  each file.
>>> +.TP
>>> +.B \-m mtab
>>> +This option will specify the file to be scanned for obtaining mounted filesystems
>>> +information. If this option is not specified, the default is to use
>>> +.B /proc/mounts .
>>> +If this file is not accessible, e4defrag will try to get required information from
>>> +.B /etc/mtab
>>>  .SH NOTES
>>>  .B e4defrag
>>>  does not support swap file, files in lost+found directory, and files allocated
>>> diff --git a/misc/e4defrag.c b/misc/e4defrag.c
>>> index 4b31d03..3567e9f 100644
>>> --- a/misc/e4defrag.c
>>> +++ b/misc/e4defrag.c
>>> @@ -123,6 +123,7 @@
>>>  #define NGMSG_FILE_OPEN              "Failed to open"
>>>  #define NGMSG_FILE_UNREG     "File is not regular file"
>>>  #define NGMSG_LOST_FOUND     "Can not process \"lost+found\""
>>> +#define _PATH_PROC_MOUNTS       "/proc/mounts"
>>>
>>>  /* Data type for filesystem-wide blocks number */
>>>  typedef unsigned long long ext4_fsblk_t;
>>> @@ -262,10 +263,8 @@ static int fallocate64(int fd, int mode, loff_t offset, loff_t len)
>>>   * @dir_path_len:    the length of directory.
>>>   */
>>>  static int get_mount_point(const char *devname, char *mount_point,
>>> -                                                     int dir_path_len)
>>> +                        int dir_path_len, const char *mtab)
>>>  {
>>> -     /* Refer to /etc/mtab */
>>> -     const char      *mtab = MOUNTED;
>>>       FILE            *fp = NULL;
>>>       struct mntent   *mnt = NULL;
>>>       struct stat64   sb;
>>> @@ -278,7 +277,7 @@ static int get_mount_point(const char *devname, char *mount_point,
>>>
>>>       fp = setmntent(mtab, "r");
>>>       if (fp == NULL) {
>>> -             perror("Couldn't access /etc/mtab");
>>> +             printf("Couldn't access %s\n", mtab);
>>>               return -1;
>>>       }
>>>
>>> @@ -313,14 +312,12 @@ static int get_mount_point(const char *devname, char *mount_point,
>>>   *
>>>   * @file:            the file's name.
>>>   */
>>> -static int is_ext4(const char *file, char *devname)
>>> +static int is_ext4(const char *file, char *devname, const char *mtab)
>>>  {
>>>       int     maxlen = 0;
>>>       int     len, ret;
>>>       FILE    *fp = NULL;
>>>       char    *mnt_type = NULL;
>>> -     /* Refer to /etc/mtab */
>>> -     const char      *mtab = MOUNTED;
>>>       char    file_path[PATH_MAX + 1];
>>>       struct mntent   *mnt = NULL;
>>>       struct statfs64 fsbuf;
>>> @@ -345,7 +342,7 @@ static int is_ext4(const char *file, char *devname)
>>>
>>>       fp = setmntent(mtab, "r");
>>>       if (fp == NULL) {
>>> -             perror("Couldn't access /etc/mtab");
>>> +             printf("Couldn't access %s", mtab);
>>>               return -1;
>>>       }
>>>
>>> @@ -374,6 +371,10 @@ static int is_ext4(const char *file, char *devname)
>>>       }
>>>
>>>       endmntent(fp);
>>> +     if (mnt_type == NULL) {
>>> +             printf("Could not get mount information from %s\n", mtab);
>>> +             return -1;
>>> +     }
>>>       if (strcmp(mnt_type, FS_EXT4) == 0) {
>>>               FREE(mnt_type);
>>>               return 0;
>>> @@ -1724,6 +1725,7 @@ int main(int argc, char *argv[])
>>>       int     success_flag = 0;
>>>       char    dir_name[PATH_MAX + 1];
>>>       char    dev_name[PATH_MAX + 1];
>>> +     char *mtab = NULL;
>>>       struct stat64   buf;
>>>       ext2_filsys fs = NULL;
>>>
>>> @@ -1731,7 +1733,7 @@ int main(int argc, char *argv[])
>>>       if (argc == 1)
>>>               goto out;
>>>
>>> -     while ((opt = getopt(argc, argv, "vc")) != EOF) {
>>> +     while ((opt = getopt(argc, argv, "vcm:")) != EOF) {
>>>               switch (opt) {
>>>               case 'v':
>>>                       mode_flag |= DETAIL;
>>> @@ -1739,11 +1741,19 @@ int main(int argc, char *argv[])
>>>               case 'c':
>>>                       mode_flag |= STATISTIC;
>>>                       break;
>>> +             case 'm':
>>> +                     mtab = optarg;
>>> +                     break;
>>>               default:
>>>                       goto out;
>>>               }
>>>       }
>>> -
>>> +     if (!mtab) {
>>> +             if (access(_PATH_PROC_MOUNTS, R_OK) == 0)
>>> +                     mtab = _PATH_PROC_MOUNTS;
>>> +             else
>>> +                     mtab = _PATH_MOUNTED;
>>> +     }
>>>       if (argc == optind)
>>>               goto out;
>>>
>>> @@ -1797,7 +1807,7 @@ int main(int argc, char *argv[])
>>>               if (S_ISBLK(buf.st_mode)) {
>>>                       /* Block device */
>>>                       strncpy(dev_name, argv[i], strnlen(argv[i], PATH_MAX));
>>> -                     if (get_mount_point(argv[i], dir_name, PATH_MAX) < 0)
>>> +                     if (get_mount_point(argv[i], dir_name, PATH_MAX, mtab) < 0)
>>>                               continue;
>>>                       if (lstat64(dir_name, &buf) < 0) {
>>>                               perror(NGMSG_FILE_INFO);
>>> @@ -1833,7 +1843,7 @@ int main(int argc, char *argv[])
>>>                * filesystem type checked in get_mount_point()
>>>                */
>>>               if (arg_type == FILENAME || arg_type == DIRNAME) {
>>> -                     if (is_ext4(argv[i], dev_name) < 0)
>>> +                     if (is_ext4(argv[i], dev_name, mtab) < 0)
>>>                               continue;
>>>                       if (realpath(argv[i], dir_name) == NULL) {
>>>                               perror("Couldn't get full path");
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ashish Sangwan - Oct. 18, 2012, 3:09 a.m.
On Thu, Oct 18, 2012 at 8:30 AM, Eric Sandeen <sandeen@redhat.com> wrote:
> On 10/17/12 9:57 PM, Ashish Sangwan wrote:
>> On Wed, Oct 17, 2012 at 10:36 PM, Eric Sandeen <sandeen@redhat.com> wrote:
>>> On 10/17/12 11:52 AM, Ashish Sangwan wrote:
>>>> Currently, e4defrag will not work on machines where /etc/mtab is not present
>>>> OR is empty.
>>>> This patch does 3 things:
>>>> a) Add option "-m mtab" to e4defrag so that user can specify any file to
>>>> be scanned for otbtaining mounted filesystems info.
>>>
>>> Under what conditions would this be useful; i.e. when is mtab information
>>> ever in a file other than /etc/mtab?
>>
>> On embedded systems usually /etc is read-only. In that case /etc/mtab,
>> though  present, is empty.
>> When we tried to run e4defrag on our system, it gave segfault due to
>> the reasons mentioned in patch.
>
> Clearly the segfault needs to be fixed, and looking at /proc/mounts makes sense.
> But I just wonder about the usefulness of being able to specify an arbitrary
> file in lieu of /etc/mtab.  Unless there's a clear usecase I'm not sure I'd
> add it.
>
>> Then we look into xfs_fsr and found that it has this -m mtab option
>> due to which it worked without any problem
>> on our system. Also, xfs_fsr scans /proc/mounts first. So we changed
>> e4defrag accordingly.
>
> Compatibility with xfs_fsr is the only reason I can think of to do it :)
>
> I just can't imagine why mount information would be anywhere other than
> /etc/mtab or /proc/mounts.
>
you are right. Actually xfs_fsr may work on more than 1 mounted XFS
filesystem in a single run.
That is why it makes _little_ sense for xfs_fsr to have this mtab
option so that user can provide
exclusive mounted xfs partitions info in a seprate file.
But e4defrag would work on a single mounted partition in a single run.
This won't be much helpful here if not for compatiblity.

> Just pushing back on the complexity a little.
>
> Thanks,
> -Eric
>
>>> Thanks,
>>> -Eric
>>>
>>>> b) If mtab option is not specified, first we try to scan /proc/mounts, if failed
>>>> to access this file, we try with /etc/mtab.
>>>> c) In function is_ext4, check if the varibale "mnt_type" is null before
>>>> calling strcmp, otherwise segfault would occur if mtab is empty.
>>>>
>>>> Signed-off-by: Ashish Sangwan <ashish.sangwan2@gmail.com>
>>>> Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
>>>> ---
>>>>  misc/e4defrag.8.in |   10 ++++++++++
>>>>  misc/e4defrag.c    |   34 ++++++++++++++++++++++------------
>>>>  2 files changed, 32 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/misc/e4defrag.8.in b/misc/e4defrag.8.in
>>>> index 75e1bc9..7090e51 100644
>>>> --- a/misc/e4defrag.8.in
>>>> +++ b/misc/e4defrag.8.in
>>>> @@ -9,6 +9,9 @@ e4defrag \- online defragmenter for ext4 filesystem
>>>>  [
>>>>  .B \-v
>>>>  ]
>>>> +[
>>>> +.B \-m mtab
>>>> +]
>>>>  .I target
>>>>  \&...
>>>>  .SH DESCRIPTION
>>>> @@ -57,6 +60,13 @@ is never defragmented.
>>>>  .B \-v
>>>>  Print error messages and the fragmentation count before and after defrag for
>>>>  each file.
>>>> +.TP
>>>> +.B \-m mtab
>>>> +This option will specify the file to be scanned for obtaining mounted filesystems
>>>> +information. If this option is not specified, the default is to use
>>>> +.B /proc/mounts .
>>>> +If this file is not accessible, e4defrag will try to get required information from
>>>> +.B /etc/mtab
>>>>  .SH NOTES
>>>>  .B e4defrag
>>>>  does not support swap file, files in lost+found directory, and files allocated
>>>> diff --git a/misc/e4defrag.c b/misc/e4defrag.c
>>>> index 4b31d03..3567e9f 100644
>>>> --- a/misc/e4defrag.c
>>>> +++ b/misc/e4defrag.c
>>>> @@ -123,6 +123,7 @@
>>>>  #define NGMSG_FILE_OPEN              "Failed to open"
>>>>  #define NGMSG_FILE_UNREG     "File is not regular file"
>>>>  #define NGMSG_LOST_FOUND     "Can not process \"lost+found\""
>>>> +#define _PATH_PROC_MOUNTS       "/proc/mounts"
>>>>
>>>>  /* Data type for filesystem-wide blocks number */
>>>>  typedef unsigned long long ext4_fsblk_t;
>>>> @@ -262,10 +263,8 @@ static int fallocate64(int fd, int mode, loff_t offset, loff_t len)
>>>>   * @dir_path_len:    the length of directory.
>>>>   */
>>>>  static int get_mount_point(const char *devname, char *mount_point,
>>>> -                                                     int dir_path_len)
>>>> +                        int dir_path_len, const char *mtab)
>>>>  {
>>>> -     /* Refer to /etc/mtab */
>>>> -     const char      *mtab = MOUNTED;
>>>>       FILE            *fp = NULL;
>>>>       struct mntent   *mnt = NULL;
>>>>       struct stat64   sb;
>>>> @@ -278,7 +277,7 @@ static int get_mount_point(const char *devname, char *mount_point,
>>>>
>>>>       fp = setmntent(mtab, "r");
>>>>       if (fp == NULL) {
>>>> -             perror("Couldn't access /etc/mtab");
>>>> +             printf("Couldn't access %s\n", mtab);
>>>>               return -1;
>>>>       }
>>>>
>>>> @@ -313,14 +312,12 @@ static int get_mount_point(const char *devname, char *mount_point,
>>>>   *
>>>>   * @file:            the file's name.
>>>>   */
>>>> -static int is_ext4(const char *file, char *devname)
>>>> +static int is_ext4(const char *file, char *devname, const char *mtab)
>>>>  {
>>>>       int     maxlen = 0;
>>>>       int     len, ret;
>>>>       FILE    *fp = NULL;
>>>>       char    *mnt_type = NULL;
>>>> -     /* Refer to /etc/mtab */
>>>> -     const char      *mtab = MOUNTED;
>>>>       char    file_path[PATH_MAX + 1];
>>>>       struct mntent   *mnt = NULL;
>>>>       struct statfs64 fsbuf;
>>>> @@ -345,7 +342,7 @@ static int is_ext4(const char *file, char *devname)
>>>>
>>>>       fp = setmntent(mtab, "r");
>>>>       if (fp == NULL) {
>>>> -             perror("Couldn't access /etc/mtab");
>>>> +             printf("Couldn't access %s", mtab);
>>>>               return -1;
>>>>       }
>>>>
>>>> @@ -374,6 +371,10 @@ static int is_ext4(const char *file, char *devname)
>>>>       }
>>>>
>>>>       endmntent(fp);
>>>> +     if (mnt_type == NULL) {
>>>> +             printf("Could not get mount information from %s\n", mtab);
>>>> +             return -1;
>>>> +     }
>>>>       if (strcmp(mnt_type, FS_EXT4) == 0) {
>>>>               FREE(mnt_type);
>>>>               return 0;
>>>> @@ -1724,6 +1725,7 @@ int main(int argc, char *argv[])
>>>>       int     success_flag = 0;
>>>>       char    dir_name[PATH_MAX + 1];
>>>>       char    dev_name[PATH_MAX + 1];
>>>> +     char *mtab = NULL;
>>>>       struct stat64   buf;
>>>>       ext2_filsys fs = NULL;
>>>>
>>>> @@ -1731,7 +1733,7 @@ int main(int argc, char *argv[])
>>>>       if (argc == 1)
>>>>               goto out;
>>>>
>>>> -     while ((opt = getopt(argc, argv, "vc")) != EOF) {
>>>> +     while ((opt = getopt(argc, argv, "vcm:")) != EOF) {
>>>>               switch (opt) {
>>>>               case 'v':
>>>>                       mode_flag |= DETAIL;
>>>> @@ -1739,11 +1741,19 @@ int main(int argc, char *argv[])
>>>>               case 'c':
>>>>                       mode_flag |= STATISTIC;
>>>>                       break;
>>>> +             case 'm':
>>>> +                     mtab = optarg;
>>>> +                     break;
>>>>               default:
>>>>                       goto out;
>>>>               }
>>>>       }
>>>> -
>>>> +     if (!mtab) {
>>>> +             if (access(_PATH_PROC_MOUNTS, R_OK) == 0)
>>>> +                     mtab = _PATH_PROC_MOUNTS;
>>>> +             else
>>>> +                     mtab = _PATH_MOUNTED;
>>>> +     }
>>>>       if (argc == optind)
>>>>               goto out;
>>>>
>>>> @@ -1797,7 +1807,7 @@ int main(int argc, char *argv[])
>>>>               if (S_ISBLK(buf.st_mode)) {
>>>>                       /* Block device */
>>>>                       strncpy(dev_name, argv[i], strnlen(argv[i], PATH_MAX));
>>>> -                     if (get_mount_point(argv[i], dir_name, PATH_MAX) < 0)
>>>> +                     if (get_mount_point(argv[i], dir_name, PATH_MAX, mtab) < 0)
>>>>                               continue;
>>>>                       if (lstat64(dir_name, &buf) < 0) {
>>>>                               perror(NGMSG_FILE_INFO);
>>>> @@ -1833,7 +1843,7 @@ int main(int argc, char *argv[])
>>>>                * filesystem type checked in get_mount_point()
>>>>                */
>>>>               if (arg_type == FILENAME || arg_type == DIRNAME) {
>>>> -                     if (is_ext4(argv[i], dev_name) < 0)
>>>> +                     if (is_ext4(argv[i], dev_name, mtab) < 0)
>>>>                               continue;
>>>>                       if (realpath(argv[i], dir_name) == NULL) {
>>>>                               perror("Couldn't get full path");
>>>>
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/misc/e4defrag.8.in b/misc/e4defrag.8.in
index 75e1bc9..7090e51 100644
--- a/misc/e4defrag.8.in
+++ b/misc/e4defrag.8.in
@@ -9,6 +9,9 @@  e4defrag \- online defragmenter for ext4 filesystem
 [
 .B \-v
 ]
+[
+.B \-m mtab
+]
 .I target
 \&...
 .SH DESCRIPTION
@@ -57,6 +60,13 @@  is never defragmented.
 .B \-v
 Print error messages and the fragmentation count before and after defrag for
 each file.
+.TP
+.B \-m mtab
+This option will specify the file to be scanned for obtaining mounted filesystems
+information. If this option is not specified, the default is to use
+.B /proc/mounts .
+If this file is not accessible, e4defrag will try to get required information from
+.B /etc/mtab
 .SH NOTES
 .B e4defrag
 does not support swap file, files in lost+found directory, and files allocated
diff --git a/misc/e4defrag.c b/misc/e4defrag.c
index 4b31d03..3567e9f 100644
--- a/misc/e4defrag.c
+++ b/misc/e4defrag.c
@@ -123,6 +123,7 @@ 
 #define NGMSG_FILE_OPEN		"Failed to open"
 #define NGMSG_FILE_UNREG	"File is not regular file"
 #define NGMSG_LOST_FOUND	"Can not process \"lost+found\""
+#define _PATH_PROC_MOUNTS       "/proc/mounts"
 
 /* Data type for filesystem-wide blocks number */
 typedef unsigned long long ext4_fsblk_t;
@@ -262,10 +263,8 @@  static int fallocate64(int fd, int mode, loff_t offset, loff_t len)
  * @dir_path_len:	the length of directory.
  */
 static int get_mount_point(const char *devname, char *mount_point,
-							int dir_path_len)
+			   int dir_path_len, const char *mtab)
 {
-	/* Refer to /etc/mtab */
-	const char	*mtab = MOUNTED;
 	FILE		*fp = NULL;
 	struct mntent	*mnt = NULL;
 	struct stat64	sb;
@@ -278,7 +277,7 @@  static int get_mount_point(const char *devname, char *mount_point,
 
 	fp = setmntent(mtab, "r");
 	if (fp == NULL) {
-		perror("Couldn't access /etc/mtab");
+		printf("Couldn't access %s\n", mtab);
 		return -1;
 	}
 
@@ -313,14 +312,12 @@  static int get_mount_point(const char *devname, char *mount_point,
  *
  * @file:		the file's name.
  */
-static int is_ext4(const char *file, char *devname)
+static int is_ext4(const char *file, char *devname, const char *mtab)
 {
 	int 	maxlen = 0;
 	int	len, ret;
 	FILE	*fp = NULL;
 	char	*mnt_type = NULL;
-	/* Refer to /etc/mtab */
-	const char	*mtab = MOUNTED;
 	char	file_path[PATH_MAX + 1];
 	struct mntent	*mnt = NULL;
 	struct statfs64	fsbuf;
@@ -345,7 +342,7 @@  static int is_ext4(const char *file, char *devname)
 
 	fp = setmntent(mtab, "r");
 	if (fp == NULL) {
-		perror("Couldn't access /etc/mtab");
+		printf("Couldn't access %s", mtab);
 		return -1;
 	}
 
@@ -374,6 +371,10 @@  static int is_ext4(const char *file, char *devname)
 	}
 
 	endmntent(fp);
+	if (mnt_type == NULL) {
+		printf("Could not get mount information from %s\n", mtab);
+		return -1;
+	}
 	if (strcmp(mnt_type, FS_EXT4) == 0) {
 		FREE(mnt_type);
 		return 0;
@@ -1724,6 +1725,7 @@  int main(int argc, char *argv[])
 	int	success_flag = 0;
 	char	dir_name[PATH_MAX + 1];
 	char	dev_name[PATH_MAX + 1];
+	char *mtab = NULL;
 	struct stat64	buf;
 	ext2_filsys fs = NULL;
 
@@ -1731,7 +1733,7 @@  int main(int argc, char *argv[])
 	if (argc == 1)
 		goto out;
 
-	while ((opt = getopt(argc, argv, "vc")) != EOF) {
+	while ((opt = getopt(argc, argv, "vcm:")) != EOF) {
 		switch (opt) {
 		case 'v':
 			mode_flag |= DETAIL;
@@ -1739,11 +1741,19 @@  int main(int argc, char *argv[])
 		case 'c':
 			mode_flag |= STATISTIC;
 			break;
+		case 'm':
+			mtab = optarg;
+			break;
 		default:
 			goto out;
 		}
 	}
-
+	if (!mtab) {
+		if (access(_PATH_PROC_MOUNTS, R_OK) == 0)
+			mtab = _PATH_PROC_MOUNTS;
+		else
+			mtab = _PATH_MOUNTED;
+	} 
 	if (argc == optind)
 		goto out;
 
@@ -1797,7 +1807,7 @@  int main(int argc, char *argv[])
 		if (S_ISBLK(buf.st_mode)) {
 			/* Block device */
 			strncpy(dev_name, argv[i], strnlen(argv[i], PATH_MAX));
-			if (get_mount_point(argv[i], dir_name, PATH_MAX) < 0)
+			if (get_mount_point(argv[i], dir_name, PATH_MAX, mtab) < 0)
 				continue;
 			if (lstat64(dir_name, &buf) < 0) {
 				perror(NGMSG_FILE_INFO);
@@ -1833,7 +1843,7 @@  int main(int argc, char *argv[])
 		 * filesystem type checked in get_mount_point()
 		 */
 		if (arg_type == FILENAME || arg_type == DIRNAME) {
-			if (is_ext4(argv[i], dev_name) < 0)
+			if (is_ext4(argv[i], dev_name, mtab) < 0)
 				continue;
 			if (realpath(argv[i], dir_name) == NULL) {
 				perror("Couldn't get full path");