[PATCH v2 2/3] virhook: support hooks placed in several files

Dmitry Nesterenko posted 3 patches 5 years, 7 months ago
[PATCH v2 2/3] virhook: support hooks placed in several files
Posted by Dmitry Nesterenko 5 years, 7 months ago
Signed-off-by: Dmitry Nesterenko <dmitry.nesterenko@virtuozzo.com>
---
 src/util/virhook.c | 106 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 97 insertions(+), 9 deletions(-)

diff --git a/src/util/virhook.c b/src/util/virhook.c
index 64ee9a2307..cd26c8c9bd 100644
--- a/src/util/virhook.c
+++ b/src/util/virhook.c
@@ -33,6 +33,7 @@
 #include "virfile.h"
 #include "configmake.h"
 #include "vircommand.h"
+#include "virstring.h"
 
 #define VIR_FROM_THIS VIR_FROM_HOOK
 
@@ -141,7 +142,11 @@ static int virHooksFound = -1;
 static int
 virHookCheck(int no, const char *driver)
 {
+    int ret;
+    DIR *dir;
+    struct dirent *entry;
     g_autofree char *path = NULL;
+    g_autofree char *dir_path = NULL;
 
     if (driver == NULL) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -158,16 +163,39 @@ virHookCheck(int no, const char *driver)
 
     if (!virFileExists(path)) {
         VIR_DEBUG("No hook script %s", path);
-        return 0;
+    } else if (!virFileIsExecutable(path)) {
+        VIR_WARN("Non-executable hook script %s", path);
+    } else {
+        VIR_DEBUG("Found hook script %s", path);
+        return 1;
     }
 
-    if (!virFileIsExecutable(path)) {
-        VIR_WARN("Non-executable hook script %s", path);
+    dir_path = g_strdup_printf("%s.d", path);
+    if ((ret = virDirOpenIfExists(&dir, dir_path)) < 0)
+        return -1;
+
+    if (!ret) {
+        VIR_DEBUG("No hook script dir %s", dir_path);
         return 0;
     }
 
-    VIR_DEBUG("Found hook script %s", path);
-    return 1;
+    while ((ret = virDirRead(dir, &entry, dir_path)) > 0) {
+        g_autofree char *entry_path = g_build_filename(dir_path,
+                                                       entry->d_name,
+                                                       NULL);
+        if (!virFileIsExecutable(entry_path)) {
+            VIR_WARN("Non-executable hook script %s", entry_path);
+            continue;
+        }
+
+        VIR_DEBUG("Found hook script %s", entry_path);
+        ret = 1;
+        break;
+    }
+
+    VIR_DIR_CLOSE(dir);
+
+    return ret;
 }
 
 /*
@@ -282,7 +310,7 @@ virRunScript(const char *path,
  * @input: extra input given to the script on stdin
  * @output: optional address of variable to store malloced result buffer
  *
- * Implement a hook call, where the external script for the driver is
+ * Implement a hook call, where the external scripts for the driver is
  * called with the given information. This is a synchronous call, we wait for
  * execution completion. If @output is non-NULL, *output is guaranteed to be
  * allocated after successful virHookCall, and is best-effort allocated after
@@ -300,11 +328,16 @@ virHookCall(int driver,
             const char *input,
             char **output)
 {
+    int ret, script_ret;
+    DIR *dir;
+    struct dirent *entry;
     g_autofree char *path = NULL;
-    g_autoptr(virCommand) cmd = NULL;
+    g_autofree char *dir_path = NULL;
+    VIR_AUTOSTRINGLIST entries = NULL;
     const char *drvstr;
     const char *opstr;
     const char *subopstr;
+    size_t i, nentries;
 
     if (output)
         *output = NULL;
@@ -368,6 +401,61 @@ virHookCall(int driver,
         return -1;
     }
 
-    return virRunScript(path, id, opstr, subopstr, extra,
-                        input, output);
+    script_ret = 1;
+
+    if (virFileIsExecutable(path)) {
+        script_ret = virRunScript(path, id, opstr, subopstr, extra,
+                                  input, output);
+    }
+
+    dir_path = g_strdup_printf("%s.d", path);
+    if ((ret = virDirOpenIfExists(&dir, dir_path)) < 0)
+        return -1;
+
+    if (!ret)
+        return script_ret;
+
+    while ((ret = virDirRead(dir, &entry, dir_path)) > 0) {
+        g_autofree char *entry_path = g_build_filename(dir_path,
+                                                       entry->d_name,
+                                                       NULL);
+        if (!virFileIsExecutable(entry_path))
+            continue;
+
+        virStringListAdd(&entries, entry_path);
+    }
+
+    VIR_DIR_CLOSE(dir);
+
+    if (ret < 0)
+        return -1;
+
+    if (!entries)
+        return script_ret;
+
+    nentries = virStringListLength((const char **)entries);
+    qsort(entries, nentries, sizeof(*entries), virStringSortCompare);
+
+    for (i = 0; i < nentries; i++) {
+        int entry_ret;
+        char *entry_input;
+        g_autofree char *entry_output = NULL;
+
+        /* Get input from previous output */
+        entry_input = (!script_ret && output &&
+                       !virStringIsEmpty(*output)) ? *output : (char *)input;
+        entry_ret = virRunScript(entries[i], id, opstr,
+                                 subopstr, extra, entry_input,
+                                 (output) ? &entry_output : NULL);
+        if (entry_ret < script_ret)
+            script_ret = entry_ret;
+
+        /* Replace output to new output from item */
+        if (!entry_ret && output && !virStringIsEmpty(entry_output)) {
+            g_free(*output);
+            *output = g_steal_pointer(&entry_output);
+        }
+    }
+
+    return script_ret;
 }
-- 
2.18.4

Re: [PATCH v2 2/3] virhook: support hooks placed in several files
Posted by Michal Privoznik 5 years, 7 months ago
On 6/23/20 4:45 PM, Dmitry Nesterenko wrote:
> Signed-off-by: Dmitry Nesterenko <dmitry.nesterenko@virtuozzo.com>
> ---
>   src/util/virhook.c | 106 +++++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 97 insertions(+), 9 deletions(-)
> 
> diff --git a/src/util/virhook.c b/src/util/virhook.c
> index 64ee9a2307..cd26c8c9bd 100644
> --- a/src/util/virhook.c
> +++ b/src/util/virhook.c
> @@ -33,6 +33,7 @@
>   #include "virfile.h"
>   #include "configmake.h"
>   #include "vircommand.h"
> +#include "virstring.h"
>   
>   #define VIR_FROM_THIS VIR_FROM_HOOK
>   
> @@ -141,7 +142,11 @@ static int virHooksFound = -1;
>   static int
>   virHookCheck(int no, const char *driver)
>   {
> +    int ret;
> +    DIR *dir;
> +    struct dirent *entry;
>       g_autofree char *path = NULL;
> +    g_autofree char *dir_path = NULL;
>   
>       if (driver == NULL) {
>           virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -158,16 +163,39 @@ virHookCheck(int no, const char *driver)
>   
>       if (!virFileExists(path)) {
>           VIR_DEBUG("No hook script %s", path);
> -        return 0;
> +    } else if (!virFileIsExecutable(path)) {
> +        VIR_WARN("Non-executable hook script %s", path);
> +    } else {
> +        VIR_DEBUG("Found hook script %s", path);
> +        return 1;
>       }
>   
> -    if (!virFileIsExecutable(path)) {
> -        VIR_WARN("Non-executable hook script %s", path);
> +    dir_path = g_strdup_printf("%s.d", path);
> +    if ((ret = virDirOpenIfExists(&dir, dir_path)) < 0)
> +        return -1;
> +
> +    if (!ret) {
> +        VIR_DEBUG("No hook script dir %s", dir_path);
>           return 0;
>       }
>   
> -    VIR_DEBUG("Found hook script %s", path);
> -    return 1;
> +    while ((ret = virDirRead(dir, &entry, dir_path)) > 0) {
> +        g_autofree char *entry_path = g_build_filename(dir_path,
> +                                                       entry->d_name,
> +                                                       NULL);
> +        if (!virFileIsExecutable(entry_path)) {
> +            VIR_WARN("Non-executable hook script %s", entry_path);
> +            continue;
> +        }
> +
> +        VIR_DEBUG("Found hook script %s", entry_path);
> +        ret = 1;
> +        break;
> +    }
> +
> +    VIR_DIR_CLOSE(dir);
> +
> +    return ret;
>   }
>   
>   /*
> @@ -282,7 +310,7 @@ virRunScript(const char *path,
>    * @input: extra input given to the script on stdin
>    * @output: optional address of variable to store malloced result buffer
>    *
> - * Implement a hook call, where the external script for the driver is
> + * Implement a hook call, where the external scripts for the driver is

s/is/are/

>    * called with the given information. This is a synchronous call, we wait for
>    * execution completion. If @output is non-NULL, *output is guaranteed to be
>    * allocated after successful virHookCall, and is best-effort allocated after

I'm documenting the $driver.d/ directory and the output chaining.

> @@ -300,11 +328,16 @@ virHookCall(int driver,
>               const char *input,
>               char **output)
>   {
> +    int ret, script_ret;
> +    DIR *dir;
> +    struct dirent *entry;
>       g_autofree char *path = NULL;
> -    g_autoptr(virCommand) cmd = NULL;
> +    g_autofree char *dir_path = NULL;
> +    VIR_AUTOSTRINGLIST entries = NULL;
>       const char *drvstr;
>       const char *opstr;
>       const char *subopstr;
> +    size_t i, nentries;
>   
>       if (output)
>           *output = NULL;
> @@ -368,6 +401,61 @@ virHookCall(int driver,
>           return -1;
>       }
>   
> -    return virRunScript(path, id, opstr, subopstr, extra,
> -                        input, output);
> +    script_ret = 1;
> +
> +    if (virFileIsExecutable(path)) {
> +        script_ret = virRunScript(path, id, opstr, subopstr, extra,
> +                                  input, output);
> +    }
> +
> +    dir_path = g_strdup_printf("%s.d", path);
> +    if ((ret = virDirOpenIfExists(&dir, dir_path)) < 0)
> +        return -1;
> +
> +    if (!ret)
> +        return script_ret;
> +
> +    while ((ret = virDirRead(dir, &entry, dir_path)) > 0) {
> +        g_autofree char *entry_path = g_build_filename(dir_path,
> +                                                       entry->d_name,
> +                                                       NULL);
> +        if (!virFileIsExecutable(entry_path))
> +            continue;
> +
> +        virStringListAdd(&entries, entry_path);
> +    }
> +
> +    VIR_DIR_CLOSE(dir);
> +
> +    if (ret < 0)
> +        return -1;
> +
> +    if (!entries)
> +        return script_ret;
> +
> +    nentries = virStringListLength((const char **)entries);
> +    qsort(entries, nentries, sizeof(*entries), virStringSortCompare);
> +
> +    for (i = 0; i < nentries; i++) {
> +        int entry_ret;
> +        char *entry_input;
> +        g_autofree char *entry_output = NULL;
> +
> +        /* Get input from previous output */
> +        entry_input = (!script_ret && output &&
> +                       !virStringIsEmpty(*output)) ? *output : (char *)input;

How about making entry_input const char* instead of typecast?

> +        entry_ret = virRunScript(entries[i], id, opstr,
> +                                 subopstr, extra, entry_input,
> +                                 (output) ? &entry_output : NULL);
> +        if (entry_ret < script_ret)
> +            script_ret = entry_ret;
> +
> +        /* Replace output to new output from item */
> +        if (!entry_ret && output && !virStringIsEmpty(entry_output)) {
> +            g_free(*output);
> +            *output = g_steal_pointer(&entry_output);
> +        }
> +    }
> +
> +    return script_ret;
>   }
> 

Michal