driver: tighten check for whether loadable module exists or not

Currently we do a access(R_OK) check to see whether a loadable module
exists, treating failure as non-fatal. This is unreasonably loose, as a
module which exists but has had incorrect permissions set will turn into
a silent skip. We only want to skip loading if the module genuinely does
not exist on disk, due to the optional package not being installed.

Furthermore, checking the return value of virDriverLoadModuleFile() is
not a suitable witness that the module does not exist. This method can
return NULL if dlopen() fails, for example due to being unable to
resolve symbols in the library. This is should always be reported as an
error because it is a sign of the bad installation where either the
module build doesn't match the libvirtd build, or where some 3rd party
libraries are missing or broken.

Both these problems can be fixed by using virFileExists in the caller
instead.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/driver.c b/src/driver.c
index ddda1e71f752fcceb887d6fe59052965076606d9..e02efe2615b3c045fb7c949b2da15b08fb6e5c2d 100644 (file)
--- a/src/driver.c
+++ b/src/driver.c
@@ -53,11 +53,6 @@ virDriverLoadModuleFile(const char *file)
 
     VIR_DEBUG("Load module file '%s'", file);
 
-    if (access(file, R_OK) < 0) {
-        VIR_INFO("Module %s not accessible", file);
-        return NULL;
-    }
-
     virUpdateSelfLastChanged(file);
 
     if (!(handle = dlopen(file, flags)))
@@ -105,11 +100,14 @@ virDriverLoadModuleFull(const char *path,
     int (*regsym)(void);
     int ret = -1;
 
-    if (!(rethandle = virDriverLoadModuleFile(path))) {
-        ret = 1;
-        goto cleanup;
+    if (!virFileExists(path)) {
+        VIR_INFO("Module '%s' does not exists", path);
+        return 1;
     }
 
+    if (!(rethandle = virDriverLoadModuleFile(path)))
+        goto cleanup;
+
     if (!(regsym = virDriverLoadModuleFunc(rethandle, regfunc)))
         goto cleanup;