Skip to content

Conversation

@sde1000
Copy link

@sde1000 sde1000 commented Oct 17, 2016

This is a re-work of my earlier pull request (now closed) regarding startup scripts for systems that need to support more than one DALI USB device being plugged in at a time, with stable mapping between physical device and TCP port number.

This pull request alters the Debian packaging rules to generate three separate binary packages: daliserver, daliserver-upstart and daliserver-systemd.

The daliserver package is unchanged, i.e. it contains the sysvinit startup script.

The daliserver-upstart package has an upstart configuration file. This configuration file runs an instance of daliserver when a DALI USB device is plugged in, and stops it when the device is unplugged. A lookup table in /etc/default/daliserver-upstart enables daliserver options to be set based on the serial number of the device that is plugged in.

The daliserver-systemd package has a systemd unit template file and a udev rule that runs an instance of daliserver when a DALI USB device is plugged in, and stops it when the device is unplugged. A lookup table in /etc/default/daliserver-systemd enables daliserver options to be set based on the serial number of the device that is plugged in. A helper script in /usr/lib/daliserver-systemd looks up the serial number, bus number and device number based on the systemd template instance name.

@onitake
Copy link
Owner

onitake commented Oct 17, 2016

Woah, nice work.
I'll still have to review and test it, but it looks like this is a good solution.
I wonder though if separate packages are really needed. Isn't it common to ship startup scripts for all supported init systems within a single package?
There may be conflicts though, as systemd picks up sysvinit scripts and automatically creates corresponding units internally.

@sde1000
Copy link
Author

sde1000 commented Oct 17, 2016

I couldn't figure out how to make a single package that did everything. systemd automatically creating a unit for the sysvinit script is one issue; having a systemd dependency in the udev rules file (which isn't required for systems using upstart or sysvinit) is another.

@onitake
Copy link
Owner

onitake commented Oct 17, 2016

You can have a choice of requirements in your dependency list, for example:
sysvinit-core | systemd | upstart
I'm not sure how other projects handle this, but it's likely something similar.

@sde1000
Copy link
Author

sde1000 commented Oct 17, 2016

Fiddling with the Debian package dependencies won't help in this case. Having /etc/init.d/daliserver in the package at the same time as /lib/systemd/system/daliserver-systemd@.service and the udev rule would mean that daliserver would be started with the default arguments by the sysvinit script (trying to drive the first device it found) and would also be started with appropriate arguments to -u for each DALI USB device attached to the system. The ordering of what would happen here is not well-defined, and at least one of those invocations of daliserver would fail.

@onitake
Copy link
Owner

onitake commented Oct 18, 2016

Good point - I came to the same conclusion after contemplating it a bit.
I still think splitting init scripts out into separate packages is wrong, though.
How about using debconf to present a dialogue to the user?

@sde1000
Copy link
Author

sde1000 commented Oct 18, 2016

If you can figure out how to make it work, go ahead. What questions would debconf ask, and what would it do after they were answered?

@onitake
Copy link
Owner

onitake commented Oct 18, 2016

My suggestion is something like this:

Would you like to run a separate instance of daliserver for each connected DALIUSB device?
Note that you need to configure a different TCP port for each instance.

If selecting yes, it could then ask for the port or something.

@sde1000
Copy link
Author

sde1000 commented Oct 19, 2016

I think that question is pointless because the answer will universally be "yes" - and then on a sysvinit system you would have to say "sorry, not possible".

Alters the Debian packaging rules to generate three separate binary
packages: daliserver, daliserver-upstart and daliserver-systemd.

The daliserver package is unchanged, i.e. it contains the
debhelper-generated sysvinit startup script.

The daliserver-upstart package has an upstart configuration file.  This
configuration file runs an instance of daliserver when a DALI USB device
is plugged in, and stops it when the device is unplugged.  A lookup
table in /etc/default/daliserver-upstart enables daliserver options to
be set based on the serial number of the device that is plugged in.

The daliserver-systemd package has a systemd unit template file and a
udev rule that runs an instance of daliserver when a DALI USB device is
plugged in, and stops it when the device is unplugged.  A lookup table
in /etc/default/daliserver-systemd enables daliserver options to be set
based on the serial number of the device that is plugged in.  A helper
script in /usr/lib/daliserver-systemd looks up the serial number, bus
number and device number based on the systemd template instance name.
Useful references:
 - systemd/systemd#7109
 - systemd/systemd#10321
 - systemd/systemd@dcebc9b

Quick summary: there are still escaping-related misfeatures in systemd
that can't be worked around using systemd-escape.  For our particular
use-case, @poettering has implemented a workaround that happens to do
what we want; this commit switches over to using it (although this
introduces a requirement that we use systemd version 236 or later).  The
general case is still broken.
@sde1000
Copy link
Author

sde1000 commented Feb 5, 2019

I've updated this pull request: I've rebased to current master, and I've fixed an escaping-related issue that stopped the systemd support working sometime after systemd release 229. See systemd#7109 and systemd#10321 for details.

@onitake
Copy link
Owner

onitake commented Feb 11, 2024

Hi @sde1000 , I just pushed a new branch that adds a basic systemd unit.
I'd like to release this first, so we get support for the common case with only one USB adapter out of the box.

Once that's merged, we can revisit your proposal of multi-device support.
One option I'm contemplating is dropping SysV init support altogether. I get that Devuan users won't be happy, but I don't want to keep special-casing new features. Are you using something else than systemd?

With only a systemd unit in place, I think it would be much simpler to just document how to create multiple versions of the unit that match different USB adapters, and configure the TCP port in each of them statically. I'd also drop /dev/default/daliserver in this case.

@sde1000
Copy link
Author

sde1000 commented Feb 11, 2024

I'm using systemd; I think I'm still using packages generated according to this pull request.

@s3lph
Copy link
Contributor

s3lph commented Feb 14, 2024

Having /etc/init.d/daliserver in the package at the same time as /lib/systemd/system/daliserver-systemd@.service and the udev rule would mean that daliserver would be started with the default arguments by the sysvinit script (trying to drive the first device it found) and would also be started with appropriate arguments to -u for each DALI USB device attached to the system.

The creation of a daliserver.service unit from systemd-sysv-generator can be prevented if daliserver.service is shipped as a masked unit (i.e. a symlink to /dev/null).

Another option would be to ship daliserver.service as a dummy unit running /bin/true, and have the template units bind to it. This would e.g. also provide the option to start or stop all daliserver instances. This approach is not uncommon, from the top of my head I can think at least of the openvpn package doing this.

@onitake
Copy link
Owner

onitake commented Feb 14, 2024

The systemd.unit man page says that an empty unit (size 0) is also treated as masked.
That could be an alternative to creating symlinks via debian/daliserver.links.

In lack of a better (read: more modern) approach, I'm inclined to accept your proposal of using /etc/default/daliserver to map device serial numbers to TCP ports, however I'd like to have some modifications to the PR:

  1. Remove the upstart units. I think upstart has run its course, and I don't want to support many different init systems.
  2. Remove the separate packages. There should be only one daliserver package that contains all files.
  3. Rename the debian/daliserver-systemd* files to debian/daliserver*
  4. Remove the extra debian/*.postint etc. maintainer scripts. I've upgraded the compat level to 10, and that will automatically take care of setting up the init scripts/units.
  5. Likewise, remove the overrides from debian/rules. compat=10 will already take care of everything.
  6. Rebase on the current master branch.
  7. Merge the comments in debian/daliserver.default and remove the log file parameter from DALISERVER_OPTS. The default should be to use the systemd journal, not extra log files. If someone wants them, they can customize the defaults on their system as needed. Your serial number case example is good, please keep that part.
  8. Install daliserver-systemd-start to /usr/libexec/daliserver/ instead of /usr/lib - the EFS designates this directory for package-internal scripts.

I get that producing journals based on the unit instances (which may not have persistent IDs) is somewhat suboptimal, but I really don't like log files if they can be avoided. It would be perfect if we could use the serial number as instance ID instead, but libusb doesn't make it easy to filter by device attributes without opening each device.

Please let me know if you'd prefer that I make these modifications myself instead, I'll rebuild the PR in that case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants