[PATCH v3] storage: Report error from VolOpen by default

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

 



Currently VolOpen notifies the user of a potentially non-fatal failure by
returning -2 and logging a VIR_WARN or VIR_INFO. Unfortunately most
callers treat -2 as fatal but don't actually report any message with
the error APIs.

Rename the VOL_OPEN_ERROR flag to VOL_OPEN_NOERROR. If NOERROR is specified,
we preserve the current behavior of returning -2 (there's only one caller
that wants this).

However in the default case, only return -1, and actually use the error
APIs. Fix up a couple callers as a result.
---
 src/storage/storage_backend.c      | 77 ++++++++++++++++++++++++--------------
 src/storage/storage_backend.h      |  7 ++--
 src/storage/storage_backend_fs.c   | 28 +++++---------
 src/storage/storage_backend_scsi.c |  3 --
 4 files changed, 62 insertions(+), 53 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 8fe3687..f44e323 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1279,8 +1279,9 @@ virStorageBackendDetectBlockVolFormatFD(virStorageSourcePtr target,
 /*
  * Allows caller to silently ignore files with improper mode
  *
- * Returns -1 on error, -2 if file mode is unexpected or the
- * volume is a dangling symbolic link.
+ * Returns -1 on error. If VIR_STORAGE_VOL_OPEN_NOERROR is passed, we
+ * return -2 if file mode is unexpected or the volume is a dangling
+ * symbolic link.
  */
 int
 virStorageBackendVolOpen(const char *path, struct stat *sb,
@@ -1288,9 +1289,10 @@ virStorageBackendVolOpen(const char *path, struct stat *sb,
 {
     int fd, mode = 0;
     char *base = last_component(path);
+    bool noerror = (flags * VIR_STORAGE_VOL_OPEN_NOERROR);
 
     if (lstat(path, sb) < 0) {
-        if (errno == ENOENT && !(flags & VIR_STORAGE_VOL_OPEN_ERROR)) {
+        if (errno == ENOENT && noerror) {
             VIR_WARN("ignoring missing file '%s'", path);
             return -2;
         }
@@ -1301,11 +1303,21 @@ virStorageBackendVolOpen(const char *path, struct stat *sb,
     }
 
     if (S_ISFIFO(sb->st_mode)) {
-        VIR_WARN("ignoring FIFO '%s'", path);
-        return -2;
+        if (noerror) {
+            VIR_WARN("ignoring FIFO '%s'", path);
+            return -2;
+        }
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Volume path '%s' is a FIFO"), path);
+        return -1;
     } else if (S_ISSOCK(sb->st_mode)) {
-        VIR_WARN("ignoring socket '%s'", path);
-        return -2;
+        if (noerror) {
+            VIR_WARN("ignoring socket '%s'", path);
+            return -2;
+        }
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Volume path '%s' is a socket"), path);
+        return -1;
     }
 
     /* O_NONBLOCK should only matter during open() for fifos and
@@ -1316,25 +1328,21 @@ virStorageBackendVolOpen(const char *path, struct stat *sb,
      * race.  */
     if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY)) < 0) {
         if ((errno == ENOENT || errno == ELOOP) &&
-            S_ISLNK(sb->st_mode)) {
+            S_ISLNK(sb->st_mode) && noerror) {
             VIR_WARN("ignoring dangling symlink '%s'", path);
             return -2;
         }
-        if (errno == ENOENT && !(flags & VIR_STORAGE_VOL_OPEN_ERROR)) {
+        if (errno == ENOENT && noerror) {
             VIR_WARN("ignoring missing file '%s'", path);
             return -2;
         }
 
-        virReportSystemError(errno,
-                             _("cannot open volume '%s'"),
-                             path);
+        virReportSystemError(errno, _("cannot open volume '%s'"), path);
         return -1;
     }
 
     if (fstat(fd, sb) < 0) {
-        virReportSystemError(errno,
-                             _("cannot stat file '%s'"),
-                             path);
+        virReportSystemError(errno, _("cannot stat file '%s'"), path);
         VIR_FORCE_CLOSE(fd);
         return -1;
     }
@@ -1351,33 +1359,46 @@ virStorageBackendVolOpen(const char *path, struct stat *sb,
         if (STREQ(base, ".") ||
             STREQ(base, "..")) {
             VIR_FORCE_CLOSE(fd);
-            VIR_INFO("Skipping special dir '%s'", base);
-            return -2;
+            if (noerror) {
+                VIR_INFO("Skipping special dir '%s'", base);
+                return -2;
+            }
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Cannot use volume path '%s'"), path);
+            return -1;
         }
     } else {
-        VIR_WARN("ignoring unexpected type for file '%s'", path);
         VIR_FORCE_CLOSE(fd);
-        return -2;
+        if (noerror) {
+            VIR_WARN("ignoring unexpected type for file '%s'", path);
+            return -2;
+        }
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("unexpected type for file '%s'"), path);
+        return -1;
     }
 
     if (virSetBlocking(fd, true) < 0) {
+        VIR_FORCE_CLOSE(fd);
+        if (noerror) {
+            VIR_WARN("unable to set blocking mode for '%s'", path);
+            return -2;
+        }
         virReportSystemError(errno, _("unable to set blocking mode for '%s'"),
                              path);
-        VIR_FORCE_CLOSE(fd);
-        return -2;
+        return -1;
     }
 
     if (!(mode & flags)) {
         VIR_FORCE_CLOSE(fd);
-        VIR_INFO("Skipping volume '%s'", path);
-
-        if (mode & VIR_STORAGE_VOL_OPEN_ERROR) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("unexpected storage mode for '%s'"), path);
-            return -1;
+        if (noerror) {
+            VIR_INFO("Skipping volume '%s'", path);
+            return -2;
         }
 
-        return -2;
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("unexpected storage mode for '%s'"), path);
+        return -1;
     }
 
     return fd;
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 89511f8..c695ef7 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -120,16 +120,15 @@ virStorageBackendPtr virStorageBackendForType(int type);
 
 /* VolOpenCheckMode flags */
 enum {
-    VIR_STORAGE_VOL_OPEN_ERROR  = 1 << 0, /* warn if unexpected type
-                                           * encountered */
+    VIR_STORAGE_VOL_OPEN_NOERROR  = 1 << 0, /* don't error if unexpected type
+                                             * encountered, just warn */
     VIR_STORAGE_VOL_OPEN_REG    = 1 << 1, /* regular files okay */
     VIR_STORAGE_VOL_OPEN_BLOCK  = 1 << 2, /* block files okay */
     VIR_STORAGE_VOL_OPEN_CHAR   = 1 << 3, /* char files okay */
     VIR_STORAGE_VOL_OPEN_DIR    = 1 << 4, /* directories okay */
 };
 
-# define VIR_STORAGE_VOL_OPEN_DEFAULT (VIR_STORAGE_VOL_OPEN_ERROR    |\
-                                       VIR_STORAGE_VOL_OPEN_REG      |\
+# define VIR_STORAGE_VOL_OPEN_DEFAULT (VIR_STORAGE_VOL_OPEN_REG      |\
                                        VIR_STORAGE_VOL_OPEN_BLOCK)
 
 int virStorageBackendVolOpen(const char *path, struct stat *sb,
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index de6521c..9b958f0 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -56,10 +56,10 @@
 
 VIR_LOG_INIT("storage.storage_backend_fs");
 
-#define VIR_STORAGE_VOL_FS_OPEN_FLAGS       (VIR_STORAGE_VOL_OPEN_DEFAULT   |\
-                                             VIR_STORAGE_VOL_OPEN_DIR)
-#define VIR_STORAGE_VOL_FS_REFRESH_FLAGS    (VIR_STORAGE_VOL_FS_OPEN_FLAGS  &\
-                                             ~VIR_STORAGE_VOL_OPEN_ERROR)
+#define VIR_STORAGE_VOL_FS_OPEN_FLAGS    (VIR_STORAGE_VOL_OPEN_DEFAULT &\
+                                          VIR_STORAGE_VOL_OPEN_DIR)
+#define VIR_STORAGE_VOL_FS_PROBE_FLAGS   (VIR_STORAGE_VOL_FS_OPEN_FLAGS &\
+                                          VIR_STORAGE_VOL_OPEN_NOERROR)
 
 static int ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
 virStorageBackendProbeTarget(virStorageSourcePtr target,
@@ -80,7 +80,7 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,
         *encryption = NULL;
 
     if ((ret = virStorageBackendVolOpen(target->path, &sb,
-                                        VIR_STORAGE_VOL_FS_REFRESH_FLAGS)) < 0)
+                                        VIR_STORAGE_VOL_FS_PROBE_FLAGS)) < 0)
         goto error; /* Take care to propagate ret, it is not always -1 */
     fd = ret;
 
@@ -902,19 +902,11 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED,
             vol->backingStore.path = backingStore;
             vol->backingStore.format = backingStoreFormat;
 
-            if (virStorageBackendUpdateVolTargetInfo(&vol->backingStore,
-                                                     false,
-                                        VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) {
-                /* The backing file is currently unavailable, the capacity,
-                 * allocation, owner, group and mode are unknown. Just log the
-                 * error and continue.
-                 * Unfortunately virStorageBackendProbeTarget() might already
-                 * have logged a similar message for the same problem, but only
-                 * if AUTO format detection was used. */
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("cannot probe backing volume info: %s"),
-                               vol->backingStore.path);
-            }
+            virStorageBackendUpdateVolTargetInfo(&vol->backingStore, false,
+                                                 VIR_STORAGE_VOL_OPEN_DEFAULT);
+            /* If this failed, the backing file is currently unavailable,
+             * the capacity, allocation, owner, group and mode are unknown.
+             * An error message was raised, but we just continue. */
         }
 
 
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index cf546fb..c448d7f 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -201,9 +201,6 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
 
     if (virStorageBackendUpdateVolInfo(vol, true,
                                        VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Failed to update volume for '%s'"),
-                       devpath);
         retval = -1;
         goto free_vol;
     }
-- 
1.9.0

--
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]