[libvirt] [PATCH v3] storage: Report error from VolOpen by default
Cole Robinson
crobinso at redhat.com
Wed Apr 2 15:54:50 UTC 2014
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
More information about the libvir-list
mailing list