GNU bug report logs -
#51878
[PATCH] installer: Rework installation device detection
Previous Next
Reported by: Josselin Poiret <dev <at> jpoiret.xyz>
Date: Mon, 15 Nov 2021 21:05:02 UTC
Severity: normal
Tags: patch
Done: Mathieu Othacehe <othacehe <at> gnu.org>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 51878 in the body.
You can then email your comments to 51878 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
guix-patches <at> gnu.org
:
bug#51878
; Package
guix-patches
.
(Mon, 15 Nov 2021 21:05:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Josselin Poiret <dev <at> jpoiret.xyz>
:
New bug report received and forwarded. Copy sent to
guix-patches <at> gnu.org
.
(Mon, 15 Nov 2021 21:05:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
Hello,
While testing the installer to add LUKS2 support in a VM, the installer
was having trouble detecting which device was the installation one, so I
decided to rewrite that part using guile-parted. The old code could not
properly detect the device as it was comparing a disk block device path (eg
`/dev/sda`) with a partition block device path (`/dev/sda2`). Instead, this
patch just iterates over all partitions of each device and tries to find if
the root partition is among one of them.
Best,
Josselin Poiret
-- >8 --
* gnu/installer/parted.scm (installation-device): Remove it.
* gnu/installer/parted.scm (installer-root-partition-path): Add it.
* gnu/installer/parted.scm (non-install-devices): Add
installation-device? predicate.
---
gnu/installer/parted.scm | 51 ++++++++++++++++++++++------------------
1 file changed, 28 insertions(+), 23 deletions(-)
diff --git a/gnu/installer/parted.scm b/gnu/installer/parted.scm
index 00de0a30fa..ea2e26ddad 100644
--- a/gnu/installer/parted.scm
+++ b/gnu/installer/parted.scm
@@ -26,6 +26,7 @@ (define-module (gnu installer parted)
#:use-module ((gnu build file-systems)
#:select (canonicalize-device-spec
find-partition-by-label
+ find-partition-by-uuid
read-partition-uuid
read-luks-partition-uuid))
#:use-module ((gnu build linux-boot)
@@ -345,35 +346,39 @@ (define (remove-logical-devices)
(with-null-output-ports
(invoke "dmsetup" "remove_all")))
-(define (installation-device)
- "Return the installation device path."
+(define (installer-root-partition-path)
+ "Return the root partition path, or #f if it could not be detected."
(let* ((cmdline (linux-command-line))
(root (find-long-option "--root" cmdline)))
(and root
- (canonicalize-device-spec (uuid root)))))
+ (or (and (access? root F_OK) root)
+ (find-partition-by-label root)
+ (and=> (uuid root)
+ find-partition-by-uuid)))))
(define (non-install-devices)
"Return all the available devices, except the install device."
- (define (read-only? device)
- (dynamic-wind
- (lambda ()
- (device-open device))
- (lambda ()
- (device-read-only? device))
- (lambda ()
- (device-close device))))
-
- ;; If parted reports that a device is read-only it is probably the
- ;; installation device. However, as this detection does not always work,
- ;; compare the device path to the installation device path read from the
- ;; command line.
- (let ((install-device (installation-device)))
- (remove (lambda (device)
- (let ((file-name (device-path device)))
- (or (read-only? device)
- (and install-device
- (string=? file-name install-device)))))
- (devices))))
+
+ (define the-intaller-root-partition-path
+ (installer-root-partition-path))
+
+ ;; Read partition table of device and compare each path to the one
+ ;; we're booting from to determine if it is the installation
+ ;; device.
+ (define (installation-device? device)
+ (let ((disk (disk-new device)))
+ (and disk
+ (let loop ((partition #f))
+ (let ((next-partition (disk-next-partition disk
+ #:partition
+ partition)))
+ (and next-partition
+ (or (string=? the-installer-root-partition-path
+ (partition-get-path
+ next-partition))
+ (loop next-partition))))))))
+
+ (remove installation-device? (devices)))
;;
--
2.33.1
Information forwarded
to
guix-patches <at> gnu.org
:
bug#51878
; Package
guix-patches
.
(Mon, 15 Nov 2021 21:34:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 51878 <at> debbugs.gnu.org (full text, mbox):
Sorry for the noise,
Josselin Poiret
-- >8 --
* gnu/installer/parted.scm (installation-device): Remove it.
* gnu/installer/parted.scm (installer-root-partition-path): Add it.
* gnu/installer/parted.scm (non-install-devices): Add
installation-device? predicate.
---
gnu/installer/parted.scm | 51 ++++++++++++++++++++++------------------
1 file changed, 28 insertions(+), 23 deletions(-)
diff --git a/gnu/installer/parted.scm b/gnu/installer/parted.scm
index 00de0a30fa..42456a1d18 100644
--- a/gnu/installer/parted.scm
+++ b/gnu/installer/parted.scm
@@ -26,6 +26,7 @@ (define-module (gnu installer parted)
#:use-module ((gnu build file-systems)
#:select (canonicalize-device-spec
find-partition-by-label
+ find-partition-by-uuid
read-partition-uuid
read-luks-partition-uuid))
#:use-module ((gnu build linux-boot)
@@ -345,35 +346,39 @@ (define (remove-logical-devices)
(with-null-output-ports
(invoke "dmsetup" "remove_all")))
-(define (installation-device)
- "Return the installation device path."
+(define (installer-root-partition-path)
+ "Return the root partition path, or #f if it could not be detected."
(let* ((cmdline (linux-command-line))
(root (find-long-option "--root" cmdline)))
(and root
- (canonicalize-device-spec (uuid root)))))
+ (or (and (access? root F_OK) root)
+ (find-partition-by-label root)
+ (and=> (uuid root)
+ find-partition-by-uuid)))))
(define (non-install-devices)
"Return all the available devices, except the install device."
- (define (read-only? device)
- (dynamic-wind
- (lambda ()
- (device-open device))
- (lambda ()
- (device-read-only? device))
- (lambda ()
- (device-close device))))
-
- ;; If parted reports that a device is read-only it is probably the
- ;; installation device. However, as this detection does not always work,
- ;; compare the device path to the installation device path read from the
- ;; command line.
- (let ((install-device (installation-device)))
- (remove (lambda (device)
- (let ((file-name (device-path device)))
- (or (read-only? device)
- (and install-device
- (string=? file-name install-device)))))
- (devices))))
+
+ (define the-installer-root-partition-path
+ (installer-root-partition-path))
+
+ ;; Read partition table of device and compare each path to the one
+ ;; we're booting from to determine if it is the installation
+ ;; device.
+ (define (installation-device? device)
+ (let ((disk (disk-new device)))
+ (and disk
+ (let loop ((partition #f))
+ (let ((next-partition (disk-next-partition disk
+ #:partition
+ partition)))
+ (and next-partition
+ (or (string=? the-installer-root-partition-path
+ (partition-get-path
+ next-partition))
+ (loop next-partition))))))))
+
+ (remove installation-device? (devices)))
;;
--
2.33.1
Information forwarded
to
guix-patches <at> gnu.org
:
bug#51878
; Package
guix-patches
.
(Wed, 17 Nov 2021 14:44:01 GMT)
Full text and
rfc822 format available.
Message #11 received at 51878 <at> debbugs.gnu.org (full text, mbox):
Hello Josselin,
> properly detect the device as it was comparing a disk block device path (eg
> `/dev/sda`) with a partition block device path (`/dev/sda2`). Instead, this
When using an ISO installer image,
> - (or (read-only? device)
> - (and install-device
> - (string=? file-name install-device)))))
> - (devices))))
file-name and install-device both equal "/dev/sr0" for the cdrom device,
which means it will be correctly filtered out. Is it handled correctly
with your patch?
When using a raw disk image, we may indeed compare devices and
partitions currently.
> +
> + (define the-intaller-root-partition-path
> + (installer-root-partition-path))
> +
> + ;; Read partition table of device and compare each path to the one
> + ;; we're booting from to determine if it is the installation
> + ;; device.
> + (define (installation-device? device)
> + (let ((disk (disk-new device)))
> + (and disk
> + (let loop ((partition #f))
> + (let ((next-partition (disk-next-partition disk
> + #:partition
> + partition)))
> + (and next-partition
> + (or (string=? the-installer-root-partition-path
> + (partition-get-path
> + next-partition))
> + (loop next-partition))))))))
Filtering the "(devices)" list can cause extra iterations compared to
your implementation, but is easier to read I think.
Thanks,
Mathieu
Information forwarded
to
guix-patches <at> gnu.org
:
bug#51878
; Package
guix-patches
.
(Tue, 23 Nov 2021 22:20:01 GMT)
Full text and
rfc822 format available.
Message #14 received at 51878 <at> debbugs.gnu.org (full text, mbox):
Hello,
You're right, I didn't take CDs into account. Here is a new version
which compares the device path itself to the --root argument as well,
which is the case for CDs. I checked both iso9660 and qcow2 in qemu,
and both only list other devices.
I didn't quite get your comment about filtering the `(devices)` list.
In both cases, we use `remove`, but here I've factored out the predicate
used for it.
Best,
Josselin
-- >8 --
* gnu/installer/parted.scm (installation-device): Remove it.
* gnu/installer/parted.scm (installer-root-partition-path): Add it.
* gnu/installer/parted.scm (non-install-devices): Add
installation-device? predicate.
---
gnu/installer/parted.scm | 53 +++++++++++++++++++++++-----------------
1 file changed, 30 insertions(+), 23 deletions(-)
diff --git a/gnu/installer/parted.scm b/gnu/installer/parted.scm
index 00de0a30fa..f665e67b35 100644
--- a/gnu/installer/parted.scm
+++ b/gnu/installer/parted.scm
@@ -26,6 +26,7 @@ (define-module (gnu installer parted)
#:use-module ((gnu build file-systems)
#:select (canonicalize-device-spec
find-partition-by-label
+ find-partition-by-uuid
read-partition-uuid
read-luks-partition-uuid))
#:use-module ((gnu build linux-boot)
@@ -345,35 +346,41 @@ (define (remove-logical-devices)
(with-null-output-ports
(invoke "dmsetup" "remove_all")))
-(define (installation-device)
- "Return the installation device path."
+(define (installer-root-partition-path)
+ "Return the root partition path, or #f if it could not be detected."
(let* ((cmdline (linux-command-line))
(root (find-long-option "--root" cmdline)))
(and root
- (canonicalize-device-spec (uuid root)))))
+ (or (and (access? root F_OK) root)
+ (find-partition-by-label root)
+ (and=> (uuid root)
+ find-partition-by-uuid)))))
(define (non-install-devices)
"Return all the available devices, except the install device."
- (define (read-only? device)
- (dynamic-wind
- (lambda ()
- (device-open device))
- (lambda ()
- (device-read-only? device))
- (lambda ()
- (device-close device))))
-
- ;; If parted reports that a device is read-only it is probably the
- ;; installation device. However, as this detection does not always work,
- ;; compare the device path to the installation device path read from the
- ;; command line.
- (let ((install-device (installation-device)))
- (remove (lambda (device)
- (let ((file-name (device-path device)))
- (or (read-only? device)
- (and install-device
- (string=? file-name install-device)))))
- (devices))))
+
+ (define the-installer-root-partition-path
+ (installer-root-partition-path))
+
+ ;; Read partition table of device and compare each path to the one
+ ;; we're booting from to determine if it is the installation
+ ;; device.
+ (define (installation-device? device)
+ (or (string=? the-installer-root-partition-path
+ (device-path device))
+ (let ((disk (disk-new device)))
+ (and disk
+ (let loop ((partition #f))
+ (let ((next-partition (disk-next-partition disk
+ #:partition
+ partition)))
+ (and next-partition
+ (or (string=? the-installer-root-partition-path
+ (partition-get-path
+ next-partition))
+ (loop next-partition)))))))))
+
+ (remove installation-device? (devices)))
;;
--
2.33.1
Information forwarded
to
guix-patches <at> gnu.org
:
bug#51878
; Package
guix-patches
.
(Thu, 25 Nov 2021 09:21:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 51878 <at> debbugs.gnu.org (full text, mbox):
Hello Josselin,
Thanks for the v2!
> I didn't quite get your comment about filtering the `(devices)` list.
> In both cases, we use `remove`, but here I've factored out the predicate
> used for it.
I was not very clear sorry about that. I meant something like that seems
a little more concise:
--8<---------------cut here---------------start------------->8---
diff --git a/gnu/installer/parted.scm b/gnu/installer/parted.scm
index 82b01d2ce1..ad7dd6bf91 100644
--- a/gnu/installer/parted.scm
+++ b/gnu/installer/parted.scm
@@ -366,19 +366,16 @@ (define the-installer-root-partition-path
;; we're booting from to determine if it is the installation
;; device.
(define (installation-device? device)
+ ;; When using CDROM based installation, the root partition path may be the
+ ;; device path.
(or (string=? the-installer-root-partition-path
(device-path device))
(let ((disk (disk-new device)))
(and disk
- (let loop ((partition #f))
- (let ((next-partition (disk-next-partition disk
- #:partition
- partition)))
- (and next-partition
- (or (string=? the-installer-root-partition-path
- (partition-get-path
- next-partition))
- (loop next-partition)))))))))
+ (any (lambda (partition)
+ (string=? the-installer-root-partition-path
+ (partition-get-path partition)))
+ (disk-partitions disk))))))
(remove installation-device? (devices)))
--8<---------------cut here---------------end--------------->8---
WDYT?
Thanks,
Mathieu
Information forwarded
to
guix-patches <at> gnu.org
:
bug#51878
; Package
guix-patches
.
(Fri, 26 Nov 2021 09:49:02 GMT)
Full text and
rfc822 format available.
Message #20 received at 51878 <at> debbugs.gnu.org (full text, mbox):
Mathieu Othacehe <othacehe <at> gnu.org> writes:
> Hello Josselin,
> I was not very clear sorry about that. I meant something like that seems
> a little more concise:
>
> --8<---------------cut here---------------start------------->8---
> snip
> --8<---------------cut here---------------end--------------->8---
>
> WDYT?
I agree that this reads better! Feel free to push with those changes.
Best,
Josselin Poiret
Reply sent
to
Mathieu Othacehe <othacehe <at> gnu.org>
:
You have taken responsibility.
(Fri, 26 Nov 2021 10:54:01 GMT)
Full text and
rfc822 format available.
Notification sent
to
Josselin Poiret <dev <at> jpoiret.xyz>
:
bug acknowledged by developer.
(Fri, 26 Nov 2021 10:54:01 GMT)
Full text and
rfc822 format available.
Message #25 received at 51878-done <at> debbugs.gnu.org (full text, mbox):
Hey,
> I agree that this reads better! Feel free to push with those changes.
Pushed as b90504cdb5ce3d1981c8d7bc8a9cc918b0d60af7.
Thanks,
Mathieu
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Fri, 24 Dec 2021 12:24:07 GMT)
Full text and
rfc822 format available.
This bug report was last modified 2 years and 85 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.