Comment 31 for bug 575469

Revision history for this message
Steve Langasek (vorlon) wrote :

Please ship the /usr/share/recovery-mode symlink in the package itself, don't just create it in the postinst; this way it gets cleaned up on package removal.

Should the friendly-recovery job be marked as a 'task'?

+ echo $(eval_gettext "Check all file systems (will exit read-only mode)")

I think it would be clearer to say:

  Check all file systems (will remount filesystems read/write when finished)

+# FIXME: Would be a lot better to just start the mountall upstart job but
+# for some reason it hangs, so instead the code below is copy/paste from mountall

Hmm; not ideal of course, but not surprising - this job is 'start on starting mountall', so trying to start the job again will naturally fail.

I don't think this is a particularly safe thing to do, anyway. As soon as mountall is run, it will start tripping off lots of other upstart jobs, including ones that will want to claim the console, so we'll no longer be single-threaded. What happens when tty1.conf starts, and wants our VT?

Maybe this should instead be handled as a special case of 'resume', causing the job to exit with a particular exit code that gets picked up by another job that's 'start on stopped mountall and stopped friendly-recovery EXIT_STATUS=magicnum' and relaunches the menu? That seems to me that it would be more robust since our new job can then take care of stopping and restarting tty1 as needed, but that means more code needs written.

Or if the intent is to mount all filesystems *without* spawning other jobs, maybe it's better to reinvent the pre-upstart mountall.sh init script to do the mounting by hand and avoid all the events until later?

Either way, I think having the whole system start to boot while friendly-recovery is running is a problem, and we ought not upload this as-is.

Other than this, the changes look good to me. Yes, this is a bit risky for this late in the cycle, but the current recovery mode is really not very usable... it's hard to make it worse at the moment, and the design proposed here looks generally sound and least likely to cause regressions provided we get the remount question sorted out.