* [bug#39698] [PATCH] file-systems: Set default value of the check? field to #f for NFS
@ 2020-02-20 17:15 Maxim Cournoyer
[not found] ` <handler.39698.B.158221893829402.ack@debbugs.gnu.org>
2020-02-22 11:17 ` [bug#39698] [PATCH] file-systems: Set default value of the check? field to #f for NFS Ludovic Courtès
0 siblings, 2 replies; 6+ messages in thread
From: Maxim Cournoyer @ 2020-02-20 17:15 UTC (permalink / raw)
To: 39698
[-- Attachment #1.1: Type: text/plain, Size: 235 bytes --]
The attached patch makes the default value of the <file-system> record
check? filed smarter, and avoids boot failures when using an NFS system
and omitting to specify "check? #f".
There are no checkers for NFS and none are required.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-file-systems-Set-default-value-of-the-check-field-to.patch --]
[-- Type: text/x-patch, Size: 1715 bytes --]
From d6b5a47036b21ec4bf4109cd781ed76208713b71 Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Date: Thu, 20 Feb 2020 11:51:37 -0500
Subject: [PATCH] file-systems: Set default value of the check? field to #f for
NFS.
Network file systems do not need to be checked locally, so provide a better
default value.
* gnu/system/file-systems.scm (<file-system>): Make the check? field thunked,
and compute the default value based on its own type.
---
gnu/system/file-systems.scm | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/gnu/system/file-systems.scm b/gnu/system/file-systems.scm
index 7b78731524..03b143fcac 100644
--- a/gnu/system/file-systems.scm
+++ b/gnu/system/file-systems.scm
@@ -101,6 +101,8 @@
(define-record-type* <file-system> %file-system
make-file-system
file-system?
+ this-file-system
+
(device file-system-device) ; string | <uuid> | <file-system-label>
(mount-point file-system-mount-point) ; string
(type file-system-type) ; string
@@ -113,7 +115,10 @@
(needed-for-boot? %file-system-needed-for-boot? ; Boolean
(default #f))
(check? file-system-check? ; Boolean
- (default #t))
+ (thunked)
+ (default (not (string-prefix-ci?
+ (file-system-type this-file-system)
+ "nfs"))))
(create-mount-point? file-system-create-mount-point? ; Boolean
(default #f))
(dependencies file-system-dependencies ; list of <file-system>
--
2.25.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [bug#39698] Acknowledgement ([PATCH] file-systems: Set default value of the check? field to #f for NFS)
[not found] ` <handler.39698.B.158221893829402.ack@debbugs.gnu.org>
@ 2020-02-20 20:29 ` Maxim Cournoyer
0 siblings, 0 replies; 6+ messages in thread
From: Maxim Cournoyer @ 2020-02-20 20:29 UTC (permalink / raw)
To: 39698
The patch sent earlier crashes 'guix system reconfigure config.scm'. It
occurs when attempting to lower the object returned by the fold-services
procedure, e.g.:
--8<---------------cut here---------------start------------->8---
,m (guix scripts system)
scheme@(guix scripts system)> (define os (load* "/path/to/config.scm" %user-module))
scheme@(guix scripts system)> (lower-object os)
$2 = #<procedure 7fd849022e40 at guix/gexp.scm:231:5 (state)>
scheme@(guix scripts system)> ,import (guix monad-repl)
scheme@(guix scripts system)> ,run-in-store $2
While executing meta-command:
Throw to key `srfi-34' with args `(#<condition &gexp-input-error [input:
#<procedure 7fd84d36ac80 at ice-9/eval.scm:333:13 (a)>] 7fd84a760e00>)'.
--8<---------------cut here---------------end--------------->8---
I'm struggling to understand why. To be continued.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [bug#39698] [PATCH] file-systems: Set default value of the check? field to #f for NFS
2020-02-20 17:15 [bug#39698] [PATCH] file-systems: Set default value of the check? field to #f for NFS Maxim Cournoyer
[not found] ` <handler.39698.B.158221893829402.ack@debbugs.gnu.org>
@ 2020-02-22 11:17 ` Ludovic Courtès
2020-02-24 16:15 ` Maxim Cournoyer
1 sibling, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2020-02-22 11:17 UTC (permalink / raw)
To: Maxim Cournoyer; +Cc: 39698
Hi,
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
> --- a/gnu/system/file-systems.scm
> +++ b/gnu/system/file-systems.scm
> @@ -101,6 +101,8 @@
> (define-record-type* <file-system> %file-system
> make-file-system
> file-system?
> + this-file-system
> +
> (device file-system-device) ; string | <uuid> | <file-system-label>
> (mount-point file-system-mount-point) ; string
> (type file-system-type) ; string
> @@ -113,7 +115,10 @@
> (needed-for-boot? %file-system-needed-for-boot? ; Boolean
> (default #f))
> (check? file-system-check? ; Boolean
> - (default #t))
> + (thunked)
> + (default (not (string-prefix-ci?
> + (file-system-type this-file-system)
> + "nfs"))))
I’d rather avoid making ‘check?’ a thunked field (it adds a bit of
overhead and breaks ‘equal?’.)
Could we instead special-case NFS in (gnu build file-systems) such that
checking is a no-op?
Thanks,
Ludo’.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [bug#39698] [PATCH] file-systems: Set default value of the check? field to #f for NFS
2020-02-22 11:17 ` [bug#39698] [PATCH] file-systems: Set default value of the check? field to #f for NFS Ludovic Courtès
@ 2020-02-24 16:15 ` Maxim Cournoyer
2020-02-26 20:41 ` Ludovic Courtès
0 siblings, 1 reply; 6+ messages in thread
From: Maxim Cournoyer @ 2020-02-24 16:15 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 39698
[-- Attachment #1.1: Type: text/plain, Size: 1426 bytes --]
Hi Ludovic!
Ludovic Courtès <ludo@gnu.org> writes:
> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> --- a/gnu/system/file-systems.scm
>> +++ b/gnu/system/file-systems.scm
>> @@ -101,6 +101,8 @@
>> (define-record-type* <file-system> %file-system
>> make-file-system
>> file-system?
>> + this-file-system
>> +
>> (device file-system-device) ; string | <uuid> | <file-system-label>
>> (mount-point file-system-mount-point) ; string
>> (type file-system-type) ; string
>> @@ -113,7 +115,10 @@
>> (needed-for-boot? %file-system-needed-for-boot? ; Boolean
>> (default #f))
>> (check? file-system-check? ; Boolean
>> - (default #t))
>> + (thunked)
>> + (default (not (string-prefix-ci?
>> + (file-system-type this-file-system)
>> + "nfs"))))
>
> I’d rather avoid making ‘check?’ a thunked field (it adds a bit of
> overhead and breaks ‘equal?’.)
>
> Could we instead special-case NFS in (gnu build file-systems) such that
> checking is a no-op?
>
> Thanks,
> Ludo’.
This actually was the first implementation I'd done, before attempting
to go fancy with the thunked default value ;-).
You'll find it attached.
Thanks!
[-- Attachment #1.2: 0001-build-file-systems-Skip-check-for-NFS-file-systems.patch --]
[-- Type: text/x-patch, Size: 2441 bytes --]
From ca38de33a7a31c7b96f7e920038b2fb6352160a8 Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Date: Mon, 24 Feb 2020 11:08:42 -0500
Subject: [PATCH] build: file-systems: Skip check for NFS file systems
* gnu/build/file-systems.scm (mount-file-system): Do not call
`check-file-system' when the file system is of NFS type.
---
gnu/build/file-systems.scm | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/gnu/build/file-systems.scm b/gnu/build/file-systems.scm
index cfa3898f83..76c143654d 100644
--- a/gnu/build/file-systems.scm
+++ b/gnu/build/file-systems.scm
@@ -5,6 +5,7 @@
;;; Copyright © 2019 Guillaume Le Vaillant <glv@posteo.net>
;;; Copyright © 2019 Tobias Geerinckx-Rice <me@tobias.gr>
;;; Copyright © 2019 David C. Trudgian <dave@trudgian.net>
+;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -663,6 +664,7 @@ corresponds to the symbols listed in FLAGS."
(string-append "," options)
"")))))
(let* ((type (file-system-type fs))
+ (nfs? (string-prefix? "nfs" type))
(fs-options (file-system-options fs))
(options (if (null? fs-options)
#f
@@ -671,7 +673,7 @@ corresponds to the symbols listed in FLAGS."
(mount-point (string-append root "/"
(file-system-mount-point fs)))
(flags (mount-flags->bit-mask (file-system-flags fs))))
- (when (file-system-check? fs)
+ (when (and (file-system-check? fs) (not nfs?))
(check-file-system source type))
;; Create the mount point. Most of the time this is a directory, but
@@ -683,11 +685,9 @@ corresponds to the symbols listed in FLAGS."
(call-with-output-file mount-point (const #t)))
(mkdir-p mount-point))
- (cond
- ((string-prefix? "nfs" type)
- (mount-nfs source mount-point type flags options))
- (else
- (mount source mount-point type flags options)))
+ (if nfs?
+ (mount-nfs source mount-point type flags options)
+ (mount source mount-point type flags options))
;; For read-only bind mounts, an extra remount is needed, as per
;; <http://lwn.net/Articles/281157/>, which still applies to Linux 4.0.
--
2.25.0
[-- Attachment #1.3: Type: text/plain, Size: 7 bytes --]
Maxim
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [bug#39698] [PATCH] file-systems: Set default value of the check? field to #f for NFS
2020-02-24 16:15 ` Maxim Cournoyer
@ 2020-02-26 20:41 ` Ludovic Courtès
2020-03-03 5:05 ` bug#39698: " Maxim Cournoyer
0 siblings, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2020-02-26 20:41 UTC (permalink / raw)
To: Maxim Cournoyer; +Cc: 39698
[-- Attachment #1: Type: text/plain, Size: 2700 bytes --]
Hi Maxim,
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
> From ca38de33a7a31c7b96f7e920038b2fb6352160a8 Mon Sep 17 00:00:00 2001
> From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
> Date: Mon, 24 Feb 2020 11:08:42 -0500
> Subject: [PATCH] build: file-systems: Skip check for NFS file systems
>
> * gnu/build/file-systems.scm (mount-file-system): Do not call
> `check-file-system' when the file system is of NFS type.
> ---
> gnu/build/file-systems.scm | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/gnu/build/file-systems.scm b/gnu/build/file-systems.scm
> index cfa3898f83..76c143654d 100644
> --- a/gnu/build/file-systems.scm
> +++ b/gnu/build/file-systems.scm
> @@ -5,6 +5,7 @@
> ;;; Copyright © 2019 Guillaume Le Vaillant <glv@posteo.net>
> ;;; Copyright © 2019 Tobias Geerinckx-Rice <me@tobias.gr>
> ;;; Copyright © 2019 David C. Trudgian <dave@trudgian.net>
> +;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer@gmail.com>
> ;;;
> ;;; This file is part of GNU Guix.
> ;;;
> @@ -663,6 +664,7 @@ corresponds to the symbols listed in FLAGS."
> (string-append "," options)
> "")))))
> (let* ((type (file-system-type fs))
> + (nfs? (string-prefix? "nfs" type))
> (fs-options (file-system-options fs))
> (options (if (null? fs-options)
> #f
> @@ -671,7 +673,7 @@ corresponds to the symbols listed in FLAGS."
> (mount-point (string-append root "/"
> (file-system-mount-point fs)))
> (flags (mount-flags->bit-mask (file-system-flags fs))))
> - (when (file-system-check? fs)
> + (when (and (file-system-check? fs) (not nfs?))
> (check-file-system source type))
Looking more closely, I see this:
--8<---------------cut here---------------start------------->8---
(define (check-file-system device type)
"Run a file system check of TYPE on DEVICE."
(define check-procedure
(cond
((string-prefix? "ext" type) check-ext2-file-system)
((string-prefix? "btrfs" type) check-btrfs-file-system)
((string-suffix? "fat" type) check-fat-file-system)
((string-prefix? "jfs" type) check-jfs-file-system)
(else #f)))
(if check-procedure
…
(format (current-error-port)
"No file system check procedure for ~a; skipping~%"
device)))
--8<---------------cut here---------------end--------------->8---
Isn’t it already taking care of not attempting to check NFS?
Or, to get rid of the warning, what about:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 475 bytes --]
diff --git a/gnu/build/file-systems.scm b/gnu/build/file-systems.scm
index ee6375515f..faf64ce304 100644
--- a/gnu/build/file-systems.scm
+++ b/gnu/build/file-systems.scm
@@ -581,6 +581,7 @@ were found."
((string-prefix? "btrfs" type) check-btrfs-file-system)
((string-suffix? "fat" type) check-fat-file-system)
((string-prefix? "jfs" type) check-jfs-file-system)
+ ((string-prefix? "nfs" type) (const 'pass))
(else #f)))
(if check-procedure
[-- Attachment #3: Type: text/plain, Size: 26 bytes --]
?
Thanks,
Ludo’.
^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#39698: [PATCH] file-systems: Set default value of the check? field to #f for NFS
2020-02-26 20:41 ` Ludovic Courtès
@ 2020-03-03 5:05 ` Maxim Cournoyer
0 siblings, 0 replies; 6+ messages in thread
From: Maxim Cournoyer @ 2020-03-03 5:05 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 39698-done
Hi again,
> Or, to get rid of the warning, what about:
>
> diff --git a/gnu/build/file-systems.scm b/gnu/build/file-systems.scm
> index ee6375515f..faf64ce304 100644
> --- a/gnu/build/file-systems.scm
> +++ b/gnu/build/file-systems.scm
> @@ -581,6 +581,7 @@ were found."
> ((string-prefix? "btrfs" type) check-btrfs-file-system)
> ((string-suffix? "fat" type) check-fat-file-system)
> ((string-prefix? "jfs" type) check-jfs-file-system)
> + ((string-prefix? "nfs" type) (const 'pass))
> (else #f)))
I've now pushed the above to master. Closing!
Maxim
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-03 5:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-20 17:15 [bug#39698] [PATCH] file-systems: Set default value of the check? field to #f for NFS Maxim Cournoyer
[not found] ` <handler.39698.B.158221893829402.ack@debbugs.gnu.org>
2020-02-20 20:29 ` [bug#39698] Acknowledgement ([PATCH] file-systems: Set default value of the check? field to #f for NFS) Maxim Cournoyer
2020-02-22 11:17 ` [bug#39698] [PATCH] file-systems: Set default value of the check? field to #f for NFS Ludovic Courtès
2020-02-24 16:15 ` Maxim Cournoyer
2020-02-26 20:41 ` Ludovic Courtès
2020-03-03 5:05 ` bug#39698: " Maxim Cournoyer
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).