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

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(a)virtuozzo.com&gt; --- 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__ */