| Message ID | 20220527045948.3672022-1-dominique.martinet@atmark-techno.com |
|---|---|
| Headers | show |
| Series | Allow scripts to write INFO and WARN messages | expand |
Hi Dominique, On 27.05.22 06:59, Dominique Martinet wrote: > This is an RFC; I've tested this and it appears to work but I'd like to > know what you think of this before really pushing this (which would > imply writing the associated documentation etc) > Ok > The commit messages are descriptive enough, but basically I've been told > a few times that "ERROR: rebooting now!" is confusing mmmhh...but this is not coming from SWUpdate. I agree this is confusing, but SWUpdate simply sends SUCCESS or FAILED. But I am maybe misunderstanding and you take this as example outside SWUpdate's context. > when I just want > to notify the user that we're going to reboot and it is expected. > > The alternative would be to not display anything, but I feel if a user > runs swupdate interactively they deserve to be told... > That being said running interactively isn't the update method we promote > the most, Neither me: it is just for debug or for an integrator. > so it doesn't affect too many users and I will likely just > remove the messages rather than keep these patches in my branch if > you're against the idea. Ok > > (I have a couple more info messages like this I'd like to display, in > particular some steps of the scripts are a bit slow and an informative > message it's going to take a bit longer than usual would be appreciated, > but if someone wants to know this kind of stuff they can just run with > TRACE as stdout is available then. OTOH, TRACE is too verbose to be > enabled by default for our users, most are not technical and too much is > printed there. I guess this is quite subjective, and if I take 10 developers, I will get 10 different and subjective answers. One point is SWUpdate even as daemon is mostly silent, and it is not doing anything (well, suricatta means polling, yes) for most time. Even if quite verbose, the messages flow into logs (syslog, systemd, whatever..) just during an update, and if something is going wrong, it is nice even in field to get as much information as possible to understand the issues. And in most cases this is due on something in the delivered SWU, and having more information help. So it depends who is the "user", and when the user becomes one of support people, they would like to know as much as possible. And restarting SWUpdate with more verbosity is mostly not possible in field (device is closed without access from outside). > There's actually too much on INFO already in my opinion, users don't > care what bootloaders or handlers are enabled: this is a static > information that doesn't change from a run to the next; This is not true. This is true for static handlers, but there are handlers written in Lua and they are loaded at the startup (currently, I have still in road map a dynamic load of handlers put in the SWU). The output then vary or can vary, or it does in case of some issue so that the Lua handler cannot be loaded (for some unknown reason), and without this information, we just get that update fails. Not only, recent improvement, mainly for Debian based devices, let to load the bootloader interface dynamically. The interface should not change from a run to a next, but if something is going wrong, it could: for example, a shared library cannot be opened again. The INFO at startup is really not very different compared to most Unix daemon: at startup, a list of plugins is showed and reported if they were initialized correctly. I can see this in any log on my Linux PC / Servers. It is not different in SWUpdate, but of course, I agree it is a question of taste. Nevertheless, I would agree if these messages are continuosly appearing in logs, but they are stored just once when system boots, and not anymore until next reboot. > it's good to > know as debug information, but it's not worth info messages.) IMHO it is, see above. > > > Anyway, please tell me what you think. > (yes, I know, that's what lua was made for...) Yes > > > Dominique Martinet (2): > core: lower some INFO messages to TRACE > run_system_cmd: create additional pipes for INFO and WARN traces > > core/bootloader.c | 4 +-- > core/handler.c | 4 +-- > core/install_from_file.c | 3 +- > core/pctl.c | 62 ++++++++++++++++++++++++++++++++++------ > core/swupdate.c | 6 ++-- > corelib/lua_interface.c | 4 +-- > 6 files changed, 65 insertions(+), 18 deletions(-) > Rest of answers in patch review. Best regards, Stefano