[Fai-commit] r6333 - trunk/lib/setup-storage

[Michael Tautschnig]

Author: mt
Date: 2011-04-14 11:11:31 +0000 (Thu, 14 Apr 2011)
New Revision: 6333

Modified:
   trunk/lib/setup-storage/Init.pm
   trunk/lib/setup-storage/Parser.pm
   trunk/lib/setup-storage/Volumes.pm
Log:
setup-storage/{Parser,Volumes,Init}.pm: Improved error reporting.

Closes: #617720, #611728
Thanks: Toomas Tamm

- setup-storage/Parser.pm: Fix error message in case of duplicate definitions
  of a single disk. Sensible diagnostics instead of just printing "Syntax
  error" for several syntax errors. Thanks Toomas Tamm for reporting this.
  Perform more advanced consistency checking for duplicate mount points
  (closes: #617720).
- setup-storage/Volumes.pm: Proper error reporting for wrong disk names (one
  die was overridden by yet another die); more verbose output in case of
  unexpected mdadm output (closes: #611728).
- setup-storage/Init.pm: Use Carp::confess to produce stack traces when
  calling internal_error.


Modified: trunk/lib/setup-storage/Init.pm
===================================================================
--- trunk/lib/setup-storage/Init.pm	2011-04-14 11:05:53 UTC (rev 6332)
+++ trunk/lib/setup-storage/Init.pm	2011-04-14 11:11:31 UTC (rev 6333)
@@ -286,7 +286,9 @@
 
   my ($error_msg) = @_;
 
-  die <<EOF;
+  use Carp;
+  $Carp::CarpLevel = 1;
+  confess <<EOF;
 INTERNAL ERROR in setup-storage:
 $error_msg
 Please report this error to the Debian Bug Tracking System.

Modified: trunk/lib/setup-storage/Parser.pm
===================================================================
--- trunk/lib/setup-storage/Parser.pm	2011-04-14 11:05:53 UTC (rev 6332)
+++ trunk/lib/setup-storage/Parser.pm	2011-04-14 11:11:31 UTC (rev 6333)
@@ -128,7 +128,7 @@
 
   # test, whether this is the first disk_config stanza to configure $disk
   defined ($FAI::configs{$FAI::device})
-    and die "Duplicate configuration for disk $FAI::disks[ $1-1 ]\n";
+    and die "Duplicate configuration for disk $disk\n";
 
   # Initialise the entry in $FAI::configs
   $FAI::configs{$FAI::device} = {
@@ -373,6 +373,7 @@
     line: <skip: qr/[ \t]*/> "\\n"
         | <skip: qr/[ \t]*/> comment "\\n"
         | <skip: qr/[ \t]*/> config "\\n"
+        | <error>
 
     comment: /^\s*#.*/
 
@@ -989,20 +990,60 @@
 ################################################################################
 sub check_config {
 
+  my %all_mount_pts = ();
+
   # loop through all configs
   foreach my $config (keys %FAI::configs) {
     if ($config =~ /^PHY_(.+)$/) {
       (scalar(keys %{ $FAI::configs{$config}{partitions} }) > 0) or
         die "Empty disk_config stanza for device $1\n";
+      foreach my $p (keys %{ $FAI::configs{$config}{partitions} }) {
+        next if (1 == $FAI::configs{$config}{partitions}{$p}{size}{extended});
+        defined($FAI::configs{$config}{partitions}{$p}{mountpoint}) or
+          &FAI::internal_error("Undefined mountpoint for non-extended partition");
+        my $this_mp = $FAI::configs{$config}{partitions}{$p}{mountpoint};
+        next if ($this_mp eq "-");
+        defined($all_mount_pts{$this_mp}) and die
+          "Mount point $this_mp used twice\n";
+        ($this_mp eq "none") or $all_mount_pts{$this_mp} = 1;
+      }
     } elsif ($config =~ /^VG_(.+)$/) {
       next if ($1 eq "--ANY--");
+      foreach my $p (keys %{ $FAI::configs{$config}{volumes} }) {
+        my $this_mp = $FAI::configs{$config}{volumes}{$p}{mountpoint};
+        next if ($this_mp eq "-");
+        defined($all_mount_pts{$this_mp}) and die
+          "Mount point $this_mp used twice\n";
+        ($this_mp eq "none") or $all_mount_pts{$this_mp} = 1;
+      }
       next;
     } elsif ($config eq "RAID") {
       (scalar(keys %{ $FAI::configs{$config}{volumes} }) > 0) or
         die "Empty RAID configuration\n";
+      foreach my $p (keys %{ $FAI::configs{$config}{volumes} }) {
+        my $this_mp = $FAI::configs{$config}{volumes}{$p}{mountpoint};
+        next if ($this_mp eq "-");
+        defined($all_mount_pts{$this_mp}) and die
+          "Mount point $this_mp used twice\n";
+        ($this_mp eq "none") or $all_mount_pts{$this_mp} = 1;
+      }
     } elsif ($config eq "CRYPT") {
+      foreach my $p (keys %{ $FAI::configs{$config}{volumes} }) {
+        my $this_mp = $FAI::configs{$config}{volumes}{$p}{mountpoint};
+        next if ($this_mp eq "-");
+        defined($all_mount_pts{$this_mp}) and die
+          "Mount point $this_mp used twice\n";
+        ($this_mp eq "none") or $all_mount_pts{$this_mp} = 1;
+      }
       next;
     } elsif ($config eq "TMPFS") {
+      foreach my $p (keys %{ $FAI::configs{$config}{volumes} }) {
+        my $this_mp = $FAI::configs{$config}{volumes}{$p}{mountpoint};
+        next if ($this_mp eq "-");
+        defined($all_mount_pts{$this_mp}) and die
+          "Mount point $this_mp used twice\n";
+        ($this_mp eq "none") or $all_mount_pts{$this_mp} = 1;
+      }
       next;
     } else {
       &FAI::internal_error("Unexpected key $config");

Modified: trunk/lib/setup-storage/Volumes.pm
===================================================================
--- trunk/lib/setup-storage/Volumes.pm	2011-04-14 11:05:53 UTC (rev 6332)
+++ trunk/lib/setup-storage/Volumes.pm	2011-04-14 11:11:31 UTC (rev 6333)
@@ -456,7 +456,9 @@
 	  }
       }
     } elsif ($line =~ /^\s*devices=(\S+)$/) {
-      defined($id) or &FAI::internal_error("mdadm ARRAY line not yet seen");
+      defined($id) or
+        &FAI::internal_error("mdadm ARRAY line not yet seen -- unexpected mdadm output:\n"
+          . join("", @mdadm_print));
       push @{ $FAI::current_raid_config{$id}{devices} }, abs_path($_)
         foreach (split (",", $1));
  
@@ -534,6 +536,8 @@
           defined ($FAI::current_config{$1}) or die
             "Can't preserve $1 because it does not exist\n";
         } else {
+          defined ($FAI::current_config{$1}) or die
+            "Can't preserve partition on $1 because $1 does not exist\n";
           defined ($FAI::current_config{$1}{partitions}{$part_id}) or die
             "Can't preserve ". &FAI::make_device_name($1, $part->{number})
               . " because it does not exist\n";