[libvirt] [PATCH RFC v3 03/15] storage pools: refactoring of fs backend

Olga Krishtal okrishtal at virtuozzo.com
Fri Dec 2 15:38:08 UTC 2016


The fs backend for storage pools works a lot with
directories and etc. The same is true for filesystem pools with
directory backend. In order to avoid rewriting the same code once again
patch moves this code to virpoolcommon.c.

Signed-off-by: Olga Krishtal <okrishtal at virtuozzo.com>
---
 po/POTFILES.in                   |  1 +
 src/libvirt_private.syms         |  3 ++
 src/storage/storage_backend.h    |  1 -
 src/storage/storage_backend_fs.c | 74 +++-------------------------------
 src/util/virpoolcommon.c         | 87 ++++++++++++++++++++++++++++++++++++++++
 src/util/virpoolcommon.h         |  7 ++++
 6 files changed, 104 insertions(+), 69 deletions(-)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 1469240..29bc45c 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -229,6 +229,7 @@ src/util/virpci.c
 src/util/virperf.c
 src/util/virpidfile.c
 src/util/virpolkit.c
+src/util/virpoolcommon.c
 src/util/virportallocator.c
 src/util/virprocess.c
 src/util/virqemu.c
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a061799..67ebe2a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2709,6 +2709,9 @@ virStoragePoolSourceFree;
 virStoragePoolSourceDeviceClear;
 virStoragePoolSourceClear;
 virStoragePoolSourceListNewSource;
+virDirPoolDelete;
+virDirItemCreate;
+virDirPoolBuild;
 
 # Let emacs know we want case-insensitive sorting
 # Local Variables:
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 28e1a65..a08a725 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -214,7 +214,6 @@ int virStorageBackendVolOpen(const char *path, struct stat *sb,
     ATTRIBUTE_RETURN_CHECK
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
-# define VIR_STORAGE_DEFAULT_POOL_PERM_MODE 0755
 # define VIR_STORAGE_DEFAULT_VOL_PERM_MODE  0600
 
 int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 6c8bae2..c21fbc8 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -45,6 +45,7 @@
 #include "storage_backend_fs.h"
 #include "storage_conf.h"
 #include "virstoragefile.h"
+#include "virpoolcommon.h"
 #include "vircommand.h"
 #include "viralloc.h"
 #include "virxml.h"
@@ -809,11 +810,7 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED,
                                  unsigned int flags)
 {
     int ret = -1;
-    char *parent = NULL;
-    char *p = NULL;
-    mode_t mode;
     bool needs_create_as_uid;
-    unsigned int dir_create_flags;
 
     virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE |
                   VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret);
@@ -822,45 +819,9 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED,
                              VIR_STORAGE_POOL_BUILD_NO_OVERWRITE,
                              error);
 
-    if (VIR_STRDUP(parent, pool->def->target.path) < 0)
-        goto error;
-    if (!(p = strrchr(parent, '/'))) {
-        virReportError(VIR_ERR_INVALID_ARG,
-                       _("path '%s' is not absolute"),
-                       pool->def->target.path);
-        goto error;
-    }
-
-    if (p != parent) {
-        /* assure all directories in the path prior to the final dir
-         * exist, with default uid/gid/mode. */
-        *p = '\0';
-        if (virFileMakePath(parent) < 0) {
-            virReportSystemError(errno, _("cannot create path '%s'"),
-                                 parent);
-            goto error;
-        }
-    }
-
-    dir_create_flags = VIR_DIR_CREATE_ALLOW_EXIST;
     needs_create_as_uid = (pool->def->type == VIR_STORAGE_POOL_NETFS);
-    mode = pool->def->target.perms.mode;
-
-    if (mode == (mode_t) -1 &&
-        (needs_create_as_uid || !virFileExists(pool->def->target.path)))
-        mode = VIR_STORAGE_DEFAULT_POOL_PERM_MODE;
-    if (needs_create_as_uid)
-        dir_create_flags |= VIR_DIR_CREATE_AS_UID;
-
-    /* Now create the final dir in the path with the uid/gid/mode
-     * requested in the config. If the dir already exists, just set
-     * the perms. */
-    if (virDirCreate(pool->def->target.path,
-                     mode,
-                     pool->def->target.perms.uid,
-                     pool->def->target.perms.gid,
-                     dir_create_flags) < 0)
-        goto error;
+    if (virDirPoolBuild(pool->def, needs_create_as_uid) < 0)
+        return ret;
 
     if (flags != 0) {
         ret = virStorageBackendMakeFileSystem(pool, flags);
@@ -869,7 +830,6 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED,
     }
 
  error:
-    VIR_FREE(parent);
     return ret;
 }
 
@@ -1054,14 +1014,8 @@ virStorageBackendFileSystemDelete(virConnectPtr conn ATTRIBUTE_UNUSED,
 
     /* XXX delete all vols first ? */
 
-    if (rmdir(pool->def->target.path) < 0) {
-        virReportSystemError(errno,
-                             _("failed to remove pool '%s'"),
-                             pool->def->target.path);
-        return -1;
-    }
+    return virDirPoolDelete(pool->def->target.path);
 
-    return 0;
 }
 
 
@@ -1084,27 +1038,11 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn ATTRIBUTE_UNUSED,
     else
         vol->type = VIR_STORAGE_VOL_FILE;
 
-    /* Volumes within a directory pools are not recursive; do not
-     * allow escape to ../ or a subdir */
-    if (strchr(vol->name, '/')) {
-        virReportError(VIR_ERR_OPERATION_INVALID,
-                       _("volume name '%s' cannot contain '/'"), vol->name);
-        return -1;
-    }
-
     VIR_FREE(vol->target.path);
-    if (virAsprintf(&vol->target.path, "%s/%s",
-                    pool->def->target.path,
-                    vol->name) == -1)
+    if (virDirItemCreate(vol->name, &vol->target.path,
+                         pool->def->target.path) < 0)
         return -1;
 
-    if (virFileExists(vol->target.path)) {
-        virReportError(VIR_ERR_OPERATION_INVALID,
-                       _("volume target path '%s' already exists"),
-                       vol->target.path);
-        return -1;
-    }
-
     VIR_FREE(vol->key);
     return VIR_STRDUP(vol->key, vol->target.path);
 }
diff --git a/src/util/virpoolcommon.c b/src/util/virpoolcommon.c
index 3ee6251..f002939 100644
--- a/src/util/virpoolcommon.c
+++ b/src/util/virpoolcommon.c
@@ -123,3 +123,90 @@ virStoragePoolSourceListNewSource(virPoolSourceListPtr list)
 
     return source;
 }
+
+int virDirPoolBuild(virPoolDefPtr pooldef, bool needs_create_as_uid)
+{
+    int ret = -1;
+    char *parent = NULL;
+    char *p = NULL;
+    mode_t mode;
+    unsigned int dir_create_flags;
+
+    if (VIR_STRDUP(parent, pooldef->target.path) < 0)
+        goto error;
+    if (!(p = strrchr(parent, '/'))) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("path '%s' is not absolute"),
+                       pooldef->target.path);
+        goto error;
+    }
+
+    if (p != parent) {
+        /* assure all directories in the path prior to the final dir
+         * exist, with default uid/gid/mode. */
+        *p = '\0';
+        if (virFileMakePath(parent) < 0) {
+            virReportSystemError(errno, _("cannot create path '%s'"),
+                                 parent);
+            goto error;
+        }
+    }
+
+    dir_create_flags = VIR_DIR_CREATE_ALLOW_EXIST;
+    mode = pooldef->target.perms.mode;
+
+    if (mode == (mode_t) -1 &&
+        (needs_create_as_uid || !virFileExists(pooldef->target.path)))
+        mode = VIR_STORAGE_DEFAULT_POOL_PERM_MODE;
+    if (needs_create_as_uid)
+        dir_create_flags |= VIR_DIR_CREATE_AS_UID;
+
+    /* Now create the final dir in the path with the uid/gid/mode
+     * requested in the config. If the dir already exists, just set
+     * the perms. */
+    if (virDirCreate(pooldef->target.path,
+                     mode,
+                     pooldef->target.perms.uid,
+                     pooldef->target.perms.gid,
+                     dir_create_flags) < 0)
+        goto error;
+     ret = 0;
+ error:
+    VIR_FREE(parent);
+    return ret;
+}
+
+int virDirPoolDelete(char *path)
+{
+    if (rmdir(path) < 0) {
+        virReportSystemError(errno,
+                             _("failed to remove pool '%s'"),
+                             path);
+        return -1;
+    }
+
+    return 0;
+}
+
+int virDirItemCreate(char *name, char **itempath, char *poolpath)
+{
+
+    if (strchr(name, '/')) {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       _("name '%s' cannot contain '/'"), name);
+        return -1;
+    }
+
+    if (virAsprintf(itempath, "%s/%s",
+                    poolpath, name) == -1)
+        return -1;
+
+    if (virFileExists(*itempath)) {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       _("target path '%s' already exists"),
+                       *itempath);
+        return -1;
+    }
+
+    return 0;
+}
diff --git a/src/util/virpoolcommon.h b/src/util/virpoolcommon.h
index c1c607f..37d642c 100644
--- a/src/util/virpoolcommon.h
+++ b/src/util/virpoolcommon.h
@@ -25,6 +25,9 @@
 # include "virthread.h"
 # include "virpci.h"
 
+
+# define VIR_STORAGE_DEFAULT_POOL_PERM_MODE 0755
+
 /*
  * For remote pools, info on how to reach the host
  */
@@ -179,4 +182,8 @@ void virStoragePoolSourceFree(virPoolSourcePtr source);
 void virStoragePoolDefFree(virPoolDefPtr def);
 virPoolSourcePtr
 virStoragePoolSourceListNewSource(virPoolSourceListPtr list);
+/*Common functions fot directory backend*/
+int virDirPoolBuild(virPoolDefPtr pooldef, bool is_ntfs);
+int virDirPoolDelete(char *path);
+int virDirItemCreate(char *name, char **itempath, char *poolname);
 # endif /* __VIR_POOL_COMMON_H__ */
-- 
1.8.3.1




More information about the libvir-list mailing list