[Nut-upsdev] Re: [nut-commits] svn commit r731

[Arjen de Korte]

Peter Selinger wrote:

[...]

> Perhaps a better solution would be to separate reading the config
> files (upsd.conf, ups.conf) from the actual opening of local sockets.
> I don't see why conf_load() should open the driver sockets; that could
> be done later.

There is not much point in keeping conf_load() if we really want/need to
parse the configuration files at different times in the startup process
of upsd. It doesn't do more than calling a couple of functions, that
also could be called straight from server/upsd.c at the right time.

>> There is one catch when reloading though. We need to close the upsd server
>> sockets and open new ones when the uspd.conf file is reloaded, since new
>> LISTEN addresses may be specified. This is doable however, provided that
>> we're not using a low port number (less than 1024). Therefor, if you need
>> a low port number, you can't use reload anymore.
> I don't much see the point of reloading anyway. It seems better to
> kill upsd and upsmon and start new ones, after making a significant
> configuration change.

I agree. Should we do away with it? We could easily drop it from
server/conf.c without impacting a lot of other stuff.

>>> Moreover, conf_load() is now called before the chroot() and
>>> become_user(), which means that ups.conf is read by root and in the
>>> raw directory tree. This is not how it was intended; perhaps even a
>>> potential security problem.
>> It has been like that for quite some time earlier on, before we decided it
>> was better to move it to a later stage, so that it was possible to check
>> permissions on the configuration files in order to make reloading the
>> configuration possible.
> OK, but whether conf_load() is before or after chroot() affects the
> location of the configuration files! Are they in the original
> filesystem, or in the chroot-ed one? It would seem to have to be the
> latter, or else reloading won't work. 

Point taken. I had not thought about that.

Best regards, Arjen