[Libguestfs] [PATCH] Improve augeas error reporting

Matthew Booth mbooth at redhat.com
Thu May 13 13:34:03 UTC 2010


Add a function to display more complete augeas errors if they occur. Update all
uses of augeas in GuestOS::RedHat to use it for error reporting.
---
 lib/Sys/VirtV2V/GuestOS/RedHat.pm |  185 ++++++++++++++++++++++++-------------
 1 files changed, 121 insertions(+), 64 deletions(-)

diff --git a/lib/Sys/VirtV2V/GuestOS/RedHat.pm b/lib/Sys/VirtV2V/GuestOS/RedHat.pm
index cf8787d..8b211f0 100644
--- a/lib/Sys/VirtV2V/GuestOS/RedHat.pm
+++ b/lib/Sys/VirtV2V/GuestOS/RedHat.pm
@@ -93,6 +93,59 @@ sub new
     return $self;
 }
 
+sub _augeas_error
+{
+    my $self = shift;
+    my ($err) = @_; # The original error message. We will emit this if there
+                    # were no augeas errors.
+
+    my $g = $self->{g};
+
+    my $msg = "";
+
+    foreach my $error ($g->aug_match('/augeas/files//error')) {
+        $error =~ /^\/augeas\/files(\/.*)\/error$/
+            or die("Unexpected return from aug_match: $error");
+        my $file = $1;
+
+        my %detail;
+        foreach my $detail_path ($g->aug_match("$error//*")) {
+            $detail_path =~ /^$error\/(.*)$/
+                or die("Unexpected return from aug_match: $detail_path");
+            $detail{$1} = $g->aug_get($detail_path);
+        }
+
+        if (defined($detail{message})) {
+            $msg .= __x("augeas error for {file}: {error}",
+                       file => $file,
+                       error => $detail{message})."\n";
+        } else {
+            $msg .= __x("augeas error for {file}",
+                       file => $file)."\n";
+        }
+
+        if (defined($detail{pos}) && defined($detail{line}) &&
+            defined($detail{char}))
+        {
+            $msg .= __x("error at line {line}, char {char}, file ".
+                             "position {pos}",
+                             line => $detail{line},
+                             char => $detail{char},
+                             pos => $detail{pos})."\n";
+        }
+
+        if (defined($detail{lens})) {
+            $msg .= __x("augeas lens: {lens}",
+                        lens => $detail{lens})."\n";
+        }
+    }
+
+    chomp($msg);
+
+    die(user_message($msg)) if (length($msg) > 0);
+    die($err);
+}
+
 # Handle SELinux for the guest
 sub _init_selinux
 {
@@ -185,7 +238,7 @@ sub _init_augeas
     };
 
     # The augeas calls will die() on any error.
-    die($@) if($@);
+    $self->_augeas_error($@) if ($@);
 }
 
 =item enable_kernel_module(device, module)
@@ -209,7 +262,7 @@ sub enable_kernel_module
     };
 
     # Propagate augeas errors
-    die($@) if($@);
+    $self->_augeas_error($@) if ($@);
 }
 
 =item update_kernel_module(device, module)
@@ -242,7 +295,7 @@ sub update_kernel_module
     };
 
     # Propagate augeas errors
-    die($@) if($@);
+    $self->_augeas_error($@) if ($@);
 }
 
 =item disable_kernel_module(device)
@@ -275,7 +328,7 @@ sub disable_kernel_module
     };
 
     # Propagate augeas errors
-    die($@) if($@);
+    $self->_augeas_error($@) if ($@);
 }
 
 =item update_display_driver(driver)
@@ -303,7 +356,7 @@ sub update_display_driver
     };
 
     # Propagate augeas errors
-    die($@) if($@);
+    $self->_augeas_error($@) if ($@);
 }
 
 # We can't rely on the index in the augeas path because it will change if
@@ -333,7 +386,7 @@ sub _check_augeas_device
     };
 
     # Propagate augeas errors
-    die($@) if($@);
+    $self->_augeas_error($@) if ($@);
 
     return $augeas;
 }
@@ -355,19 +408,28 @@ sub get_default_kernel
     eval {
         $default = $g->aug_get('/files/boot/grub/menu.lst/default');
     };
+    # Doesn't matter if get fails
 
     # Get the grub filesystem
     my $grub = $self->{desc}->{boot}->{grub_fs};
 
     # Look for a kernel, starting with the default
     my @paths;
-    push(@paths, $g->aug_match("/files/boot/grub/menu.lst/".
-                               "title[$default]/kernel")) if defined($default);
-    push(@paths, $g->aug_match('/files/boot/grub/menu.lst/title/kernel'));
+    eval {
+        push(@paths, $g->aug_match("/files/boot/grub/menu.lst/".
+                                   "title[$default]/kernel"))
+            if defined($default);
+        push(@paths, $g->aug_match('/files/boot/grub/menu.lst/title/kernel'));
+    };
+
+    $self->_augeas_error($@) if ($@);
 
     my $kernel;
     foreach my $path (@paths) {
-        $kernel = $g->aug_get($path);
+        eval {
+            $kernel = $g->aug_get($path);
+        };
+        $self->_augeas_error($@) if ($@);
 
         # Prepend the grub filesystem to the kernel path
         $kernel = "$grub$kernel" if(defined($grub));
@@ -521,7 +583,11 @@ sub add_kernel
     $self->_install_rpms(0, ($app));
 
     # Make augeas reload so it'll find the new kernel
-    $g->aug_load();
+    eval {
+        $g->aug_load();
+    };
+
+    $self->_augeas_error($@) if ($@);
 
     return $version;
 }
@@ -603,17 +669,15 @@ sub remove_kernel
         unless(defined($version));
 
     my $g = $self->{g};
-    eval {
-        # Work out which rpm contains the kernel
-        my @output =
-            $g->command_lines(['rpm', '-qf', "/boot/vmlinuz-$version"]);
-        $g->command(['rpm', '-e', $output[0]]);
-    };
-
-    die($@) if($@);
+    # Work out which rpm contains the kernel
+    my @output = $g->command_lines(['rpm', '-qf', "/boot/vmlinuz-$version"]);
+    $g->command(['rpm', '-e', $output[0]]);
 
     # Make augeas reload so it knows the kernel's gone
-    $g->aug_load();
+    eval {
+        $g->aug_load();
+    };
+    $self->_augeas_error($@) if ($@);
 }
 
 =item add_application(label)
@@ -928,13 +992,14 @@ sub remove_application
     my $name = shift;
 
     my $g = $self->{g};
+    $g->command(['rpm', '-e', $name]);
+
+    # Make augeas reload in case the removal changed anything
     eval {
-        $g->command(['rpm', '-e', $name]);
+        $g->aug_load();
     };
-    die($@) if($@);
 
-    # Make augeas reload in case the removal changed anything
-    $g->aug_load();
+    $self->_augeas_error($@) if ($@);
 }
 
 =item get_application_owner(file)
@@ -969,15 +1034,14 @@ sub _install_rpms
     @rpms = map { $_ = $self->_transfer_path($_) } @rpms;
 
     my $g = $self->{g};
+    $g->command(['rpm', $upgrade == 1 ? '-U' : '-i', @rpms]);
+
+    # Reload augeas in case the rpm installation changed anything
     eval {
-        $g->command(['rpm', $upgrade == 1 ? '-U' : '-i', @rpms]);
+        $g->aug_load();
     };
 
-    # Propagate command failure
-    die($@) if($@);
-
-    # Reload augeas in case the rpm installation changed anything
-    $g->aug_load();
+    $self->_augeas_error($@) if($@);
 }
 
 # Get full, local path of a file on the transfer mount
@@ -1049,13 +1113,17 @@ sub remap_block_devices
     if ($libata) {
         # Look for IDE and SCSI devices in fstab for the guest
         my %guestif;
-        foreach my $spec ($g->aug_match('/files/etc/fstab/*/spec')) {
-            my $device = $g->aug_get($spec);
+        eval {
+            foreach my $spec ($g->aug_match('/files/etc/fstab/*/spec')) {
+                my $device = $g->aug_get($spec);
 
-            next unless($device =~ m{^/dev/(sd|hd)([a-z]+)});
-            $guestif{$1} ||= {};
-            $guestif{$1}->{$1.$2} = 1;
-        }
+                next unless($device =~ m{^/dev/(sd|hd)([a-z]+)});
+                $guestif{$1} ||= {};
+                $guestif{$1}->{$1.$2} = 1;
+            }
+        };
+
+        $self->_augeas_error($@) if ($@);
 
         # If fstab contains references to sdX, these could refer to IDE or SCSI
         # devices. We may need to update them.
@@ -1115,19 +1183,23 @@ sub remap_block_devices
         $letter++;
     }
 
-    # Go through fstab again, updating bare device references
-    foreach my $spec ($g->aug_match('/files/etc/fstab/*/spec')) {
-        my $device = $g->aug_get($spec);
+    eval {
+        # Go through fstab again, updating bare device references
+        foreach my $spec ($g->aug_match('/files/etc/fstab/*/spec')) {
+            my $device = $g->aug_get($spec);
 
-        next unless($device =~ m{^/dev/([a-z]+)(\d*)});
-        my $name = $1;
-        my $part = $2;
+            next unless($device =~ m{^/dev/([a-z]+)(\d*)});
+            my $name = $1;
+            my $part = $2;
 
-        next unless(exists($map{$name}));
+            next unless(exists($map{$name}));
 
-        $g->aug_set($spec, "/dev/".$map{$name}.$part);
-    }
-    $g->aug_save();
+            $g->aug_set($spec, "/dev/".$map{$name}.$part);
+        }
+        $g->aug_save();
+    };
+
+    $self->_augeas_error($@) if ($@);
 
     # Delete cached (and now out of date) blkid info if it exists
     foreach my $blkidtab ('/etc/blkid/blkid.tab', '/etc/blkid.tab') {
@@ -1292,21 +1364,11 @@ sub prepare_bootable
             }
         }
 
-        eval {
-            $g->aug_save();
-        };
-
-        if ($@) {
-            my $msg = '';
-            foreach my $error ($g->aug_match('/augeas//error')) {
-                $msg .= $error.': '.$g->aug_get($error)."\n";
-            }
-            die($msg);
-        }
+        $g->aug_save();
     };
 
     # Propagate augeas failure
-    die($@) if($@);
+    $self->_augeas_error($@) if ($@);
 
     if(!defined($initrd)) {
         print STDERR user_message(__x("WARNING: Kernel version {version} ".
@@ -1330,12 +1392,7 @@ sub prepare_bootable
             $g->aug_save();
         };
 
-        if($@) {
-            foreach my $error ($g->aug_match('/augeas//error/*')) {
-                print STDERR "$error: ".$g->aug_get($error)."\n";
-            }
-            die($@);
-        }
+        $self->_augeas_error($@) if ($@);
 
         # We explicitly modprobe ext2 here. This is required by mkinitrd on RHEL
         # 3, and shouldn't hurt on other OSs. We don't care if this fails.
-- 
1.6.6.1




More information about the Libguestfs mailing list