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

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

[Daniel Ehrenberg]

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 returns one of the
error status codes. 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)

Signed-off-by: Dan Ehrenberg <dehrenberg@chromium.org>
---
 drivers/mtd/ubi/block.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
index 8876c7d..32eeeee 100644
--- a/drivers/mtd/ubi/block.c
+++ b/drivers/mtd/ubi/block.c
@@ -596,10 +596,11 @@ static int __init ubiblock_create_from_param(void)

                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));
+                       ubi_err(
+                               "block: can't open volume on ubi%d_%d, err=%ld",
+                               p->ubi_num, p->vol_id, PTR_ERR(desc));
                        ret = PTR_ERR(desc);
-                       break;
+                       continue;
                }

                ubi_get_volume_info(desc, &vi);
@@ -607,9 +608,10 @@ 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;


--
2.2.0.rc0.207.ga3a616c

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

[Richard Weinberger]

On Thu, Dec 18, 2014 at 12:16 AM, Daniel Ehrenberg
<dehrenberg@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 returns one of the
> error status codes. 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)
>
> Signed-off-by: Dan Ehrenberg <dehrenberg@chromium.org>

Makes sense to me.
Reviewed-by: Richard Weinberger <richard@nod.at>

-- 
Thanks,
//richard

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

[Ezequiel Garcia]

On 12/17/2014 08:16 PM, Daniel Ehrenberg 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 returns one of the
> error status codes. 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)
> 
> Signed-off-by: Dan Ehrenberg <dehrenberg@chromium.org>
> ---
>  drivers/mtd/ubi/block.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
> index 8876c7d..32eeeee 100644
> --- a/drivers/mtd/ubi/block.c
> +++ b/drivers/mtd/ubi/block.c
> @@ -596,10 +596,11 @@ static int __init ubiblock_create_from_param(void)
>

Do you think we can add a comment here to explain how errors are treated?

>                 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));
> +                       ubi_err(
> +                               "block: can't open volume on ubi%d_%d, err=%ld",
> +                               p->ubi_num, p->vol_id, PTR_ERR(desc));

Is it me, or this line break is unneeded here?

Also, it seems something is wrong with the patch format. Patchwork shows
some oddities:

http://patchwork.ozlabs.org/patch/422408/

-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar

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

[Daniel Ehrenberg]

On Thu, Dec 18, 2014 at 3:25 AM, Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
> On 12/17/2014 08:16 PM, Daniel Ehrenberg 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 returns one of the
>> error status codes. 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)
>>
>> Signed-off-by: Dan Ehrenberg <dehrenberg@chromium.org>
>> ---
>>  drivers/mtd/ubi/block.c | 14 ++++++++------
>>  1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
>> index 8876c7d..32eeeee 100644
>> --- a/drivers/mtd/ubi/block.c
>> +++ b/drivers/mtd/ubi/block.c
>> @@ -596,10 +596,11 @@ static int __init ubiblock_create_from_param(void)
>>
>
> Do you think we can add a comment here to explain how errors are treated?

Sent a v2 with that comment and another change.
>
>>                 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));
>> +                       ubi_err(
>> +                               "block: can't open volume on ubi%d_%d, err=%ld",
>> +                               p->ubi_num, p->vol_id, PTR_ERR(desc));
>
> Is it me, or this line break is unneeded here?

checkpatch.pl mandates that (a) strings must not be broken onto
multiple lines, even if that makes the line longer than 80 characters
(b) if a string exceeds 80 characters, it has to be the first thing
besides whitespace on that line.
>
> Also, it seems something is wrong with the patch format. Patchwork shows
> some oddities:
>
> http://patchwork.ozlabs.org/patch/422408/

What oddity do you see there? Looks like it wraps my long string
around, but it's not wrapped in my email (is it?).
>
> --
> Ezequiel Garcia, VanguardiaSur
> www.vanguardiasur.com.ar

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

[Grant Grundler]

[resending as plain text...gmail..grrrh. :( ]

On Thu, Dec 18, 2014 at 3:25 AM, Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
...
>> +                       ubi_err(
>> +                               "block: can't open volume on ubi%d_%d, err=%ld",
>> +                               p->ubi_num, p->vol_id, PTR_ERR(desc));
>
> Is it me, or this line break is unneeded here?

gmail injected this. I've helped Daniel to setup/use git-sendmail.

>
> Also, it seems something is wrong with the patch format. Patchwork shows
> some oddities:
>
> http://patchwork.ozlabs.org/patch/422408/

Oh dear...yes, it's more than just extra line break. Hopefully the
"v3" he just sent fairs better.

thanks,
grant