Re: [libvirt] [PATCH v4 02/20] util: ignore EACCESS in virDirOpenIfExists
On 9/13/19 8:50 AM, marcandre.lureau(a)redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau(a)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(a)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