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

[Michal Privoznik]

On 6/23/20 4:45 PM, Dmitry Nesterenko wrote:
> Signed-off-by: Dmitry Nesterenko <dmitry.nesterenko at 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