GNU bug report logs - #71352
branch master updated: services: nix: Mount Nix store read only.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 71352 in the body.
You can then email your comments to 71352 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: bug-guix <bug-guix <at> gnu.org>
Cc: Oleg Pykhalov <go.wigust <at> gmail.com>
Subject: Re: branch master updated: services: nix: Mount Nix store read only.
Date: Mon, 03 Jun 2024 22:34:35 -0400

Hello,

guix-commits <at> gnu.org writes:

>     services: nix: Mount Nix store read only.
>     
>     * gnu/services/nix.scm (nix-shepherd-service): Add requirements.
>     (%nix-store-directory): New variable.
>     (nix-service-type): Add file-system-service-type extension.
>     
>     Change-Id: I18a5d58c92c1f2b5b6dcecc3d5b439cc15bf4e49

This commit unfortunately appears to introduce a regression where
reconfiguring a system with the read-only /nix/store causes the
following error:

--8<---------------cut here---------------start------------->8---
guix system: error: chown: Système de fichiers accessible en lecture seulement
--8<---------------cut here---------------end--------------->8---

With the accompanying strace output:

--8<---------------cut here---------------start------------->8---
20261 close(17)                         = 0
20261 chown("/nix/store", 0, 981)       = -1 EROFS (Système de fichiers accessible en lecture seulement)
20261 close(13)                         = 0
20261 write(2, "guix system: \33[1;31merror: \33[0m\33[1mchown\33[0m: Syst\303\250me de fichiers accessible en lecture seulement\n", 99) = 99
--8<---------------cut here---------------end--------------->8---

Are these chown still useful in the activation snippet?

--8<---------------cut here---------------start------------->8---
(define (nix-activation _)
  ;; Return the activation gexp.
  #~(begin
      (use-modules (guix build utils)
                   (srfi srfi-26))
      (for-each (cut mkdir-p <>) '("/nix/store" "/nix/var/log"
                                   "/nix/var/nix/gcroots/per-user"
                                   "/nix/var/nix/profiles/per-user"))
      (chown "/nix/store"
             (passwd:uid (getpw "root")) (group:gid (getpw "nixbld01")))
      (chmod "/nix/store" #o775)
      (for-each (cut chmod <> #o777) '("/nix/var/nix/profiles"
                                       "/nix/var/nix/profiles/per-user"))))
--8<---------------cut here---------------end--------------->8---

If they are useful only on the first time, perhaps we could catch the
exceptions for when it runs on an already read-only mounted /nix/store?

-- 
Thanks,
Maxim

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Oleg Pykhalov <go.wigust <at> gmail.com>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: bug-guix <bug-guix <at> gnu.org>
Subject: Re: branch master updated: services: nix: Mount Nix store read only.
Date: Tue, 04 Jun 2024 11:34:24 +0300

[Message part 1 (text/plain, inline)]

Hello Maxim,

Thank you for your report.  Apologize for any inconvenience caused by
the unexpected breakage.

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:

> Hello,
>
> guix-commits <at> gnu.org writes:
>
>>     services: nix: Mount Nix store read only.
>>     
>>     * gnu/services/nix.scm (nix-shepherd-service): Add requirements.
>>     (%nix-store-directory): New variable.
>>     (nix-service-type): Add file-system-service-type extension.
>>     
>>     Change-Id: I18a5d58c92c1f2b5b6dcecc3d5b439cc15bf4e49
>
> This commit unfortunately appears to introduce a regression where
> reconfiguring a system with the read-only /nix/store causes the
> following error:
>
> guix system: error: chown: Système de fichiers accessible en lecture seulement
>
>
> With the accompanying strace output:
>
> 20261 close(17)                         = 0
> 20261 chown("/nix/store", 0, 981)       = -1 EROFS (Système de fichiers accessible en lecture seulement)
> 20261 close(13)                         = 0
> 20261 write(2, "guix system: \33[1;31merror: \33[0m\33[1mchown\33[0m: Syst\303\250me de fichiers accessible en lecture seulement\n", 99) = 99
>
>
> Are these chown still useful in the activation snippet?
>
> (define (nix-activation _)
>   ;; Return the activation gexp.
>   #~(begin
>       (use-modules (guix build utils)
>                    (srfi srfi-26))
>       (for-each (cut mkdir-p <>) '("/nix/store" "/nix/var/log"
>                                    "/nix/var/nix/gcroots/per-user"
>                                    "/nix/var/nix/profiles/per-user"))
>       (chown "/nix/store"
>              (passwd:uid (getpw "root")) (group:gid (getpw "nixbld01")))
>       (chmod "/nix/store" #o775)
>       (for-each (cut chmod <> #o777) '("/nix/var/nix/profiles"
>                                        "/nix/var/nix/profiles/per-user"))))
>
> If they are useful only on the first time, perhaps we could catch the
> exceptions for when it runs on an already read-only mounted /nix/store?

Indeed, it is a good idea.

A hotfix for the issue was discussed and implemented. It has already
been pushed to the master branch. The fix involves a simple
'file-exists?' check. You can find more details in the discussion at
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=71320

What do you think is preferable in this scenario – catching exceptions
or sticking with '(unless (file-exists? ...))'?  Your thoughts on the
best approach here?


Regards,
Oleg.

[signature.asc (application/pgp-signature, inline)]

Message #11 received at 71352 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Oleg Pykhalov <go.wigust <at> gmail.com>
Cc: 71352 <at> debbugs.gnu.org
Subject: Re: branch master updated: services: nix: Mount Nix store read only.
Date: Wed, 05 Jun 2024 22:03:33 -0400

Hi Oleg,

[...]

>> Are these chown still useful in the activation snippet?
>>
>> (define (nix-activation _)
>>   ;; Return the activation gexp.
>>   #~(begin
>>       (use-modules (guix build utils)
>>                    (srfi srfi-26))
>>       (for-each (cut mkdir-p <>) '("/nix/store" "/nix/var/log"
>>                                    "/nix/var/nix/gcroots/per-user"
>>                                    "/nix/var/nix/profiles/per-user"))
>>       (chown "/nix/store"
>>              (passwd:uid (getpw "root")) (group:gid (getpw "nixbld01")))
>>       (chmod "/nix/store" #o775)
>>       (for-each (cut chmod <> #o777) '("/nix/var/nix/profiles"
>>                                        "/nix/var/nix/profiles/per-user"))))
>>
>> If they are useful only on the first time, perhaps we could catch the
>> exceptions for when it runs on an already read-only mounted /nix/store?
>
> Indeed, it is a good idea.
>
> A hotfix for the issue was discussed and implemented. It has already
> been pushed to the master branch. The fix involves a simple
> 'file-exists?' check. You can find more details in the discussion at
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=71320
>
> What do you think is preferable in this scenario – catching exceptions
> or sticking with '(unless (file-exists? ...))'?  Your thoughts on the
> best approach here?

Exceptions are usually better than 'check then do' as they avoid the
TOCTTOU (time-of-check to time-of-use) class of bugs/vulnerabilities.

By the way, 'Reported-by:' is a fine git trailer to use :-).  I also use
'Fixes:' as a git trailer (trailer means they should be found at the
bottom of the commit message -- these can be parsed with the 'git
interpret-trailers' command)

-- 
Thanks,
Maxim

Message #16 received at 71352-done <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Oleg Pykhalov <go.wigust <at> gmail.com>
Cc: 71352-done <at> debbugs.gnu.org
Subject: Re: bug#71352: branch master updated: services: nix: Mount Nix
 store read only.
Date: Sun, 23 Jun 2024 22:47:33 -0400

Hi Oleg,

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:

> Hi Oleg,
>
> [...]
>
>>> Are these chown still useful in the activation snippet?
>>>
>>> (define (nix-activation _)
>>>   ;; Return the activation gexp.
>>>   #~(begin
>>>       (use-modules (guix build utils)
>>>                    (srfi srfi-26))
>>>       (for-each (cut mkdir-p <>) '("/nix/store" "/nix/var/log"
>>>                                    "/nix/var/nix/gcroots/per-user"
>>>                                    "/nix/var/nix/profiles/per-user"))
>>>       (chown "/nix/store"
>>>              (passwd:uid (getpw "root")) (group:gid (getpw "nixbld01")))
>>>       (chmod "/nix/store" #o775)
>>>       (for-each (cut chmod <> #o777) '("/nix/var/nix/profiles"
>>>                                        "/nix/var/nix/profiles/per-user"))))
>>>
>>> If they are useful only on the first time, perhaps we could catch the
>>> exceptions for when it runs on an already read-only mounted /nix/store?
>>
>> Indeed, it is a good idea.
>>
>> A hotfix for the issue was discussed and implemented. It has already
>> been pushed to the master branch. The fix involves a simple
>> 'file-exists?' check. You can find more details in the discussion at
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=71320
>>
>> What do you think is preferable in this scenario – catching exceptions
>> or sticking with '(unless (file-exists? ...))'?  Your thoughts on the
>> best approach here?
>
> Exceptions are usually better than 'check then do' as they avoid the
> TOCTTOU (time-of-check to time-of-use) class of bugs/vulnerabilities.

I'm closing this for now; I'm satisfied that working order has been
restored :-).

-- 
Thanks,
Maxim

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.