[libvirt PATCH] util: fix accessibility check for hook directory

Ján Tomko posted 1 patch 3 years, 8 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/a94ea6b47ecf71bd8af1784350867c404187bbc1.1594124053.git.jtomko@redhat.com
src/util/virhook.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
[libvirt PATCH] util: fix accessibility check for hook directory
Posted by Ján Tomko 3 years, 8 months ago
virFileIsAccessible does not return true on accessible
directories. Check whether it set EISDIR and only
then assume the directory is inaccessible.

Return 0 (not found) instead of 1 (found),
since the bridge driver taints the network based on
this return value, not whether the hook actually ran.

Remove the bogus check from virHookCall, since it already
checks the virHooksFound bitmap that was filled before
by virHookCheck.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 7fa7f7eeb6e969e002845928e155914da2fc8cd0
Closes: https://gitlab.com/libvirt/libvirt/-/issues/47
---
 src/util/virhook.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/util/virhook.c b/src/util/virhook.c
index d1ac518c24..119ad1aae6 100644
--- a/src/util/virhook.c
+++ b/src/util/virhook.c
@@ -172,9 +172,9 @@ virHookCheck(int no, const char *driver)
 
     dir_path = g_strdup_printf("%s.d", path);
 
-    if (!virFileIsExecutable(dir_path)) {
+    if (!virFileIsExecutable(dir_path) && errno != EISDIR) {
         VIR_DEBUG("Hook dir %s is not accessible", dir_path);
-        return 1;
+        return 0;
     }
 
     if ((ret = virDirOpenIfExists(&dir, dir_path)) < 0)
@@ -422,9 +422,6 @@ virHookCall(int driver,
 
     dir_path = g_strdup_printf("%s.d", path);
 
-    if (!virFileIsExecutable(dir_path))
-        return script_ret;
-
     if ((ret = virDirOpenIfExists(&dir, dir_path)) < 0)
         return -1;
 
-- 
2.25.4

Re: [libvirt PATCH] util: fix accessibility check for hook directory
Posted by Daniel Henrique Barboza 3 years, 8 months ago

On 7/7/20 9:14 AM, Ján Tomko wrote:
> virFileIsAccessible does not return true on accessible
> directories. Check whether it set EISDIR and only
> then assume the directory is inaccessible.
> 
> Return 0 (not found) instead of 1 (found),
> since the bridge driver taints the network based on
> this return value, not whether the hook actually ran.
> 
> Remove the bogus check from virHookCall, since it already
> checks the virHooksFound bitmap that was filled before
> by virHookCheck.
> 
> Signed-off-by: Ján Tomko <jtomko@redhat.com>
> Fixes: 7fa7f7eeb6e969e002845928e155914da2fc8cd0
> Closes: https://gitlab.com/libvirt/libvirt/-/issues/47
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   src/util/virhook.c | 7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/src/util/virhook.c b/src/util/virhook.c
> index d1ac518c24..119ad1aae6 100644
> --- a/src/util/virhook.c
> +++ b/src/util/virhook.c
> @@ -172,9 +172,9 @@ virHookCheck(int no, const char *driver)
>   
>       dir_path = g_strdup_printf("%s.d", path);
>   
> -    if (!virFileIsExecutable(dir_path)) {
> +    if (!virFileIsExecutable(dir_path) && errno != EISDIR) {
>           VIR_DEBUG("Hook dir %s is not accessible", dir_path);
> -        return 1;
> +        return 0;
>       }
>   
>       if ((ret = virDirOpenIfExists(&dir, dir_path)) < 0)
> @@ -422,9 +422,6 @@ virHookCall(int driver,
>   
>       dir_path = g_strdup_printf("%s.d", path);
>   
> -    if (!virFileIsExecutable(dir_path))
> -        return script_ret;
> -
>       if ((ret = virDirOpenIfExists(&dir, dir_path)) < 0)
>           return -1;
>   
>