On 18.09.2015 20:20, John Ferlan wrote:
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).
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_fs.c | 20 +++++++++----------- src/util/virfile.c | 23 +++++++++++++++++------ 2 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c index fe9f8d4..7c285d9 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2316,7 +2316,7 @@ virFileOpenAs(const char *path, int openflags, mode_t mode,
/* virFileUnlink: - * @path: file to unlink + * @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) * @@ -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:
Since this function is now able to work with dirs too, maybe it should be renamed to something like virFileDirRemove? But that can be saved for a follow up patch. Not a show stopper to me. Michal