bug#59823: [1.4.0rc1] Installer fails to identify installation device on Ventoy-made images

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

Hi!

Mathieu Othacehe <othacehe@gnu.org> skribis:

>>From 0afda5b3ed32e73bece9db96ab970d83f9f2e74b Mon Sep 17 00:00:00 2001
> From: Mathieu Othacehe <othacehe@gnu.org>
> Date: Thu, 8 Dec 2022 13:24:02 +0100
> Subject: [PATCH 1/1] installer: Detect mapped installation devices.
>
> Fixes: <https://issues.guix.gnu.org/59823>
>
> * gnu/installer/parted.scm (mapped-device?,
> mapped-device->parent-partition-path): New procedures.
> (eligible-devices): Detect mapped installation devices using the new
> procedures.

Well done!

> +(define (mapped-device->parent-partition-path device)
> +  "Return the parent partition path of the mapped DEVICE."

Maybe ‘mapped-device-parent-partition’?  (“Path” being used for search
paths, not for file names.)

> +  (let* ((command `("dmsetup" "deps" ,device "-o" "devname"))
> +         (parent #f)
> +         (handler
> +          (lambda (input)
> +            (let ((result
> +                   (string-match "\\(([^\\)]+)\\)"
> +                                 (get-string-all input))))
> +              (and result
> +                   (set! parent
> +                         (format #f "/dev/~a"
> +                                 (match:substring result 1))))))))
> +    (run-external-command-with-handler handler command)
> +    parent))

The advantage of ‘run-external-command-with-handler’ is logging I guess,
but that leads to an awkward style compared to using ‘open-input-pipe’,
which I would favor.

Then we want to parse a line that looks like this:

--8<---------------cut here---------------start------------->8---
1 dependencies  : (sda2)
--8<---------------cut here---------------end--------------->8---

I’m not super confident relying on this format.

‘dmsetup deps …’ does this (here /dev/dm-0 is the same as
/dev/mapper/root):

--8<---------------cut here---------------start------------->8---
ioctl(3, DM_VERSION, {version=[4, 0, 0], data_size=16384, flags=DM_EXISTS_FLAG} => {version=[4, 47, 0], data_size=16384, flags=DM_EXISTS_FLAG}) = 0
newfstatat(AT_FDCWD, "/dev/dm-0", {st_mode=S_IFBLK|0660, st_rdev=makedev(0xfd, 0), ...}, 0) = 0
newfstatat(AT_FDCWD, "/dev/mapper/dm-0", 0x7ffe78f99020, 0) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/dev/mapper", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = 4
newfstatat(4, "", {st_mode=S_IFDIR|0755, st_size=80, ...}, AT_EMPTY_PATH) = 0
getdents64(4, 0x563c1d72f8b0 /* 4 entries */, 32768) = 104
newfstatat(AT_FDCWD, "/dev/mapper/root", {st_mode=S_IFBLK|0600, st_rdev=makedev(0xfd, 0), ...}, 0) = 0
close(4)                                = 0
ioctl(3, DM_TABLE_DEPS, {version=[4, 0, 0], data_size=16384, data_start=312, name="root", flags=DM_EXISTS_FLAG} => {version=[4, 47, 0], data_size=328, data_start=312, dev=makedev(0xfd, 0), name="root", uuid="CRYPT-LUKS1-xxxxxx-root", target_count=1, open_count=1, event_nr=0, flags=DM_EXISTS_FLAG|DM_ACTIVE_PRESENT_FLAG, ...}) = 0
newfstatat(1, "", {st_mode=S_IFCHR|0622, st_rdev=makedev(0x88, 0), ...}, AT_EMPTY_PATH) = 0
readlink("/sys/dev/block/8:2", "../../devices/pci0000:00/0000:00:17.0/ata3/host2/target2:0:0/2:0:0:0/block/sda/sda2", 4095) = 83
write(1, "1 dependencies\t: (sda2)\n", 24) = 24
--8<---------------cut here---------------end--------------->8---

We could implement those ioctls, but not in the time frame we’re talking
about…

The libdevmapper interface doesn’t look more pleasant than the raw
ioctl.

Anyway, all in all, calling out to dmsetup looks reasonable for now; I
have a slight preference for using ‘open-pipe* OPEN_READ’, but no big
deal.  Perhaps add a comment showing what the line we’re parsing should
look like.

>  (define (eligible-devices)
>    "Return all the available devices except the install device and the devices
>  which are smaller than %MIN-DEVICE-SIZE."
>  
>    (define the-installer-root-partition-path
> -    (installer-root-partition-path))
> +    (let ((root-path
> +           (installer-root-partition-path)))

Just ‘root’.  :-)

Thanks a lot for fixing this!  You can push to ‘version-1.4.0’.
Hopefully I’ll prepare RC2 tomorrow evening.

Ludo’.