[PATCH v2] storage: Check for invalid storage mode before opening

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



If a directory pool contains pipes or sockets, a pool start can fail or hang:

https://bugzilla.redhat.com/show_bug.cgi?id=589577

We already try to avoid these special files, but only attempt after
opening the path, which is where the problems lie. Unify volume opening
into a single function which runs stat() before any open() call. Directory
pools can then proceed along, ignoring the invalid files.

v2: Use ATTRIBUTE_NONNULL. Drop stat check, just open with
    O_NONBLOCK|O_NOCTTY.

Signed-off-by: Cole Robinson <crobinso@xxxxxxxxxx>
---
 src/storage/storage_backend.c         |   23 +++++++++++++++++------
 src/storage/storage_backend.h         |    2 ++
 src/storage/storage_backend_fs.c      |    7 ++-----
 src/storage/storage_backend_logical.c |    9 +++------
 src/storage/storage_backend_mpath.c   |    9 +++------
 src/storage/storage_backend_scsi.c    |   17 ++++++++---------
 6 files changed, 35 insertions(+), 32 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index f4124df..bbd3375 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -872,6 +872,20 @@ virStorageBackendForType(int type) {
     return NULL;
 }
 
+int
+virStorageBackendVolOpen(const char *path)
+{
+    int fd;
+
+    if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY)) < 0) {
+        virReportSystemError(errno,
+                             _("cannot open volume '%s'"),
+                             path);
+        return -1;
+    }
+
+    return fd;
+}
 
 int
 virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target,
@@ -880,13 +894,10 @@ virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target,
 {
     int ret, fd;
 
-    if ((fd = open(target->path, O_RDONLY)) < 0) {
-        virReportSystemError(errno,
-                             _("cannot open volume '%s'"),
-                             target->path);
-        return -1;
-    }
+    if ((ret = virStorageBackendVolOpen(target->path)) < 0)
+        return ret;
 
+    fd = ret;
     ret = virStorageBackendUpdateVolTargetInfoFD(target,
                                                  fd,
                                                  allocation,
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 907c4bc..45683ec 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -80,6 +80,8 @@ struct _virStorageBackend {
 
 virStorageBackendPtr virStorageBackendForType(int type);
 
+int virStorageBackendVolOpen(const char *path)
+ATTRIBUTE_NONNULL(1);
 
 int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
                                    int withCapacity);
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index c96c4f3..1650177 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -61,12 +61,9 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
     if (encryption)
         *encryption = NULL;
 
-    if ((fd = open(target->path, O_RDONLY)) < 0) {
-        virReportSystemError(errno,
-                             _("cannot open volume '%s'"),
-                             target->path);
+    if ((ret = virStorageBackendVolOpen(target->path)) < 0)
         return -1;
-    }
+    fd = ret;
 
     if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd,
                                                       allocation,
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
index 06238d1..616ca1a 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -559,7 +559,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
                                   virStoragePoolObjPtr pool,
                                   virStorageVolDefPtr vol)
 {
-    int fd = -1;
+    int fdret, fd = -1;
     char size[100];
     const char *cmdargvnew[] = {
         LVCREATE, "--name", vol->name, "-L", size,
@@ -602,12 +602,9 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
     if (virRun(cmdargv, NULL) < 0)
         return -1;
 
-    if ((fd = open(vol->target.path, O_RDONLY)) < 0) {
-        virReportSystemError(errno,
-                             _("cannot read path '%s'"),
-                             vol->target.path);
+    if ((fdret = virStorageBackendVolOpen(vol->target.path)) < 0)
         goto cleanup;
-    }
+    fd = fdret;
 
     /* We can only chown/grp if root */
     if (getuid() == 0) {
diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c
index 8d0a92a..a62dc3d 100644
--- a/src/storage/storage_backend_mpath.c
+++ b/src/storage/storage_backend_mpath.c
@@ -44,14 +44,11 @@ virStorageBackendMpathUpdateVolTargetInfo(virStorageVolTargetPtr target,
                                           unsigned long long *capacity)
 {
     int ret = -1;
-    int fd = -1;
+    int fdret, fd = -1;
 
-    if ((fd = open(target->path, O_RDONLY)) < 0) {
-        virReportSystemError(errno,
-                             _("cannot open volume '%s'"),
-                             target->path);
+    if ((fdret = virStorageBackendVolOpen(target->path)) < 0)
         goto out;
-    }
+    fd = fdret;
 
     if (virStorageBackendUpdateVolTargetInfoFD(target,
                                                fd,
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index 40f4fd8..71492cf 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -135,14 +135,12 @@ virStorageBackendSCSIUpdateVolTargetInfo(virStorageVolTargetPtr target,
                                          unsigned long long *allocation,
                                          unsigned long long *capacity)
 {
-    int fd, ret = -1;
+    int fdret, fd = -1;
+    int ret = -1;
 
-    if ((fd = open(target->path, O_RDONLY)) < 0) {
-        virReportSystemError(errno,
-                             _("cannot open volume '%s'"),
-                             target->path);
-        return -1;
-    }
+    if ((fdret = virStorageBackendVolOpen(target->path)) < 0)
+        goto cleanup;
+    fd = fdret;
 
     if (virStorageBackendUpdateVolTargetInfoFD(target,
                                                fd,
@@ -155,8 +153,9 @@ virStorageBackendSCSIUpdateVolTargetInfo(virStorageVolTargetPtr target,
 
     ret = 0;
 
-  cleanup:
-    close(fd);
+cleanup:
+    if (fd >= 0)
+        close(fd);
 
     return ret;
 }
-- 
1.6.6.1

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]