[OpenWrt-Devel] [PATCH procd] instance: Improve missing jail binary message

[Petr Štetiar]

Kevin Darbyshire-Bryant <ldir at darbyshire-bryant.me.uk> [2020-01-30 17:37:23]:

Hi Kevin,

thanks for looking into that.

> Signed-off-by: Kevin Darbyshire-Bryant <ldir at darbyshire-bryant.me.uk>
> ---
>  service/instance.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/service/instance.c b/service/instance.c
> index e872ba0..b78a65f 100644
> --- a/service/instance.c
> +++ b/service/instance.c
> @@ -824,7 +824,8 @@ instance_jail_parse(struct service_instance *in, struct blob_attr *attr)
>  
>  	r = stat(UJAIL_BIN_PATH, &s);
>  	if (r < 0) {
> -		ERROR("unable to find %s: %m (%d)\n", UJAIL_BIN_PATH, r);
> +		ULOG_WARN("Cannot jail service %s::%s. %s: %m (%d) Are jails enabled?\n",
> +				in->srv->name, in->name, UJAIL_BIN_PATH, r);

I added this message in commit 557f11b3a20f ("instance: provide error feedback
if ujail binary is missing") because I've spent non trivial amount of time on
finding this, but back then I actually didn't realized, that this code path is
probed every time, leading to this error messages in cases where the ujail
binary is absent (most of the time as of now).

This change with service name/instance is indeed helpful, but it doesn't solve
current issue, that we produce this perhaps misleading error/warning every
time when procd (re)starts service which contains jails features in its init
script.

I'm not sure if it makes sense to waste more time on this as there is a plan
to make jails enabled by default soon. If there is a will to make the UX
better, then I see following solutions:

 1. turn that ULOG_WARN into DEBUG, because I think, that most of us increases
    procd's debug level while debugging the init foo issues, we wont pollute
    logs anymore

 2. move the ujail detection logic into procd's init library and send service jail
    params to procd only if it makes sense and adjust procd code accordingly
    if necessary

Just my two cents.

-- ynezz

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel