[PATCH 1/4] storage: Use virFileUnlink instead of rmdir

[John Ferlan <jferlan@xxxxxxxxxx>]

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@xxxxxxxxxx>
---
 src/storage/storage_backend_fs.c | 20 +++++++++-----------
 src/util/virfile.c               | 23 +++++++++++++++++------
 2 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index f41a41e..388ac7e 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:
+    case VIR_STORAGE_VOL_DIR:
         if (virFileUnlink(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..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:
-- 
2.1.0

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list