Bug 506578 - Review Request: rtkit - Realtime Policy and Watchdog Daemon
Review Request: rtkit - Realtime Policy and Watchdog Daemon
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Matthias Clasen
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-06-17 15:56 EDT by Lennart Poettering
Modified: 2009-06-19 11:55 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-06-19 11:55:30 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mclasen: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Lennart Poettering 2009-06-17 15:56:45 EDT
Spec URL: http://0pointer.de/public/rtkit.spec
SRPM URL: http://0pointer.de/public/rtkit-0.1-1.fc11.src.rpm
Description: Realtime Policy and Watchdog Daemon

This finally allows us us to securely enable RT scheduling by default for user software such as PulseAudio which really could benefit from it.

This is a dependency of upcoming PA 0.9.16.

This all depends on a kernel patch that is already in Ingo's tree on the way into mainline and Kyle wants to integrate into the Rawhide kernel very soon. On systems that lack this patch you'll just get a bit of spew in syslog, and clients won't be allowed to get RT scheduling, but nothing should fail beyond that.
Comment 1 Matthias Clasen 2009-06-18 10:41:11 EDT
Builds fine in mock.

rpmlint output:

[mclasen@planemask ~]$ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm
rtkit.x86_64: W: non-conffile-in-etc /etc/dbus-1/system.d/org.freedesktop.RealtimeKit1.conf
3 packages and 0 specfiles checked; 0 errors, 1 warnings.
Comment 2 Matthias Clasen 2009-06-18 12:13:19 EDT
package name: ok
spec file name: ok
 
[...to be continued...]
Comment 3 Matthias Clasen 2009-06-18 12:27:39 EDT
packaging guidelines: should follow 
  https://fedoraproject.org/wiki/Packaging:UsersAndGroups for adding users/groups
  - add a home dir, maybe
  - not sure the getent foo is really essential, but you need to make sure that %pre does exit with 0 even if the user/group are already there
license: ok
license field: ok, not sure if you need to add BSD there for the included sources, probably yes, with a comment explaining what it applies to, see https://fedoraproject.org/wiki/Packaging/LicensingGuidelines
license file: ok
spec language: ok
spec legible: yes
upstream sources: please put the tarball where the source url says
buildable: ok
excludearch: ok
build deps: ok
locale handling: n/a
shared libs: n/a
relocatable: n/a
directory ownership: ok

[...to be continued...]
Comment 4 Lennart Poettering 2009-06-18 12:48:45 EDT
(In reply to comment #1)

> [mclasen@planemask ~]$ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm
> rtkit.x86_64: W: non-conffile-in-etc
> /etc/dbus-1/system.d/org.freedesktop.RealtimeKit1.conf
> 3 packages and 0 specfiles checked; 0 errors, 1 warnings.  

Although this is placed in /etc the stuff in /etc/dbus-1/system.d isn't really config files. In fact I would argue that all this stuff should be moved to /usr/share in some way. But that's another story.
Comment 5 Lennart Poettering 2009-06-18 12:57:06 EDT
(In reply to comment #3)
> packaging guidelines: should follow 
>   https://fedoraproject.org/wiki/Packaging:UsersAndGroups for adding
> users/groups
>   - add a home dir, maybe
>   - not sure the getent foo is really essential, but you need to make sure that
> %pre does exit with 0 even if the user/group are already there

We use /proc as homedir (i.e. -c /proc). This is the only thing that remotely makes sense, given that rtkit chroot()s into /proc after starting up. 

The code in the spec file does almost exactly what the user/groups documentation you linked suggests, except that we use groupadd -f instead of the seperate getent check when creating the group, and for the user we check the availability with 'id' insted of getent.

I have however added a final "exit 0" now.

> license: ok
> license field: ok, not sure if you need to add BSD there for the included
> sources, probably yes, with a comment explaining what it applies to, see
> https://fedoraproject.org/wiki/Packaging/LicensingGuidelines

Fixed.

> license file: ok
> spec language: ok
> spec legible: yes
> upstream sources: please put the tarball where the source url says

Done.

> buildable: ok
> excludearch: ok
> build deps: ok
> locale handling: n/a
> shared libs: n/a
> relocatable: n/a
> directory ownership: ok
> 
> [...to be continued...]  

I have now updated the spec file in-place.
Comment 6 Matthias Clasen 2009-06-18 13:32:17 EDT
Ok, restarting with the things that got fixed:

packaging guidelines: ok now
license field: ok
upstream sources: ok
Comment 7 Matthias Clasen 2009-06-18 14:20:09 EDT
duplicate files: ok
permissions: ok, although I'm sure that somebody will re-review and complain that you didn't follow the packaging guideline to the letter...
%clean: ok
macro use: ok
permissible content: ok
large docs: n/a
%doc content: ok
headers: n/a
static libs: n/a
shared libs: n/a
pc files: n/a
devel deps: n/a
libtool archives: n/a
gui: n/a
directory ownership: ok
utf8 filenames: ok

Looking all good now, approved.
Comment 8 Lennart Poettering 2009-06-18 14:30:36 EDT
New Package CVS Request
=======================
Package Name: rtkit
Short Description: Realtime Policy and Watchdog Daemon
Owners: lennart
Branches: devel
Comment 9 Jason Tibbitts 2009-06-18 16:30:19 EDT
CVS done.

Note You need to log in before you can comment on or make changes to this bug.