[UIFe] [FFe] recovery mode mounts filesystems read-write rather than read-only

They are mounted RW because of recovery-mode options like dpkg "Repair broken packages" (actually an upgrade as per bug #452222). There is an fsck option that is invisible on Ubuntu 10.04 (Lucid Lynx) due to bug #566200. It creates a flag file, /forcefsck, that is checked for by the mountall.conf Upstart job to force a fsck at next boot. Creating the flag file probably is relatively safe as / gets checked before mounting.

A workaround to boot with the root mounted read-only is to edit the grub command line so it includes "init=/bin/bash". Obviously non-ideal, but it lets me run zerofree at least.

Ok, so looking at the current code, the problem is that friendly-recovery is started after mountall mounted / as read-write, actually everything that's in my fstab is mounted when friendly-reocvery starts.

Most of the options also require read/write access to the filesystem and indeed need pretty much everything to be mounted (in the case where /usr or /var are on another partition).

I can perfectly see cases where you want to drop into a shell to fix a broken filesystem before it gets mounted or modified any further. I don't think the current friendly-recovery is the right place to do that though.

Ideally I'd like to see the recovery boot sequence works like this:
 - Select recovery in grub (as it's currently)
 - Start in verbose mode and without splash (as it's currently)
 - When mountall is triggered, it'll know it's in recovery mode and will trigger a first stage recovery screen asking the user if he wants to drop into a root shell or mount the filesystems. (that'd be the new part)
 - If the user chooses to let mountall mount the filesystems, then the usual friendly-recovery would start and let the user repair dpkg, make some free space, repair grub, ... (as usual)

I'd think adding this extra step to allow read/only recovery is indeed something we need, but that's a change in a core part of the boot sequence not to mention a UI change (as we'd get another prompt that'd need to be translated and documented) so if we choose to go with this, it'll have to be deferred to P (I'm happy to implement it then).

The alternative is to switch the current code to read-only, essentially making the recovery mode bypass mountall entirely and then modifying all the friendly-recovery scripts to trigger mountall when needed. I wouldn't recommend doing this as it'd be a pretty significant change in behavior and I'm not particularly fond of the idea of bypassing mountall especially this late in the cycle.

Do we really need to change the ui, here? What if we used the existing friendly-recovery options to decide whether or not to tell mountall to mount read-write? I guess the choice of recovery options that require read-write disk is rather clear.

I also am not nearly as worried about the UI change as I would be about the new code being written for mountall (which would violate FFe). We would need a UI freeze exception, true, but UI Freeze is primarily about *coordinating* changes, not about forbidding them entirely - and this is a significant enough bug in the recovery mode that I think all users are better off with a possibly-untranslated facility than with not having the facility at all. Whereas the mountall changes are risky and could make recovery mode less stable / usable for everyone.

Can you not tell mountall to force ro instead of rw? It's just that ro->rw remounts are easy and safe, while rw->ro remounts are not (you have no guarantee that the filesystem is actually ro in this state). To me it would have seemed like an obvious thing to mount the filesystems ro and remount them rw when needed, and there are lots of things where rw isn't needed. If it would be a big change, then it seems obvious to me that it's been doing it wrong from the outset.

I just had a quick look at mountall and can't find any easy way to mount everything read only. So bypassing mountall at this point means having only the root filesystem mounted in read-only mode, which should work in most cases when people stick to default partitioning.
It'll lead to a non working rescue environment for people who have split their /usr or /var to another partition though.

A quick look at the current scripts gives me that list:
 - apt-snapshots (not exactly sure what it needs)
 - clean (requires at least write access to /var)
 - dpkg (requires write access to everything)
 - failsafeX (probably requires everything to be mounted, it's calling gdm directly and won't work with lightdm)
 - fsck (requires write access to / as it's just touching /forcefsck)
 - grub (requires write access to /boot)
 - netroot (requires write access to /var/lib/dhcp/ for the dhclient lease file)
 - root (safe to run in read-only mode)

So to avoid introducing another script prompting the user about either getting an early root shell or starting to mount everything read/write and start friendly-recovery, I think we have two ways of doing it:

1)
  - Add a flag to mountall to force it to mount everything read-only and another to remount everything as read/write.
  - We could then have mountall's init script check if we are in a rescue environment, if that's the case, mount all local storage read-only and ignore any network storage.
  - Then modify all existing friendly-recovery scripts to ask mountall to remount everything read/write and write a new one for a "read-only root shell" that'd only show up if none of the others have been called yet.
  - Exiting friendly-recovery should also trigger a remount of everything in read/write so the system can boot properly.

2)
  - Move all of friendly-recovery out of /usr
  - Modify mountall's script not to do anything when in recovery mode unless a specific environment variable is set
  - Then modify all existing friendly-recovery scripts to call mountall to mount everything read/write and write a new one for a "read-only root shell" that'd only show up if none of the others have been called yet.
  - Exiting friendly-recovery should also trigger mountall in case none of the other scripts did it already.

So far I only had a quick look at mntctl and mountall itself and I couldn't find any flag to force it to mount everything read-only.

Option 2 would also get us rid of bug 234409 in the process.

I still like the idea of having read-only be an option of mountall as it'd be pretty handy to have in other cases but I guess option 2 is the most likely to be implemented for Oneiric. It's still a pretty significant amount of changes that'd be required for it but at least it doesn't require modifying mountall (only its upstart jobs).

Working on implementing option 2, I noticed that friendly-recovery depends on whiptail which is in /usr/bin.
We're not going to be moving whiptail and libnewt out of /usr this cycle but it's definitely something that should happen for next cycle (opening bug tasks for these two).

For now, friendly-recovery itself will be in /lib and will simply drop to a root shell if /urs/bin/whiptail can't be accessed.

Whenever we move whiptail to /bin we should also make sure nothing currently refers to it with an hardcoded path as friendly-recovery used to do.

Sorry for the mess above, I just remembered we already had bug 234409 for the /usr part of this bug.

Ok, so I have the initial implementation of the fix.

The current chosen solution is:
 - Move friendly-recovery to /lib/recovery-mode
 - Make it ship its own upstart job starting on "recovery-mode"
 - Update initramfs-tools to support a new boot parameter "startup-event" that's passed to init
 - Update grub2 to have update-grub set startup-event=recovery-mode instead of setting "single" for recovery entries
 - Update upstart to stop calling friendly-recovery from its rcS.conf job

I also updated all the scripts that generate output to wait for user confirmation before going back to the menu.
I introduced a new "remount" script that will trigger mountall to remount all the filesystems in read-write mode.
The "fsck" script has also been changed to no longer require a reboot and to simply trigger the "remount" script but forcing fsck.

Complete changelog for friendly-recovery is:
  * Move everything to /lib/recovery-mode/ (LP: #xxxxxx)
  * Don't use the fullpath to whiptail
  * Small packaging refresh:
    - Bump standard to 3.9.2, no change needed
    - Drop simple patchsys (no patches)
    - No need to depend on bash (essential package)
    - Add ${misc:Depends} to dependencies
  * Add new init script starting on recovery-mode
  * If whiptail can't be found, just start sulogin
  * Add a script to start mountall (remount everything read/write)
  * Export READONLY to all scripts so they can test on the read/write state
  * Disable most scripts when the system is read only
  * Change file system check to happen when remount is called
  * Wait after all scripts returning output so the user can read it

Test packages are available in my experimental PPA at: https://launchpad.net/~stgraber/+archive/experimental
(please note it might take a while until everything is done building)

Attaching the debdiffs:
 - upstart: Remove any reference to friendly-recovery from rcS.conf
 - initramfs-tools: Add support for startup-event parameter (forwarded to init)
 - grub2: Replace "single" by "startup-event=recovery-mode"
 - friendly-recovery: Move from /usr to /lib, start from recovery-mode instead of single, split menu into read-only and read-write mode. See above for full changelog

Below are the updated changelogs:

upstart:
  * Update rcS.conf to only start single user mode but not friendly-recovery.
    (LP: #575469)

initramfs-tools:
  * init: When given startup-event on cmdline, forward it to init.
    This is needed for friendly-recovery. (LP: #575469)

grub2:
  * Replace "single" by "startup-event=recovery-mode"
    to trigger friendly-recovery (LP: #575469)

friendly-recovery:
  * Move everything to /lib/recovery-mode/ (LP: #234409)
  * Don't use the fullpath to whiptail
  * Add new init script starting on recovery-mode (LP: #459376)
  * If whiptail can't be found, just start sulogin
  * Add a script to start mountall (remount everything read/write)
    (LP: #575469, LP: #651782)
  * Export READONLY to all scripts so they can test on the read/write state
  * Disable most scripts when the system is read only
  * Change file system check to happen when remount is called
  * Wait after all scripts returning output so the user can read it
  * Small packaging refresh:
    - Bump standard to 3.9.2, no change needed
    - Drop simple patchsys (no patches)
    - No need to depend on bash (essential package)
    - Add ${misc:Depends} to dependencies

Feature Freeze exception:
 - Adds a new feature to initramfs-tools and use it in grub2 to allow passing the "startup-event" flag to upstart. This is required to start friendly-recovery using upstart before any of the regular "on startup" jobs. Otherwise we'd have had to patch all the upstart jobs declaring "start on startup" or avoid using upstart entirely.
 - Add support for read-only and read/write recovery into friendly-recovery. Previous friendly-recovery would essentially start once most of the system services are already running and using read/write access to the FS.

UI Freeze Exception:
 - The recovery menu after this change is quite different. Instead of getting all the available options, the user is offered with only the subset that can run in read-only mode and an extra option to switch to read/write mode. Selecting that option then shows the usual set of options. I don't know if our documentation contains screenshot of the recovery mode, if it does, then these will need updating.

The attachment "upstart.debdiff" of this bug report has been identified as being a patch in the form of a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. In the event that this is in fact not a patch you can resolve this situation by removing the tag 'patch' from the bug report and editing the attachment so that it is not flagged as a patch. Additionally, if you are member of the ubuntu-sponsors please also unsubscribe the team from this bug report.

[This is an automated message performed by a Launchpad user owned by Brian Murray. Please contact him regarding any issues with the action taken in this bug report.]

= Code review =

In general I think this is broadly right; recovery software clearly doesn't belong in /usr, and I can see why it simplifies things to have this guaranteeably start before mountall rather than having to change mountall.

However, the only other 'start on startup' job I see is hostname, and I think we ought to run that even in recovery mode. Any job that's 'start on startup' already has to run with a read-only root filesystem, since that's how the initramfs typically leaves things and it isn't until mountall runs that the root filesystem becomes read-write. Thus, why not simply make friendly-recovery be 'start on starting mountall'? That way, you could drop the grub2 and initramfs-tools changes entirely.

Can we make /usr/share/recovery-mode a symlink to /lib/recovery-mode for compatibility, just in case third-party packages have added entries here? The postinst will need to move any remaining files from /usr/share/recovery-mode/ to /lib/recovery-mode/ and then install a symlink, since dpkg won't replace a directory with a symlink by itself.

I think I'd slightly prefer the Upstart job to look like this:

  script
          if [ -x /lib/recovery-mode/recovery-menu ]; then
                  exec /lib/recovery-mode/recovery-menu
          else
                  exec /sbin/sulogin
          fi
  end script

  post-stop exec initctl emit startup

That way Upstart is directly supervising the process that's actually doing the work; it's a detail but it seems more logical to me.

"Remount / in read/write" should read "Remount / read/write". Also, I think it would be helpful to indicate somewhere in the UI that this enables more recovery mode options, otherwise people will be rather confused why some option they were vaguely looking for doesn't appear. Perhaps check this with a designer?

I feel there ought to be some dependency relation between upstart and friendly-recovery, perhaps Breaks in one direction or both, to make sure partial upgrades don't leave people without recovery mode. This would be simpler if you could take my 'start on starting mountall' suggestion so that we don't also need dependency relations with grub-common and initramfs-tools.

The following items apply only if you decide not to take my 'start on starting mountall' suggestion above:

What happens if friendly-recovery isn't installed? After all, it's only Priority: standard. Can we use *both* single and startup-event=recovery-mode safely; or, failing that, have the relevant grub.d script test for the presence of /lib/recovery-mode/recovery-menu and add either single or startup-event=recovery-mode as appropriate? I think it's important to test this both with and without friendly-recovery installed.

To reduce code duplication, I'd prefer the initramfs-tools code to do something like:

  exec run-init ${rootmnt} ${init} "$@" ${STARTUP_EVENT:+--startup-event=$STARTUP_EVENT} ...

= Feature freeze exception =

On the one hand, it would be lovely to get this in: the bug makes recovery mode unusable in a significant set of situations, and that's serious. On the other, I don't like contemplating something unforeseen making recovery mode worse. So I'm on the...

Interesting, I didn't think of "start on starting mountall", that seems like a good way around a good part of my problem :)
Thanks for all the other comments, I'll update the debdiffs accordingly.

Hmm, thinking a bit more about it, I think we'll still need a change to update-grub, replacing "single" by "rescue" so we can then have the friendly-recovery look for "rescue" on the command line.

"single" is already caught by rc-sysvinit.conf and triggers a root shell, that'd make the resume option of friendly-recovery start a root shell rather than actually resuming the standard boot sequence. I don't think that's what we want.

So I think it'd be reasonable to:
 - Replace "single" by "rescue" in update-grub
 - Have friendly-recovery changed to "start on starting mountall" and run only if rescue is found in cmdline

This way we can still drop the initramfs-tools change and keep a working single user mode (if a user explicitly boot appending single to the command line).

We'd then need Breaks added to:
 - upstart (breaks old friendly-recovery that needed rcS.conf to start)
 - grub2 (breaks old friendly-recovery that needed single to start)

New friendly-recovery debdiff including:
 - Addition of Breaks entry for upstart and grub2
 - Postinst script to move any script that's in /usr/share/recovery-mode and replace by a symlink
 - Change upstart script to use "start on starting mountall" and check for "recovery" in cmdline
 - Change "Remount / in read/write" by "Remount / read/write"
 - Replace menu title by "Recovery Menu (limited read-only menu)" by default, changing to "Recovery Menu" when remounting the file systems read/write

New debdiff for grub2, moving my change into its own patch, replace "single" by "recovery".
Add Breaks on the old friendly-recovery

New upstart debdiff, added a "Breaks" entry for the old friendly-recovery

No more changes to initramfs-tools are required. The changes I just pasted haven't been tested yet as I'm busy using my VMs for other things. I'll be pushing these to my PPA now and will have them tested later tonight.

Update debdiff for grub2, replacing "single" by "recovery" only when friendly recovery is present.

Is this all needed and appropriate for post-beta design changes. I don't pretend to understand all the moving parts here, but renaming single user mode sounds a bit scary.

Yes it's quite a few changes to introduce pretty late in the cycle, the main reason though is getting recovery mode to actually act like one.
What we currently have basically shows you friendly-recovery at the end of the boot sequence when most of your services are already running and file systems are already mounted. I think we definitely need to fix that to make our recovery boot useful.

The renaming from single to recovery is needed to avoid additional changes into upstart, I think it's a good idea to split actual "single user mode" and "recovery mode". This essentially gives us:
 - when booting with "single" only => start some services + sysvinit scripts for runlevel S and give the user a root shell
 - when booting with "recovery" only => start friendly-recovery with a read-only filesystem, on resume, just continue a regular boot sequence
 - when booting with "single" and "recovery" => start friendly-recovery, on resume continue in single mode and give the user a root shell

I think this makes sense and the changes to grub and upstart are really limited and I think are pretty safe.
The changes to friendly-recovery itself are quite substantial to implement the read-only mode and switch to read/write but it's very localized change that affects only friendly-recovery and that I think can't get us a worse experience than what we had in the past.

So are you proposing no "single user" for Oneiric and add a proper single user
back for P (since it's being renamed)?

Well, the patches above essentially split single user and recovery as it was the easiest way of making the "resume" option of friendly-recovery work properly.

So if we think the changes I proposed are reasonable we'll get both a proper recovery mode and some kind of single user mode for Oneiric. With single user mode here meaning, the boot sequence ends with a root shell. I don't think it strictly matches the regular definition of single user mode (as IIRC you're not supposed to start networking or services in single user mode but upstart does it anyway).

For P, we could then spend a bit of time trying to improve upstart's behaviour in single mode and make the distinction between recovery and single clearer to the user (didn't want to change strings in grub for Oneiric).

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.

Indeed, the upstart job should be a task, updated in my copy.
Sadly the upstart job doesn't work at the moment, I tried a few variations without much luck and e-mailed James Hunt to see if "start on starting mountall" is supposed to work at all in this particular case.

It seems like I'd need to start friendly-recovery from the pre-start script if I want to make sure mountall doesn't start or I'd need to do a "stop $JOB" to prevent it from starting. Neither of these work, both successfully block mountall but I end up with an empty console and no recovery menu.

That FIXME indeed doesn't make much sense anymore, I added it before I switched to "start on starting".
I think for now it's safe to just keep the current duplicate code (that had to be changed quite a bit anyway) and I'll change the FIXME for another comment saying why we're doing it this way.

I'll have these changed in my local copy and wait for feedback from James on getting the upstart job working. Worst case, I'll revert to using the startup-event boot parameter which needs a change in initramfs-tools but works perfectly.

Oh, forgot to respond to your comment about:
+ echo $(eval_gettext "Check all file systems (will exit read-only mode)")

The reason not to go with something longer like "Check all file systems (will remount filesystems read/write when finished)" was that screen space is limited and whiptail doesn't seem to wrap properly (text just overflows).
I had to change a few strings to be shorted so they don't mess up the menu (at least on my system), would be nice if we could teach whiptail to properly wrap.

Refreshed patches after IRC discussion and decision to go with startup-event for now. This re-introduce a change to initramfs-tools.

Please ignore previous friendly-recovery debdiff (typo and missing files)

It's regrettable that we have to go the route of patching initramfs-tools here, but all of these changes stand up to scrutiny. FFe granted, provided that the final version of the patches gets fully tested before upload.

This bug was fixed in the package friendly-recovery - 0.2.13

---------------
friendly-recovery (0.2.13) oneiric; urgency=low

  * Move everything to /lib/recovery-mode/ (LP: #234409)
  * Don't use the fullpath to whiptail
  * Add new init script starting on recovery-mode (LP: #459376)
  * If whiptail can't be found, just start sulogin
  * Add a script to start mountall (remount everything read/write)
    (LP: #575469, LP: #651782)
  * Export READONLY to all scripts so they can test on the read/write state
  * Disable most scripts when the system is read only
  * Change file system check to happen when remount is called
  * Wait after all scripts returning output so the user can read it
  * Add postinst script to move any existing script to /lib/recovery-mode
    and make /usr/share/recovery-mode a symlink
  * Mark as breaking on older grub2, upstart and initramfs-tools
  * Small packaging refresh:
    - Bump standard to 3.9.2, no change needed
    - Drop simple patchsys (no patches)
    - No need to depend on bash (essential package)
    - Add ${misc:Depends} to dependencies
 -- Stephane Graber <email address hidden> Wed, 07 Sep 2011 20:18:53 -0400