[PATCH v3] UBI: block: Continue creating ubiblocks after an initialization error

[Richard Weinberger]

On Thu, Dec 18, 2014 at 11:39 PM, Dan Ehrenberg <dehrenberg at chromium.org> wrote:
> If one ubi volume is corrupted but another is not, it should be
> possible to initialize that ubiblock from a kernel commandline which
> includes both of them. This patch changes the error handling behavior
> in initializing ubiblock to ensure that all parameters are attempted
> even if one fails. If there is a failure, it is logged on dmesg.
> It also makes error messages more descriptive by including the
> name of the UBI volume that failed.
>
> Tested: Formatted ubi volume /dev/ubi5_0 in a corrupt way and
> dev/ubi3_0 properly and included "ubi.block=5,0 ubi.block=3,0" on
> the kernel command line. At boot, I see the following in the console:
> [   21.082420] UBI error: ubiblock_create_from_param: block: can't open volume on ubi5_0, err=-19
> [   21.084268] UBI: ubiblock3_0 created from ubi3:0(rootfs)
>
> Change-Id: I85a5e756429314f317174efe4beee2f2532e8b73

Please remove this line.

> Signed-off-by: Dan Ehrenberg <dehrenberg at chromium.org>
> ---
> Changes in v2:
> - Added comments in the code explaning the intent of the error
>   handling strategy.
> - Fixed the code surrounding ubiblock initialization to not
>   shut down the block devices after starting them up. In my
>   test case, the initialization happened to not get called
>   because the return value happened to be overwritten by the
>   second successful case, but if the cases were reversed it
>   would be removed. In the new code, ubiblock_create_from_param
>   is simply a void function because no caller cares if one of
>   the blocks failed to initialize.
> Changes in v3:
> - Tried to fix the wraparound in sending the email
> - Moved the 'changes' section to below the ---
>
>  drivers/mtd/ubi/block.c | 37 ++++++++++++++++++++++---------------
>  1 file changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
> index 8876c7d..f9fd9048 100644
> --- a/drivers/mtd/ubi/block.c
> +++ b/drivers/mtd/ubi/block.c
> @@ -584,22 +584,28 @@ open_volume_desc(const char *name, int ubi_num, int vol_id)
>                 return ubi_open_volume(ubi_num, vol_id, UBI_READONLY);
>  }
>
> -static int __init ubiblock_create_from_param(void)
> +static void __init ubiblock_create_from_param(void)
>  {
> -       int i, ret;
> +       int i, ret = 0;
>         struct ubiblock_param *p;
>         struct ubi_volume_desc *desc;
>         struct ubi_volume_info vi;
>
> +       /*
> +        * If there is an error creating one of the ubiblocks, continue on to
> +        * create the following ubiblocks. This helps in a circumstance where
> +        * the kernel command-line specifies multiple block devices and some
> +        * may be broken, but we still want the working ones to come up.
> +        */
>         for (i = 0; i < ubiblock_devs; i++) {
>                 p = &ubiblock_param[i];
>
>                 desc = open_volume_desc(p->name, p->ubi_num, p->vol_id);
>                 if (IS_ERR(desc)) {
> -                       ubi_err("block: can't open volume, err=%ld\n",
> -                               PTR_ERR(desc));
> -                       ret = PTR_ERR(desc);
> -                       break;
> +                       ubi_err(
> +                               "block: can't open volume on ubi%d_%d, err=%ld",
> +                               p->ubi_num, p->vol_id, PTR_ERR(desc));
> +                       continue;
>                 }
>
>                 ubi_get_volume_info(desc, &vi);
> @@ -607,12 +613,12 @@ static int __init ubiblock_create_from_param(void)
>
>                 ret = ubiblock_create(&vi);
>                 if (ret) {
> -                       ubi_err("block: can't add '%s' volume, err=%d\n",
> -                               vi.name, ret);
> -                       break;
> +                       ubi_err(
> +                               "block: can't add '%s' volume on ubi%d_%d, err=%d",
> +                               vi.name, p->ubi_num, p->vol_id, ret);
> +                       continue;
>                 }
>         }
> -       return ret;
>  }
>
>  static void ubiblock_remove_all(void)
> @@ -640,10 +646,12 @@ int __init ubiblock_init(void)
>         if (ubiblock_major < 0)
>                 return ubiblock_major;
>
> -       /* Attach block devices from 'block=' module param */
> -       ret = ubiblock_create_from_param();
> -       if (ret)
> -               goto err_remove;
> +       /*
> +        * Attach block devices from 'block=' module param.
> +        * Even if one block device in the param list fails to come up,
> +        * still allow the module to load and leave any others up.
> +        */
> +       ubiblock_create_from_param();
>
>         /*
>          * Block devices are only created upon user requests, so we ignore
> @@ -656,7 +664,6 @@ int __init ubiblock_init(void)
>
>  err_unreg:
>         unregister_blkdev(ubiblock_major, "ubiblock");
> -err_remove:
>         ubiblock_remove_all();
>         return ret;
>  }
> --
> 2.2.0.rc0.207.ga3a616c
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Thanks,
//richard