bug#56209: Shepherd 0.9 not cleanly unmounting root

["Ludovic Courtès" <ludo@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 1370 bytes --]

Hi,

angry rectangle <angryrectangle@cock.li> skribis:

> Since the upgrade to shepherd 0.9, I get "recovering journal" every single time I start my computer.
> To be specific, "recovering journal" appears after I enter my encryption password in the initrd.
> I assume this means the filesystem wasn't cleanly unmounted.
> I am doing a proper shutdown, using either "reboot" or "halt." 

I can see that as well.

> The guix commit 400c9ed3d779308e56038305d40cd93acb496180 is the specific commit that upgrades shepherd and causes me this problem. The previous commit is fine.
> I'm can confirm that it's still broken on recent commits. I'm on 696e2cc345f015c32f211bf0d0330c04b1cf5f15.

Preliminary investigation suggests this is because shepherd doesn’t
close log files beforehand (in 0.9, those specified as #:log-file to
‘make-forkexec-constructor’ & co. are opened by PID 1; conversely,
shepherd 0.8 would open them in the child process.)

To be continued…

Thanks for reporting the issue and finding the offending commit!

Ludo’.

PS: Below my (ugly) debugging tricks for posterity.  To see those
    messages, you typically need to start a VM with ‘-serial stdio’ and
    pass “console=ttyS0” to the kernel.  (It’s best to start a
    standalone VM with an image created by ‘guix system image -t
    qcow2’.)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 4488 bytes --]

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index d58afb27e3..25d747d226 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -299,6 +299,9 @@ (define %root-file-system-shepherd-service
    (stop #~(lambda _
              ;; Return #f if successfully stopped.
              (sync)
+             (call-with-port (open-file "/dev/console" "w0")
+               (lambda (port)
+                 (display "This is my last message.\n" port)))
 
              (call-with-blocked-asyncs
               (lambda ()
@@ -314,11 +317,24 @@ (define %root-file-system-shepherd-service
                   ;; Close /dev/console.
                   (for-each close-fdes '(0 1 2))
 
-                  ;; At this point, there are no open files left, so the
-                  ;; root file system can be re-mounted read-only.
-                  (mount #f "/" #f
-                         (logior MS_REMOUNT MS_RDONLY)
-                         #:update-mtab? #f)
+                  (open-fdes "/dev/null" O_RDONLY)
+                  (open-fdes "/dev/console" O_WRONLY)
+                  (open-fdes "/dev/console" O_WRONLY)
+                  (current-output-port (fdopen 1 "w0"))
+                  (current-error-port (fdopen 2 "w0"))
+                  (pk 'umount-root)
+
+                  (catch 'system-error
+                    (lambda ()
+                      ;; At this point, there are no open files left, so the
+                      ;; root file system can be re-mounted read-only.
+                      (mount #f "/" #f
+                             (logior MS_REMOUNT MS_RDONLY)
+                             #:update-mtab? #f))
+                    (lambda args
+                      (pk 'umount-root-error args)
+                      #f))
+                  (pk 'done-umount-root)
 
                   #f)))))
    (respawn? #f)))
@@ -406,7 +422,28 @@ (define (file-system-shepherd-service file-system)
                       ;; Make sure PID 1 doesn't keep TARGET busy.
                       (chdir "/")
 
-                      (umount #$target)
+                      (call-with-port (open-file "/dev/console" "w0")
+                        (lambda (port)
+                          (parameterize ((current-output-port port)
+                                         (current-error-port port))
+                            (pk 'umount #$target)
+                            #$(if (file-system-mount-may-fail? file-system)
+                                  #~(catch 'system-error
+                                      (lambda ()
+                                        (umount #$target))
+                                      (const #f))
+                                  #~(catch 'system-error
+                                      (lambda ()
+                                        (umount #$target))
+                                      (lambda args
+                                        (pk 'umount-error args)
+                                        (system* #$(file-append (@ (gnu
+                                                                    packages
+                                                                    lsof)
+                                                                   lsof)
+                                                                "/bin/lsof"))
+                                        #f)))
+                            (pk 'done-umount #$target))))
                       #f))
 
             ;; We need additional modules.
diff --git a/gnu/system/examples/bare-bones.tmpl b/gnu/system/examples/bare-bones.tmpl
index 387e4b12ba..1f9012c167 100644
--- a/gnu/system/examples/bare-bones.tmpl
+++ b/gnu/system/examples/bare-bones.tmpl
@@ -2,8 +2,8 @@
 ;; for a "bare bones" setup, with no X11 display server.
 
 (use-modules (gnu))
-(use-service-modules networking ssh)
-(use-package-modules screen ssh)
+(use-service-modules networking ssh shepherd)
+(use-package-modules screen ssh admin)
 
 (operating-system
   (host-name "komputilo")
@@ -38,6 +38,13 @@
                                         "audio" "video")))
                %base-user-accounts))
 
+  (essential-services
+   (modify-services (operating-system-default-essential-services
+                     this-operating-system)
+     (shepherd-root-service-type
+      config => (shepherd-configuration
+                 (shepherd shepherd-0.8)))))
+
   ;; Globally-installed packages.
   (packages (cons screen %base-packages))