Re: [libvirt] [PATCH v2 5/5] storage: fs: Only force directory permissions if required

Only set directory permissions at pool build time, if: - User explicitly requested a mode via the XML - The directory needs to be created - We need to do the crazy NFS root-squash workaround This allows qemu:///session to call build on an existing directory like /tmp. --- v2: Fix style issue pointed out by pkrempa Skip chmod if mode == -1 for the fork/nfs case as well src/storage/storage_backend_fs.c | 16 +++++++++++----- src/util/virfile.c | 4 ++-- 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index ed56935..3f42a5b 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -769,6 +769,8 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, int err, ret = -1; char *parent = NULL; char *p = NULL; + mode_t mode; + bool needs_create_as_uid; virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE | VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret); @@ -797,18 +799,22 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, } } + 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; + /* 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 ((err = virDirCreate(pool->def->target.path, - (pool->def->target.perms.mode == (mode_t) -1 ? - VIR_STORAGE_DEFAULT_POOL_PERM_MODE : - pool->def->target.perms.mode), + mode, pool->def->target.perms.uid, pool->def->target.perms.gid, VIR_DIR_CREATE_ALLOW_EXIST | - (pool->def->type == VIR_STORAGE_POOL_NETFS - ? VIR_DIR_CREATE_AS_UID : 0))) < 0) { + (needs_create_as_uid ? + VIR_DIR_CREATE_AS_UID : 0))) < 0) { goto error; } diff --git a/src/util/virfile.c b/src/util/virfile.c index 63eafdf..5ff4668 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2311,7 +2311,7 @@ virDirCreateNoFork(const char *path, path, (unsigned int) uid, (unsigned int) gid); goto error; } - if (chmod(path, mode) < 0) { + if (mode != (mode_t) -1 && chmod(path, mode) < 0) { ret = -errno; virReportSystemError(errno, _("cannot set mode of '%s' to %04o"), @@ -2424,7 +2424,7 @@ virDirCreate(const char *path, path, (unsigned int) gid); goto childerror; } - if (chmod(path, mode) < 0) { + if (mode != (mode_t) -1 && chmod(path, mode) < 0) { virReportSystemError(errno, _("cannot set mode of '%s' to %04o"), path, mode);