Ubuntu
libcgroup package

Merge lp:~serge-hallyn/ubuntu/precise/libcgroup/fix-startup-after-remove into lp:ubuntu/precise/libcgroup

  1. Precise (12.04)
  2. fix-startup-after-remove
  3. Merge into precise
Proposed by Serge Hallyn
Status: Needs review
Proposed branch: lp:~serge-hallyn/ubuntu/precise/libcgroup/fix-startup-after-remove
Merge into: lp:ubuntu/precise/libcgroup
Diff against target: 87 lines (+25/-2)
5 files modified
debian/cgroup-bin.cgconfig.upstart (+6/-1)
debian/cgroup-bin.cgred.upstart (+5/-0)
debian/cgroup-bin.libvirt-cgconfig-wait.upstart (+2/-0)
debian/cgroup-lite.upstart (+5/-1)
debian/changelog (+7/-0)
To merge this branch: bzr merge lp:~serge-hallyn/ubuntu/precise/libcgroup/fix-startup-after-remove
Reviewer Review Type Date Requested Status
Alex Muntada (community) Approve
Review via email: mp+86280@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Alex Muntada (alex.muntada) wrote :

I'm wondering why there's no "stop;" in debian/cgroup-bin.libvirt-cgconfig-wait.upstart. All instances of "exit 0;" in upstart scripts i've seen in /etc/init are preceeded by "stop;", but i'm no expert at all.

OTOH, i'm not sure if post-stop script in debian/cgroup-bin.cgconfig.upstart should also test -x /usr/sbin/cgclear, just in case.

review: Needs Information
Reply
lp:~serge-hallyn/ubuntu/precise/libcgroup/fix-startup-after-remove updated
16. By Serge Hallyn

address comments by alex.muntada - thanks for the feedback!

Revision history for this message
Serge Hallyn (serge-hallyn) wrote :

@Alex,

thanks for the review.

It's my (potentially mistaken) understanding that 'stop;' before exit is mainly for in pre-start script sections, to prevent the main action from being started. The libvirt-cgconfig-wait.upstart simply exits from the main script to allow libvirt to start (and has been doing that all along further down in the file) so that part is fine.

Your other comment I agree with, and have pushed a new tree with that addressed. The new tree also fixes the same issues in cgroup-lite.conf.

Thanks again!

Reply
Revision history for this message
Alex Muntada (alex.muntada) wrote :

Reviewed again and approving.

review: Approve
Reply

Unmerged revisions

16. By Serge Hallyn

address comments by alex.muntada - thanks for the feedback!

15. By Serge Hallyn

Don't run upstart jobs if package has been removed but not purged.
(LP: #875525)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/cgroup-bin.cgconfig.upstart'
2--- debian/cgroup-bin.cgconfig.upstart 2011-09-02 16:35:03 +0000
3+++ debian/cgroup-bin.cgconfig.upstart 2012-01-03 17:07:25 +0000
4@@ -6,6 +6,8 @@
5 console output
6
7 pre-start script
8+ test -x /usr/sbin/cgconfigparser || { stop; exit 0; }
9+
10 CREATE_DEFAULT="yes"
11 CGCONFIG=/etc/cgconfig.conf
12 if [ -r /etc/default/cgconfig ]; then
13@@ -28,6 +30,9 @@
14 end script
15
16 post-stop script
17- /usr/sbin/cgclear
18+ if [ -x /usr/sbin/cgclear ]
19+ then
20+ /usr/sbin/cgclear
21+ fi
22 umount /sys/fs/cgroup || true
23 end script
24
25=== modified file 'debian/cgroup-bin.cgred.upstart'
26--- debian/cgroup-bin.cgred.upstart 2011-02-17 18:03:17 +0000
27+++ debian/cgroup-bin.cgred.upstart 2012-01-03 17:07:25 +0000
28@@ -6,7 +6,12 @@
29 start on started cgconfig
30 stop on stopped cgconfig
31
32+pre-start script
33+ test -x /usr/sbin/cgrulesengd || { stop; exit 0; }
34+end script
35+
36 script
37+
38 # get default options
39 OPTIONS=""
40 CGRED_CONF=/etc/cgrules.conf
41
42=== modified file 'debian/cgroup-bin.libvirt-cgconfig-wait.upstart'
43--- debian/cgroup-bin.libvirt-cgconfig-wait.upstart 2011-09-02 16:35:03 +0000
44+++ debian/cgroup-bin.libvirt-cgconfig-wait.upstart 2012-01-03 17:07:25 +0000
45@@ -12,6 +12,8 @@
46 task
47
48 script
49+ test -x /usr/sbin/cgconfigparser || { exit 0; }
50+
51 # If cgconfig started before we were triggered, then we can stop
52 # so that libvirt-bin can start
53 if status cgconfig 2>/dev/null | grep -q "start/running"; then
54
55=== modified file 'debian/cgroup-lite.upstart'
56--- debian/cgroup-lite.upstart 2011-08-25 20:02:42 +0000
57+++ debian/cgroup-lite.upstart 2012-01-03 17:07:25 +0000
58@@ -4,9 +4,13 @@
59 start on mounted MOUNTPOINT=/sys
60
61 pre-start script
62+ test -x /usr/bin/cgroups-mount || { stop; exit 0; }
63 /usr/bin/cgroups-mount
64 end script
65
66 post-stop script
67- /usr/bin/cgroups-umount
68+ if [ -x /usr/bin/cgroups-umount ]
69+ then
70+ /usr/bin/cgroups-umount
71+ fi
72 end script
73
74=== modified file 'debian/changelog'
75--- debian/changelog 2011-11-22 12:26:40 +0000
76+++ debian/changelog 2012-01-03 17:07:25 +0000
77@@ -1,3 +1,10 @@
78+libcgroup (0.37.1-1ubuntu9) precise; urgency=low
79+
80+ * Don't run upstart jobs if package has been removed but not purged.
81+ (LP: #875525)
82+
83+ -- Serge Hallyn <serge.hallyn@ubuntu.com> Mon, 19 Dec 2011 12:06:46 -0600
84+
85 libcgroup (0.37.1-1ubuntu8) precise; urgency=low
86
87 * debian/cgroup-lite.cgroups-mount: don't run if cgroups are mounted by

Subscribers

People subscribed via source and target branches

to all changes: