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@xxxxxxxxxxxx> >>> >>> 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list