unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings
@ 2020-05-09  1:12 tsmish
  2020-05-09  1:22 ` [bug#41143] [PATCH 2/2] mapped-devices: Add 'lvm-device-mapping' tsmish
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: tsmish @ 2020-05-09  1:12 UTC (permalink / raw)
  To: 41143

(let ...) stuff should be in function, but I don't know in which
module it should go.
Code is somewhat untested, proceed with caution.

---
 gnu/services/base.scm         |  5 ++++-
 gnu/system.scm                | 13 ++++++++-----
 gnu/system/mapped-devices.scm |  2 +-
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 0c154d1c4e..3d09e8220c 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -408,7 +408,10 @@ FILE-SYSTEM."
 (define (mapped-device->shepherd-service-name md)
   "Return the symbol that denotes the shepherd service of MD, a
<mapped-device>."
   (symbol-append 'device-mapping-
-                 (string->symbol (mapped-device-target md))))
+                 (string->symbol (string-join
+                                  (let ((t (mapped-device-target md)))
+                                    (if (list? t) t (list t)))
+                                  "-"))))

 (define dependency->shepherd-service-name
   (match-lambda
diff --git a/gnu/system.scm b/gnu/system.scm
index 01baa248a2..75632c5e8a 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -390,9 +390,10 @@ marked as 'needed-for-boot'."
     (let ((device (file-system-device fs)))
       (if (string? device)                        ;title is 'device
           (filter (lambda (md)
-                    (string=? (string-append "/dev/mapper/"
-                                             (mapped-device-target md))
-                              device))
+                    (any (cut string=? device <>)
+                         (map (cut string-append "/dev/mapper" <>)
+                              (let ((t (mapped-device-target md)))
+                                (if (list? t) t (list t))))))
                   (operating-system-mapped-devices os))
           '())))

@@ -412,11 +413,13 @@ marked as 'needed-for-boot'."

 (define (mapped-device-users device file-systems)
   "Return the subset of FILE-SYSTEMS that use DEVICE."
-  (let ((target (string-append "/dev/mapper/" (mapped-device-target device))))
+  (let ((targets (map (cut string-append "/dev/mapper/" <>)
+                      (let ((t (mapped-device-target device)))
+                        (if (list? t) t (list t))))))
     (filter (lambda (fs)
               (or (member device (file-system-dependencies fs))
                   (and (string? (file-system-device fs))
-                       (string=? (file-system-device fs) target))))
+                       (any (cut string=? (file-system-device fs) <>)
targets))))
             file-systems)))

 (define (operating-system-user-mapped-devices os)
diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scm
index 7c58f876a3..3339e509e0 100644
--- a/gnu/system/mapped-devices.scm
+++ b/gnu/system/mapped-devices.scm
@@ -72,7 +72,7 @@
   make-mapped-device
   mapped-device?
   (source    mapped-device-source)                ;string | list of strings
-  (target    mapped-device-target)                ;string
+  (target    mapped-device-target)                ;string | list of strings
   (type      mapped-device-type)                  ;<mapped-device-kind>
   (location  mapped-device-location
              (default (current-source-location)) (innate)))
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [bug#41143] [PATCH 2/2] mapped-devices: Add 'lvm-device-mapping'
  2020-05-09  1:12 [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings tsmish
@ 2020-05-09  1:22 ` tsmish
  2020-09-09 20:48   ` Ludovic Courtès
  2020-05-14 22:53 ` [bug#41143] Some clarification Mikhail Tsykalov
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: tsmish @ 2020-05-09  1:22 UTC (permalink / raw)
  To: 41143

"vgscan --mknodes" is a bit of a hack. Everyone else relies on udev to
create files in /dev/mapper, but since initrd doesn't have working
udevd, they need to be created this way.
Also, while this code is able to boot from root on LVM, grub in
current configuration can't find required files, This can be fixed by
placing (format port "insmod lvm") in grub configuration builder, but
this is somewhat hacky.

---
 gnu/system/mapped-devices.scm | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scm
index 3339e509e0..03bc7c782d 100644
--- a/gnu/system/mapped-devices.scm
+++ b/gnu/system/mapped-devices.scm
@@ -34,7 +34,7 @@
   #:autoload   (gnu build linux-modules)
                  (missing-modules)
   #:autoload   (gnu packages cryptsetup) (cryptsetup-static)
-  #:autoload   (gnu packages linux) (mdadm-static)
+  #:autoload   (gnu packages linux) (mdadm-static lvm2-static)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-34)
@@ -59,7 +59,8 @@
             check-device-initrd-modules           ;XXX: needs a better place

             luks-device-mapping
-            raid-device-mapping))
+            raid-device-mapping
+            lvm-device-mapping))

 ;;; Commentary:
 ;;;
@@ -269,4 +270,28 @@ TARGET (e.g., \"/dev/md0\"), using 'mdadm'."
    (open open-raid-device)
    (close close-raid-device)))

+(define (open-lvm-device source target)
+  #~(begin
+       (use-modules
+        (srfi srfi-1)
+        (srfi srfi-26))
+       (and
+        (zero? (system* #$(file-append lvm2-static "/sbin/lvm")
+                        "vgchange" "--activate" "y" #$source))
+        (zero? (system* #$(file-append lvm2-static "/sbin/lvm")
+                         "vgscan" "--mknodes")) ; make /dev/mapper
nodes when in initrd
+        (every file-exists? (map (cut string-append "/dev/mapper/" <>)
+                                 (let ((t '#$target))
+                                   (if (list? t) t (list t))))))))
+
+
+(define (close-lvm-device source target)
+  #~(zero? (system* #$(file-append lvm2-static "/sbin/lvm")
+                    "vgchange" "--activate" "n" #$source)))
+
+(define lvm-device-mapping
+  (mapped-device-kind
+   (open open-lvm-device)
+   (close close-lvm-device)))
+
 ;;; mapped-devices.scm ends here
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [bug#41143] Some clarification
  2020-05-09  1:12 [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings tsmish
  2020-05-09  1:22 ` [bug#41143] [PATCH 2/2] mapped-devices: Add 'lvm-device-mapping' tsmish
@ 2020-05-14 22:53 ` Mikhail Tsykalov
  2020-05-15  1:17 ` [bug#41143] [PATCH] mapped-devices: Document lvm-mapping-device Mikhail Tsykalov
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Mikhail Tsykalov @ 2020-05-14 22:53 UTC (permalink / raw)
  To: 41143

Hi,

In LVM one volume group usually haves more than one logical volume, so 
target need to be a list. Just setting target to list in config doesn't 
work because that field is actually used for checking when (and whether 
at all) the device needs to be mounted. Because we still need to support 
using a string as target, we need to handle two situations: target is a 
string and target is list of strings. Patch converts plain string target 
to single element list and proceeds with assuming a list. grep by 
mapped-device-target revealed two uses in gnu/system.scm and one in 
gnu/services/base.scm. First two checked if any filesystems used a 
device, so I replaced plain string comparison with comparison with every 
element of list and returning true if any of them matched. Last one used 
target for building service name for shepherd, so I replaced plain 
target with concatenation of every target that mapped device has.

Moving on to second patch. Basically to mount LVM you need to activate 
volume group(s) with command "vgchange -ay". It worked fine with 
non-root mounts, but when it got moved in initrd it crashed. Quick look 
around found that while /dev/dm-X devices were being created, 
/dev/mapper/ nodes were not. Looking at man pages (and internet) "vgscan 
--mknodes" looked like what I wanted (there is also dmsetup mknodes, but 
basically first just calls it). After adding it, boot with root on LVM 
started working, so I left it. Also I check that targets actually exist, 
so you get crash in a bit more appropriate place. This check should 
probably be moved to somewhere else, but I haven't found better solution.

Looking at 
https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/6/html/logical_volume_manager_administration/udev_device_manager 
10-dm.rules looks like what creates /dev/mapper nodes in normal systems, 
so probably more proper solution will be to add udevd to initrd, but I 
don't know how to run daemons in initrd properly. dracut also creates 
file /etc/lvm/lvm.conf, which doesn't seem to be very important, but I 
probably should look closer to it. Also both dracut and LVM add 
"--ignorelockingfailure" flag to vgchange, which I forgot and will 
change in future patches.

There seems to be close to no resources about mounting LVM in initrd, so 
I'm adding code from other initrd generators:

Dracut: 
https://github.com/dracutdevs/dracut/blob/master/modules.d/90lvm/lvm_scan.sh

LVM: 
https://github.com/lvmteam/lvm2/blob/master/scripts/lvm2create_initrd/lvm2create_initrd





^ permalink raw reply	[flat|nested] 22+ messages in thread

* [bug#41143] [PATCH] mapped-devices: Document lvm-mapping-device.
  2020-05-09  1:12 [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings tsmish
  2020-05-09  1:22 ` [bug#41143] [PATCH 2/2] mapped-devices: Add 'lvm-device-mapping' tsmish
  2020-05-14 22:53 ` [bug#41143] Some clarification Mikhail Tsykalov
@ 2020-05-15  1:17 ` Mikhail Tsykalov
  2020-06-06 13:40 ` [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings Lars-Dominik Braun
  2020-09-09 20:38 ` Ludovic Courtès
  4 siblings, 0 replies; 22+ messages in thread
From: Mikhail Tsykalov @ 2020-05-15  1:17 UTC (permalink / raw)
  To: 41143

---
  doc/guix.texi | 30 ++++++++++++++++++++++++++----
  1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index d6fbd85fde..612a9b25e5 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -11738,7 +11738,6 @@ Guix extends this notion by considering any 
device or set of devices that
  are @dfn{transformed} in some way to create a new device; for instance,
  RAID devices are obtained by @dfn{assembling} several other devices, such
  as hard disks or partitions, into a new one that behaves as one partition.
-Other examples, not yet implemented, are LVM logical volumes.

  Mapped devices are declared using the @code{mapped-device} form,
  defined as follows; for examples, see below.
@@ -11751,15 +11750,20 @@ the system boots up.
  @item source
  This is either a string specifying the name of the block device to be 
mapped,
  such as @code{"/dev/sda3"}, or a list of such strings when several devices
-need to be assembled for creating a new one.
+need to be assembled for creating a new one. In case of LVM this is a
+string specifying name of the volume group to be mapped.

  @item target
-This string specifies the name of the resulting mapped device. For
-kernel mappers such as encrypted devices of type 
@code{luks-device-mapping},
+This is either a string specifying the name of the resulting mapped
+device, or a list of such strings in case there are several, which is
+common while using LVM.
+For kernel mappers such as encrypted devices of type 
@code{luks-device-mapping},
  specifying @code{"my-partition"} leads to the creation of
  the @code{"/dev/mapper/my-partition"} device.
  For RAID devices of type @code{raid-device-mapping}, the full device name
  such as @code{"/dev/md0"} needs to be given.
+LVM logical volumes of type @code{lvm-device-mapping} need to
+be specified as @code{"VGNAME-LVNAME"}.

  @item type
  This must be a @code{mapped-device-kind} object, which specifies how
@@ -11780,6 +11784,11 @@ module for the appropriate RAID level to be 
loaded, such as @code{raid456}
  for RAID-4, RAID-5 or RAID-6, or @code{raid10} for RAID-10.
  @end defvr

+@defvr {Scheme Variable} lvm-device-mapping
+This defines LVM logical volume(s). Volume group is activated by
+@command{vgchange} command from the package @code{lvm2}.
+@end defvr
+
  @cindex disk encryption
  @cindex LUKS
  The following example specifies a mapping from @file{/dev/sda3} to
@@ -11837,6 +11846,19 @@ Note that the RAID level need not be given; it 
is chosen during the
  initial creation and formatting of the RAID device and is determined
  automatically later.

+LVM logical volumes ``alpha'' and ``beta'' from volume group ``vg0'' can
+be declared as follows:
+
+@lisp
+(mapped-device
+  (source "vg0")
+  (target (list "vg0-alpha" "vg0-beta"))
+  (type lvm-device-mapping))
+@end lisp
+
+Devices @file{/dev/mapper/vg0-alpha} and @file{/dev/mapper/vg0-beta} can
+then be used as the @code{device} of a @code{file-system} declaration
+(@pxref{File Systems}).

  @node User Accounts
  @section User Accounts
-- 
2.26.2





^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings
  2020-05-09  1:12 [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings tsmish
                   ` (2 preceding siblings ...)
  2020-05-15  1:17 ` [bug#41143] [PATCH] mapped-devices: Document lvm-mapping-device Mikhail Tsykalov
@ 2020-06-06 13:40 ` Lars-Dominik Braun
  2020-06-06 20:16   ` Mikhail Tsykalov
  2020-09-09 20:38 ` Ludovic Courtès
  4 siblings, 1 reply; 22+ messages in thread
From: Lars-Dominik Braun @ 2020-06-06 13:40 UTC (permalink / raw)
  To: tsmish; +Cc: 41143

Hi,

I’ve tried the patches, but `guix system reconfigure` fails(?) with

	building /gnu/store/cy6d2a4b8bcqcjiaz8bm367j7vbdrslc-upgrade-shepherd-services.scm.drv...
	guix system: error: exception caught while executing 'start' on service 'device-mapping-disk3-data':
	error: <>: unbound variable

Relevant part of my config.scm:

	  (mapped-devices
		  (list (mapped-device
				  (source "disk3")
				  (target "disk3-data")
				  (type lvm-device-mapping))))
	  (file-systems (append
					 (list …
							(file-system
							 (device "/dev/mapper/disk3-data")
							 (mount-point "/storage/disk3")
							 (type "ext4")
							 (dependencies mapped-devices))
						   )
					 …))

I’m also curious why sources is a list of VG’s as opposed to a list of device
nodes (i.e. /dev/sdX), like the other mappings. Does `pvscan --cache` not work
here?

Lars





^ permalink raw reply	[flat|nested] 22+ messages in thread

* [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings
  2020-06-06 13:40 ` [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings Lars-Dominik Braun
@ 2020-06-06 20:16   ` Mikhail Tsykalov
  2020-06-07  6:48     ` Lars-Dominik Braun
  0 siblings, 1 reply; 22+ messages in thread
From: Mikhail Tsykalov @ 2020-06-06 20:16 UTC (permalink / raw)
  To: Lars-Dominik Braun; +Cc: 41143

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

Hi,

Thanks for the report, guess that's what I get by assuming instead of 
testing.

> I’ve tried the patches, but `guix system reconfigure` fails(?) with
>
> 	building /gnu/store/cy6d2a4b8bcqcjiaz8bm367j7vbdrslc-upgrade-shepherd-services.scm.drv...
> 	guix system: error: exception caught while executing 'start' on service 'device-mapping-disk3-data':
> 	error: <>: unbound variable

Can you test with the attached patch, it seems like `cut` macros is 
undefined while shepherd service is expanded(?). I fixed it by replacing 
it with a plain lambda. I managed to mount a flash disk with LVM using 
this patch, so I think it should be working now.

> I’m also curious why sources is a list of VG’s as opposed to a list of device
> nodes (i.e. /dev/sdX), like the other mappings. Does `pvscan --cache` not work
> here?

I always thought that it wasn't possible to mount LVM by PV's, but 
looking at man, `pvscan --cache -aay` seems to do the thing. Still, I 
think that using VG's instead of PV's is better, since you must `pvscan` 
all PV's in a VG in order to activate it, so you'll still need to think 
in terms of VG's. This also can be a source of errors, for example 
adding a device to a VG will result in entire VG not being activated 
until the configuration is changed and the system is reconfigured. Also 
I think that LVM doesn't put that much importance on individual PV's as 
it does on VG's as a place where LV's live.

All of this goes out of the window if you use LVM on only one PV, so it 
may be possible to have a heuristic that will check if source is a 
device node as opposed to a VG name and perform appropriate actions, but 
I'm not sure if the resulting complexity is worth it.

Mikhail


[-- Attachment #2: 0002-mapped-devices-Add-lvm-device-mapping.patch --]
[-- Type: text/x-patch, Size: 2188 bytes --]

From e9a2b441190fe18ebc4f5a8c73707edab3459d58 Mon Sep 17 00:00:00 2001
From: Mikhail Tsykalov <tsymsh@gmail.com>
Date: Sat, 9 May 2020 03:27:13 +0300
Subject: [PATCH 2/2] mapped-devices: Add 'lvm-device-mapping'

---
 gnu/system/mapped-devices.scm | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scm
index 3339e509e0..641cddc146 100644
--- a/gnu/system/mapped-devices.scm
+++ b/gnu/system/mapped-devices.scm
@@ -34,7 +34,7 @@
   #:autoload   (gnu build linux-modules)
                  (missing-modules)
   #:autoload   (gnu packages cryptsetup) (cryptsetup-static)
-  #:autoload   (gnu packages linux) (mdadm-static)
+  #:autoload   (gnu packages linux) (mdadm-static lvm2-static)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-34)
@@ -59,7 +59,8 @@
             check-device-initrd-modules           ;XXX: needs a better place
 
             luks-device-mapping
-            raid-device-mapping))
+            raid-device-mapping
+            lvm-device-mapping))
 
 ;;; Commentary:
 ;;;
@@ -269,4 +270,26 @@ TARGET (e.g., \"/dev/md0\"), using 'mdadm'."
    (open open-raid-device)
    (close close-raid-device)))
 
+(define (open-lvm-device source target)
+  #~(begin
+       (use-modules (srfi srfi-1))
+       (and
+        (zero? (system* #$(file-append lvm2-static "/sbin/lvm")
+                        "vgchange" "--activate" "ay" #$source))
+        (zero? (system* #$(file-append lvm2-static "/sbin/lvm")
+                         "vgscan" "--mknodes")) ; make /dev/mapper nodes when in initrd
+        (every file-exists? (map (lambda (file) (string-append "/dev/mapper/" file))
+                                 (let ((t '#$target))
+                                   (if (list? t) t (list t))))))))
+
+
+(define (close-lvm-device source target)
+  #~(zero? (system* #$(file-append lvm2-static "/sbin/lvm")
+                    "vgchange" "--activate" "n" #$source)))
+
+(define lvm-device-mapping
+  (mapped-device-kind
+   (open open-lvm-device)
+   (close close-lvm-device)))
+
 ;;; mapped-devices.scm ends here
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings
  2020-06-06 20:16   ` Mikhail Tsykalov
@ 2020-06-07  6:48     ` Lars-Dominik Braun
  0 siblings, 0 replies; 22+ messages in thread
From: Lars-Dominik Braun @ 2020-06-07  6:48 UTC (permalink / raw)
  To: Mikhail Tsykalov; +Cc: 41143

Hi,

> Can you test with the attached patch, it seems like `cut` macros is 
> undefined while shepherd service is expanded(?). I fixed it by replacing 
> it with a plain lambda. I managed to mount a flash disk with LVM using 
> this patch, so I think it should be working now.
yes, both `guix system reconfigure` and a reboot are working properly
now. (This is a non-boot disk though.) Thanks!

> Still, I think that using VG's instead of PV's is better, since you
> must `pvscan` all PV's in a VG in order to activate it, so you'll
> still need to think in terms of VG's.
I agree, using device nodes would probably break guix’ rollback if you
decide to change the VG.

Lars





^ permalink raw reply	[flat|nested] 22+ messages in thread

* [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings
  2020-05-09  1:12 [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings tsmish
                   ` (3 preceding siblings ...)
  2020-06-06 13:40 ` [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings Lars-Dominik Braun
@ 2020-09-09 20:38 ` Ludovic Courtès
  2020-09-24 16:09   ` Mikhail Tsykalov
  4 siblings, 1 reply; 22+ messages in thread
From: Ludovic Courtès @ 2020-09-09 20:38 UTC (permalink / raw)
  To: tsmish; +Cc: 41143

Hi Mikhail,

Sorry for the very late reply!  Vacations came by, and by now this entry
is at the bottom of the patch tracker.  :-)

People repeatedly ask for LVM support, so I guess you’ll make them all
happy!  Great you got it into shape.

tsmish <tsymsh@gmail.com> skribis:

> (let ...) stuff should be in function, but I don't know in which
> module it should go.
> Code is somewhat untested, proceed with caution.
>
> ---
>  gnu/services/base.scm         |  5 ++++-
>  gnu/system.scm                | 13 ++++++++-----
>  gnu/system/mapped-devices.scm |  2 +-
>  3 files changed, 13 insertions(+), 7 deletions(-)

Side note: We’ll a commit log that follows our conventions¹ but that’s
something I can help with.

¹ https://guix.gnu.org/manual/en/html_node/Submitting-Patches.html

> diff --git a/gnu/services/base.scm b/gnu/services/base.scm
> index 0c154d1c4e..3d09e8220c 100644
> --- a/gnu/services/base.scm
> +++ b/gnu/services/base.scm
> @@ -408,7 +408,10 @@ FILE-SYSTEM."
>  (define (mapped-device->shepherd-service-name md)
>    "Return the symbol that denotes the shepherd service of MD, a
> <mapped-device>."
>    (symbol-append 'device-mapping-
> -                 (string->symbol (mapped-device-target md))))
> +                 (string->symbol (string-join
> +                                  (let ((t (mapped-device-target md)))
> +                                    (if (list? t) t (list t)))
> +                                  "-"))))

To avoid duplicating the (if (list? t) …) everywhere, I propose instead
the following approach:

  1. Rename ‘target’ to ‘targets’ (plural) and likewise for the
     accessor, and agree that it always contains a list;

  2. Rename ‘mapped-device’ to ‘%mapped-device’ and add a
     ‘mapped-device’ backward-compatibility macro that allows for a
     ‘target’ (singular) field and automatically turns its value into a
     list.  See the ‘origin’ macro in (guix packages) for an example of
     how to do that (that macro allows users to specify ‘sha256’ instead
     of ‘hash’).

  3. Add a deprecated ‘mapped-device-target’ (singular) that returns the
     first element returned by ‘mapped-device-targets’.

We’ll need to adjust doc/guix.texi accordingly.

How does that sound?

Thanks,
Ludo’.




^ permalink raw reply	[flat|nested] 22+ messages in thread

* [bug#41143] [PATCH 2/2] mapped-devices: Add 'lvm-device-mapping'
  2020-05-09  1:22 ` [bug#41143] [PATCH 2/2] mapped-devices: Add 'lvm-device-mapping' tsmish
@ 2020-09-09 20:48   ` Ludovic Courtès
  0 siblings, 0 replies; 22+ messages in thread
From: Ludovic Courtès @ 2020-09-09 20:48 UTC (permalink / raw)
  To: tsmish; +Cc: 41143

tsmish <tsymsh@gmail.com> skribis:

> "vgscan --mknodes" is a bit of a hack. Everyone else relies on udev to
> create files in /dev/mapper, but since initrd doesn't have working
> udevd, they need to be created this way.

Oh, I guess that’s fine.  Could you place this as a comment above the
“vgscan” invocation?

> Also, while this code is able to boot from root on LVM, grub in
> current configuration can't find required files, This can be fixed by
> placing (format port "insmod lvm") in grub configuration builder, but
> this is somewhat hacky.

Uh, future work.  :-)

> +(define (open-lvm-device source target)
> +  #~(begin
> +       (use-modules
> +        (srfi srfi-1)
> +        (srfi srfi-26))

Since this gets spliced into the initrd expression (not at the top
level), we cannot have ‘use-modules’ here (well, it may not work as
expected, as you found out with srfi-26 macros).

I’d suggest adding (srfi srfi-1) to the ‘use-modules’ form in
‘raw-initrd’, in (gnu system linux-initrd), so you can rely on it
(srfi-26 is already there).

It would be great to have a system test for LVM support.  We have tests
for Btrfs, RAID with mdadm, etc., but these are system installation
tests in (gnu tests install).  Do you think we could have either an
installation test or maybe a less expensive test for LVM?

Thanks for your work!

Ludo’.




^ permalink raw reply	[flat|nested] 22+ messages in thread

* [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings
  2020-09-09 20:38 ` Ludovic Courtès
@ 2020-09-24 16:09   ` Mikhail Tsykalov
  2020-09-25  9:34     ` Ludovic Courtès
  0 siblings, 1 reply; 22+ messages in thread
From: Mikhail Tsykalov @ 2020-09-24 16:09 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 41143

Hi Ludovic,

On 09.09.2020 23:38, Ludovic Courtès wrote:
>> diff --git a/gnu/services/base.scm b/gnu/services/base.scm
>> index 0c154d1c4e..3d09e8220c 100644
>> --- a/gnu/services/base.scm
>> +++ b/gnu/services/base.scm
>> @@ -408,7 +408,10 @@ FILE-SYSTEM."
>>   (define (mapped-device->shepherd-service-name md)
>>     "Return the symbol that denotes the shepherd service of MD, a
>> <mapped-device>."
>>     (symbol-append 'device-mapping-
>> -                 (string->symbol (mapped-device-target md))))
>> +                 (string->symbol (string-join
>> +                                  (let ((t (mapped-device-target md)))
>> +                                    (if (list? t) t (list t)))
>> +                                  "-"))))
> To avoid duplicating the (if (list? t) …) everywhere, I propose instead
> the following approach:
>
>    1. Rename ‘target’ to ‘targets’ (plural) and likewise for the
>       accessor, and agree that it always contains a list;
>
>    2. Rename ‘mapped-device’ to ‘%mapped-device’ and add a
>       ‘mapped-device’ backward-compatibility macro that allows for a
>       ‘target’ (singular) field and automatically turns its value into a
>       list.  See the ‘origin’ macro in (guix packages) for an example of
>       how to do that (that macro allows users to specify ‘sha256’ instead
>       of ‘hash’).
>
>    3. Add a deprecated ‘mapped-device-target’ (singular) that returns the
>       first element returned by ‘mapped-device-targets’.

While this looks like a good idea, doesn't this break code that 
implements mapped-device and assumes that target is a string. Suddenly 
passing a string to a mapped-device constructor results in a list passed 
to open/close. Also, what functions should do if they expect a string 
but get a list of them? Ignore everything but the first item? Implement 
mandatory check function? Doesn't this change push complexity out of 
mapped-device to implementations of it.

Basically, this change will break all current implementations of 
mapped-devices, so I wanted to hear your thoughts on it before continuing.

Mikhail





^ permalink raw reply	[flat|nested] 22+ messages in thread

* [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings
  2020-09-24 16:09   ` Mikhail Tsykalov
@ 2020-09-25  9:34     ` Ludovic Courtès
  2020-09-25 13:36       ` Mikhail Tsykalov
  0 siblings, 1 reply; 22+ messages in thread
From: Ludovic Courtès @ 2020-09-25  9:34 UTC (permalink / raw)
  To: Mikhail Tsykalov; +Cc: 41143

Hi Mikhail,

Mikhail Tsykalov <tsymsh@gmail.com> skribis:

> On 09.09.2020 23:38, Ludovic Courtès wrote:
>>> diff --git a/gnu/services/base.scm b/gnu/services/base.scm
>>> index 0c154d1c4e..3d09e8220c 100644
>>> --- a/gnu/services/base.scm
>>> +++ b/gnu/services/base.scm
>>> @@ -408,7 +408,10 @@ FILE-SYSTEM."
>>>   (define (mapped-device->shepherd-service-name md)
>>>     "Return the symbol that denotes the shepherd service of MD, a
>>> <mapped-device>."
>>>     (symbol-append 'device-mapping-
>>> -                 (string->symbol (mapped-device-target md))))
>>> +                 (string->symbol (string-join
>>> +                                  (let ((t (mapped-device-target md)))
>>> +                                    (if (list? t) t (list t)))
>>> +                                  "-"))))
>> To avoid duplicating the (if (list? t) …) everywhere, I propose instead
>> the following approach:
>>
>>    1. Rename ‘target’ to ‘targets’ (plural) and likewise for the
>>       accessor, and agree that it always contains a list;
>>
>>    2. Rename ‘mapped-device’ to ‘%mapped-device’ and add a
>>       ‘mapped-device’ backward-compatibility macro that allows for a
>>       ‘target’ (singular) field and automatically turns its value into a
>>       list.  See the ‘origin’ macro in (guix packages) for an example of
>>       how to do that (that macro allows users to specify ‘sha256’ instead
>>       of ‘hash’).
>>
>>    3. Add a deprecated ‘mapped-device-target’ (singular) that returns the
>>       first element returned by ‘mapped-device-targets’.
>
> While this looks like a good idea, doesn't this break code that
> implements mapped-device and assumes that target is a string. Suddenly 
> passing a string to a mapped-device constructor results in a list
> passed to open/close. Also, what functions should do if they expect a
> string but get a list of them? Ignore everything but the first item?
> Implement mandatory check function? Doesn't this change push
> complexity out of mapped-device to implementations of it.

The intent of what I propose above is (1) to not break existing code,
and (2) to avoid duplicating checks and conversions at every call site.

#1 is achieved by providing a deprecated ‘mapped-device-target’
(singular) procedure, for example.

Does that make sense?

Thanks,
Ludo’.




^ permalink raw reply	[flat|nested] 22+ messages in thread

* [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings
  2020-09-25  9:34     ` Ludovic Courtès
@ 2020-09-25 13:36       ` Mikhail Tsykalov
  2020-09-25 16:20         ` Ludovic Courtès
  0 siblings, 1 reply; 22+ messages in thread
From: Mikhail Tsykalov @ 2020-09-25 13:36 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 41143

Hi, Ludovic

On 25.09.2020 12:34, Ludovic Courtès wrote:
> Hi Mikhail,
>
> Mikhail Tsykalov <tsymsh@gmail.com> skribis:
>
>> On 09.09.2020 23:38, Ludovic Courtès wrote:
>>>> diff --git a/gnu/services/base.scm b/gnu/services/base.scm
>>>> index 0c154d1c4e..3d09e8220c 100644
>>>> --- a/gnu/services/base.scm
>>>> +++ b/gnu/services/base.scm
>>>> @@ -408,7 +408,10 @@ FILE-SYSTEM."
>>>>    (define (mapped-device->shepherd-service-name md)
>>>>      "Return the symbol that denotes the shepherd service of MD, a
>>>> <mapped-device>."
>>>>      (symbol-append 'device-mapping-
>>>> -                 (string->symbol (mapped-device-target md))))
>>>> +                 (string->symbol (string-join
>>>> +                                  (let ((t (mapped-device-target md)))
>>>> +                                    (if (list? t) t (list t)))
>>>> +                                  "-"))))
>>> To avoid duplicating the (if (list? t) …) everywhere, I propose instead
>>> the following approach:
>>>
>>>     1. Rename ‘target’ to ‘targets’ (plural) and likewise for the
>>>        accessor, and agree that it always contains a list;
>>>
>>>     2. Rename ‘mapped-device’ to ‘%mapped-device’ and add a
>>>        ‘mapped-device’ backward-compatibility macro that allows for a
>>>        ‘target’ (singular) field and automatically turns its value into a
>>>        list.  See the ‘origin’ macro in (guix packages) for an example of
>>>        how to do that (that macro allows users to specify ‘sha256’ instead
>>>        of ‘hash’).
>>>
>>>     3. Add a deprecated ‘mapped-device-target’ (singular) that returns the
>>>        first element returned by ‘mapped-device-targets’.
>> While this looks like a good idea, doesn't this break code that
>> implements mapped-device and assumes that target is a string. Suddenly
>> passing a string to a mapped-device constructor results in a list
>> passed to open/close. Also, what functions should do if they expect a
>> string but get a list of them? Ignore everything but the first item?
>> Implement mandatory check function? Doesn't this change push
>> complexity out of mapped-device to implementations of it.
> The intent of what I propose above is (1) to not break existing code,
> and (2) to avoid duplicating checks and conversions at every call site.
>
> #1 is achieved by providing a deprecated ‘mapped-device-target’
> (singular) procedure, for example.
>
> Does that make sense?

I'm sorry if I didn't make myself clear, but it doesn't seem like 
open/close functions even use any mapped-device-* procedures, they just 
get passed source and target field directly. What I meant was this 
change will require changes to luks-device-mapping, raid-device-mapping 
and all other device mappings that users may have implemented in their 
local trees/config.

To be fair, after thinking about it for a bit, I think that this issue 
can be solved by renaming mapped-device-kind and providing compatibility 
macros similar to %mapped-device. Still question remains about what 
should we do if a list gets passed to a kind that doesn't expect it, but 
I think we can just raise an error in macro if that's the case. Does 
this sound fine to you?

Thanks,
Mikhail





^ permalink raw reply	[flat|nested] 22+ messages in thread

* [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings
  2020-09-25 13:36       ` Mikhail Tsykalov
@ 2020-09-25 16:20         ` Ludovic Courtès
  2020-10-01 22:48           ` [bug#41143] [PATCH v2 " Mikhail Tsykalov
  2020-10-01 23:15           ` [bug#41143] [PATCH " Mikhail Tsykalov
  0 siblings, 2 replies; 22+ messages in thread
From: Ludovic Courtès @ 2020-09-25 16:20 UTC (permalink / raw)
  To: Mikhail Tsykalov; +Cc: 41143

Hi,

Mikhail Tsykalov <tsymsh@gmail.com> skribis:

[...]

>>> While this looks like a good idea, doesn't this break code that
>>> implements mapped-device and assumes that target is a string. Suddenly
>>> passing a string to a mapped-device constructor results in a list
>>> passed to open/close. Also, what functions should do if they expect a
>>> string but get a list of them? Ignore everything but the first item?
>>> Implement mandatory check function? Doesn't this change push
>>> complexity out of mapped-device to implementations of it.
>> The intent of what I propose above is (1) to not break existing code,
>> and (2) to avoid duplicating checks and conversions at every call site.
>>
>> #1 is achieved by providing a deprecated ‘mapped-device-target’
>> (singular) procedure, for example.
>>
>> Does that make sense?
>
> I'm sorry if I didn't make myself clear, but it doesn't seem like
> open/close functions even use any mapped-device-* procedures, they
> just get passed source and target field directly. What I meant was
> this change will require changes to luks-device-mapping,
> raid-device-mapping and all other device mappings that users may have
> implemented in their local trees/config.

Ah yes, got it.

I tend to think it’s OK though, if we assume all the implementations are
in-tree, which would be consistent with the fact that the internals (how
to implement a mapped device type) are undocumented.

WDYT?

IOW, I think we must provide compatibility for users (people writing
their OS config, including perhaps service definitions) while leaving us
the ability to change internal interfaces.

Thanks,
Ludo’.




^ permalink raw reply	[flat|nested] 22+ messages in thread

* [bug#41143] [PATCH v2 1/2] mapped-devices: Allow target to be list of strings.
  2020-09-25 16:20         ` Ludovic Courtès
@ 2020-10-01 22:48           ` Mikhail Tsykalov
  2020-10-01 22:49             ` [bug#41143] [PATCH v2 2/2] mapped-devices: Add 'lvm-device-mapping' Mikhail Tsykalov
  2020-10-04 10:28             ` [bug#41143] [PATCH v2 1/2] mapped-devices: Allow target to be list of strings Ludovic Courtès
  2020-10-01 23:15           ` [bug#41143] [PATCH " Mikhail Tsykalov
  1 sibling, 2 replies; 22+ messages in thread
From: Mikhail Tsykalov @ 2020-10-01 22:48 UTC (permalink / raw)
  To: ludo; +Cc: 41143, Mikhail Tsykalov

* gnu/system/mapped-devices.scm (<mapped-device>): Rename constructor to
%mapped-device.
[target]: Remove field.
[targets]: New field. Adjust users.
(mapped-device-compatibility-helper, mapped-device): New macros.
(mapped-device-target): New deprecated procedure.
---
 gnu/services/base.scm         |  3 ++-
 gnu/system.scm                | 11 +++++-----
 gnu/system/linux-initrd.scm   |  2 +-
 gnu/system/mapped-devices.scm | 40 ++++++++++++++++++++++++++++-------
 4 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 04bc991356..4aa14ebf99 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -291,7 +291,8 @@ FILE-SYSTEM."
 (define (mapped-device->shepherd-service-name md)
   "Return the symbol that denotes the shepherd service of MD, a <mapped-device>."
   (symbol-append 'device-mapping-
-                 (string->symbol (mapped-device-target md))))
+                 (string->symbol (string-join
+                                  (mapped-device-targets md) "-"))))
 
 (define dependency->shepherd-service-name
   (match-lambda
diff --git a/gnu/system.scm b/gnu/system.scm
index bdb696fe2e..1bb812256f 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -444,9 +444,9 @@ marked as 'needed-for-boot'."
     (let ((device (file-system-device fs)))
       (if (string? device)                        ;title is 'device
           (filter (lambda (md)
-                    (string=? (string-append "/dev/mapper/"
-                                             (mapped-device-target md))
-                              device))
+                    (any (cut string=? device <>)
+                         (map (cut string-append "/dev/mapper" <>)
+                              (mapped-device-targets md))))
                   (operating-system-mapped-devices os))
           '())))
 
@@ -466,11 +466,12 @@ marked as 'needed-for-boot'."
 
 (define (mapped-device-users device file-systems)
   "Return the subset of FILE-SYSTEMS that use DEVICE."
-  (let ((target (string-append "/dev/mapper/" (mapped-device-target device))))
+  (let ((targets (map (cut string-append "/dev/mapper/" <>)
+                      (mapped-device-targets device))))
     (filter (lambda (fs)
               (or (member device (file-system-dependencies fs))
                   (and (string? (file-system-device fs))
-                       (string=? (file-system-device fs) target))))
+                       (any (cut string=? (file-system-device fs) <>) targets))))
             file-systems)))
 
 (define (operating-system-user-mapped-devices os)
diff --git a/gnu/system/linux-initrd.scm b/gnu/system/linux-initrd.scm
index b8a30c0abc..db02059a26 100644
--- a/gnu/system/linux-initrd.scm
+++ b/gnu/system/linux-initrd.scm
@@ -196,7 +196,7 @@ upon error."
     ;; List of gexps to open the mapped devices.
     (map (lambda (md)
            (let* ((source (mapped-device-source md))
-                  (target (mapped-device-target md))
+                  (target (mapped-device-targets md))
                   (type   (mapped-device-type md))
                   (open   (mapped-device-kind-open type)))
              (open source target)))
diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scm
index 31c50c4e40..8622418fcf 100644
--- a/gnu/system/mapped-devices.scm
+++ b/gnu/system/mapped-devices.scm
@@ -28,6 +28,7 @@
                           formatted-message
                           &fix-hint
                           &error-location))
+  #:use-module (guix deprecation)
   #:use-module (gnu services)
   #:use-module (gnu services shepherd)
   #:use-module (gnu system uuid)
@@ -42,10 +43,12 @@
   #:use-module (srfi srfi-35)
   #:use-module (ice-9 match)
   #:use-module (ice-9 format)
-  #:export (mapped-device
+  #:export (%mapped-device
+            mapped-device
             mapped-device?
             mapped-device-source
             mapped-device-target
+            mapped-device-targets
             mapped-device-type
             mapped-device-location
 
@@ -70,15 +73,36 @@
 ;;;
 ;;; Code:
 
-(define-record-type* <mapped-device> mapped-device
+(define-record-type* <mapped-device> %mapped-device
   make-mapped-device
   mapped-device?
   (source    mapped-device-source)                ;string | list of strings
-  (target    mapped-device-target)                ;string
+  (targets   mapped-device-targets)               ;list of strings
   (type      mapped-device-type)                  ;<mapped-device-kind>
   (location  mapped-device-location
              (default (current-source-location)) (innate)))
 
+(define-syntax mapped-device-compatibility-helper
+  (syntax-rules (target)
+    ((_ () (fields ...))
+     (%mapped-device fields ...))
+    ((_ ((target exp) rest ...) (others ...))
+     (%mapped-device others ...
+                      (targets (list exp))
+                      rest ...))
+    ((_ (field rest ...) (others ...))
+     (mapped-device-compatibility-helper (rest ...)
+                                         (others ... field)))))
+
+(define-syntax-rule (mapped-device fields ...)
+  "Build an <mapped-device> record, automatically converting 'target' field
+specifications to 'targets'."
+  (mapped-device-compatibility-helper (fields ...) ()))
+
+(define-deprecated (mapped-device-target md)
+  mapped-device-targets
+  (car (mapped-device-targets md)))
+
 (define-record-type* <mapped-device-type> mapped-device-kind
   make-mapped-device-kind
   mapped-device-kind?
@@ -100,7 +124,7 @@
      (($ <mapped-device> source target
                          ($ <mapped-device-type> open close))
       (shepherd-service
-       (provision (list (symbol-append 'device-mapping- (string->symbol target))))
+       (provision (list (symbol-append 'device-mapping- (string->symbol (string-join target "-")))))
        (requirement '(udev))
        (documentation "Map a device node using Linux's device mapper.")
        (start #~(lambda () #$(open source target)))
@@ -198,12 +222,12 @@ option of @command{guix system}.\n")
                                 (error "LUKS partition not found" source))
                             source)
 
-                        #$target)))))
+                        #$(car target))))))
 
 (define (close-luks-device source target)
   "Return a gexp that closes TARGET, a LUKS device."
   #~(zero? (system* #$(file-append cryptsetup-static "/sbin/cryptsetup")
-                    "close" #$target)))
+                    "close" #$(car target))))
 
 (define* (check-luks-device md #:key
                             needed-for-boot?
@@ -259,12 +283,12 @@ TARGET (e.g., \"/dev/md0\"), using 'mdadm'."
       ;; Use 'mdadm-static' rather than 'mdadm' to avoid pulling its whole
       ;; closure (80 MiB) in the initrd when a RAID device is needed for boot.
       (zero? (apply system* #$(file-append mdadm-static "/sbin/mdadm")
-                    "--assemble" #$target sources))))
+                    "--assemble" #$(car target) sources))))
 
 (define (close-raid-device sources target)
   "Return a gexp that stops the RAID device TARGET."
   #~(zero? (system* #$(file-append mdadm-static "/sbin/mdadm")
-                    "--stop" #$target)))
+                    "--stop" #$(car target))))
 
 (define raid-device-mapping
   ;; The type of RAID mapped devices.
-- 
2.28.0





^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [bug#41143] [PATCH v2 2/2] mapped-devices: Add 'lvm-device-mapping'.
  2020-10-01 22:48           ` [bug#41143] [PATCH v2 " Mikhail Tsykalov
@ 2020-10-01 22:49             ` Mikhail Tsykalov
  2020-10-04 10:34               ` Ludovic Courtès
  2020-10-04 10:28             ` [bug#41143] [PATCH v2 1/2] mapped-devices: Allow target to be list of strings Ludovic Courtès
  1 sibling, 1 reply; 22+ messages in thread
From: Mikhail Tsykalov @ 2020-10-01 22:49 UTC (permalink / raw)
  To: ludo; +Cc: 41143, Mikhail Tsykalov

* gnu/system/mapped-devices.scm (lvm-device-mapping, open-lvm-device,
close-lvm-device): New variables.

* gnu/tests/install.scm (%lvm-separate-home-os,
%lvm-separate-home-os-source, %lvm-separate-home-installation-script,
%test-lvm-separate-home-os): New variables.

* gnu/system/linux-initrd.scm (raw-initrd): Add (srfi srfi-1) to initrd expression.
---
 gnu/system/linux-initrd.scm   |  1 +
 gnu/system/mapped-devices.scm | 25 +++++++++-
 gnu/tests/install.scm         | 87 +++++++++++++++++++++++++++++++++++
 3 files changed, 111 insertions(+), 2 deletions(-)

diff --git a/gnu/system/linux-initrd.scm b/gnu/system/linux-initrd.scm
index db02059a26..41cbe28c3c 100644
--- a/gnu/system/linux-initrd.scm
+++ b/gnu/system/linux-initrd.scm
@@ -217,6 +217,7 @@ upon error."
                       (gnu system file-systems)
                       ((guix build utils) #:hide (delete))
                       (guix build bournish)   ;add the 'bournish' meta-command
+                      (srfi srfi-1)
                       (srfi srfi-26)
 
                       ;; FIXME: The following modules are for
diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scm
index 8622418fcf..8088bd99a3 100644
--- a/gnu/system/mapped-devices.scm
+++ b/gnu/system/mapped-devices.scm
@@ -36,7 +36,7 @@
   #:autoload   (gnu build linux-modules)
                  (missing-modules)
   #:autoload   (gnu packages cryptsetup) (cryptsetup-static)
-  #:autoload   (gnu packages linux) (mdadm-static)
+  #:autoload   (gnu packages linux) (mdadm-static lvm2-static)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-34)
@@ -64,7 +64,8 @@
             check-device-initrd-modules           ;XXX: needs a better place
 
             luks-device-mapping
-            raid-device-mapping))
+            raid-device-mapping
+            lvm-device-mapping))
 
 ;;; Commentary:
 ;;;
@@ -296,4 +297,24 @@ TARGET (e.g., \"/dev/md0\"), using 'mdadm'."
    (open open-raid-device)
    (close close-raid-device)))
 
+(define (open-lvm-device source targets)
+  #~(and
+     (zero? (system* #$(file-append lvm2-static "/sbin/lvm")
+                     "vgchange" "--activate" "ay" #$source))
+     ; /dev/mapper nodes are usually created by udev, but udev may be unavailable at the time we run this. So we create them here.
+     (zero? (system* #$(file-append lvm2-static "/sbin/lvm")
+                     "vgscan" "--mknodes"))
+     (every file-exists? (map (lambda (file) (string-append "/dev/mapper/" file))
+                              '#$targets))))
+
+
+(define (close-lvm-device source targets)
+  #~(zero? (system* #$(file-append lvm2-static "/sbin/lvm")
+                    "vgchange" "--activate" "n" #$source)))
+
+(define lvm-device-mapping
+  (mapped-device-kind
+   (open open-lvm-device)
+   (close close-lvm-device)))
+
 ;;; mapped-devices.scm ends here
diff --git a/gnu/tests/install.scm b/gnu/tests/install.scm
index dee2b870e8..84fe8bda78 100644
--- a/gnu/tests/install.scm
+++ b/gnu/tests/install.scm
@@ -65,6 +65,7 @@
             %test-btrfs-root-on-subvolume-os
             %test-jfs-root-os
             %test-f2fs-root-os
+            %test-lvm-separate-home-os
 
             %test-gui-installed-os
             %test-gui-installed-os-encrypted
@@ -794,6 +795,92 @@ build (current-guix) and then store a couple of full system images.")
       (run-basic-test %encrypted-root-os command "encrypted-root-os"
                       #:initialization enter-luks-passphrase)))))
 
+\f
+;;;
+;;; Separate /home on LVM
+;;;
+
+; Since LVM support in guix currently doesn't allow root-on-LVM we use /home on LVM
+(define-os-with-source (%lvm-separate-home-os %lvm-separate-home-os-source)
+  (use-modules (gnu) (gnu tests))
+
+  (operating-system
+    (host-name "separate-home-on-lvm")
+    (timezone "Europe/Paris")
+    (locale "en_US.utf8")
+
+    (bootloader (bootloader-configuration
+                 (bootloader grub-bootloader)
+                 (target "/dev/vdb")))
+    (kernel-arguments '("console=ttyS0"))
+
+    (mapped-devices (list (mapped-device
+                           (source "vg0")
+                           (target "vg0-home")
+                           (type lvm-device-mapping))))
+    (file-systems (cons* (file-system
+                           (device (file-system-label "root-fs"))
+                           (mount-point "/")
+                           (type "ext4"))
+                         (file-system
+                           (device "/dev/mapper/vg0-home")
+                           (mount-point "/home")
+                           (type "ext4")
+                           (dependencies mapped-devices))
+                        %base-file-systems))
+    (users %base-user-accounts)
+    (services (cons (service marionette-service-type
+                             (marionette-configuration
+                              (imported-modules '((gnu services herd)
+                                                  (guix combinators)))))
+                    %base-services))))
+
+(define %lvm-separate-home-installation-script
+  "\
+. /etc/profile
+set -e -x
+guix --version
+
+export GUIX_BUILD_OPTIONS=--no-grafts
+parted --script /dev/vdb mklabel gpt \\
+  mkpart primary ext2 1M 3M \\
+  mkpart primary ext2 3M 1.6G \\
+  mkpart primary 1.6G 3.2G \\
+  set 1 boot on \\
+  set 1 bios_grub on
+pvcreate /dev/vdb3
+vgcreate vg0 /dev/vdb3
+lvcreate -L 1.6G -n home vg0
+vgchange -ay
+mkfs.ext4 -L root-fs /dev/vdb2
+mkfs.ext4 /dev/mapper/vg0-home
+mount /dev/vdb2 /mnt
+mkdir /mnt/home
+mount /dev/mapper/vg0-home /mnt/home
+df -h /mnt /mnt/home
+herd start cow-store /mnt
+mkdir /mnt/etc
+cp /etc/target-config.scm /mnt/etc/config.scm
+guix system init /mnt/etc/config.scm /mnt --no-substitutes
+sync
+reboot\n")
+
+(define %test-lvm-separate-home-os
+  (system-test
+   (name "lvm-separate-home-os")
+   (description
+    "Test functionality of an OS installed with a LVM /home partition")
+   (value
+    (mlet* %store-monad ((image   (run-install %lvm-separate-home-os
+                                               %lvm-separate-home-os-source
+                                               #:script
+                                               %lvm-separate-home-installation-script
+                                               #:packages (list lvm2-static)
+                                               #:target-size (* 3200 MiB)))
+                         (command (qemu-command/writable-image image)))
+      (run-basic-test %lvm-separate-home-os
+                      `(,@command) "lvm-separate-home-os")))))
+
 \f
 ;;;
 ;;; Btrfs root file system.
-- 
2.28.0





^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings
  2020-09-25 16:20         ` Ludovic Courtès
  2020-10-01 22:48           ` [bug#41143] [PATCH v2 " Mikhail Tsykalov
@ 2020-10-01 23:15           ` Mikhail Tsykalov
  1 sibling, 0 replies; 22+ messages in thread
From: Mikhail Tsykalov @ 2020-10-01 23:15 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 41143

Hi,

Hoping I didn't mess up too much with patches. System tests seem to be 
somewhat unstable and really long, but I think raid-root-os and 
encrypted-root-os pass. Somewhat unsure about adding "(srfi srfi-1)" to 
raw-initrd since current implementation shouldn't ever end up in initrd, 
but this would be helpful if we actually implement booting from LVM and 
I think I saw request to add it there in open-raid-device.

I also tend not to answer to things I agree with and was a bit busy with 
work recently, so I hope you understand me taking so long to answer.

Thanks,
Mikhail





^ permalink raw reply	[flat|nested] 22+ messages in thread

* [bug#41143] [PATCH v2 1/2] mapped-devices: Allow target to be list of strings.
  2020-10-01 22:48           ` [bug#41143] [PATCH v2 " Mikhail Tsykalov
  2020-10-01 22:49             ` [bug#41143] [PATCH v2 2/2] mapped-devices: Add 'lvm-device-mapping' Mikhail Tsykalov
@ 2020-10-04 10:28             ` Ludovic Courtès
  2020-11-05  9:48               ` Ludovic Courtès
  1 sibling, 1 reply; 22+ messages in thread
From: Ludovic Courtès @ 2020-10-04 10:28 UTC (permalink / raw)
  To: Mikhail Tsykalov; +Cc: 41143

Hi Mikhail,

Mikhail Tsykalov <tsymsh@gmail.com> skribis:

> * gnu/system/mapped-devices.scm (<mapped-device>): Rename constructor to
> %mapped-device.
> [target]: Remove field.
> [targets]: New field. Adjust users.
> (mapped-device-compatibility-helper, mapped-device): New macros.
> (mapped-device-target): New deprecated procedure.

Thanks for following up.  I think we’re almost done, some comments
below:

> --- a/gnu/system/linux-initrd.scm
> +++ b/gnu/system/linux-initrd.scm
> @@ -196,7 +196,7 @@ upon error."
>      ;; List of gexps to open the mapped devices.
>      (map (lambda (md)
>             (let* ((source (mapped-device-source md))
> -                  (target (mapped-device-target md))
> +                  (target (mapped-device-targets md))

I think we should write ‘targets’ (plural) everywhere.  That can help
avoid confusion IMO.

> -                        #$target)))))
> +                        #$(car target))))))
>  
>  (define (close-luks-device source target)
>    "Return a gexp that closes TARGET, a LUKS device."
>    #~(zero? (system* #$(file-append cryptsetup-static "/sbin/cryptsetup")
> -                    "close" #$target)))
> +                    "close" #$(car target))))

As per our coding convention (info "(guix) Data Types and Pattern
Matching"), I’d recommend using ‘match’

  (define (close-luks-device source targets)
    (match targets
      ((target)
       #~(zero? (system* … #$target)))))

That has the added benefit that it errors out if TARGETS is not exactly
a one-element list.

>  (define (close-raid-device sources target)
>    "Return a gexp that stops the RAID device TARGET."
>    #~(zero? (system* #$(file-append mdadm-static "/sbin/mdadm")
> -                    "--stop" #$target)))
> +                    "--stop" #$(car target))))

Same here.

Could you also update “Mapped Devices” in doc/guix.texi to mention the
new ‘targets’ field?

Thanks,
Ludo’.




^ permalink raw reply	[flat|nested] 22+ messages in thread

* [bug#41143] [PATCH v2 2/2] mapped-devices: Add 'lvm-device-mapping'.
  2020-10-01 22:49             ` [bug#41143] [PATCH v2 2/2] mapped-devices: Add 'lvm-device-mapping' Mikhail Tsykalov
@ 2020-10-04 10:34               ` Ludovic Courtès
  0 siblings, 0 replies; 22+ messages in thread
From: Ludovic Courtès @ 2020-10-04 10:34 UTC (permalink / raw)
  To: Mikhail Tsykalov; +Cc: 41143

Mikhail Tsykalov <tsymsh@gmail.com> skribis:

> * gnu/system/mapped-devices.scm (lvm-device-mapping, open-lvm-device,
> close-lvm-device): New variables.
>
> * gnu/tests/install.scm (%lvm-separate-home-os,
> %lvm-separate-home-os-source, %lvm-separate-home-installation-script,
> %test-lvm-separate-home-os): New variables.
>
> * gnu/system/linux-initrd.scm (raw-initrd): Add (srfi srfi-1) to initrd expression.

Nice!

> +++ b/gnu/system/linux-initrd.scm
> @@ -217,6 +217,7 @@ upon error."
>                        (gnu system file-systems)
>                        ((guix build utils) #:hide (delete))
>                        (guix build bournish)   ;add the 'bournish' meta-command
> +                      (srfi srfi-1)

Maybe add a comment saying this is for ‘lvm-device-mapping’.

> +(define %test-lvm-separate-home-os
> +  (system-test
> +   (name "lvm-separate-home-os")
> +   (description
> +    "Test functionality of an OS installed with a LVM /home partition")

Great.

Do you know what it would take to have a LVM on root?  (As future work,
of course.)

Could you document ‘lvm-device-mapping’ under “Mapped Devices” in
doc/guix.texi?  Essentially adding a @defvr for ‘lvm-device-mapping’ and
a paragraph with an example.

There’s also a sentence somewhere in guix.texi that says LVM is not
supported, we can probably remove it.  :-)

Last: you can add an entry in etc/news.scm to mention LVM support since
that’s a long awaited feature!

Thanks!

Ludo’.




^ permalink raw reply	[flat|nested] 22+ messages in thread

* [bug#41143] [PATCH v2 1/2] mapped-devices: Allow target to be list of strings.
  2020-10-04 10:28             ` [bug#41143] [PATCH v2 1/2] mapped-devices: Allow target to be list of strings Ludovic Courtès
@ 2020-11-05  9:48               ` Ludovic Courtès
  2020-11-06  9:47                 ` [bug#41143] [PATCH v3 " Mikhail Tsykalov
  0 siblings, 1 reply; 22+ messages in thread
From: Ludovic Courtès @ 2020-11-05  9:48 UTC (permalink / raw)
  To: Mikhail Tsykalov; +Cc: 41143

Hi Mikhail,

Did you have a chance to look into the proposed changes?

  https://issues.guix.gnu.org/41143

Would be nice to have LVM support integrated!

TIA,
Ludo’.

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

> Hi Mikhail,
>
> Mikhail Tsykalov <tsymsh@gmail.com> skribis:
>
>> * gnu/system/mapped-devices.scm (<mapped-device>): Rename constructor to
>> %mapped-device.
>> [target]: Remove field.
>> [targets]: New field. Adjust users.
>> (mapped-device-compatibility-helper, mapped-device): New macros.
>> (mapped-device-target): New deprecated procedure.
>
> Thanks for following up.  I think we’re almost done, some comments
> below:
>
>> --- a/gnu/system/linux-initrd.scm
>> +++ b/gnu/system/linux-initrd.scm
>> @@ -196,7 +196,7 @@ upon error."
>>      ;; List of gexps to open the mapped devices.
>>      (map (lambda (md)
>>             (let* ((source (mapped-device-source md))
>> -                  (target (mapped-device-target md))
>> +                  (target (mapped-device-targets md))
>
> I think we should write ‘targets’ (plural) everywhere.  That can help
> avoid confusion IMO.
>
>> -                        #$target)))))
>> +                        #$(car target))))))
>>  
>>  (define (close-luks-device source target)
>>    "Return a gexp that closes TARGET, a LUKS device."
>>    #~(zero? (system* #$(file-append cryptsetup-static "/sbin/cryptsetup")
>> -                    "close" #$target)))
>> +                    "close" #$(car target))))
>
> As per our coding convention (info "(guix) Data Types and Pattern
> Matching"), I’d recommend using ‘match’
>
>   (define (close-luks-device source targets)
>     (match targets
>       ((target)
>        #~(zero? (system* … #$target)))))
>
> That has the added benefit that it errors out if TARGETS is not exactly
> a one-element list.
>
>>  (define (close-raid-device sources target)
>>    "Return a gexp that stops the RAID device TARGET."
>>    #~(zero? (system* #$(file-append mdadm-static "/sbin/mdadm")
>> -                    "--stop" #$target)))
>> +                    "--stop" #$(car target))))
>
> Same here.
>
> Could you also update “Mapped Devices” in doc/guix.texi to mention the
> new ‘targets’ field?
>
> Thanks,
> Ludo’.




^ permalink raw reply	[flat|nested] 22+ messages in thread

* [bug#41143] [PATCH v3 1/2] mapped-devices: Allow target to be list of strings.
  2020-11-05  9:48               ` Ludovic Courtès
@ 2020-11-06  9:47                 ` Mikhail Tsykalov
  2020-11-06  9:47                   ` [bug#41143] [PATCH v3 2/2] mapped-devices: Add 'lvm-device-mapping' Mikhail Tsykalov
  2020-11-25 23:09                   ` bug#41143: [PATCH v3 1/2] mapped-devices: Allow target to be list of strings Ludovic Courtès
  0 siblings, 2 replies; 22+ messages in thread
From: Mikhail Tsykalov @ 2020-11-06  9:47 UTC (permalink / raw)
  To: ludo; +Cc: 41143, Mikhail Tsykalov

* gnu/system/mapped-devices.scm (<mapped-device>): Rename constructor to
%mapped-device.
[target]: Remove field.
[targets]: New field. Adjust users.
(mapped-device-compatibility-helper, mapped-device): New macros.
(mapped-device-target): New deprecated procedure.
---
 doc/guix.texi                 |   3 +
 gnu/services/base.scm         |   3 +-
 gnu/system.scm                |  11 ++-
 gnu/system/linux-initrd.scm   |  10 +-
 gnu/system/mapped-devices.scm | 174 ++++++++++++++++++++--------------
 5 files changed, 119 insertions(+), 82 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 79c79b6a96..02b92a9b69 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -12735,6 +12735,9 @@ specifying @code{"my-partition"} leads to the creation of
 the @code{"/dev/mapper/my-partition"} device.
 For RAID devices of type @code{raid-device-mapping}, the full device name
 such as @code{"/dev/md0"} needs to be given.
+@item targets
+This list of strings specifies names of the resulting mapped devices in case
+there are several. The format is identical to @var{target}.
 
 @item type
 This must be a @code{mapped-device-kind} object, which specifies how
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 04bc991356..4aa14ebf99 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -291,7 +291,8 @@ FILE-SYSTEM."
 (define (mapped-device->shepherd-service-name md)
   "Return the symbol that denotes the shepherd service of MD, a <mapped-device>."
   (symbol-append 'device-mapping-
-                 (string->symbol (mapped-device-target md))))
+                 (string->symbol (string-join
+                                  (mapped-device-targets md) "-"))))
 
 (define dependency->shepherd-service-name
   (match-lambda
diff --git a/gnu/system.scm b/gnu/system.scm
index bdb696fe2e..1bb812256f 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -444,9 +444,9 @@ marked as 'needed-for-boot'."
     (let ((device (file-system-device fs)))
       (if (string? device)                        ;title is 'device
           (filter (lambda (md)
-                    (string=? (string-append "/dev/mapper/"
-                                             (mapped-device-target md))
-                              device))
+                    (any (cut string=? device <>)
+                         (map (cut string-append "/dev/mapper" <>)
+                              (mapped-device-targets md))))
                   (operating-system-mapped-devices os))
           '())))
 
@@ -466,11 +466,12 @@ marked as 'needed-for-boot'."
 
 (define (mapped-device-users device file-systems)
   "Return the subset of FILE-SYSTEMS that use DEVICE."
-  (let ((target (string-append "/dev/mapper/" (mapped-device-target device))))
+  (let ((targets (map (cut string-append "/dev/mapper/" <>)
+                      (mapped-device-targets device))))
     (filter (lambda (fs)
               (or (member device (file-system-dependencies fs))
                   (and (string? (file-system-device fs))
-                       (string=? (file-system-device fs) target))))
+                       (any (cut string=? (file-system-device fs) <>) targets))))
             file-systems)))
 
 (define (operating-system-user-mapped-devices os)
diff --git a/gnu/system/linux-initrd.scm b/gnu/system/linux-initrd.scm
index b8a30c0abc..3e2f1282cc 100644
--- a/gnu/system/linux-initrd.scm
+++ b/gnu/system/linux-initrd.scm
@@ -195,11 +195,11 @@ upon error."
   (define device-mapping-commands
     ;; List of gexps to open the mapped devices.
     (map (lambda (md)
-           (let* ((source (mapped-device-source md))
-                  (target (mapped-device-target md))
-                  (type   (mapped-device-type md))
-                  (open   (mapped-device-kind-open type)))
-             (open source target)))
+           (let* ((source  (mapped-device-source md))
+                  (targets (mapped-device-targets md))
+                  (type    (mapped-device-type md))
+                  (open    (mapped-device-kind-open type)))
+             (open source targets)))
          mapped-devices))
 
   (define kodir
diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scm
index 31c50c4e40..8b5aec983d 100644
--- a/gnu/system/mapped-devices.scm
+++ b/gnu/system/mapped-devices.scm
@@ -28,6 +28,7 @@
                           formatted-message
                           &fix-hint
                           &error-location))
+  #:use-module (guix deprecation)
   #:use-module (gnu services)
   #:use-module (gnu services shepherd)
   #:use-module (gnu system uuid)
@@ -42,10 +43,12 @@
   #:use-module (srfi srfi-35)
   #:use-module (ice-9 match)
   #:use-module (ice-9 format)
-  #:export (mapped-device
+  #:export (%mapped-device
+            mapped-device
             mapped-device?
             mapped-device-source
             mapped-device-target
+            mapped-device-targets
             mapped-device-type
             mapped-device-location
 
@@ -70,15 +73,36 @@
 ;;;
 ;;; Code:
 
-(define-record-type* <mapped-device> mapped-device
+(define-record-type* <mapped-device> %mapped-device
   make-mapped-device
   mapped-device?
   (source    mapped-device-source)                ;string | list of strings
-  (target    mapped-device-target)                ;string
+  (targets   mapped-device-targets)               ;list of strings
   (type      mapped-device-type)                  ;<mapped-device-kind>
   (location  mapped-device-location
              (default (current-source-location)) (innate)))
 
+(define-syntax mapped-device-compatibility-helper
+  (syntax-rules (target)
+    ((_ () (fields ...))
+     (%mapped-device fields ...))
+    ((_ ((target exp) rest ...) (others ...))
+     (%mapped-device others ...
+                      (targets (list exp))
+                      rest ...))
+    ((_ (field rest ...) (others ...))
+     (mapped-device-compatibility-helper (rest ...)
+                                         (others ... field)))))
+
+(define-syntax-rule (mapped-device fields ...)
+  "Build an <mapped-device> record, automatically converting 'target' field
+specifications to 'targets'."
+  (mapped-device-compatibility-helper (fields ...) ()))
+
+(define-deprecated (mapped-device-target md)
+  mapped-device-targets
+  (car (mapped-device-targets md)))
+
 (define-record-type* <mapped-device-type> mapped-device-kind
   make-mapped-device-kind
   mapped-device-kind?
@@ -97,14 +121,14 @@
   (shepherd-service-type
    'device-mapping
    (match-lambda
-     (($ <mapped-device> source target
+     (($ <mapped-device> source targets
                          ($ <mapped-device-type> open close))
       (shepherd-service
-       (provision (list (symbol-append 'device-mapping- (string->symbol target))))
+       (provision (list (symbol-append 'device-mapping- (string->symbol (string-join targets "-")))))
        (requirement '(udev))
        (documentation "Map a device node using Linux's device mapper.")
-       (start #~(lambda () #$(open source target)))
-       (stop #~(lambda _ (not #$(close source target))))
+       (start #~(lambda () #$(open source targets)))
+       (stop #~(lambda _ (not #$(close source targets))))
        (respawn? #f))))))
 
 (define (device-mapping-service mapped-device)
@@ -162,48 +186,52 @@ option of @command{guix system}.\n")
 ;;; Common device mappings.
 ;;;
 
-(define (open-luks-device source target)
+(define (open-luks-device source targets)
   "Return a gexp that maps SOURCE to TARGET as a LUKS device, using
 'cryptsetup'."
   (with-imported-modules (source-module-closure
                           '((gnu build file-systems)))
-    #~(let ((source #$(if (uuid? source)
-                          (uuid-bytevector source)
-                          source)))
-        ;; XXX: 'use-modules' should be at the top level.
-        (use-modules (rnrs bytevectors)           ;bytevector?
-                     ((gnu build file-systems)
-                      #:select (find-partition-by-luks-uuid)))
-
-        ;; Use 'cryptsetup-static', not 'cryptsetup', to avoid pulling the
-        ;; whole world inside the initrd (for when we're in an initrd).
-        (zero? (system* #$(file-append cryptsetup-static "/sbin/cryptsetup")
-                        "open" "--type" "luks"
-
-                        ;; Note: We cannot use the "UUID=source" syntax here
-                        ;; because 'cryptsetup' implements it by searching the
-                        ;; udev-populated /dev/disk/by-id directory but udev may
-                        ;; be unavailable at the time we run this.
-                        (if (bytevector? source)
-                            (or (let loop ((tries-left 10))
-                                  (and (positive? tries-left)
-                                       (or (find-partition-by-luks-uuid source)
-                                           ;; If the underlying partition is
-                                           ;; not found, try again after
-                                           ;; waiting a second, up to ten
-                                           ;; times.  FIXME: This should be
-                                           ;; dealt with in a more robust way.
-                                           (begin (sleep 1)
-                                                  (loop (- tries-left 1))))))
-                                (error "LUKS partition not found" source))
-                            source)
-
-                        #$target)))))
-
-(define (close-luks-device source target)
+    (match targets
+      ((target)
+       #~(let ((source #$(if (uuid? source)
+                             (uuid-bytevector source)
+                             source)))
+           ;; XXX: 'use-modules' should be at the top level.
+           (use-modules (rnrs bytevectors) ;bytevector?
+                        ((gnu build file-systems)
+                         #:select (find-partition-by-luks-uuid)))
+
+           ;; Use 'cryptsetup-static', not 'cryptsetup', to avoid pulling the
+           ;; whole world inside the initrd (for when we're in an initrd).
+           (zero? (system* #$(file-append cryptsetup-static "/sbin/cryptsetup")
+                           "open" "--type" "luks"
+
+                           ;; Note: We cannot use the "UUID=source" syntax here
+                           ;; because 'cryptsetup' implements it by searching the
+                           ;; udev-populated /dev/disk/by-id directory but udev may
+                           ;; be unavailable at the time we run this.
+                           (if (bytevector? source)
+                               (or (let loop ((tries-left 10))
+                                     (and (positive? tries-left)
+                                          (or (find-partition-by-luks-uuid source)
+                                              ;; If the underlying partition is
+                                              ;; not found, try again after
+                                              ;; waiting a second, up to ten
+                                              ;; times.  FIXME: This should be
+                                              ;; dealt with in a more robust way.
+                                              (begin (sleep 1)
+                                                     (loop (- tries-left 1))))))
+                                   (error "LUKS partition not found" source))
+                               source)
+
+                           #$target)))))))
+
+(define (close-luks-device source targets)
   "Return a gexp that closes TARGET, a LUKS device."
-  #~(zero? (system* #$(file-append cryptsetup-static "/sbin/cryptsetup")
-                    "close" #$target)))
+  (match targets
+    ((target)
+     #~(zero? (system* #$(file-append cryptsetup-static "/sbin/cryptsetup")
+                       "close" #$target)))))
 
 (define* (check-luks-device md #:key
                             needed-for-boot?
@@ -235,36 +263,40 @@ option of @command{guix system}.\n")
    (close close-luks-device)
    (check check-luks-device)))
 
-(define (open-raid-device sources target)
+(define (open-raid-device sources targets)
   "Return a gexp that assembles SOURCES (a list of devices) to the RAID device
 TARGET (e.g., \"/dev/md0\"), using 'mdadm'."
-  #~(let ((sources '#$sources)
-
-          ;; XXX: We're not at the top level here.  We could use a
-          ;; non-top-level 'use-modules' form but that doesn't work when the
-          ;; code is eval'd, like the Shepherd does.
-          (every   (@ (srfi srfi-1) every))
-          (format  (@ (ice-9 format) format)))
-      (let loop ((attempts 0))
-        (unless (every file-exists? sources)
-          (when (> attempts 20)
-            (error "RAID devices did not show up; bailing out"
-                   sources))
-
-          (format #t "waiting for RAID source devices~{ ~a~}...~%"
-                  sources)
-          (sleep 1)
-          (loop (+ 1 attempts))))
-
-      ;; Use 'mdadm-static' rather than 'mdadm' to avoid pulling its whole
-      ;; closure (80 MiB) in the initrd when a RAID device is needed for boot.
-      (zero? (apply system* #$(file-append mdadm-static "/sbin/mdadm")
-                    "--assemble" #$target sources))))
-
-(define (close-raid-device sources target)
+  (match targets
+    ((target)
+     #~(let ((sources '#$sources)
+
+             ;; XXX: We're not at the top level here.  We could use a
+             ;; non-top-level 'use-modules' form but that doesn't work when the
+             ;; code is eval'd, like the Shepherd does.
+             (every   (@ (srfi srfi-1) every))
+             (format  (@ (ice-9 format) format)))
+         (let loop ((attempts 0))
+           (unless (every file-exists? sources)
+             (when (> attempts 20)
+               (error "RAID devices did not show up; bailing out"
+                      sources))
+
+             (format #t "waiting for RAID source devices~{ ~a~}...~%"
+                     sources)
+             (sleep 1)
+             (loop (+ 1 attempts))))
+
+         ;; Use 'mdadm-static' rather than 'mdadm' to avoid pulling its whole
+         ;; closure (80 MiB) in the initrd when a RAID device is needed for boot.
+         (zero? (apply system* #$(file-append mdadm-static "/sbin/mdadm")
+                       "--assemble" #$target sources))))))
+
+(define (close-raid-device sources targets)
   "Return a gexp that stops the RAID device TARGET."
-  #~(zero? (system* #$(file-append mdadm-static "/sbin/mdadm")
-                    "--stop" #$target)))
+  (match targets
+    ((target)
+     #~(zero? (system* #$(file-append mdadm-static "/sbin/mdadm")
+                       "--stop" #$target)))))
 
 (define raid-device-mapping
   ;; The type of RAID mapped devices.
-- 
2.20.1





^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [bug#41143] [PATCH v3 2/2] mapped-devices: Add 'lvm-device-mapping'.
  2020-11-06  9:47                 ` [bug#41143] [PATCH v3 " Mikhail Tsykalov
@ 2020-11-06  9:47                   ` Mikhail Tsykalov
  2020-11-25 23:09                   ` bug#41143: [PATCH v3 1/2] mapped-devices: Allow target to be list of strings Ludovic Courtès
  1 sibling, 0 replies; 22+ messages in thread
From: Mikhail Tsykalov @ 2020-11-06  9:47 UTC (permalink / raw)
  To: ludo; +Cc: 41143, Mikhail Tsykalov

* gnu/system/mapped-devices.scm (lvm-device-mapping, open-lvm-device,
close-lvm-device): New variables.

* gnu/tests/install.scm (%lvm-separate-home-os,
%lvm-separate-home-os-source, %lvm-separate-home-installation-script,
%test-lvm-separate-home-os): New variables.

* gnu/system/linux-initrd.scm (raw-initrd): Add (srfi srfi-1) to initrd expression.
---
 doc/guix.texi                 | 25 +++++++++-
 gnu/system/linux-initrd.scm   |  1 +
 gnu/system/mapped-devices.scm | 25 +++++++++-
 gnu/tests/install.scm         | 87 +++++++++++++++++++++++++++++++++++
 4 files changed, 134 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 02b92a9b69..acb6453165 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -12713,7 +12713,6 @@ Guix extends this notion by considering any device or set of devices that
 are @dfn{transformed} in some way to create a new device; for instance,
 RAID devices are obtained by @dfn{assembling} several other devices, such
 as hard disks or partitions, into a new one that behaves as one partition.
-Other examples, not yet implemented, are LVM logical volumes.
 
 Mapped devices are declared using the @code{mapped-device} form,
 defined as follows; for examples, see below.
@@ -12726,7 +12725,8 @@ the system boots up.
 @item source
 This is either a string specifying the name of the block device to be mapped,
 such as @code{"/dev/sda3"}, or a list of such strings when several devices
-need to be assembled for creating a new one.
+need to be assembled for creating a new one. In case of LVM this is a
+string specifying name of the volume group to be mapped.
 
 @item target
 This string specifies the name of the resulting mapped device.  For
@@ -12735,6 +12735,9 @@ specifying @code{"my-partition"} leads to the creation of
 the @code{"/dev/mapper/my-partition"} device.
 For RAID devices of type @code{raid-device-mapping}, the full device name
 such as @code{"/dev/md0"} needs to be given.
+LVM logical volumes of type @code{lvm-device-mapping} need to
+be specified as @code{"VGNAME-LVNAME"}.
+
 @item targets
 This list of strings specifies names of the resulting mapped devices in case
 there are several. The format is identical to @var{target}.
@@ -12758,6 +12761,11 @@ module for the appropriate RAID level to be loaded, such as @code{raid456}
 for RAID-4, RAID-5 or RAID-6, or @code{raid10} for RAID-10.
 @end defvr
 
+@defvr {Scheme Variable} lvm-device-mapping
+This defines LVM logical volume(s). Volume group is activated by
+@command{vgchange} command from the package @code{lvm2}.
+@end defvr
+
 @cindex disk encryption
 @cindex LUKS
 The following example specifies a mapping from @file{/dev/sda3} to
@@ -12815,6 +12823,19 @@ Note that the RAID level need not be given; it is chosen during the
 initial creation and formatting of the RAID device and is determined
 automatically later.
 
+LVM logical volumes ``alpha'' and ``beta'' from volume group ``vg0'' can
+be declared as follows:
+
+@lisp
+(mapped-device
+  (source "vg0")
+  (target (list "vg0-alpha" "vg0-beta"))
+  (type lvm-device-mapping))
+@end lisp
+
+Devices @file{/dev/mapper/vg0-alpha} and @file{/dev/mapper/vg0-beta} can
+then be used as the @code{device} of a @code{file-system} declaration
+(@pxref{File Systems}).
 
 @node User Accounts
 @section User Accounts
diff --git a/gnu/system/linux-initrd.scm b/gnu/system/linux-initrd.scm
index 3e2f1282cc..85e493fecb 100644
--- a/gnu/system/linux-initrd.scm
+++ b/gnu/system/linux-initrd.scm
@@ -217,6 +217,7 @@ upon error."
                       (gnu system file-systems)
                       ((guix build utils) #:hide (delete))
                       (guix build bournish)   ;add the 'bournish' meta-command
+                      (srfi srfi-1)           ;for lvm-device-mapping
                       (srfi srfi-26)
 
                       ;; FIXME: The following modules are for
diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scm
index 8b5aec983d..559c27bb28 100644
--- a/gnu/system/mapped-devices.scm
+++ b/gnu/system/mapped-devices.scm
@@ -36,7 +36,7 @@
   #:autoload   (gnu build linux-modules)
                  (missing-modules)
   #:autoload   (gnu packages cryptsetup) (cryptsetup-static)
-  #:autoload   (gnu packages linux) (mdadm-static)
+  #:autoload   (gnu packages linux) (mdadm-static lvm2-static)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-34)
@@ -64,7 +64,8 @@
             check-device-initrd-modules           ;XXX: needs a better place
 
             luks-device-mapping
-            raid-device-mapping))
+            raid-device-mapping
+            lvm-device-mapping))
 
 ;;; Commentary:
 ;;;
@@ -304,4 +305,24 @@ TARGET (e.g., \"/dev/md0\"), using 'mdadm'."
    (open open-raid-device)
    (close close-raid-device)))
 
+(define (open-lvm-device source targets)
+  #~(and
+     (zero? (system* #$(file-append lvm2-static "/sbin/lvm")
+                     "vgchange" "--activate" "ay" #$source))
+     ; /dev/mapper nodes are usually created by udev, but udev may be unavailable at the time we run this. So we create them here.
+     (zero? (system* #$(file-append lvm2-static "/sbin/lvm")
+                     "vgscan" "--mknodes"))
+     (every file-exists? (map (lambda (file) (string-append "/dev/mapper/" file))
+                              '#$targets))))
+
+
+(define (close-lvm-device source targets)
+  #~(zero? (system* #$(file-append lvm2-static "/sbin/lvm")
+                    "vgchange" "--activate" "n" #$source)))
+
+(define lvm-device-mapping
+  (mapped-device-kind
+   (open open-lvm-device)
+   (close close-lvm-device)))
+
 ;;; mapped-devices.scm ends here
diff --git a/gnu/tests/install.scm b/gnu/tests/install.scm
index dee2b870e8..2d99638ecc 100644
--- a/gnu/tests/install.scm
+++ b/gnu/tests/install.scm
@@ -65,6 +65,7 @@
             %test-btrfs-root-on-subvolume-os
             %test-jfs-root-os
             %test-f2fs-root-os
+            %test-lvm-separate-home-os
 
             %test-gui-installed-os
             %test-gui-installed-os-encrypted
@@ -795,6 +796,92 @@ build (current-guix) and then store a couple of full system images.")
                       #:initialization enter-luks-passphrase)))))
 
 \f
+;;;
+;;; Separate /home on LVM
+;;;
+
+;; Since LVM support in guix currently doesn't allow root-on-LVM we use /home on LVM
+(define-os-with-source (%lvm-separate-home-os %lvm-separate-home-os-source)
+  (use-modules (gnu) (gnu tests))
+
+  (operating-system
+    (host-name "separate-home-on-lvm")
+    (timezone "Europe/Paris")
+    (locale "en_US.utf8")
+
+    (bootloader (bootloader-configuration
+                 (bootloader grub-bootloader)
+                 (target "/dev/vdb")))
+    (kernel-arguments '("console=ttyS0"))
+
+    (mapped-devices (list (mapped-device
+                           (source "vg0")
+                           (target "vg0-home")
+                           (type lvm-device-mapping))))
+    (file-systems (cons* (file-system
+                           (device (file-system-label "root-fs"))
+                           (mount-point "/")
+                           (type "ext4"))
+                         (file-system
+                           (device "/dev/mapper/vg0-home")
+                           (mount-point "/home")
+                           (type "ext4")
+                           (dependencies mapped-devices))
+                        %base-file-systems))
+    (users %base-user-accounts)
+    (services (cons (service marionette-service-type
+                             (marionette-configuration
+                              (imported-modules '((gnu services herd)
+                                                  (guix combinators)))))
+                    %base-services))))
+
+(define %lvm-separate-home-installation-script
+  "\
+. /etc/profile
+set -e -x
+guix --version
+
+export GUIX_BUILD_OPTIONS=--no-grafts
+parted --script /dev/vdb mklabel gpt \\
+  mkpart primary ext2 1M 3M \\
+  mkpart primary ext2 3M 1.6G \\
+  mkpart primary 1.6G 3.2G \\
+  set 1 boot on \\
+  set 1 bios_grub on
+pvcreate /dev/vdb3
+vgcreate vg0 /dev/vdb3
+lvcreate -L 1.6G -n home vg0
+vgchange -ay
+mkfs.ext4 -L root-fs /dev/vdb2
+mkfs.ext4 /dev/mapper/vg0-home
+mount /dev/vdb2 /mnt
+mkdir /mnt/home
+mount /dev/mapper/vg0-home /mnt/home
+df -h /mnt /mnt/home
+herd start cow-store /mnt
+mkdir /mnt/etc
+cp /etc/target-config.scm /mnt/etc/config.scm
+guix system init /mnt/etc/config.scm /mnt --no-substitutes
+sync
+reboot\n")
+
+(define %test-lvm-separate-home-os
+  (system-test
+   (name "lvm-separate-home-os")
+   (description
+    "Test functionality of an OS installed with a LVM /home partition")
+   (value
+    (mlet* %store-monad ((image   (run-install %lvm-separate-home-os
+                                               %lvm-separate-home-os-source
+                                               #:script
+                                               %lvm-separate-home-installation-script
+                                               #:packages (list lvm2-static)
+                                               #:target-size (* 3200 MiB)))
+                         (command (qemu-command/writable-image image)))
+      (run-basic-test %lvm-separate-home-os
+                      `(,@command) "lvm-separate-home-os")))))
+
+\f
 ;;;
 ;;; Btrfs root file system.
 ;;;
-- 
2.20.1





^ permalink raw reply related	[flat|nested] 22+ messages in thread

* bug#41143: [PATCH v3 1/2] mapped-devices: Allow target to be list of strings.
  2020-11-06  9:47                 ` [bug#41143] [PATCH v3 " Mikhail Tsykalov
  2020-11-06  9:47                   ` [bug#41143] [PATCH v3 2/2] mapped-devices: Add 'lvm-device-mapping' Mikhail Tsykalov
@ 2020-11-25 23:09                   ` Ludovic Courtès
  1 sibling, 0 replies; 22+ messages in thread
From: Ludovic Courtès @ 2020-11-25 23:09 UTC (permalink / raw)
  To: Mikhail Tsykalov; +Cc: 41143-done

Hi Mikhail,

Mikhail Tsykalov <tsymsh@gmail.com> skribis:

> * gnu/system/mapped-devices.scm (<mapped-device>): Rename constructor to
> %mapped-device.
> [target]: Remove field.
> [targets]: New field. Adjust users.
> (mapped-device-compatibility-helper, mapped-device): New macros.
> (mapped-device-target): New deprecated procedure.

[...]

> * gnu/system/mapped-devices.scm (lvm-device-mapping, open-lvm-device,
> close-lvm-device): New variables.
>
> * gnu/tests/install.scm (%lvm-separate-home-os,
> %lvm-separate-home-os-source, %lvm-separate-home-installation-script,
> %test-lvm-separate-home-os): New variables.
>
> * gnu/system/linux-initrd.scm (raw-initrd): Add (srfi srfi-1) to initrd expression.

Pushed as a9a2fdaabcc78e7a54d9a6bcfa4ee3de308e9a90!

Sorry for the delay, the release process took longer than one could hope
for.  :-)

I added a news entry so users will learn about it upon ‘guix pull’.

Thank you!

Ludo’.




^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2020-11-25 23:10 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-09  1:12 [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings tsmish
2020-05-09  1:22 ` [bug#41143] [PATCH 2/2] mapped-devices: Add 'lvm-device-mapping' tsmish
2020-09-09 20:48   ` Ludovic Courtès
2020-05-14 22:53 ` [bug#41143] Some clarification Mikhail Tsykalov
2020-05-15  1:17 ` [bug#41143] [PATCH] mapped-devices: Document lvm-mapping-device Mikhail Tsykalov
2020-06-06 13:40 ` [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings Lars-Dominik Braun
2020-06-06 20:16   ` Mikhail Tsykalov
2020-06-07  6:48     ` Lars-Dominik Braun
2020-09-09 20:38 ` Ludovic Courtès
2020-09-24 16:09   ` Mikhail Tsykalov
2020-09-25  9:34     ` Ludovic Courtès
2020-09-25 13:36       ` Mikhail Tsykalov
2020-09-25 16:20         ` Ludovic Courtès
2020-10-01 22:48           ` [bug#41143] [PATCH v2 " Mikhail Tsykalov
2020-10-01 22:49             ` [bug#41143] [PATCH v2 2/2] mapped-devices: Add 'lvm-device-mapping' Mikhail Tsykalov
2020-10-04 10:34               ` Ludovic Courtès
2020-10-04 10:28             ` [bug#41143] [PATCH v2 1/2] mapped-devices: Allow target to be list of strings Ludovic Courtès
2020-11-05  9:48               ` Ludovic Courtès
2020-11-06  9:47                 ` [bug#41143] [PATCH v3 " Mikhail Tsykalov
2020-11-06  9:47                   ` [bug#41143] [PATCH v3 2/2] mapped-devices: Add 'lvm-device-mapping' Mikhail Tsykalov
2020-11-25 23:09                   ` bug#41143: [PATCH v3 1/2] mapped-devices: Allow target to be list of strings Ludovic Courtès
2020-10-01 23:15           ` [bug#41143] [PATCH " Mikhail Tsykalov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).