[PATCH 2/5] lxc: process: Rework reading errors from the log file

[Peter Krempa]

Introduce 'virLXCProcessReportStartupLogError' which simplifies the
error handling on startup of the LXC process when reading of the error
log is needed.

This function has unusual return value semantics but it helps to make
the callers simpler.

This patch also removes 2 1k stack'd buffers from virLXCProcessStart.

Signed-off-by: Peter Krempa <pkrempa at redhat.com>
---
 src/lxc/lxc_process.c | 90 ++++++++++++++++++++++++++-----------------
 1 file changed, 55 insertions(+), 35 deletions(-)

diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 39dcf53c67..2a753ae1da 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1096,6 +1096,43 @@ virLXCProcessReadLogOutput(virDomainObj *vm,
 }


+/**
+ * virLXCProcessReportStartupLogError:
+ * @vm: domain object
+ * @logfile: path to the VM logfile
+ * @pos: position in @logfile to look for errors
+ *
+ * Looks for the error message from the LXC container process.
+ * Returns:
+ * -1: - When reading of error message failed. Reports appropriate error
+ *     - When successfully read a non-empty error message. Reports an error with
+ *       the following message
+ *          'guest failed to start: ' with the error from the log appended
+ *
+ *  0: - When reading the error was successful but the error log was empty.
+ */
+static int
+virLXCProcessReportStartupLogError(virDomainObj *vm,
+                                   char *logfile,
+                                   off_t pos)
+{
+    size_t buflen = 1024;
+    g_autofree char *errbuf = g_new0(char, buflen);
+    int rc;
+
+    if ((rc = virLXCProcessReadLogOutput(vm, logfile, pos, errbuf, buflen)) < 0)
+        return -1;
+
+    if (rc == 0)
+        return 0;
+
+    virReportError(VIR_ERR_INTERNAL_ERROR,
+                   _("guest failed to start: %s"), errbuf);
+
+    return -1;
+}
+
+
 static int
 virLXCProcessEnsureRootFS(virDomainObj *vm)
 {
@@ -1151,7 +1188,6 @@ int virLXCProcessStart(virLXCDriver * driver,
     g_auto(GStrv) veths = NULL;
     int handshakefds[4] = { -1, -1, -1, -1 }; /* two pipes */
     off_t pos = -1;
-    char ebuf[1024];
     g_autofree char *timestamp = NULL;
     int nsInheritFDs[VIR_LXC_DOMAIN_NAMESPACE_LAST];
     g_autoptr(virCommand) cmd = NULL;
@@ -1368,28 +1404,28 @@ int virLXCProcessStart(virLXCDriver * driver,
         goto cleanup;

     if (status != 0) {
-        if (virLXCProcessReadLogOutput(vm, logfile, pos, ebuf,
-                                       sizeof(ebuf)) <= 0) {
-            if (WIFEXITED(status))
-                g_snprintf(ebuf, sizeof(ebuf), _("unexpected exit status %d"),
+        if (virLXCProcessReportStartupLogError(vm, logfile, pos) < 0)
+            goto cleanup;
+
+        /* In case there isn't an error in the logs report one based on the exit status */
+        if (WIFEXITED(status)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("guest failed to start: unexpected exit status %d"),
                            WEXITSTATUS(status));
-            else
-                g_snprintf(ebuf, sizeof(ebuf), "%s", _("terminated abnormally"));
+        } else {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("guest failed to start: terminated abnormally"));
         }
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("guest failed to start: %s"), ebuf);
         goto cleanup;
     }

     /* It has started running, so get its pid */
     if ((r = virPidFileReadPath(pidfile, &vm->pid)) < 0) {
-        if (virLXCProcessReadLogOutput(vm, logfile, pos, ebuf, sizeof(ebuf)) > 0)
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("guest failed to start: %s"), ebuf);
-        else
-            virReportSystemError(-r,
-                                 _("Failed to read pid file %s"),
-                                 pidfile);
+        if (virLXCProcessReportStartupLogError(vm, logfile, pos) < 0)
+            goto cleanup;
+
+        /* In case there isn't an error in the logs report that we failed to read the pidfile */
+        virReportSystemError(-r, _("Failed to read pid file %s"), pidfile);
         goto cleanup;
     }

@@ -1422,13 +1458,7 @@ int virLXCProcessStart(virLXCDriver * driver,

     /* The first synchronization point is when the controller creates CGroups. */
     if (lxcContainerWaitForContinue(handshakefds[0]) < 0) {
-        char out[1024];
-
-        if (!(virLXCProcessReadLogOutput(vm, logfile, pos, out, 1024) < 0)) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("guest failed to start: %s"), out);
-        }
-
+        virLXCProcessReportStartupLogError(vm, logfile, pos);
         goto cleanup;
     }

@@ -1461,13 +1491,7 @@ int virLXCProcessStart(virLXCDriver * driver,
     /* The second synchronization point is when the controller finished
      * creating the container. */
     if (lxcContainerWaitForContinue(handshakefds[0]) < 0) {
-        char out[1024];
-
-        if (!(virLXCProcessReadLogOutput(vm, logfile, pos, out, 1024) < 0)) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("guest failed to start: %s"), out);
-        }
-
+        virLXCProcessReportStartupLogError(vm, logfile, pos);
         goto cleanup;
     }

@@ -1476,11 +1500,7 @@ int virLXCProcessStart(virLXCDriver * driver,
         /* Intentionally overwrite the real monitor error message,
          * since a better one is almost always found in the logs
          */
-        if (virLXCProcessReadLogOutput(vm, logfile, pos, ebuf, sizeof(ebuf)) > 0) {
-            virResetLastError();
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("guest failed to start: %s"), ebuf);
-        }
+        virLXCProcessReportStartupLogError(vm, logfile, pos);
         goto cleanup;
     }

-- 
2.37.1