On Sun, Mar 26, 2017 at 6:45 PM, Alexandru Ardelean <ardeleana...@gmail.com> wrote: > On Sun, Mar 26, 2017 at 7:38 PM, Hans Dedecker <dedec...@gmail.com> wrote: >> On Sun, Mar 26, 2017 at 6:21 PM, Alexandru Ardelean >> <ardeleana...@gmail.com> wrote: >>> On Sun, Mar 26, 2017 at 7:06 PM, Hans Dedecker <dedec...@gmail.com> wrote: >>>> On Mon, Mar 20, 2017 at 3:08 PM, Alexandru Ardelean >>>> <ardeleana...@gmail.com> wrote: >>>>> From: Alexandru Ardelean <ardeleana...@gmail.com> >>>>> >>>>> The context is that we generate some of the UCI config >>>>> for netifd via scripts/programs. >>>>> >>>>> Every once in a while, there's a goof when doing that >>>>> UCI generation, and netifd prints out the error at >>>>> stderr, but returns 0 (success) err-code. >>>>> >>>>> This change will fail the ubus call if UCI config >>>>> is invalid or missing for /etc/config/network. >>>>> >>>>> Signed-off-by: Alexandru Ardelean <ardeleana...@gmail.com> >>>>> --- >>>>> config.c | 10 ++++++++-- >>>>> config.h | 9 ++++++++- >>>>> main.c | 4 ++-- >>>>> netifd.h | 2 +- >>>>> ubus.c | 8 ++++++-- >>>>> 5 files changed, 25 insertions(+), 8 deletions(-) >>>>> >>>>> diff --git a/config.c b/config.c >>>>> index 0d965d3..3b1af82 100644 >>>>> --- a/config.c >>>>> +++ b/config.c >>>>> @@ -393,16 +393,20 @@ config_init_wireless(void) >>>>> vlist_flush(&wdev->interfaces); >>>>> } >>>>> >>>>> -void >>>>> +int >>>>> config_init_all(void) >>>>> { >>>>> + int ret = CONFIG_INIT_OK; >>>>> + >>>>> uci_network = config_init_package("network"); >>>>> if (!uci_network) { >>>>> fprintf(stderr, "Failed to load network config\n"); >>>>> - return; >>>>> + return CONFIG_INIT_ERR_NO_NETWORK; >>>>> } >>>>> >>>>> uci_wireless = config_init_package("wireless"); >>>>> + if (!uci_wireless) >>>>> + ret = CONFIG_INIT_ERR_NO_WIRELESS; >>>>> >>>>> vlist_update(&interfaces); >>>>> config_init = true; >>>>> @@ -426,4 +430,6 @@ config_init_all(void) >>>>> interface_refresh_assignments(false); >>>>> interface_start_pending(); >>>>> wireless_start_pending(); >>>>> + >>>>> + return ret; >>>>> } >>>>> diff --git a/config.h b/config.h >>>>> index 5adaca6..df30b64 100644 >>>>> --- a/config.h >>>>> +++ b/config.h >>>>> @@ -19,6 +19,13 @@ >>>>> >>>>> extern bool config_init; >>>>> >>>>> -void config_init_all(void); >>>>> +enum { >>>>> + CONFIG_INIT_OK, >>>>> + CONFIG_INIT_ERR_NO_WIRELESS, >>>>> + CONFIG_INIT_ERR_NO_NETWORK, >>>>> + __CONFIG_INIT_LAST >>>>> +}; >>>>> + >>>>> +int config_init_all(void); >>>>> >>>>> #endif >>>>> diff --git a/main.c b/main.c >>>>> index 5717b81..c173cef 100644 >>>>> --- a/main.c >>>>> +++ b/main.c >>>>> @@ -208,9 +208,9 @@ static void netifd_do_restart(struct uloop_timeout >>>>> *timeout) >>>>> execvp(global_argv[0], global_argv); >>>>> } >>>>> >>>>> -void netifd_reload(void) >>>>> +int netifd_reload(void) >>>>> { >>>>> - config_init_all(); >>>>> + return config_init_all(); >>>>> } >>>>> >>>>> void netifd_restart(void) >>>>> diff --git a/netifd.h b/netifd.h >>>>> index 5a90858..e565423 100644 >>>>> --- a/netifd.h >>>>> +++ b/netifd.h >>>>> @@ -98,6 +98,6 @@ struct interface; >>>>> extern const char *main_path; >>>>> extern const char *config_path; >>>>> void netifd_restart(void); >>>>> -void netifd_reload(void); >>>>> +int netifd_reload(void); >>>>> >>>>> #endif >>>>> diff --git a/ubus.c b/ubus.c >>>>> index 1b1a4cd..31e535c 100644 >>>>> --- a/ubus.c >>>>> +++ b/ubus.c >>>>> @@ -44,8 +44,12 @@ netifd_handle_reload(struct ubus_context *ctx, struct >>>>> ubus_object *obj, >>>>> struct ubus_request_data *req, const char *method, >>>>> struct blob_attr *msg) >>>>> { >>>>> - netifd_reload(); >>>>> - return 0; >>>>> + switch (netifd_reload()) { >>>>> + case CONFIG_INIT_ERR_NO_NETWORK: >>>>> + return UBUS_STATUS_UNKNOWN_ERROR; >>>> Is there any reason why only an ubus error code is returned for a >>>> network uci failure and not for a wireless uci failure ? >>>> >>>> Hans >>>>> + default: >>>>> + return UBUS_STATUS_OK; >>>>> + } >>>>> } >>>>> >>>>> enum { >>>>> -- >>>>> 2.7.4 >>>>> >>> >>> >>> That would make the "ubus call network reload" return non-zero. >>> Hmm, I guess that would not be a problem. >>> I interpreted Felix's comment a bit differently regarding not failing >>> the wireless config. >> I understood Felix's comment as still load the network config in case >> a wireless uci error is raised; but I would still return an error >> code.. This would make the CONFIG_INIT enum definition superfluous and >> will simplify the code. > > ack ; will update > > the main thing i would like to get some input on atm [before > re-spinning], is the base-files change in rc.common [ regarding reload > & restart ] > i'm not too confident about it ; and it's a subtle change that can > have a big impact on processes OK I will check with Felix
Hans > > > Alex > >> >> Hans >>> >>> Alex _______________________________________________ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev