[Merge] lp:~jamesodhunt/upstart/bug-980917-reworked into lp:upstart

[Steve Langasek]

Review: Disapprove

I think there's some very unfortunate scope creep here.  My previous revision was addressing what would have been a regression for users of static /dev, due to the incorrect overmounting with devtmpfs.  This subsequent change doesn't address any problems related to the /dev/pts case at all, instead adding checks for additional devices that have never been the source of error reports.

+ if (system_check_file ("/dev/null", S_IFCHR, makedev (1, 3)) < 0)
+ needs_devtmpfs = 1;
+
+ if (system_check_file ("/dev/console", S_IFCHR, makedev (5, 1)) < 0)
+ needs_devtmpfs = 1;
+
+ if (system_check_file ("/dev/tty", S_IFCHR, makedev (5, 0)) < 0)
+ needs_devtmpfs = 1;
+

This is a completely unnecessary check.  These three device nodes are guaranteed to *always* be present at boot time.  Either they're set up by the initramfs, or they're required to be part of /dev on the root filesystem, or the system must be configured for the kernel to automount devtmpfs at boot time.  It is therefore *wrong* for upstart to take action when these device nodes are missing, as that means the system is badly broken and requiring admin intervention, and papering over one of the symptoms will only make it harder for the problem to be fixed at its source.  This adds complexity where none is needed.

+ if (system_check_file ("/dev/kmsg", S_IFCHR, makedev (1, 11)) < 0)
+ needs_devtmpfs = 1;

This is a device that could possibly be missing (it's not included in the "std" set from MAKEDEV), but it also seems optional... if missing, you just don't get messages logged, right?

I think it's borderline as to whether upstart should do anything to create /dev/kmsg.  If it's not present at boot time, chances are that it will be present soon afterwards.  On the other hand, if it's going to become "present soon afterwards", it means we have a dynamic /dev, so there's probably no *harm* in mounting /dev here.  But I think it's still orthogonal to the problem this branch was supposed to solve and should be treated separately.

So the only part of this that I would include is the fix of the mknod() argument ordering.  (Which, btw, you probably never noticed problems with when testing because the node is created by the kernel at the same time as /dev/null, so in practice it's always present by the time devtmpfs is mounted.)
-- 
https://code.launchpad.net/~jamesodhunt/upstart/bug-980917-reworked/+merge/118132
Your team Upstart Reviewers is subscribed to branch lp:upstart.