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 Alex > > Hans >> >> Alex _______________________________________________ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev