[libvirt] [PATCH v2 1/4] virfile: Rename virFileUnlink to virFileRemove

[John Ferlan]

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 at redhat.com>
---
 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