Merge lp:~jamesodhunt/ubuntu/trusty/sysvinit/log-open-files-on-shutdown into lp:ubuntu/trusty/sysvinit

Proposed by James Hunt
Status: Work in progress
Proposed branch: lp:~jamesodhunt/ubuntu/trusty/sysvinit/log-open-files-on-shutdown
Merge into: lp:ubuntu/trusty/sysvinit
Diff against target: 86 lines (+50/-0)
3 files modified
debian/changelog (+10/-0)
debian/src/initscripts/etc/init.d/umountfs (+20/-0)
debian/src/initscripts/etc/init.d/umountroot (+20/-0)
To merge this branch: bzr merge lp:~jamesodhunt/ubuntu/trusty/sysvinit/log-open-files-on-shutdown
Reviewer Review Type Date Requested Status
Steve Langasek Needs Fixing
Review via email: mp+196141@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Steve Langasek (vorlon) wrote :

Hi James,

Apologies again for the delay in reviewing. Some comments:
 - lsof and awk are both in /usr/bin. If /usr is on a separate filesystem, this function is going to fail from /etc/init.d/umountroot. We should really try to make it more robust. Should we be using fuser -vm here (part of psmisc, more likely to be installed), instead of lsof? fuser is located in /bin, and more directly addresses what we're looking to find out.
 - Your /etc/init.d/umountroot change defines the do_log() function but does not invoke it. This looks like a bug to me.
 - If we're going to print anything to the console, then we should also include some sort of pause to give the user a chance to react before the system hard-reboots out from under them. Perhaps either a 'sleep 5' or a 'Press any key to continue' && read in the error case.
 - I think we want to invoke this logging code *only* when there is a failure unmounting the filesystem or mounting the root filesystem read-write. If the root filesystem is successfully mounted ro, and all the other filesystems are successfully unmounted, then there's no reason for us to care what files are open, and spamming this to the console on shutdown (because there will always be a non-zero number of files) is going to be unwelcome noise. And in all the cases where it matters, we know in fact that the root filesystem is *not* remounted ro, so we can reliably log to the root filesystem instead of the console which is better for debugging purposes.

Can you please adjust your patch in light of the above?

review: Needs Fixing

Unmerged revisions

193. By James Hunt

* debian/src/initscripts/etc/init.d/umountfs: Log open files on root
  filesystem just prior to unmounting filesystems to aid diagnosis of
  shutdown issues.
* debian/src/initscripts/etc/init.d/umountroot: Check again for open files
  and log to console.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2013-06-04 08:53:47 +0000
3+++ debian/changelog 2013-11-21 15:31:05 +0000
4@@ -1,3 +1,13 @@
5+sysvinit (2.88dsf-41ubuntu4) UNRELEASED; urgency=low
6+
7+ * debian/src/initscripts/etc/init.d/umountfs: Log open files on root
8+ filesystem just prior to unmounting filesystems to aid diagnosis of
9+ shutdown issues.
10+ * debian/src/initscripts/etc/init.d/umountroot: Check again for open files
11+ and log to console.
12+
13+ -- James Hunt <james.hunt@ubuntu.com> Thu, 21 Nov 2013 15:19:02 +0000
14+
15 sysvinit (2.88dsf-41ubuntu3) saucy; urgency=low
16
17 * Exit the ondemand script silently if we are on a system that has
18
19=== modified file 'debian/src/initscripts/etc/init.d/umountfs'
20--- debian/src/initscripts/etc/init.d/umountfs 2013-05-17 06:03:10 +0000
21+++ debian/src/initscripts/etc/init.d/umountfs 2013-11-21 15:31:05 +0000
22@@ -16,6 +16,25 @@
23
24 umask 022
25
26+# Before unmounting filesystems, log all files open for writing on the root
27+# filesystem for possible analysis on the next boot since such files
28+# will cause an unclean shutdown.
29+do_log () {
30+ if command -v lsof >/dev/null 2>&1; then
31+ # capture list of open files on root mountpoint as quickly as possible
32+ output=$(lsof -lnPR /)
33+
34+ # look for first occurence of file open for writing
35+ open_files=$(echo "$output"|awk '$5 ~ /^[0-9][0-9]*[uw]/ {print;exit 0}')
36+
37+ if [ -n "$open_files" ]; then
38+ # log full lsof output
39+ echo "$output" > /var/log/shutdown-lsof.log 2>/dev/null
40+ sync
41+ fi
42+ fi
43+}
44+
45 do_stop () {
46 PROTECTED_MOUNTS="$(sed -n ':a;/^[^ ]* \/ /!{H;n;ba};{H;s/.*//;x;s/\n//;p}' /proc/mounts)"
47 WEAK_MTPTS="" # be gentle, don't use force
48@@ -119,6 +138,7 @@
49 exit 3
50 ;;
51 stop)
52+ do_log
53 do_stop
54 ;;
55 *)
56
57=== modified file 'debian/src/initscripts/etc/init.d/umountroot'
58--- debian/src/initscripts/etc/init.d/umountroot 2013-05-17 06:03:10 +0000
59+++ debian/src/initscripts/etc/init.d/umountroot 2013-11-21 15:31:05 +0000
60@@ -14,6 +14,26 @@
61
62 . /lib/lsb/init-functions
63
64+# Before unmounting root, log all files open for writing to the console
65+# (since no other locations are available for writing and root may be
66+# read-only).
67+do_log () {
68+ if command -v lsof >/dev/null 2>&1; then
69+ # capture list of open files on root mountpoint as quickly as possible
70+ output=$(lsof -lnPR /)
71+ open_files=$(echo "$output"|awk '$5 ~ /^[0-9][0-9]*[uw]/ {print}')
72+
73+ if [ -n "$open_files" ]; then
74+ echo "INFO: files open at shutdown"
75+ # show lsof header
76+ echo "$output"|head -n1
77+ # just show the open files to avoid completely
78+ # flooding the console.
79+ echo "$open_files" > /dev/console
80+ fi
81+ fi
82+}
83+
84 do_stop () {
85 [ "$VERBOSE" = no ] || log_action_begin_msg "Mounting root filesystem read-only"
86 # Ask init to re-exec itself before we go down if it has been

Subscribers

People subscribed via source and target branches

to all changes: