bug#27212: 203a9455c4695152fc5d0085bffeead9ce3216c2 broke system boot

bug#27212: 203a9455c4695152fc5d0085bffeead9ce3216c2 broke system boot

[ng0]

Otherwise perfectly working systems I have *all* break after
https://git.savannah.gnu.org/cgit/guix.git/commit/?id=203a9455c4695152fc5d0085bffeead9ce3216c2

and the last thing  they display before guile repl is the
message in that commit.

Yes, I use labels. No, I don't use complicated setups.
Please test such changes before applying them in the wild..
I just hope this happened here and the break is a mistake.
-- 
ng0
OpenPG: A88C8ADD129828D7EAC02E52E22F9BBFEE348588

bug#27212: 203a9455c4695152fc5d0085bffeead9ce3216c2 broke system boot

[Marius Bakke]

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

ng0 <ng0@pragmatique.xyz> writes:

> Otherwise perfectly working systems I have *all* break after
> https://git.savannah.gnu.org/cgit/guix.git/commit/?id=203a9455c4695152fc5d0085bffeead9ce3216c2
>
> and the last thing  they display before guile repl is the
> message in that commit.
>
> Yes, I use labels. No, I don't use complicated setups.
> Please test such changes before applying them in the wild..
> I just hope this happened here and the break is a mistake.

I *just* noticed the same thing. I also tried reverting this commit, but
that just entered a (re)boot loop. Not sure what's up, but two of my
GuixSD systems are affected.

On my way out now, but will investigate more tomorrow. CC Danny in case
of insight (which report by Chris did this address?).

Older generations are fine.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#27212: 203a9455c4695152fc5d0085bffeead9ce3216c2 broke system boot

[ng0]

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

Marius Bakke transcribed 1.4K bytes:
> ng0 <ng0@pragmatique.xyz> writes:
> 
> > Otherwise perfectly working systems I have *all* break after
> > https://git.savannah.gnu.org/cgit/guix.git/commit/?id=203a9455c4695152fc5d0085bffeead9ce3216c2
> >
> > and the last thing  they display before guile repl is the
> > message in that commit.
> >
> > Yes, I use labels. No, I don't use complicated setups.
> > Please test such changes before applying them in the wild..
> > I just hope this happened here and the break is a mistake.
> 
> I *just* noticed the same thing. I also tried reverting this commit, but
> that just entered a (re)boot loop. Not sure what's up, but two of my
> GuixSD systems are affected.
> 
> On my way out now, but will investigate more tomorrow. CC Danny in case
> of insight (which report by Chris did this address?).
> 
> Older generations are fine.

Thanks.

Results like this should be avoided. As long as this happens
there is a reason for the 'beta' label.

It might be usable in all kinds of environments, but we
have to have rules and guidelines when something can be
applied just like this after QA or when a certain set
of people have to aprove the change as they did
test the change and it worked.

I consider file-system one of these cases.
If the commit touches the way towards boot, approval
is the way to go.

-- 
ng0
OpenPG: A88C8ADD129828D7EAC02E52E22F9BBFEE348588

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#27212: [PATCH] file-systems: Improve error handling in the iso9660 case - fixes boot problem.

[Danny Milosavljevic]

* gnu/build/file-systems.scm (read-iso9660-superblock): Modify.
---
 gnu/build/file-systems.scm | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gnu/build/file-systems.scm b/gnu/build/file-systems.scm
index 3e0873377..740c37124 100644
--- a/gnu/build/file-systems.scm
+++ b/gnu/build/file-systems.scm
@@ -260,7 +260,11 @@ volume descriptor from ~s"
   "Return the raw contents of DEVICE's iso9660 primary volume descriptor
 as a bytevector, or #f if DEVICE does not contain an iso9660 file system."
   ;; Start reading at sector 16.
-  (read-iso9660-primary-volume-descriptor device (* 2048 16)))
+  ;; Since we are not sure that the device contains an ISO9660 filesystem,
+  ;; we have to find that out first.
+  (if (read-superblock device (* 2048 16) 2048 iso9660-superblock?)
+       (read-iso9660-primary-volume-descriptor device (* 2048 16))
+       #f)) ; Device does not contain an iso9660 filesystem.
 
 (define (iso9660-superblock-uuid sblock)
   "Return the modification time of an iso9660 primary volume descriptor

bug#27212: [PATCH] file-systems: Improve error handling in the iso9660 case - fixes boot problem.

[Danny Milosavljevic]

Hi,

sorry for the slip-up.

The error handling in 203a9455c4695152fc5d0085bffeead9ce3216c2 was improved for the case when there's no iso9660 primary volume descriptor anywhere and no terminator either.  In that case the CD is broken.

But if there's no iso9660 volume descriptor AT ALL (primary or not) it's not a fatal error for guix gnu/build/file-systems.scm - it just means we picked the wrong filesystem type and should try the next one.  This case was not handled correctly and this patch addresses this.

I'd like help with testing this patch.  If someone with a fast computer could apply this patch, then run make check-system, and then, if everything worked, run guix system reconfigure /etc/config.scm , that would be great (my computer is building guix master for the last 3 days and probably needs 4 more - so no idea whether the patch works).

@Marius: I don't understand why reverting 203a9455c4695152fc5d0085bffeead9ce3216c2 didn't fix it.  It should have.  (I can't try it myself because I'm in forced-system-update purgatory - argh).

bug#27212: 203a9455c4695152fc5d0085bffeead9ce3216c2 broke system boot

[jah]

The same issue affects a GuixSD installed on a GPT partitioned Qemu image.  This was after doing guix pull and guix system reconfigure to a fresh install of 0.13.0.x86_64-linux.

The following is a screenshot of the backtrace:-

https://jahboite.co.uk/files/bab/backtrace_file-systems-scm.png

jah

bug#27212: [PATCH] file-systems: Improve error handling in the iso9660 case - fixes boot problem.

[Leo Famulari]

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

On Sat, Jun 03, 2017 at 08:25:00PM +0200, Danny Milosavljevic wrote:
> Hi,
> 
> sorry for the slip-up.
> 
> The error handling in 203a9455c4695152fc5d0085bffeead9ce3216c2 was improved
> for the case when there's no iso9660 primary volume descriptor anywhere and no
> terminator either.  In that case the CD is broken.
> 
> But if there's no iso9660 volume descriptor AT ALL (primary or not) it's not a
> fatal error for guix gnu/build/file-systems.scm - it just means we picked the
> wrong filesystem type and should try the next one.  This case was not handled
> correctly and this patch addresses this.
> 
> I'd like help with testing this patch.  If someone with a fast computer could
> apply this patch, then run make check-system, and then, if everything worked,
> run guix system reconfigure /etc/config.scm , that would be great (my computer
> is building guix master for the last 3 days and probably needs 4 more - so no
> idea whether the patch works).

I don't have access to a fast GuixSD system today.

How about reverting the original change now, so that we can reduce the number of
people who hit the bug. Then we can test the revised commit afterwards.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#27212: [PATCH] file-systems: Improve error handling in the iso9660 case - fixes boot problem.

[Jelle Licht]

Leo Famulari <leo@famulari.name> writes:

> On Sat, Jun 03, 2017 at 08:25:00PM +0200, Danny Milosavljevic wrote:
>> Hi,
>>
>> sorry for the slip-up.
>>
>> The error handling in 203a9455c4695152fc5d0085bffeead9ce3216c2 was improved
>> for the case when there's no iso9660 primary volume descriptor anywhere and no
>> terminator either.  In that case the CD is broken.
>>
>> But if there's no iso9660 volume descriptor AT ALL (primary or not) it's not a
>> fatal error for guix gnu/build/file-systems.scm - it just means we picked the
>> wrong filesystem type and should try the next one.  This case was not handled
>> correctly and this patch addresses this.
>>
>> I'd like help with testing this patch.  If someone with a fast computer could
>> apply this patch, then run make check-system, and then, if everything worked,
>> run guix system reconfigure /etc/config.scm , that would be great (my computer
>> is building guix master for the last 3 days and probably needs 4 more - so no
>> idea whether the patch works).
>
> I don't have access to a fast GuixSD system today.
>
> How about reverting the original change now, so that we can reduce the number of
> people who hit the bug. Then we can test the revised commit afterwards.

Good news! My system boots again after applying your patch and
reconfiguring. The make check-system fails a network related test, but
only because of an unrelated problem, as this already happened before
any of these problems started.

- Jelle

bug#27212: [PATCH] file-systems: Improve error handling in the iso9660 case - fixes boot problem.

[Danny Milosavljevic]

Pushed as fb03f44bb117226e7d67a85401ffbb54ad8858ed.

bug#27212: [PATCH] file-systems: Improve error handling in the iso9660 case - fixes boot problem.

[Danny Milosavljevic]

Hi Leo,

On Sat, 3 Jun 2017 15:45:55 -0400
Leo Famulari <leo@famulari.name> wrote:

> How about reverting the original change now

According to Marius that doesn't work either so it wouldn't help.

So I've applied the new patch to master after Jelle's successful test.  Should be fixed now.

bug#27212: 203a9455c4695152fc5d0085bffeead9ce3216c2 broke system boot

[ng0]

ng0 transcribed 0.5K bytes:
> Otherwise perfectly working systems I have *all* break after
> https://git.savannah.gnu.org/cgit/guix.git/commit/?id=203a9455c4695152fc5d0085bffeead9ce3216c2
> 
> and the last thing  they display before guile repl is the
> message in that commit.
> 
> Yes, I use labels. No, I don't use complicated setups.
> Please test such changes before applying them in the wild..
> I just hope this happened here and the break is a mistake.
> -- 
> ng0
> OpenPG: A88C8ADD129828D7EAC02E52E22F9BBFEE348588

Okay, this has been fixed. Thanks!
I just booted into a functional generation.

I'm closing this bug, in the future let's come up
with ideas how to prevent this.
It is not good PR to collectively send everyone into
the guile repl.

I started discussing about solutions to this offline already
and in other channels.

Thanks!
-- 
ng0
OpenPG: A88C8ADD129828D7EAC02E52E22F9BBFEE348588