[Ocfs2-devel] [PATCH] ocfs2-tools: add error message to mount.ocfs2 when mount point is invalid.

[Ocfs2-devel] [PATCH] ocfs2-tools: add error message to mount.ocfs2 when mount point is invalid.

[Coly Li]

Adds error messages to mount.ocfs2 when mount point is invalid in three conditions:
1) mount point does not exist.
2) mount point is a symbolic link.
3) mount point is not a directory.
Adding these error messages can help ocfs2console provides more helpful feedback information to end 
use. Similar information can be found when mounting ext[234] file systems.

Patch is generated against upstream ocfs2-tools.

Signed-off-by: Coly Li <coyli@suse.de>
Cc: Joel Becker <Joel.Becker@oracle.com>
Cc: Tao Ma <tao.ma@oracle.com>
Cc: Tiger Yang <tiger.yang@oracle.com>
---
diff --git a/mount.ocfs2/mount.ocfs2.c b/mount.ocfs2/mount.ocfs2.c
index a97b4cb..eb8e336 100644
--- a/mount.ocfs2/mount.ocfs2.c
+++ b/mount.ocfs2/mount.ocfs2.c
@@ -382,9 +382,21 @@ int main(int argc, char **argv)
  			o2cb_complete_group_join(&cluster, &desc, errno);
  		}
  		block_signals (SIG_UNBLOCK);
-		com_err(progname, ret, "while mounting %s on %s. "
-			"Check 'dmesg' for more information on this error.",
-			mo.dev, mo.dir);
+
+		/* complain mount failure */
+		if (lstat(mo.dir, &statbuf))
+			com_err(progname, 0, "mount point %s does not "
+				"exist", mo.dir);
+		else if (stat(mo.dir, &statbuf))
+			com_err(progname, 0, "mount point %s is a "
+				"symbolic link to nowhere", mo.dir);
+		else if (stat(mo.dir, &statbuf) || !S_ISDIR(statbuf.st_mode))
+			com_err(progname, 0, "mount point %s is not "
+				"a directory", mo.dir);
+		else
+			com_err(progname, ret, "while mounting %s on %s. "
+				"Check 'dmesg' for more information on this "
+				"error.", mo.dev, mo.dir);
  		goto bail;
  	}
  	if (hb_started) {

-- 
Coly Li
SuSE PRC Labs

[Ocfs2-devel] [PATCH] ocfs2-tools: add error message to mount.ocfs2 when mount point is invalid.

[Sunil Mushran]

Maybe change "symbolic link to nowhere" to "broken symbolic link".

Looks good otherwise.

Coly Li wrote:
> Adds error messages to mount.ocfs2 when mount point is invalid in three conditions:
> 1) mount point does not exist.
> 2) mount point is a symbolic link.
> 3) mount point is not a directory.
> Adding these error messages can help ocfs2console provides more helpful feedback information to end 
> use. Similar information can be found when mounting ext[234] file systems.
>
> Patch is generated against upstream ocfs2-tools.
>
> Signed-off-by: Coly Li <coyli@suse.de>
> Cc: Joel Becker <Joel.Becker@oracle.com>
> Cc: Tao Ma <tao.ma@oracle.com>
> Cc: Tiger Yang <tiger.yang@oracle.com>
> ---
> diff --git a/mount.ocfs2/mount.ocfs2.c b/mount.ocfs2/mount.ocfs2.c
> index a97b4cb..eb8e336 100644
> --- a/mount.ocfs2/mount.ocfs2.c
> +++ b/mount.ocfs2/mount.ocfs2.c
> @@ -382,9 +382,21 @@ int main(int argc, char **argv)
>   			o2cb_complete_group_join(&cluster, &desc, errno);
>   		}
>   		block_signals (SIG_UNBLOCK);
> -		com_err(progname, ret, "while mounting %s on %s. "
> -			"Check 'dmesg' for more information on this error.",
> -			mo.dev, mo.dir);
> +
> +		/* complain mount failure */
> +		if (lstat(mo.dir, &statbuf))
> +			com_err(progname, 0, "mount point %s does not "
> +				"exist", mo.dir);
> +		else if (stat(mo.dir, &statbuf))
> +			com_err(progname, 0, "mount point %s is a "
> +				"symbolic link to nowhere", mo.dir);
> +		else if (stat(mo.dir, &statbuf) || !S_ISDIR(statbuf.st_mode))
> +			com_err(progname, 0, "mount point %s is not "
> +				"a directory", mo.dir);
> +		else
> +			com_err(progname, ret, "while mounting %s on %s. "
> +				"Check 'dmesg' for more information on this "
> +				"error.", mo.dev, mo.dir);
>   		goto bail;
>   	}
>   	if (hb_started) {
>
>   

[Ocfs2-devel] [PATCH] ocfs2-tools: add error message to mount.ocfs2 when mount point is invalid.

[Joel Becker]

On Thu, Jul 31, 2008 at 09:40:21PM +0800, Coly Li wrote:
> diff --git a/mount.ocfs2/mount.ocfs2.c b/mount.ocfs2/mount.ocfs2.c
> index a97b4cb..eb8e336 100644
> --- a/mount.ocfs2/mount.ocfs2.c
> +++ b/mount.ocfs2/mount.ocfs2.c
> @@ -382,9 +382,21 @@ int main(int argc, char **argv)
>   			o2cb_complete_group_join(&cluster, &desc, errno);
>   		}
>   		block_signals (SIG_UNBLOCK);
> -		com_err(progname, ret, "while mounting %s on %s. "
> -			"Check 'dmesg' for more information on this error.",
> -			mo.dev, mo.dir);
> +
> +		/* complain mount failure */
> +		if (lstat(mo.dir, &statbuf))
> +			com_err(progname, 0, "mount point %s does not "
> +				"exist", mo.dir);
> +		else if (stat(mo.dir, &statbuf))
> +			com_err(progname, 0, "mount point %s is a "
> +				"symbolic link to nowhere", mo.dir);
> +		else if (stat(mo.dir, &statbuf) || !S_ISDIR(statbuf.st_mode))
  +		else if (!S_ISDIR(statbuf.st_mode))

	You successfully called stat in the previous "else if" clause.

> +			com_err(progname, 0, "mount point %s is not "
> +				"a directory", mo.dir);
> +		else
> +			com_err(progname, ret, "while mounting %s on %s. "
> +				"Check 'dmesg' for more information on this "
> +				"error.", mo.dev, mo.dir);

	Also, I agree with sunil's "broken symbolic link" text.

Joel

-- 

"I have never let my schooling interfere with my education."
        - Mark Twain

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH] ocfs2-tools: add error message to mount.ocfs2 when mount point is invalid.

[Coly Li]

Joel Becker Wrote:
> On Thu, Jul 31, 2008 at 09:40:21PM +0800, Coly Li wrote:
>> diff --git a/mount.ocfs2/mount.ocfs2.c b/mount.ocfs2/mount.ocfs2.c
>> index a97b4cb..eb8e336 100644
>> --- a/mount.ocfs2/mount.ocfs2.c
>> +++ b/mount.ocfs2/mount.ocfs2.c
>> @@ -382,9 +382,21 @@ int main(int argc, char **argv)
>>   			o2cb_complete_group_join(&cluster, &desc, errno);
>>   		}
>>   		block_signals (SIG_UNBLOCK);
>> -		com_err(progname, ret, "while mounting %s on %s. "
>> -			"Check 'dmesg' for more information on this error.",
>> -			mo.dev, mo.dir);
>> +
>> +		/* complain mount failure */
>> +		if (lstat(mo.dir, &statbuf))
>> +			com_err(progname, 0, "mount point %s does not "
>> +				"exist", mo.dir);
>> +		else if (stat(mo.dir, &statbuf))
>> +			com_err(progname, 0, "mount point %s is a "
>> +				"symbolic link to nowhere", mo.dir);
>> +		else if (stat(mo.dir, &statbuf) || !S_ISDIR(statbuf.st_mode))
>   +		else if (!S_ISDIR(statbuf.st_mode))
> 
> 	You successfully called stat in the previous "else if" clause.
wow, thanks for catching this :)

> 
>> +			com_err(progname, 0, "mount point %s is not "
>> +				"a directory", mo.dir);
>> +		else
>> +			com_err(progname, ret, "while mounting %s on %s. "
>> +				"Check 'dmesg' for more information on this "
>> +				"error.", mo.dev, mo.dir);
> 
> 	Also, I agree with sunil's "broken symbolic link" text.
patch modified and resend to ocfs2-tools-devel again.

-- 
Coly Li
SuSE PRC Labs