From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:470:142:3::10]:37458) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1j6GDD-00028J-NW for guix-patches@gnu.org; Mon, 24 Feb 2020 11:04:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1j6GDB-0003mE-Un for guix-patches@gnu.org; Mon, 24 Feb 2020 11:04:03 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:47986) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1j6GDB-0003mA-R4 for guix-patches@gnu.org; Mon, 24 Feb 2020 11:04:01 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1j6GDB-0006Nv-Mt for guix-patches@gnu.org; Mon, 24 Feb 2020 11:04:01 -0500 Subject: [bug#37305] [PATCH V2] Allow booting from a Btrfs subvolume. Resent-Message-ID: From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <87sgpby4p9.fsf@gmail.com> <87y2yg3t3s.fsf@gnu.org> <87k14sfaz7.fsf@gmail.com> <87lfp6b5cs.fsf_-_@gmail.com> <8736bdf5il.fsf@gnu.org> <87blpzozz7.fsf@gmail.com> Date: Mon, 24 Feb 2020 17:02:49 +0100 In-Reply-To: <87blpzozz7.fsf@gmail.com> (Maxim Cournoyer's message of "Sun, 16 Feb 2020 00:36:28 -0500") Message-ID: <87imjwkm6u.fsf@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Maxim Cournoyer Cc: 37305@debbugs.gnu.org Hi Maxim, Resuming review of this series=E2=80=A6 Sorry for the delay! Maxim Cournoyer skribis: >>> From 97d8a635eba34c7cf0708e99bf77ef9bad1344bf Mon Sep 17 00:00:00 2001 >>> From: Maxim Cournoyer >>> Date: Tue, 11 Feb 2020 12:57:29 -0500 >>> Subject: [PATCH 2/9] gnu: linux-boot: Ensure volatile root is mounted >>> read-only. >>> >>> * gnu/build/linux-boot.scm (mount-root-file-system): Ensure MS_RDONLY is >>> present among the root file system flags when VOLATILE-ROOT? is #t. >> >> (You can drop the =E2=80=9Cgnu:=E2=80=9D prefix.) > > Done. > > I never know before looking at past logs (and then sometimes it's a > mixed bag). Is there any mechanical process for selecting the right > commit prefix? :-) =E2=80=9Cgnu:=E2=80=9D is for changes to (gnu packages). The idea is that = the prefix should reflect what subsystem the commit is modifying. But yeah, looking at =E2=80=98git log=E2=80=99 can be inspiring. :-) >>> @item --root=3D@var{root} >>> -Mount @var{root} as the root file system. @var{root} can be a >>> -device name like @code{/dev/sda1}, a file system label, or a file syst= em >>> -UUID. >>> +Mount @var{root} as the root file system. @var{root} can be a device >>> +name like @code{/dev/sda1}, a file system label, or a file system UUID. >>> +When unspecified, the device name from the root file system of the >>> +operating system declaration is used. >> >> Oh! Does it always work? That makes me wonder why we=E2=80=99ve been c= arrying >> =E2=80=98--root=E2=80=99 and I=E2=80=99m not sure if I=E2=80=99m forgett= ing a good reason to do it that >> way. > > If the documentation is accurate, it should :-), given that --root gets > written as a string to the GRUB configuration file, and that the doc > says it's possible to give it as a device name, label or UUID. Yes, =E2=80=98--root=E2=80=99 can resolve labels and UUIDs; my question was= more about why we have it in the first place. > About why providing options such as --root or --root-options in the > first place; I pondered about this as well, especially after making the > file systems from operating system able to be mounted with all their > (file system independent -- more on that later) options. A reason I > came up with was that it allows to experiment at the GRUB command line > and change the root device, or perhaps the root options. One use case > would be debugging the right options to pass to a file system driver in > case of a mistake in the operating system declaration. Yes, that makes sense. It=E2=80=99s certainly useful to have =E2=80=98--ro= ot=E2=80=99 at least as an option. >> The main issue I see with this change is that mount(2) takes raw strings >> for the options. There=E2=80=99s a convention to have those strings loo= k like >> =E2=80=9CKEY1=3DVALUE1,KEY2=3DVALUE2=E2=80=9D, but it=E2=80=99s just a c= onvention. >> >> As a rule of thumb, I=E2=80=99d rather have our interface be as close as >> possible to the actual mount(2) interface, which means taking strings. >> >> Now, we can surely add helper procedures to parse options that follow >> the above conventions. >> >> WDYT? > > To me, it's an implementation detail that I'd rather abstract away (or > make optional, like in this patch). Just like we provide a higher level > configuration for services instead of requiring the user to input the > configuration in the native format of the tool (or allowing for both). > The idea for this format was taken from a discussion here: > http://issues.guix.info/issue/33517#3. > > Are we really targeting mount(2)? The commit > 9d3053819dfd834a1c29a03427c41d8524b8a7d5 (which you co-authored :-)) > mentions 'man 8 mount' for the file system options. Right, mount(8) documents file system options that can be passed to mount(2). What does it mean to target mount(8) vs. mount(2)? To me, mount(8) is a CLI to mount(2) that provides additional features to make the CLI more convenient: the =E2=80=9Cdefaults=E2=80=9D option, a way to pass mount(2) f= lags as options (like =E2=80=9Cro=E2=80=9D, =E2=80=9Cremount=E2=80=9D, =E2=80=9Cbin= d=E2=80=9D), /etc/fstab handling, etc. Guix System handles /etc/fstab differently and =E2=80=9Cdefaults=E2=80=9D m= akes little sense in our API (one can just use leave the default value of the =E2=80=98options=E2=80=99 field.) I think mount(8) is actually a good illustration of what not to do. It ends up mixing things that are separate in the mount(2) API, and that doesn=E2=80=99t improve clarity and future-proof-ness (what if a file syste= m has a =E2=80=9Cbind=E2=80=9D option, etc.). But again, I think the helper procedures that you propose to move back and forth between the string and the alist representations are very welcome. I just wouldn=E2=80=99t hard-code that directly in our API. WDYT? >>> From 67135c925b07f2e077b4cd852e07178691a10164 Mon Sep 17 00:00:00 2001 >>> From: Maxim Cournoyer >>> Date: Tue, 11 Feb 2020 14:14:36 -0500 >>> Subject: [PATCH 6/9] gnu: linux-boot: Honor the "--root-options" kernel >>> argument. >>> >>> * gnu/build/linux-boot.scm (boot-system): Parse the "--root-options" ke= rnel >>> argument, and use it when calling `mount-root-file-system'. Update doc. >>> * doc/guix.texi (Initial RAM Disk): Document the use of the "--root-opt= ions" >>> argument. >> >> Hmm do we really need this extra option? :-) > > It is not strictly needed but allows the user to experiment/troubleshoot > with the init RAM disk from GRUB as discussed earlier for --root. Do > you think it has enough value to be kept? I=E2=80=99d rather avoid it for now. Less code is better. :-) >> (Also, in hindsight, I think it was a mistake to call them >> =E2=80=98--something=E2=80=99. Following the common naming convention, = we should rather >> call these options =E2=80=98gnu.something=E2=80=99.) > > Is this convention detailed somewhere? I haven't found it in 'Standards'. It=E2=80=99s a convention of the Linux kernel, I don=E2=80=99t know if it= =E2=80=99s documented. That=E2=80=99s it! Ludo=E2=80=99.