On 03/31/2014 12:50 PM, Cole Robinson wrote:
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()
*/
Is this comment still accurate?
char *base = last_component(path);
+ bool raise_error = (flags & VIR_STORAGE_VOL_OPEN_ERROR);
Hmm - are the semantics of this flag backwards? If most users always
want an error, and only one specific caller wants the error suppressed
because it is prepared to deal with a -2 return, shouldn't the default
be errors when flags==0, and the one caller pass a non-zero flag for
VIR_STORAGE_VOL_OPEN_NOERROR?
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;
This is an unconditional error, even when the flags say the user doesn't
want errors; do we need to tweak the docs to mention that we are only
suppressing SOME errors, not all?
@@ -82,8 +82,11 @@
virStorageBackendProbeTarget(virStorageVolTargetPtr target,
*encryption = NULL;
if ((ret = virStorageBackendVolOpen(target->path, &sb,
- VIR_STORAGE_VOL_FS_REFRESH_FLAGS)) < 0)
- goto error; /* Take care to propagate ret, it is not always -1 */
+ VIR_STORAGE_VOL_FS_REFRESH_FLAGS)) < 0) {
+ /* ret == -2 is non-fatal, propage the return code so the caller
s/propage/propagate/
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org