Re: [OpenWrt-Devel] [PATCH] libfstools: Output error in case of loop blkdev failure
Jo-Philipp Wich [2019-03-27 08:05:50]:
Hi,
> I suggest to rephrase the subject to something like "print error in
> case". I kept reading "output error" and wondered what went wrong with
> the output.
Good point.
> > - if (!p->loop_name[0] && rootdisk_create_loop(p) != 0)
> > + if (!p->loop_name[0] && rootdisk_create_loop(p) != 0) {
> > + ULOG_ERR("unable to create loop device\n");
>
> Do we have a valid errno value here? Would be useful to append the
> strerror(3) description as well to see whether its ENOSYS, EINVAL etc.
I was thinking about the same, but I wasn't 100% sure, that the errno after
rootdisk_create_loop() would be always valid and I think, that in some corner
cases errno won't be set at all (strcmp, offset comparison), thus I've chosen
to go with generic error, rather then misleading one.
In order to fix this properly, we would probably need to refactor
rootdisk_create_loop, potentialy open some regressions in some corner cases,
increasing the flash footprint and I'm not sure if it's worth the effort,
since this error (missing loop blkdev kernel module) usually happens only
during the development.
-- ynezz
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] libfstools: Output error in case of loop blkdev failure
Hi Petr,
I suggest to rephrase the subject to something like "print error in
case". I kept reading "output error" and wondered what went wrong with
the output.
> It took me quite some time today(while fixing squashfs+overlay on
> armvirt) to find out, that I was missing support for loop block device
> in kernel, so I'm adding error message which might be helpful for
> someone else in the future as well.
>
> Signed-off-by: Petr Štetiar
> ---
> libfstools/rootdisk.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/libfstools/rootdisk.c b/libfstools/rootdisk.c
> index dd00c1b..68a6296 100644
> --- a/libfstools/rootdisk.c
> +++ b/libfstools/rootdisk.c
> @@ -258,8 +258,10 @@ static int rootdisk_volume_init(struct volume *v)
> char str[128];
> int ret = 0;
>
> - if (!p->loop_name[0] && rootdisk_create_loop(p) != 0)
> + if (!p->loop_name[0] && rootdisk_create_loop(p) != 0) {
> + ULOG_ERR("unable to create loop device\n");
Do we have a valid errno value here? Would be useful to append the
strerror(3) description as well to see whether its ENOSYS, EINVAL etc.
> return -1;
> + }
>
> v->type = BLOCKDEV;
> v->blk = p->loop_name;
>
~ Jo
signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
