xfs
[Top] [All Lists]

Re: [PATCH 2/2] xfsprogs: fix return value of verify_set_primary_sb()

To: Li Zhong <zhong@xxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH 2/2] xfsprogs: fix return value of verify_set_primary_sb()
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Thu, 26 Sep 2013 09:43:18 -0500
Cc: xfsprogs <xfs@xxxxxxxxxxx>, Mark Tinguely <tinguely@xxxxxxx>, Chandra Seetharaman <sekharan@xxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1380178092.2983.14.camel@ThinkPad-T5421>
References: <1379829679.4089.2.camel@ThinkPad-T5421> <5241E125.7010902@xxxxxxx> <1380094327.2526.5.camel@ThinkPad-T5421> <5242F31B.4060902@xxxxxxxxxxx> <1380177932.2983.11.camel@ThinkPad-T5421> <1380178092.2983.14.camel@ThinkPad-T5421>
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) Gecko/20130801 Thunderbird/17.0.8
On 9/26/13 1:48 AM, Li Zhong wrote:
> If get_sb() fails because of EOF, it will return with retval 1, which will
> then be interpreted as XR_BAD_MAGIC("bad magic number") in phase1() when
> warning the user.
> 
> This patch fix it by using XR_EOF here, so it would be interpreted correctly.
> Also change the associated comments about the return value.
> 
> Signed-off-by: Li Zhong <zhong@xxxxxxxxxxxxxxxxxx>

I think this is OK.  I had originally thought we should return whatever XR_*
code get_sb() returned, but I wasn't thinking straight; we ignore those
other codes, and only stop if we see XR_EOF.

So this is fine as far as it goes; I noticed something else in the process
though so I'll send a patch for that.  :)

Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx>

> ---
>  repair/sb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/repair/sb.c b/repair/sb.c
> index d34d7a2..2e35a4c 100644
> --- a/repair/sb.c
> +++ b/repair/sb.c
> @@ -665,7 +665,7 @@ get_sb_geometry(fs_geometry_t *geo, xfs_sb_t *sbp)
>   * primary and compare the geometries in the secondaries against
>   * the geometry indicated by the primary.
>   *
> - * returns 1 if bad, 0 if ok
> + * returns 0 if ok, else error code (XR_EOF, XR_INSUFF_SEC_SB, etc).
>   */
>  int
>  verify_set_primary_sb(xfs_sb_t               *rsb,
> @@ -732,7 +732,7 @@ verify_set_primary_sb(xfs_sb_t            *rsb,
>                       checked[agno] = 1;
>  
>                       if (get_sb(sb, off, size, agno) == XR_EOF)  {
> -                             retval = 1;
> +                             retval = XR_EOF;
>                               goto out_free_list;
>                       }
>  
> 

<Prev in Thread] Current Thread [Next in Thread>