https://bugzilla.redhat.com/show_bug.cgi?id=2126785 Björn Persson <bjorn@xxxxxxxxxxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(bjorn@xn--rombobj | |rn-67a.se) | |needinfo?(bjorn@xn--rombobj | |rn-67a.se) | --- Comment #22 from Björn Persson <bjorn@xxxxxxxxxxxxxxxxxxxx> --- (In reply to Darryl Bond from comment #19) > If the daemon is manually started without a relay plugged in, the daemon > exits but systemd restarts it the configured number of times. Because the service file specifies Restart=always. It would make more sense to have the daemon restarted if it crashes, but not when it will just terminate again. See "Restart=" in "man 5 systemd.service". You might want on-failure or on-abnormal, and maybe set RestartPreventExitStatus to an exit code that the daemon returns when there are no relays. > Does systemd_postun_with_restart only restart the daemon if it was running? It wouldn't hurt to have that documented, but anything else would be a bug in SystemD. At https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_systemd you'll find this policy: | On upgrade, a package may only restart a service if it is running; it may not | start it if it is off. Also, the service may not enable itself if it is | currently disabled. It would then be stupid if the very next section would tell you to use something that violates that policy. (In reply to Darryl Bond from comment #21) > I can > find nothing in systemd that will prevent the daemon attempting to run if > the USB device isn't present. That should be possible but I think it's fine to have the daemon check that itself. I just think it should be enough to do it once, not repeatedly until the restart limit is reached. > The correct fix is to fix the daemon so it keeps running without the > hardware and probe for it periodically. Polling is rarely the best solution. It wastes resources and conflicts with things like low-power modes. If a daemon initializes itself quickly enough to avoid an annoying delay, then starting it on demand is fine. When a daemon is running, it's best to have it just wait until it's woken up by some kind of event, such as a network request or a USB device being inserted. But that's getting a bit too far beyond the scope of a package review. > Please find the latest version of the spec file here: > https://github.com/darrylb123/usbrelay/blob/master/rpm/usbrelay.spec For the record, I'm now looking at this version of the spec: https://raw.githubusercontent.com/darrylb123/usbrelay/2793c5e1d6fb9875e8718a8b529a29a14a8bffc9/rpm/usbrelay.spec There's still an empty %pre scriptlet, but now it's at least visible that it's empty. I expected you to understand by yourself that "%pre" could be removed when its content was gone. Other than that, it looks like all the blocking issues have been resolved. With the leftover "%pre" removed, I think this package can be approved. I hope to be back soon with the full checklist and a formal approval. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=2126785 _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
