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

[Joel Becker]

On Thu, Jun 02, 2011 at 12:43:40PM +0800, Tiger Yang wrote:
> 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
> 
> 

> >From 36951746bb68250ce13a62fd2a3290fa8af30c54 Mon Sep 17 00:00:00 2001
> From: Tiger Yang <tiger.yang at oracle.com>
> Date: Thu, 2 Jun 2011 11:27:11 +0800
> Subject: [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>

This patch is now part of the fixes branch of ocfs2.git.

Joel

> ---
>  fs/ocfs2/quota_global.c |   11 +++--------
>  1 files changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/ocfs2/quota_global.c b/fs/ocfs2/quota_global.c
> index 92fcd57..cb0f017 100644
> --- a/fs/ocfs2/quota_global.c
> +++ b/fs/ocfs2/quota_global.c
> @@ -358,10 +358,8 @@ int ocfs2_global_read_info(struct super_block *sb, int type)
>  	oinfo->dqi_gqi_count = 0;
>  	oinfo->dqi_gqinode = gqinode;
>  	status = ocfs2_lock_global_qf(oinfo, 0);
> -	if (status < 0) {
> -		mlog_errno(status);
> +	if (status < 0)
>  		goto out_err;
> -	}
>  
>  	status = ocfs2_extent_map_get_blocks(gqinode, 0, &oinfo->dqi_giblk,
>  					     &pcount, NULL);
> @@ -381,7 +379,6 @@ int ocfs2_global_read_info(struct super_block *sb, int type)
>  		     status);
>  		if (status >= 0)
>  			status = -EIO;
> -		mlog_errno(status);
>  		goto out_err;
>  	}
>  	info->dqi_bgrace = le32_to_cpu(dinfo.dqi_bgrace);
> @@ -398,14 +395,12 @@ int ocfs2_global_read_info(struct super_block *sb, int type)
>  	schedule_delayed_work(&oinfo->dqi_sync_work,
>  			      msecs_to_jiffies(oinfo->dqi_syncms));
>  
> -out_err:
> -	if (status)
> -		mlog_errno(status);
>  	return status;
>  out_unlock:
>  	ocfs2_unlock_global_qf(oinfo, 0);
> +out_err:
>  	mlog_errno(status);
> -	goto out_err;
> +	return status;
>  }
>  
>  /* Write information to global quota file. Expects exlusive lock on quota
> -- 
> 1.7.4.4
> 


-- 

"The opposite of a correct statement is a false statement. The
 opposite of a profound truth may well be another profound truth."
         - Niels Bohr 

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