Similar to commit id '35847860', it's possible to attempt to create a 'netfs' directory in an NFS root-squash environment which will cause the 'vol-delete' command to fail. It's also possible error paths from the 'vol-create' would result in an error to remove a created directory if the permissions were incorrect (and disallowed root access). Thus rename the virFileUnlink to be virFileRemove to match the C API functionality, adjust the code to following using rmdir or unlink depending on the path type, and then use/call it for the VIR_STORAGE_VOL_DIR Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> --- src/libvirt_private.syms | 2 +- src/storage/storage_backend_fs.c | 22 ++++++++++------------ src/util/virfile.c | 29 ++++++++++++++++++++--------- src/util/virfile.h | 2 +- 4 files changed, 32 insertions(+), 23 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1bb41f7..8c1f388 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1454,6 +1454,7 @@ virFileReadAllQuiet; virFileReadHeaderFD; virFileReadLimFD; virFileRelLinkPointsTo; +virFileRemove; virFileResolveAllLinks; virFileResolveLink; virFileRewrite; @@ -1461,7 +1462,6 @@ virFileSanitizePath; virFileSkipRoot; virFileStripSuffix; virFileTouch; -virFileUnlink; virFileUnlock; virFileUpdatePerm; virFileWaitForDevices; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index f41a41e..99ea394 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1203,25 +1203,23 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, switch ((virStorageVolType) vol->type) { case VIR_STORAGE_VOL_FILE: - if (virFileUnlink(vol->target.path, vol->target.perms->uid, + case VIR_STORAGE_VOL_DIR: + if (virFileRemove(vol->target.path, vol->target.perms->uid, vol->target.perms->gid) < 0) { /* Silently ignore failures where the vol has already gone away */ if (errno != ENOENT) { - virReportSystemError(errno, - _("cannot unlink file '%s'"), - vol->target.path); + if (vol->type == VIR_STORAGE_VOL_FILE) + virReportSystemError(errno, + _("cannot unlink file '%s'"), + vol->target.path); + else + virReportSystemError(errno, + _("cannot remove directory '%s'"), + vol->target.path); return -1; } } break; - case VIR_STORAGE_VOL_DIR: - if (rmdir(vol->target.path) < 0) { - virReportSystemError(errno, - _("cannot remove directory '%s'"), - vol->target.path); - return -1; - } - break; case VIR_STORAGE_VOL_BLOCK: case VIR_STORAGE_VOL_NETWORK: case VIR_STORAGE_VOL_NETDIR: diff --git a/src/util/virfile.c b/src/util/virfile.c index fe9f8d4..668daf8 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2315,8 +2315,8 @@ virFileOpenAs(const char *path, int openflags, mode_t mode, } -/* virFileUnlink: - * @path: file to unlink +/* virFileRemove: + * @path: file to unlink or directory to remove * @uid: uid that was used to create the file (not required) * @gid: gid that was used to create the file (not required) * @@ -2327,7 +2327,7 @@ virFileOpenAs(const char *path, int openflags, mode_t mode, * from the child. */ int -virFileUnlink(const char *path, +virFileRemove(const char *path, uid_t uid, gid_t gid) { @@ -2341,8 +2341,12 @@ virFileUnlink(const char *path, * the file/volume, then use unlink directly */ if ((geteuid() != 0) || - ((uid == (uid_t) -1) && (gid == (gid_t) -1))) - return unlink(path); + ((uid == (uid_t) -1) && (gid == (gid_t) -1))) { + if (virFileIsDir(path)) + return rmdir(path); + else + return unlink(path); + } /* Otherwise, we have to deal with the NFS root-squash craziness * to run under the uid/gid that created the volume in order to @@ -2406,9 +2410,16 @@ virFileUnlink(const char *path, goto childerror; } - if (unlink(path) < 0) { - ret = errno; - goto childerror; + if (virFileIsDir(path)) { + if (rmdir(path) < 0) { + ret = errno; + goto childerror; + } + } else { + if (unlink(path) < 0) { + ret = errno; + goto childerror; + } } childerror: @@ -2643,7 +2654,7 @@ virDirCreate(const char *path ATTRIBUTE_UNUSED, } int -virFileUnlink(const char *path, +virFileRemove(const char *path, uid_t uid ATTRIBUTE_UNUSED, gid_t gid ATTRIBUTE_UNUSED) { diff --git a/src/util/virfile.h b/src/util/virfile.h index 797ca65..312f226 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -219,7 +219,7 @@ int virFileOpenAs(const char *path, int openflags, mode_t mode, uid_t uid, gid_t gid, unsigned int flags) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; -int virFileUnlink(const char *path, uid_t uid, gid_t gid); +int virFileRemove(const char *path, uid_t uid, gid_t gid); enum { VIR_DIR_CREATE_NONE = 0, -- 2.1.0 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list