[libvirt] [PATCH] storage: silently ignore missing files on pool refresh
From: Wei Zhou <w.zhou@leaseweb.com> Make virStorageBackendVolOpenCheckMode return -2 instead of -1 if volume file is missing. https://bugzilla.redhat.com/show_bug.cgi?id=977706 --- src/storage/storage_backend.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index e2527c9..f063601 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1084,7 +1084,7 @@ virStorageBackendForType(int type) * 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. + * volume is a dangling symbolic link or file is missing. */ int virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) @@ -1097,7 +1097,7 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) virReportSystemError(errno, _("cannot stat file '%s'"), path); - return -1; + return -2; } if (S_ISFIFO(sb.st_mode)) { -- 1.8.1.5
On 10.07.2013 16:57, Ján Tomko wrote:
From: Wei Zhou <w.zhou@leaseweb.com>
Make virStorageBackendVolOpenCheckMode return -2 instead of -1 if volume file is missing.
https://bugzilla.redhat.com/show_bug.cgi?id=977706 --- src/storage/storage_backend.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index e2527c9..f063601 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1084,7 +1084,7 @@ virStorageBackendForType(int type) * 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. + * volume is a dangling symbolic link or file is missing. */ int virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) @@ -1097,7 +1097,7 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) virReportSystemError(errno, _("cannot stat file '%s'"), path);
If we want to *silently* ignore missing file, why do we virReportSystemError() here?
- return -1; + return -2; }
if (S_ISFIFO(sb.st_mode)) {
Michal
On Wed, Jul 10, 2013 at 05:00:59PM +0200, Michal Privoznik wrote:
On 10.07.2013 16:57, Ján Tomko wrote:
From: Wei Zhou <w.zhou@leaseweb.com>
Make virStorageBackendVolOpenCheckMode return -2 instead of -1 if volume file is missing.
https://bugzilla.redhat.com/show_bug.cgi?id=977706 --- src/storage/storage_backend.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index e2527c9..f063601 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1084,7 +1084,7 @@ virStorageBackendForType(int type) * 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. + * volume is a dangling symbolic link or file is missing. */ int virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) @@ -1097,7 +1097,7 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) virReportSystemError(errno, _("cannot stat file '%s'"), path);
If we want to *silently* ignore missing file, why do we virReportSystemError() here?
- return -1; + return -2; }
Well returning -1 vs -2 from this function isn't ignoring the error. It is just providing the caller a way to detect a specific error scenario. Thus if the caller decides to ignore the error when ret == -2, then the caller should call virResetLastError() to clear it. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 07/10/2013 06:07 PM, Daniel P. Berrange wrote:
On Wed, Jul 10, 2013 at 05:00:59PM +0200, Michal Privoznik wrote:
On 10.07.2013 16:57, Ján Tomko wrote:
From: Wei Zhou <w.zhou@leaseweb.com>
Make virStorageBackendVolOpenCheckMode return -2 instead of -1 if volume file is missing.
https://bugzilla.redhat.com/show_bug.cgi?id=977706 --- src/storage/storage_backend.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index e2527c9..f063601 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1084,7 +1084,7 @@ virStorageBackendForType(int type) * 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. + * volume is a dangling symbolic link or file is missing. */ int virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) @@ -1097,7 +1097,7 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) virReportSystemError(errno, _("cannot stat file '%s'"), path);
If we want to *silently* ignore missing file, why do we virReportSystemError() here?
- return -1; + return -2; }
Well returning -1 vs -2 from this function isn't ignoring the error. It is just providing the caller a way to detect a specific error scenario. Thus if the caller decides to ignore the error when ret == -2, then the caller should call virResetLastError() to clear it.
Does this imply a NACK to v2 [1], where I changed it to VIR_WARN to match the other cases of returning -2? Resetting the error won't unlog it. On the other hand, there are paths (like in virStorageBackendFileSystemVolRefresh) where the return value of '-2' would get propagated without setting an error (which is pre-existing for the other cases, but could be a problem in this case). Jan [1] https://www.redhat.com/archives/libvir-list/2013-July/msg00639.html
participants (3)
-
Daniel P. Berrange -
Ján Tomko -
Michal Privoznik