Re: [patch 9/9] Implement better error reporting

[Mark McLoughlin <markmc@xxxxxxxxxx>]

On Mon, 2007-02-19 at 18:00 +0000, Mark McLoughlin wrote:

> 	Okay, here's an attempt to clean up all this code - mostly to coalesce
> the two system vs. user code paths into a single function, but also to
> recursively create config dirs and fix up the problems you spotted.

	I went ahead and committed that assuming no responses meant no-one had
a problem with it.

	Here's a further patch to make us only attempt to create config dirs
when configs are being saved.

Cheers,
Mark.

Index: libvirt/qemud/conf.c
===================================================================
--- libvirt.orig/qemud/conf.c	2007-02-20 14:58:51.000000000 +0000
+++ libvirt.orig/qemud/conf.c	2007-02-20 14:58:51.000000000 +0000
@@ -138,31 +138,32 @@ qemudMakeConfigPath(const char *configDi
 }
 
 static int
-qemudEnsureConfigDir(struct qemud_server *server,
-                     const char *configDir) {
-    struct stat sb;
-
-    /* XXX: needs to be recursive */
-
-    if (stat(configDir, &sb) < 0) {
-        if (errno == ENOENT) {
-            if (mkdir(configDir, 0700) < 0) {
-                qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
-                                 "cannot create config directory %s: %s",
-                                 configDir, strerror(errno));
-                return -1;
-            }
-        } else {
-            qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
-                             "cannot stat config directory %s",
-                             configDir, strerror(errno));
-            return -1;
-        }
-    } else if (!S_ISDIR(sb.st_mode)) {
-        qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
-                         "config directory %s is not a directory", configDir);
-        return -1;
-    }
+qemudEnsureDir(const char *path)
+{
+    struct stat st;
+    char parent[PATH_MAX];
+    char *p;
+    int err;
+
+    if (stat(path, &st) >= 0)
+        return 0;
+
+    strncpy(parent, path, PATH_MAX);
+    parent[PATH_MAX - 1] = '\0';
+
+    if (!(p = strrchr(parent, '/')))
+        return EINVAL;
+
+    if (p == parent)
+        return EPERM;
+
+    *p = '\0';
+
+    if ((err = qemudEnsureDir(parent)))
+        return err;
+
+    if (mkdir(path, 0777) < 0 && errno != EEXIST)
+        return errno;
 
     return 0;
 }
@@ -1138,12 +1139,16 @@ static int qemudSaveConfig(struct qemud_
     char *xml;
     int fd = -1, ret = -1;
     int towrite;
+    int err;
 
     if (!(xml = qemudGenerateXML(server, vm, 0))) {
         return -1;
     }
 
-    if (qemudEnsureConfigDir(server, server->configDir) < 0) {
+    if ((err = qemudEnsureDir(server->configDir))) {
+        qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
+                         "cannot create config directory %s: %s",
+                         server->configDir, strerror(err));
         goto cleanup;
     }
 
@@ -1268,12 +1273,16 @@ static int qemudSaveNetworkConfig(struct
     char *xml;
     int fd, ret = -1;
     int towrite;
+    int err;
 
     if (!(xml = qemudGenerateNetworkXML(server, network, 0))) {
         return -1;
     }
 
-    if (qemudEnsureConfigDir(server, server->networkConfigDir) < 0) {
+    if ((err = qemudEnsureDir(server->networkConfigDir))) {
+        qemudReportError(server, VIR_ERR_INTERNAL_ERROR,
+                         "cannot create config directory %s: %s",
+                         server->networkConfigDir, strerror(err));
         goto cleanup;
     }
 
Index: libvirt/qemud/qemud.c
===================================================================
--- libvirt.orig/qemud/qemud.c	2007-02-20 14:58:51.000000000 +0000
+++ libvirt.orig/qemud/qemud.c	2007-02-20 14:58:51.000000000 +0000
@@ -350,37 +350,6 @@ static int qemudListenUnix(struct qemud_
     return 0;
 }
 
-static int
-qemudEnsureDir(const char *path)
-{
-    struct stat st;
-    char parent[PATH_MAX];
-    char *p;
-    int err;
-
-    if (stat(path, &st) >= 0)
-        return 0;
-
-    strncpy(parent, path, PATH_MAX);
-    parent[PATH_MAX - 1] = '\0';
-
-    if (!(p = strrchr(parent, '/')))
-        return EINVAL;
-
-    if (p == parent)
-        return EPERM;
-
-    *p = '\0';
-
-    if ((err = qemudEnsureDir(parent)))
-        return err;
-
-    if (mkdir(path, 0777) < 0 && errno != EEXIST)
-        return errno;
-
-    return 0;
-}
-
 static int qemudInitPaths(int sys,
                           char *configDir,
                           char *networkConfigDir,
@@ -388,7 +357,6 @@ static int qemudInitPaths(int sys,
                           char *roSockname,
                           int maxlen) {
     uid_t uid;
-    int err;
 
     uid = geteuid();
 
@@ -432,18 +400,6 @@ static int qemudInitPaths(int sys,
             goto snprintf_error;
     }
 
-    if ((err = qemudEnsureDir(configDir))) {
-        qemudLog(QEMUD_ERR, "Failed to create directory '%s': %s",
-                 configDir, strerror(err));
-        return -1;
-    }
-
-    if ((err = qemudEnsureDir(networkConfigDir))) {
-        qemudLog(QEMUD_ERR, "Failed to create directory '%s': %s",
-                 networkConfigDir, strerror(err));
-        return -1;
-    }
-
     return 0;
 
  snprintf_error: