Libguestfs - Oct 2015 - [PATCH v2] inspect: check for errors in files parsed with augeas (RHBZ#1229119)

During the inspection phase, check for errors after aug_load: if any of
the errors happened in any of the requested files, then report the error
straight away; ignoring the error means that information would be
silently lost.  For example, a malformed /etc/fstab would have caused
the inspection to not handle any of the additional mount points, giving
cryptic errors later on when trying to access files in any of the mount
points.

Now guests with invalid files such as /etc/fstab, /etc/mdadm.conf, and
/etc/sysconfig/network will cause the inspection to fail, instead of
being reported with a single mount point ('/').
---
 src/inspect-fs-unix.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/src/inspect-fs-unix.c b/src/inspect-fs-unix.c
index 3d19276..968aa40 100644
--- a/src/inspect-fs-unix.c
+++ b/src/inspect-fs-unix.c
@@ -1983,6 +1983,9 @@ inspect_with_augeas (guestfs_h *g, struct inspect_fs *fs,
   int64_t size;
   int r;
   CLEANUP_FREE char *pathexpr = NULL;
+  CLEANUP_FREE_STRING_LIST char **matches = NULL;
+  char **match;
+  size_t len;
 
   /* Security: Refuse to do this if a config file is too large. */
   for (i = 0; configfiles[i] != NULL; ++i) {
@@ -2019,6 +2022,22 @@ inspect_with_augeas (guestfs_h *g, struct inspect_fs *fs,
   if (guestfs_aug_load (g) == -1)
     goto out;
 
+  /* Check that augeas did not get a parse error for any of the configfiles,
+   * otherwise we are silently missing information. */
+  matches = guestfs_aug_match (g, "/augeas/files//error");
+  for (match = matches; *match != NULL; ++match) {
+    for (i = 0; configfiles[i] != NULL; ++i) {
+      len = strlen (configfiles[i]);
+      if (strlen (*match) == (13 /* len(/augeas/files) */ + len + 6 /*
len(/error) */) &&
+          STRPREFIX(*match, "/augeas/files") &&
+          STREQLEN(*match + 13, configfiles[i], len) &&
+          STREQ(*match + 13 + len, "/error")) {
+        error (g, _("augeas could not parse %s"), configfiles[i]);
+        goto out;
+      }
+    }
+  }
+
   r = f (g, fs);
 
  out:
-- 
2.1.0

On Thu, Oct 15, 2015 at 06:15:28PM +0200, Pino Toscano
wrote:
> During the inspection phase, check for errors after aug_load: if any of
> the errors happened in any of the requested files, then report the error
> straight away; ignoring the error means that information would be
> silently lost.  For example, a malformed /etc/fstab would have caused
> the inspection to not handle any of the additional mount points, giving
> cryptic errors later on when trying to access files in any of the mount
> points.
> 
> Now guests with invalid files such as /etc/fstab, /etc/mdadm.conf, and
> /etc/sysconfig/network will cause the inspection to fail, instead of
> being reported with a single mount point ('/').
> ---
>  src/inspect-fs-unix.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/src/inspect-fs-unix.c b/src/inspect-fs-unix.c
> index 3d19276..968aa40 100644
> --- a/src/inspect-fs-unix.c
> +++ b/src/inspect-fs-unix.c
> @@ -1983,6 +1983,9 @@ inspect_with_augeas (guestfs_h *g, struct inspect_fs
*fs,
>    int64_t size;
>    int r;
>    CLEANUP_FREE char *pathexpr = NULL;
> +  CLEANUP_FREE_STRING_LIST char **matches = NULL;
> +  char **match;
> +  size_t len;
>  
>    /* Security: Refuse to do this if a config file is too large. */
>    for (i = 0; configfiles[i] != NULL; ++i) {
> @@ -2019,6 +2022,22 @@ inspect_with_augeas (guestfs_h *g, struct inspect_fs
*fs,
>    if (guestfs_aug_load (g) == -1)
>      goto out;
>  
> +  /* Check that augeas did not get a parse error for any of the
configfiles,
> +   * otherwise we are silently missing information. */
> +  matches = guestfs_aug_match (g, "/augeas/files//error");
> +  for (match = matches; *match != NULL; ++match) {
> +    for (i = 0; configfiles[i] != NULL; ++i) {
> +      len = strlen (configfiles[i]);
> +      if (strlen (*match) == (13 /* len(/augeas/files) */ + len + 6 /*
len(/error) */) &&
> +          STRPREFIX(*match, "/augeas/files") &&
> +          STREQLEN(*match + 13, configfiles[i], len) &&
> +          STREQ(*match + 13 + len, "/error")) {
> +        error (g, _("augeas could not parse %s"),
configfiles[i]);
> +        goto out;
> +      }
> +    }
> +  }
> +
ACK .. and ugh!  Augeas should make this easier.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/