https://bugzilla.redhat.com/show_bug.cgi?id=1814682 --- Comment #49 from lsun@xxxxxxxxxxxx --- Thanks Michal for the comments. Please see my response below. (In reply to Michal Schmidt from comment #47) > (In reply to lsun from comment #32) > > As for the "systemd-units", it appears to be required by koji. I got some > > failures like below once removed it. So I'll leave it for now (please > > correct me if I am incorrect). > > https://koji.fedoraproject.org/koji/taskinfo?taskID=43163313 > > OK, in that case leaving a BuildRequires there is acceptable. > Note that the "systemd-units" package was merged into the main "systemd" > package in 2012 (before Fedora 18). Fedora packaging guidelines removed the > last reference to "systemd-units" in 2016, keeping "BuildRequires: systemd" > as the preferred way. > > There is another option. You can remove the BR and instead tell your > configure script to not autodetect the systemd units directory, using: > %configure --with-systemdsystemunitdir=%{_unitdir} > The minor advantage of this would be a smaller buildroot. > > Use whatever option you prefer there. Will update in next post and use "BuildRequires: systemd" as suggested. > > (In reply to lsun from comment #46) > > Spec URL: https://github.com/Mellanox/rshim-user-space/releases/download/rshim-2.0.3/rshim.spec > > > Requires: psmisc > > Why does the rshim.service use killall in the first place? There is: > > KillMode=process > > Why this mode? Do you need child processes to be left running in the cgroup > after the service is stopped? > If yes, commenting on it in the unit file would be nice. It's a copy/paste error. I would like to stop all processes when service stops. > > > ExecStop=/usr/bin/killall -SIGKILL rshim > > Referencing processes to kill by name is not good. It would kill unrelated > processes with the same name. > Is none of systemd's kill modes suitable for stopping the service without > additional help? > And why SIGKILL? Does it not stop on SIGTERM? Will update in next posted version with the following: - Remove the "Requires: psmisc" - Remove "ExecStop". - Use "KillMode=control-group" - Support SIGTERM [Service] Restart=always Type=forking ExecStart=-/usr/sbin/rshim $OPTIONS KillMode=control-group > > > Requires: kernel-modules-extra > > This is always going to be imperfect, because nothing guarantees that the > installed package corresponds to the actually running kernel (different > versions, variants like -debug, custom unpackaged kernels, ...). > There is precedent for depending on kernel-modules-extra in Fedora packages > (usbip, xl2tpd, ...) though. > I would just make it more explicit which module the package needs by instead > using this: > > Requires: kmod(cuse.ko) > # Hint for dnf to prefer kernel-modules-extra over > kernel-debug-modules-extra: > Suggests: kernel-modules-extra Will update in next posted version as suggested: Requires: kmod(cuse.ko) Suggests: kernel-modules-extra > > > BTW, is it necessary to call 'system("modprobe cuse");' in src/rshim.c? I > would expect the module to get autoloaded during the call to > cuse_lowlevel_setup. It seems not loading 'cuse' automatically. Below is what I got on CentOS-7 when calling cuse_lowlevel_setup() with cuse not loaded. "cuse: device not found, try 'modprobe cuse' first". -- 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 _______________________________________________ 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
