[Nut-upsdev] NUT Windows port sources review

[Charles Lepple]

On Sun, Apr 17, 2016 at 3:56 PM, Denis Serov <intrudo at outlook.com> wrote:
> Hello!
>
> I performed lightweight review of Windows port source code (Windows-v2.6.5-7 branch). I have some ideas I would like to share with NUT community.
>
> First of all, Frederic did great job to port NUT sources to Windows platform and we have the path in dark forest. Unfortunately, the port looks like experimental. I think that the next major iteration is required.
>
> Here are many words, sorry about it :)

Here are a few more :)

> Denis
>
> REQUIREMENTS
> ------------
> I could not find any requirements for Windows port version of NUT. I have arranged some items, which I see currently:
>
> 1. Windows port must be reliable and stable. It should provide enough level of logging and debugging to have good technical support.
>
> 2. Windows port must support all devices and programs, which Linux version does.
>
> 3. Windows port must be done for x86 and x64 platforms. The source code supports both platforms. To get the binaries, it is enough to configure and compile the sources, nothing more.

How critical is 32-bit x86 these days? (Especially if we were to
support a minimum of something later than XP)

> 4. Windows port must support installation via Windows Installer. There should be two MSI setups for both platforms or single one with automatic platform detection. The setup system should provide selecting the features (programs, drivers, etc.).

Not sure if there is a solution for building MSIs already, but it
would be nice to have the option to cross-compile from Linux.

> 5. Windows port components (x86, x64) must be compatible with Linux versions in any combinations. For example, Windows version of upsmon should be compatible with Linux version of upsd and so on. It allows to have mixed environment without any problem.
>
> It seems there are no system requirements for minimal supported Windows version. As result, no one checks the version of operating system: neither NUT programs nor MSI setup. For example, Windows port uses OpenSCManager function, but it is available from Windows XP/Windows 2003 only. As result, the programs will crash in environment of older operating systems.
>
> I think that minimal supported Windows version must be Windows XP/Windows 2003 or even more modern one. Setup and source code should check operating system at startup.
>
> GIT BRANCHES
> ------------
> As I understand, master and Windows-v2.6.5-7 branches separated in last years and they contain different commits. In the end of Windows port implementation there should be one single branch for Windows port and Linux version (master). After it, Windows and Linux versions will be released simultaneously under equal versions.
> That is the big question what branch should be used for next development iteration of Windows port.
>
> I think Windows port must be stabilized in Windows-v2.6.5-7 branch and then merged to master one. Sure, the merge will 100% affect Linux version and may be bring some regression. On the other hand, it is single operation and it does not look very hard.

It might not be hard to do in git-space, but I do not want the
duplicated history from our failed attempts to merge the original
windows_port branch (that Windows-v2.6.7 is derived from). As I
mentioned earlier, we do not know how many regressions were introduced
in the windows_port branch due to the use of inexact SVN revisions
('HEAD') when merging.

Given some of your recommendations about refactoring for platform
abstraction, I think the extra history from the old branches will be a
distraction.

TL;DR: I do not want this branch merged as-is. Rebasing or
reconstructing is fine, though.

> PLATFORM ABSTRACTION
> --------------------
> Current Windows port sources contain mixed Linux and Windows code in form of compiler predefines like #ifdef WIN32. It makes the sources unreadable and not maintainable.
>
> I think the solution is implementation of abstraction layer that hides platform specific from algorithms and business logic. Saying the truth, something like abstraction layer already exists in wincompat.c file, but it is not full.
>
> It would be great to have the abstraction layer not for Windows port only but for Linux version too. For example, the algorithms call the functions from interface like C-header. There are different implementations of this interface in static linked libraries. The libraries are building from different source code for specified target platform.
>
> -------------                  ---------------
> Linux version                  Windows version
> ----------------------------------------------
>     Common algorithms and business logic
> ----------------------------------------------
>          Software abstraction layer
> ----------------------------------------------
> Linux implementation    Windows implementation
> --------------------    ----------------------
>
> LOGGING AND DEBUGGING
> ---------------------
> When I tried to install Windows port in my environment, I had some problems to understand what is going on and what is wrong: it is quite hard to increase logging level and it is uncomfortable to work with Windows Event Log. I think that logging architecture should be improved.

You might want to check with @zykh on Github:
https://github.com/networkupstools/nut/issues/211

It is slightly tangential to the problems you are trying to solve, but
the NUT logging code could use some cleanup, and I wouldn't want
someone to have to do this twice. I personally like the idea of more
descriptive log levels, but have not had a chance to map this out
myself.

> Linux version contains several logging function like upslogx, upslog_with_errno, upsdebugx, etc. All of them call vupslog function inside. This function can write messages into STDERR stream and send them into syslog (writing to a log file is not required because Linux has syslogd, logrotation, etc.).
>
> Windows port contains the same function. It can write messages into STDERR stream too, but as there is no native syslog support in Windows, Windows port provides special implementation of syslog function. It sends message via named pipe into a service called nut.exe (NUT service). The service writes them into Windows Event Log. In other words, NUT service provides simple "syslogd" feature.
>
> This solution has different issues.
>
> 1. Sure, it is common recommendation to use Windows Event Log in Windows world as replacement of Linux syslog. I think it is not proper equivalent: Windows Event Log is more complex to use; it designed to log periodical significant events, not heavy debugging output; it is not useful for direct logs analysis, quite slow and problematic with support of users.
>
> 2. If NUT service is not available (stopped or crashed), logging is stopped automatically. I think that important software like NUT has to provide more reliable logging.
>
> 3. Unfortunately, "syslogd" implementation is unstable in Windows port. It eats syslog messages periodically and they do not appear in Windows Event Log (I did not find the bug yet, but something is wrong with new pipe connections).
>
> 4. Design of Windows Event Log bases on event identifiers and message table resources. Different notifications should have different Event ID and message body. Now, Windows port supports one predefined Event ID for formatted message "%1". It means that all messages have equal Event ID. This is not a big problem, but removes charm of Windows Event Log: no way to filter Windows port events and it is not transparent to external software analyzers. Moreover, Windows Event Log locks binaries with message table resources, so, nut.exe file becomes locked if somebody is viewing NUT messages in Event Viewer.
>
> 5. Currently, mixed NUT environment (Linux version and current Windows port) provides mixed logging: syslog and Windows Event Log. The people who use such environments will try to have single mechanism (just imagine). They will have to install additional software converters from syslog to Windows Event Log or vice versa and it looks at least strange.

I certainly can't speak for all of the NUT users, but I suspect that
most would care more about the server side of NUT. They might dive
into client-side logs if something goes wrong, but I don't see many
users trying to continuously aggregate the logs from NUT Windows
clients.

> It seems that "syslogd" feature is elimination candidate and it should be removed from NUT service.
> I think that Windows port may provide file-based logging instead of "syslogd" implementation. Every program may write to their own file or in the common one. File logging is more stable; the programs do not depend from "central point" availability. Thereby, the users can simply send log files to developers during support cases and nothing more.

I am not an expert in Windows file handling, but I suspect that there
will need to be coordination between the daemons for things like log
rotation. One log file per component might work better in that case.

> It would be nice to provide classic implementation of syslog function in Windows port (for example, RFC 5424). This is not hard and such support resolve mixed NUT environment issue.
>
> On the other hand, Windows administrators are used to looking into Windows Event Log for important notifications. It would be great if Windows port can additionally write the significant messages to Windows Event Log. Every program may write them directly in special points of the source code. The messages can be located in resource DLL module to avoid "lock" problem.
>
> I suggests the following logging architecture:
>
> Linux version          Windows version
> -------------------    ----------------------------
> STDERR (no changes)    STDERR (no changes)
> syslog (no changes)    syslog (new implementation)
>                        filelog (new implementation)
>                        Windows Event Log (redesign)
>
> WINDOWS PORT ARCHITECTURE
> -------------------------
> NUT drivers are daemons in Linux version. They are starting and are stopping via controller called upsdrvctl (it is not daemon). Network server (upsd) and monitoring client (upsmon) are daemons too. Special "init.d" scripts start and stop all the programs in required order.
>
> Linux architecture is following:
>
> ---------------------------------
>          upsmon (daemon)
> ---------------------------------
>           upsd (daemon)
> ---------------------------------
>         Drivers (daemons)
> apcsmart  blazer  ...  usbhid-ups
> --------  ------  ---  ----------
>
> In contrast to Linux version, Windows port contains NUT service that is "central point". The main idea of NUT service is providing functions like "syslogd" (I described it above) and "init.d", because their equivalents are absent in Windows world.
>
> Drivers, network server and monitoring client are implemented as console applications. The service just starts and stops them as hidden processes in required order.
>
> Windows port architecture is following:
>
> ---------------------------------
>           nut (service)
> ---------------------------------
>   upsmon (console application)
> ---------------------------------
>    upsd (console application)
> ---------------------------------
>  Drivers (console applications)
> apcsmart  blazer  ...  usbhid-ups
> --------  ------  ---  ----------
>
> This design has some issues.
>
> 1. NUT service does not control the programs it has started. It is just a kind of starter. In the case of any failure (for example, upsd has crashed or terminated unexpectedly), the service does not restart failed program and thinks that everything is OK, but the system becomes unavailable. I have seen such behavior several times and I think that Windows port has to provide the mechanisms making the whole system more stable (watchdogs, etc.) because functionality of NUT is very important. For example, NUT service can be watchdog for all programs it has started.
>
> 2. Then upsmon executing shutdown procedure, it sends stop signal to NUT service and the service will stop everything it has started before, including upsmon. Therefore, it looks like a scheme where no main component is but chaotic calls are. It cannot be stable by default. What will be if NUT service is stopped? It seems nothing good.
>
> This is common recommendation to port Linux daemons to Windows services. I think that upsd and upsmon should be implemented as Windows services (sure, they can be started as usual applications manually). As result, NUT service is not required anymore and it can be discarded.
>
> Such scheme has many advantages:
>
> 1. It moves the responsibility of starting and stopping upsd and upsmon from NUT service to operating system.
>
> 2. Windows provides service recovery feature: in the case of failure, system will restart the service automatically. Service recovery function is a kind of watchdog I have told before. It will raise product stability and availability.
>
> 3. Additionally, the users will get standard system notifications and easy way to control and check what is running or not (via Control Panel).
>
> As there are many drivers, there is no reason to implement them as services. They can stay as the simple console applications.
>
> To control the drivers, the following function are required:
>
> 1. Start and stop the drivers at the operating system startup/shutdown (as upsdrvctl controller does).
>
> 2. Provide watchdog function for started driver. If driver process is finished by unknown reasons, it will be restarted automatically.
>
> I think a new service should be implemented for these functions or it is possible to extend upsdrvctl source code to be as service. Another way is integrate the functions into upsd service, but it is quite different from Linux version.
>
> The setup can register all the services in operating system. Moreover, it may create the dependencies between services and the system will start and stop them in required order. As result, there is no necessity to "init.d" function in NUT service.

It would be good to get feedback from others on this, since it sounds
like there are at least two options here (service for a single
upsdrvctl/watchdog, or let upsdrvctl create service entries for
drivers dynamically).

> New Windows port architecture can look like:
>
> ---------------------------------
>         upsmon (service)
> ---------------------------------
>          upsd (service)
> ---------------------------------
>   Drivers controller (service)
> ---------------------------------
>  Drivers (console applications)
> apcsmart  blazer  ...  usbhid-ups
> --------  ------  ---  ----------
>
> CONFIG FILES
> ------------
> Some parameters in configuration files must contain double backslashes. For example, "\\\\.\\COM1" should be used instead of "\\.\COM1". I am not sure, but configuration files parsing looks like a candidate to improve in Windows version. It can be moved to abstraction layer I have described above.
>
> SETUP AND BINARIES
> ------------------
> I think that currently used solution based on WIX is the best. I have found some issues, they are not critical, but should be fixed in future.
>
> 1. It seems that Windows port releases have been built under MinGW in Windows environment. Probably, the first versions were based at MSYS (not MINGW32), because setup contains files like msys-*.dll. Now, they can be removed from setup (bin/sbin directories), because no one uses them.
>
> 2. Windows port has been built with SSL support, because some binaries require libeay32.dll and ssleay32.dll. These files should be added to setup (bin/sbin directories).

Is there any way to use Netscape's NSS? There are licensing issues
with distributing OpenSSL with software built from GPL'd source code.

> 3. upsmon depends on libgcc_s_dw2-1.dll file, but it missed in sbin directory and should be added to setup too.

Is there a way to automatically check the installation for
dependencies? Something like running 'ldd upsmon' on Linux, then
verifying that the needed libraries are either part of the OS (goes
back to the previous question about minimum supported version) or are
included with the install.

On a related note, if it's not too hard, I would like to be able to
cross-compile this from Linux, at least for continuous integration (if
it is too hard to cross-compile WIX for packages, assuming that even
makes any sense). Debian has some mingw-* packages that could be used
for this, but at the moment, I think there are still a few assumptions
that the build and target platforms are the same.

> 4. bin directory contains upssched-cmd file without extension. It is WIN32 executable file and there is its duplicate in sbin directory called upssched.exe. As result, upssched-cmd file should be removed. Unfortunately, I didn’t check what is the right place of upssched.exe file, may be it should be relocated too.
>
> 5. The setup contains lib directory with libupsclient.a and libupsclient.la files. I think they are not interesting for most of users and can be removed.
>
> 6. When I tried to build Windows port branch, I have found that neon and net-snmp libraries can be linked statically. Currently, they represent as dynamic libraries with quite big size and they are exporting many useless functions. I could reduce the setup size using static linking during my experiments.

Either way, NUT will need to ensure that updates for any dependencies
are tracked closely. Linking statically will make it harder for a
sysadmin to scan for old libraries during audits.

> 7. The installed executable files should have VERSIONINFO resource by Windows Installer rules. I think it is easy to add proper version and description to every executable file.

Agreed. I do not think the Windows branches automatically update the
version string from Git the way the master branch does, but that
should not be too hard. Let me (and the list) know if you have
questions about that code.

> 8. I am not sure that StartService.bat/ StopService.bat files are necessary. They just start and stop NUT service. These actions can be performed via Control Panel.
>
> 9. Binary files contain debug information that can be stripped. It will significantly decrease setup file.

I am not opposed to stripping debug information if it is available
somewhere, but what will be debug procedure? Ideally, a developer
without access to a Windows system should be able to look at a crash
log and pinpoint the source code for the error location.

> I told before about one single setup with automatic Windows platform detection (x86 or x64). Current setup installs all files at once without any customization. The main idea is to group the files to drivers, client files and server ones. After platform detection, the setup will ask the user about working mode (standalone, netserver, netclient).
>
> If netclient mode is selected, the setup will install the client files only (upsmon, etc.).
>
> For netserver mode, the setup will install the server files (upsd, etc.). The client files can be optionally installed by additional user select.
>
> For standalone mode, the setup will install both client and server files.
>
> The most important thing is that netserver and standalone modes require driver installation. For these modes, the user can select which drivers he needs or install all. Also, the setup can provide automatic detection of available devices via nut-scanner.

I am not a fan of nut-scanner, but I think that it does "lazy linking"
against some of the libraries to simplify things on *nix systems. Your
static linking proposal might run into trouble here.

> In the end of installation, the setup writes selected configuration into *.conf files, registers all necessary services and starts them. As result, the product is ready to use.

Thanks for taking the time to write this up. Hopefully, we can get
some other users and developers to chime in, too.

-- 
- Charles Lepple