[PATCH 1/2] Add 'lvm-device-mapping'

[PATCH 1/2] mapped-devices: Allow target to be list of strings

Recipients:(address . guix-patches@gnu.org)

Message-ID:CAMaATaPqFZTB-j9MyhS4+vrn8sjA25xzu2+DD71nE541yCiRwQ@mail.gmail.com

(let ...) stuff should be in function, but I don't know in whichmodule 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(-)

Toggle diff (67 lines)diff --git a/gnu/services/base.scm b/gnu/services/base.scmindex 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-lambdadiff --git a/gnu/system.scm b/gnu/system.scmindex 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.scmindex 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

tsmish wrote on 9 May 03:22 +0200

[PATCH 2/2] mapped-devices: Add 'lvm-device-mapping'

Recipients:(address . 41143@debbugs.gnu.org)

Message-ID:CAMaATaNkBUP-2sg-QX=AXQoP4z=2HcM_c9gpbL7PPLJ+sxDnrg@mail.gmail.com

"vgscan --mknodes" is a bit of a hack. Everyone else relies on udev tocreate files in /dev/mapper, but since initrd doesn't have workingudevd, they need to be created this way.Also, while this code is able to boot from root on LVM, grub incurrent configuration can't find required files, This can be fixed byplacing (format port "insmod lvm") in grub configuration builder, butthis is somewhat hacky.
--- gnu/system/mapped-devices.scm | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-)

Toggle diff (55 lines)diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scmindex 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/mappernodes 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

Mikhail Tsykalov wrote on 15 May 00:53 +0200

Some clarification

Recipients:(address . 41143@debbugs.gnu.org)

Message-ID:7e96c284-c69c-965f-6bae-9546b1c711ad@gmail.com

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_manager10-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

Mikhail Tsykalov wrote on 15 May 03:17 +0200

[PATCH] mapped-devices: Document lvm-mapping-device.

Recipients:(address . 41143@debbugs.gnu.org)

Message-ID:b68a706e-a554-8dd8-13d1-963de34cb260@gmail.com

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

Toggle diff (76 lines)diff --git a/doc/guix.texi b/doc/guix.texiindex 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

Ludovic Courtès wrote on 16 May 19:38 +0200

control message for bug #41143

Recipients:(address . control@debbugs.gnu.org)

Message-ID:87pnb3pypo.fsf@gnu.org

retitle 41143 [PATCH 1/2] Add 'lvm-device-mapping'quit

Lars-Dominik Braun wrote on 6 Jun 15:40 +0200

Re: [PATCH 1/2] mapped-devices: Allow target to be list of strings

Recipients:(name . tsmish)(address . tsymsh@gmail.com)(address . guix-patches@gnu.org)

Message-ID:20200606134015.GA204960@noor.fritz.box

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 devicenodes (i.e. /dev/sdX), like the other mappings. Does `pvscan --cache` not workhere?
Lars

Mikhail Tsykalov wrote on 6 Jun 22:16 +0200

Recipients:(name . Lars-Dominik Braun)(address . lars@6xq.net)(address . 41143@debbugs.gnu.org)

Message-ID:8c852805-38e6-92b2-15cf-43d47818782e@gmail.com

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

Toggle quote (6 lines)

> 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.

Toggle quote (4 lines)

> 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

From e9a2b441190fe18ebc4f5a8c73707edab3459d58 Mon Sep 17 00:00:00 2001
--- gnu/system/mapped-devices.scm | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-)

Toggle diff (52 lines)diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scmindex 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

Lars-Dominik Braun wrote on 7 Jun 08:48 +0200

Recipients:(name . Mikhail Tsykalov)(address . tsymsh@gmail.com)(address . 41143@debbugs.gnu.org)

Message-ID:20200607064852.GA1408@noor

Hi,

Toggle quote (4 lines)

> 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 properlynow. (This is a non-boot disk though.) Thanks!

Toggle quote (3 lines)

> 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 youdecide to change the VG.
Lars

Ludovic Courtès wrote on 9 Sep 22:38 +0200

Re: [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings

Recipients:(name . tsmish)(address . tsymsh@gmail.com)(address . 41143@debbugs.gnu.org)

Message-ID:874ko6n0s5.fsf@gnu.org

Hi Mikhail,
Sorry for the very late reply! Vacations came by, and by now this entryis at the bottom of the patch tracker. :-)
People repeatedly ask for LVM support, so I guess you’ll make them allhappy! Great you got it into shape.
tsmish <tsymsh@gmail.com> skribis:

Toggle quote (10 lines)> (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’ssomething I can help with.
¹ https://guix.gnu.org/manual/en/html_node/Submitting-Patches.html

Toggle quote (15 lines)> 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 insteadthe 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’.

Ludovic Courtès wrote on 9 Sep 22:48 +0200

Re: [bug#41143] [PATCH 2/2] mapped-devices: Add 'lvm-device-mapping'

Recipients:(name . tsmish)(address . tsymsh@gmail.com)(address . 41143@debbugs.gnu.org)

Message-ID:87sgbqllrd.fsf@gnu.org

tsmish <tsymsh@gmail.com> skribis:

Toggle quote (4 lines)

> "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?

Toggle quote (5 lines)

> 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. :-)

Toggle quote (6 lines)

> +(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 toplevel), we cannot have ‘use-modules’ here (well, it may not work asexpected, 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 testsfor Btrfs, RAID with mdadm, etc., but these are system installationtests in (gnu tests install). Do you think we could have either aninstallation test or maybe a less expensive test for LVM?
Thanks for your work!
Ludo’.

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date