Re: [libvirt] [PATCH v2 4/4] storage: Report error from VolOpen if proper flag is passed

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. Change VolOpen to report an error if the VOL_OPEN_ERROR flag is passed. The ony caller that doesn't use this flag is the one that is explicitly handling -2 return code, so it fits the pattern. Tweak some of the other call sites to propagate the newly raised error. --- src/storage/storage_backend.c | 60 ++++++++++++++++++++++++----------- src/storage/storage_backend_fs.c | 21 ++++++------ src/storage/storage_backend_logical.c | 6 +++- src/storage/storage_backend_scsi.c | 4 +-- 4 files changed, 59 insertions(+), 32 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 42bd445..7c1220e 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1273,10 +1273,11 @@ virStorageBackendDetectBlockVolFormatFD(virStorageVolTargetPtr 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. + * + * -2 can be an ignorable error, but callers have to make sure to + * virResetLastError() */ int virStorageBackendVolOpen(const char *path, struct stat *sb, @@ -1284,22 +1285,30 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, { int fd, mode = 0; char *base = last_component(path); + bool raise_error = (flags & VIR_STORAGE_VOL_OPEN_ERROR);

if (lstat(path, sb) < 0) { - if (errno == ENOENT && !(flags & VIR_STORAGE_VOL_OPEN_ERROR)) { + if (errno == ENOENT && !raise_error) { VIR_WARN("ignoring missing file '%s'", path); return -2; } - virReportSystemError(errno, - _("cannot stat file '%s'"), - path); + + virReportSystemError(errno, _("cannot stat file '%s'"), path); return -1; } if (S_ISFIFO(sb->st_mode)) { + if (raise_error) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Volume path '%s' is a FIFO"), path); + } VIR_WARN("ignoring FIFO '%s'", path);

return -2; } else if (S_ISSOCK(sb->st_mode)) { + if (raise_error) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Volume path '%s' is a socket"), path); + } VIR_WARN("ignoring socket '%s'", path); return -2; }

@@ -1366,13 +1386,11 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, if (!(mode & flags)) { VIR_FORCE_CLOSE(fd); VIR_INFO("Skipping volume '%s'", path); - - if (mode & VIR_STORAGE_VOL_OPEN_ERROR) {

+ if (raise_error) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected storage mode for '%s'"), path); return -1; } - return -2; }

@@ -1389,8 +1407,12 @@ virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target, int ret, fd = -1; struct stat sb; - if ((ret = virStorageBackendVolOpen(target->path, &sb, openflags)) < 0) + if ((ret = virStorageBackendVolOpen(target->path, &sb, openflags)) < 0) { + /* ret == -2 may be a non-fatal error, but if callers want that + * behavior they should call VolOpen manually */ + ret = -1; goto cleanup;

+ } fd = ret; if ((ret = virStorageBackendUpdateVolTargetInfoFD(target,