diff mbox series

[bug#44169,3/3] installer: Use UUIDs in the 'swap-devices' field.

Message ID 871rhle1eo.fsf@gnu.org
State Accepted
Headers show
Series None | expand

Checks

Context Check Description
cbaines/submitting builds fail
cbaines/issue success View issue
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job

Commit Message

Ludovic Courtès Oct. 26, 2020, 10:18 a.m. UTC 
Hi!

Mathieu Othacehe <othacehe@gnu.org> skribis:

>> +              `((swap-devices (list ,@(map (lambda (uuid)
>> +                                             `(uuid ,uuid))
>> +                                           uuids))))))
>
> I fear that this will break the "gui-installed-os" test. The
> "installation-target-os-for-gui-tests" procedure declares "/dev/vda2" as
> swap-device, while the installer will declare it using an UUID.
>
> This mismatch will probably make "shepherd services" test unhappy. We
> could maybe set a swap label during installation and use it in the
> installer tests?

Good point, I had overlooked that.

I came up with the following trick: during installation, once user
partitions are formatted, we set a pre-defined UUID on /dev/vda2, and we
use that label in ‘installation-target-os-for-gui-tests’.

It works, but I think it’s racy: the config file could be generated
before we’ve changed the UUID.

Problem is that since formatting is the last step that occurs before
config file generation, there’s no synchronization point where the
installer could wait for the client (i.e., wait until the client has run
‘swaplabel’.)

Ideas?

Thanks,
Ludo’.

Comments

Mathieu Othacehe Oct. 26, 2020, 12:06 p.m. UTC | #1 
Hey,

> I came up with the following trick: during installation, once user
> partitions are formatted, we set a pre-defined UUID on /dev/vda2, and we
> use that label in ‘installation-target-os-for-gui-tests’.
>
> It works, but I think it’s racy: the config file could be generated
> before we’ve changed the UUID.

Mmh that's problematic then. What about using swaplabel to set a label,
during the installation, only when there are connected clients and use
an UUID otherwise?

Thanks,

Mathieu
Ludovic Courtès Oct. 28, 2020, 1:58 p.m. UTC | #2 
Hi!

Mathieu Othacehe <othacehe@gnu.org> skribis:

> Mmh that's problematic then. What about using swaplabel to set a label,
> during the installation, only when there are connected clients and use
> an UUID otherwise?

I unleashed my creativity, leading to the v2 I just sent.  The trick is
to add an explicit request/reply sequence with clients right before the
config file is generated.

I also added the comments regarding PAGE_SIZE as you suggested.

I think it’s probably OK for 1.2, though another round of testing for
the ISO would be welcome.

WDYT?

Thanks,
Ludo’.
Ludovic Courtès Oct. 30, 2020, 12:17 a.m. UTC | #3 
Hi,

Ludovic Courtès <ludo@gnu.org> skribis:

> I unleashed my creativity, leading to the v2 I just sent.  The trick is
> to add an explicit request/reply sequence with clients right before the
> config file is generated.
>
> I also added the comments regarding PAGE_SIZE as you suggested.
>
> I think it’s probably OK for 1.2, though another round of testing for
> the ISO would be welcome.

I went ahead and pushed it as 1c6d98533153bc8e0e36236e7fbcf1eb5e178d26.

I’ll update the ‘guix’ package as a followup.

Thanks,
Ludo’.
Mathieu Othacehe Nov. 1, 2020, 6:18 p.m. UTC | #4 
Hey Ludo,

> I went ahead and pushed it as 1c6d98533153bc8e0e36236e7fbcf1eb5e178d26.

Sorry for not replying earlier, this looks fine :).

Thanks,

Mathieu
diff mbox series

Patch

diff --git a/gnu/installer/newt/partition.scm b/gnu/installer/newt/partition.scm
index ed38287fe8..1854b35656 100644
--- a/gnu/installer/newt/partition.scm
+++ b/gnu/installer/newt/partition.scm
@@ -777,5 +777,9 @@  by pressing the Exit button.~%~%")))
     ;; Make sure the disks are not in use before proceeding to formatting.
     (free-parted non-install-devices)
     (format-user-partitions user-partitions-with-pass)
+    (syslog "user partitions formatted~%")
+
+    ;; Let clients know that the partitions are ready.
+    (send-to-clients '(partitions-formatted))
     (destroy-form-and-pop form)
     user-partitions))
diff --git a/gnu/installer/tests.scm b/gnu/installer/tests.scm
index 58bf0a2700..5962391e61 100644
--- a/gnu/installer/tests.scm
+++ b/gnu/installer/tests.scm
@@ -286,8 +286,8 @@  instrumented for further testing."
                                edit-configuration-file))
   "Converse over PORT to choose the partitioning method.  When ENCRYPTED? is
 true, choose full-disk encryption with PASSPHRASE as the LUKS passphrase.
-This conversation goes past the final dialog box that shows the configuration
-file, actually starting the installation process."
+This conversation stops when the user partitions have been formatted, right
+before the final dialog box that shows the configuration file."
   (converse port
     ((list-selection (title "Partitioning method")
                      (multiple-choices? #f)
@@ -330,15 +330,19 @@  file, actually starting the installation process."
      #t)
     ((info (title "Preparing partitions") _ ...)
      (values))                                    ;nothing to return
+    ((partitions-formatted)
+     (values))))
+
+(define (conclude-installation port)
+  "Conclude the installation by checking over PORT that we get the generated
+configuration file, accepting it and starting the installation, and then
+receiving the final messages once the 'guix system init' process has
+completed."
+  (converse port
     ((file-dialog (title "Configuration file")
                   (text _)
                   (file ,configuration-file))
-     (edit-configuration-file configuration-file))))
-
-(define (conclude-installation port)
-  "Conclude the installation by checking over PORT that we get the final
-messages once the 'guix system init' process has completed."
-  (converse port
+     (edit-configuration-file configuration-file))
     ((pause)                                      ;"Press Enter to continue."
      #t)
     ((installation-complete)                      ;congratulations!
diff --git a/gnu/tests/install.scm b/gnu/tests/install.scm
index 86bd93966b..0dc5f6ad94 100644
--- a/gnu/tests/install.scm
+++ b/gnu/tests/install.scm
@@ -1211,6 +1211,14 @@  build (current-guix) and then store a couple of full system images.")
                         #$marionette)
       (screenshot "installer-run.ppm")
 
+      (unless #$encrypted?
+        ;; Choose a fixed UUID for the swap partition that matches what
+        ;; 'installation-target-os-for-gui-tests' expects.
+        (marionette-eval* '(invoke #$(file-append util-linux "/sbin/swaplabel")
+                                   "-U" "11111111-2222-3333-4444-123456789abc"
+                                   "/dev/vda2")
+                          #$marionette))
+
       (marionette-eval* '(conclude-installation installer-socket)
                         #$marionette)
 
@@ -1257,8 +1265,12 @@  build (current-guix) and then store a couple of full system images.")
                            '("wheel" "audio" "video"))))
                    %base-user-accounts))
     ;; The installer does not create a swap device in guided mode with
-    ;; encryption support.
-    (swap-devices (if encrypted? '() '("/dev/vda2")))
+    ;; encryption support.  The installer produces a UUID for the partition;
+    ;; this "UUID" is explicitly set in 'gui-test-program' to the value shown
+    ;; below.
+    (swap-devices (if encrypted?
+                      '()
+                      (list (uuid "11111111-2222-3333-4444-123456789abc"))))
     (services (cons (service dhcp-client-service-type)
                     (operating-system-user-services %minimal-os-on-vda)))))