On 9/13/19 8:50 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Whether a directory exists or is not readable shouldn't make a big diffence.
This removes errors when firmare or vhost-user config is looked up from a user session libvirt if /etc/libvirt is not readable.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/util/virfile.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
In some cases this makes sense, in others it isn't clear. For example: [:~] $ chmod 000 .config/libvirt/storage/ [:~] $ libvirtd ... 2019-09-20 19:45:13.691+0000: 327223: error : virDirOpenInternal:2846 : cannot open directory '/home/crobinso/.config/libvirt/storage': Permission denied 2019-09-20 19:45:13.691+0000: 327223: error : virStateInitialize:656 : Initialization of storage state driver failed: cannot open directory '/home/crobinso/.config/libvirt/storage': Permission denied 2019-09-20 19:45:13.691+0000: 327223: error : daemonRunStateInit:790 : Driver state initialization failed After the patches, this case doesn't explicitly fail, but the driver won't report any existing storage pools so it causes silent issues. I think erroring incase permissions are wrong is better, because libvirt doesn't have any code to attempt to fix that situation, unlike for missing directory Not sure if it's realistic for this case to happen but I noticed it through code inspection. Since this only applies to your particular case in the code in one area, you can do (untested) if ((rc = virDirOpenIfExists(&dirp, dir)) < 0) { /* descriptive comment */ if (virLastErrorIsSystemErrno(EACCES)) { virResetLastError(); return 0; } return -1; }
diff --git a/src/util/virfile.c b/src/util/virfile.c index bb844c64e5..4d1fe50efc 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2855,7 +2855,8 @@ virFileRemove(const char *path, #endif /* WIN32 */
static int -virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT, bool quiet) +virDirOpenInternal(DIR **dirp, const char *name, + bool ignoreENOENT, bool ignoreEACCESS, bool quiet) { *dirp = opendir(name); /* exempt from syntax-check */ if (!*dirp) { @@ -2864,6 +2865,8 @@ virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT, bool quiet)
if (ignoreENOENT && errno == ENOENT) return 0; + if (ignoreEACCESS && errno == EACCES) + return 0; virReportSystemError(errno, _("cannot open directory '%s'"), name); return -1; } @@ -2881,7 +2884,7 @@ virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT, bool quiet) int virDirOpen(DIR **dirp, const char *name) { - return virDirOpenInternal(dirp, name, false, false); + return virDirOpenInternal(dirp, name, false, false, false); }
/** @@ -2890,13 +2893,13 @@ virDirOpen(DIR **dirp, const char *name) * @name: path of the directory * * Returns 1 on success. - * If opendir returns ENOENT, 0 is returned without reporting an error. + * If opendir returns ENOENT or EACCES, 0 is returned without reporting an error. * On other errors, -1 is returned and an error is reported. */ int virDirOpenIfExists(DIR **dirp, const char *name) { - return virDirOpenInternal(dirp, name, true, false); + return virDirOpenInternal(dirp, name, true, true, false); }
/** @@ -2912,7 +2915,7 @@ virDirOpenIfExists(DIR **dirp, const char *name) int virDirOpenQuiet(DIR **dirp, const char *name) { - return virDirOpenInternal(dirp, name, false, true); + return virDirOpenInternal(dirp, name, false, false, true); }
/**
- Cole