Ocfs2 devel - May 2011 - [PATCH 1/1] ocfs2: remove redundant and incorrect mlog_error

We have already mlog all error and positive status is not error.

Signed-off-by: Tiger Yang <tiger.yang at oracle.com>
---
 fs/ocfs2/quota_global.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/fs/ocfs2/quota_global.c b/fs/ocfs2/quota_global.c
index 92fcd57..0a86e30 100644
--- a/fs/ocfs2/quota_global.c
+++ b/fs/ocfs2/quota_global.c
@@ -399,8 +399,6 @@ int ocfs2_global_read_info(struct super_block *sb, int type)
 			      msecs_to_jiffies(oinfo->dqi_syncms));
 
 out_err:
-	if (status)
-		mlog_errno(status);
 	return status;
 out_unlock:
 	ocfs2_unlock_global_qf(oinfo, 0);
-- 
1.7.4.4

On Sat, May 28, 2011 at 12:34:52AM +0800, Tiger Yang
wrote:
> We have already mlog all error and positive status is not error.
	Positive status is turned into -EIO.  There are actually a
couple of places in this function that do not mlog_errno(status) and
rely on this print.  I think you should add them to your patch.
	For example:

371         status = ocfs2_qinfo_lock(oinfo, 0);
372         if (status < 0)
373                 goto out_unlock;

Joel
 
> Signed-off-by: Tiger Yang <tiger.yang at oracle.com>
> ---
>  fs/ocfs2/quota_global.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ocfs2/quota_global.c b/fs/ocfs2/quota_global.c
> index 92fcd57..0a86e30 100644
> --- a/fs/ocfs2/quota_global.c
> +++ b/fs/ocfs2/quota_global.c
> @@ -399,8 +399,6 @@ int ocfs2_global_read_info(struct super_block *sb, int
type)
>  			      msecs_to_jiffies(oinfo->dqi_syncms));
>  
>  out_err:
> -	if (status)
> -		mlog_errno(status);
>  	return status;
>  out_unlock:
>  	ocfs2_unlock_global_qf(oinfo, 0);
> -- 
> 1.7.4.4
> 
-- 

"Copy from one, it's plagiarism; copy from two, it's
research."
        - Wilson Mizner

			http://www.jlbec.org/
			jlbec at evilplan.org

On 06/01/2011 09:43 AM, Joel Becker wrote:
> On Sat, May 28, 2011 at 12:34:52AM +0800, Tiger Yang wrote:
>> We have already mlog all error and positive status is not error.
> 	Positive status is turned into -EIO.  There are actually a
> couple of places in this function that do not mlog_errno(status) and
> rely on this print.  I think you should add them to your patch.
> 	For example:
>
> 371         status = ocfs2_qinfo_lock(oinfo, 0);
> 372         if (status<  0)
> 373                 goto out_unlock;
>
> Joel
>
>
Hi, Joel,
There is something devious about this function.
1 If status == sizeof(struct ocfs2_global_disk_dqinfo)) then positive 
status will be return.
2 After goto out_unlock, they goto out_err again.
I make a new one to fix this.  please review the attached patch.

Thanks,
Tiger


-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-ocfs2-remove-redundant-and-incorrect-mlog_error.patch
Url:
http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20110602/88479be9/attachment.pl