Extract the per-directory hook checking logic into a new virHookCheckDir() helper function, separating it from the input validation done in virHookCheck(). This prepares for supporting multiple hook directories by making virHookCheckDir() reusable per directory. No functional change. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/util/virhook.c | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/src/util/virhook.c b/src/util/virhook.c index d012bb182576..9cbe55faa03e 100644 --- a/src/util/virhook.c +++ b/src/util/virhook.c @@ -129,17 +129,17 @@ VIR_ENUM_IMPL(virHookBhyveOp, static int virHooksFound = -1; /** - * virHookCheck: + * virHookCheckDir: + * @hook_dir: the directory to check for hooks * @driver: the driver name "daemon", "qemu", "lxc"... * - * Check is there is an installed hook for the given driver, if this - * is the case register it. Then subsequent calls to virHookCall - * will call the hook if found. + * Check if there is an installed hook for the given driver in the + * specified directory. * * Returns 1 if found, 0 if not found, and -1 in case of error */ static int -virHookCheck(int no, const char *driver) +virHookCheckDir(const char *hook_dir, const char *driver) { int ret; g_autoptr(DIR) dir = NULL; @@ -147,13 +147,7 @@ virHookCheck(int no, const char *driver) g_autofree char *path = NULL; g_autofree char *dir_path = NULL; - if (driver == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid hook name for #%1$d"), no); - return -1; - } - - path = g_build_filename(LIBVIRT_HOOK_DIR, driver, NULL); + path = g_build_filename(hook_dir, driver, NULL); if (!virFileExists(path)) { VIR_DEBUG("No hook script %s", path); @@ -196,6 +190,27 @@ virHookCheck(int no, const char *driver) return ret; } +/** + * virHookCheck: + * @no: the driver number + * @driver: the driver name "daemon", "qemu", "lxc"... + * + * Check if there is an installed hook for the given driver. + * + * Returns 1 if found, 0 if not found, and -1 in case of error + */ +static int +virHookCheck(int no, const char *driver) +{ + if (driver == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid hook name for #%1$d"), no); + return -1; + } + + return virHookCheckDir(LIBVIRT_HOOK_DIR, driver); +} + /* * virHookInitialize: * -- 2.56.0.rc0.29.g47ce80527c56

For non-root (session) daemons, hooks were previously only searched in /etc/libvirt/hooks. Add support for also searching in the user directory (~/.config/libvirt/hooks), with the system directory checked first. Introduce virHookGetDirs() which returns an ordered list of hook directories to search. Update virHookCheck() and virHookCall() to iterate over all directories, chaining hook output across them. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/util/virhook.c | 169 ++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 115 insertions(+), 54 deletions(-) diff --git a/src/util/virhook.c b/src/util/virhook.c index bd206f8cf1c2..f62ac902bb40 100644 --- a/src/util/virhook.c +++ b/src/util/virhook.c @@ -33,6 +33,7 @@ #include "vircommand.h" #include "virstring.h" #include "virglibutil.h" +#include "virutil.h" #define VIR_FROM_THIS VIR_FROM_HOOK @@ -40,6 +41,34 @@ VIR_LOG_INIT("util.hook"); #define LIBVIRT_HOOK_DIR SYSCONFDIR "/libvirt/hooks" + +/** + * virHookGetDirs: + * + * Returns a NULL-terminated array of hook directories to search, + * in priority order. For root, only /etc/libvirt/hooks is returned. + * For non-root users, both /etc/libvirt/hooks and + * ~/.config/libvirt/hooks are returned (system first, then user). + * + * The caller must free the result with g_strfreev(). + */ +static char ** +virHookGetDirs(void) +{ + g_autofree char *configdir = NULL; + GPtrArray *dirs = g_ptr_array_new(); + + g_ptr_array_add(dirs, g_strdup(LIBVIRT_HOOK_DIR)); + + if (geteuid() != 0) { + configdir = virGetUserConfigDirectory(); + g_ptr_array_add(dirs, g_build_filename(configdir, "hooks", NULL)); + } + + g_ptr_array_add(dirs, NULL); + return (char **) g_ptr_array_free(dirs, FALSE); +} + VIR_ENUM_DECL(virHookDriver); VIR_ENUM_DECL(virHookDaemonOp); VIR_ENUM_DECL(virHookSubop); @@ -205,13 +234,22 @@ virHookCheckDir(const char *hook_dir, const char *driver) static int virHookCheck(int no, const char *driver) { + g_auto(GStrv) hook_dirs = virHookGetDirs(); + size_t i; + if (driver == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid hook name for #%1$d"), no); return -1; } - return virHookCheckDir(LIBVIRT_HOOK_DIR, driver); + for (i = 0; hook_dirs[i]; i++) { + int ret = virHookCheckDir(hook_dirs[i], driver); + if (ret != 0) + return ret; + } + + return 0; } /* @@ -332,11 +370,12 @@ virRunScript(const char *path, * allocated after successful virHookCall, and is best-effort allocated after * failed virHookCall; the caller is responsible for freeing *output. * - * The script from LIBVIRT_HOOK_DIR is executed the first, followed by scripts - * found under "$driver.d/" directory (sorted alphabetically. If output from - * the hook script is expected, then the output produced by LIBVIRT_HOOK_DIR - * script is fed as input to the first script from the "$driver.d/" directory - * and its output is fed as input to the second and so on. + * For each hook directory (system /etc/libvirt/hooks first, then user + * ~/.config/libvirt/hooks for non-root), the main script is executed first, + * followed by scripts found under "$driver.d/" directory (sorted + * alphabetically). If output from the hook script is expected, then the + * output produced by each script is fed as input to the next script and + * so on, chaining across directories. * * Returns: 0 if the execution succeeded, 1 if the script was not found or * invalid parameters, and -1 if script returned an error @@ -351,15 +390,11 @@ virHookCall(int driver, char **output) { int ret, script_ret; - g_autoptr(DIR) dir = NULL; - struct dirent *entry; - g_autofree char *path = NULL; - g_autofree char *dir_path = NULL; - g_autoptr(virGSListString) entries = NULL; + g_auto(GStrv) hook_dirs = NULL; const char *drvstr; const char *opstr; const char *subopstr; - GSList *next; + size_t i; if (output) *output = NULL; @@ -416,60 +451,86 @@ virHookCall(int driver, if (extra == NULL) extra = "-"; - path = g_build_filename(LIBVIRT_HOOK_DIR, drvstr, NULL); + hook_dirs = virHookGetDirs(); script_ret = 1; - if (virFileIsExecutable(path)) { - script_ret = virRunScript(path, id, opstr, subopstr, extra, - input, output); - } + for (i = 0; hook_dirs[i]; i++) { + g_autofree char *path = NULL; + g_autofree char *dir_path = NULL; + g_autoptr(DIR) dir = NULL; + struct dirent *entry; + g_autoptr(virGSListString) entries = NULL; + GSList *next; - dir_path = g_strdup_printf("%s.d", path); + path = g_build_filename(hook_dirs[i], drvstr, NULL); - if ((ret = virDirOpenIfExists(&dir, dir_path)) < 0) - return -1; + if (virFileIsExecutable(path)) { + const char *cur_input; + int hook_ret; - if (!ret) - return script_ret; + if (!script_ret && output && !virStringIsEmpty(*output)) + cur_input = *output; + else + cur_input = input; - 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)) + hook_ret = virRunScript(path, id, opstr, subopstr, extra, + cur_input, output); + if (hook_ret < script_ret) + script_ret = hook_ret; + } + + dir_path = g_strdup_printf("%s.d", path); + + if ((ret = virDirOpenQuiet(&dir, dir_path)) < 0) { + if (errno == ENOENT || errno == EACCES) { + VIR_DEBUG("Hook dir %s is not accessible: %s", + dir_path, g_strerror(errno)); + continue; + } + virReportSystemError(errno, + _("cannot open directory '%1$s'"), dir_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)) + continue; + + entries = g_slist_prepend(entries, g_steal_pointer(&entry_path)); + } + + if (ret < 0) + return -1; + + if (!entries) continue; - entries = g_slist_prepend(entries, g_steal_pointer(&entry_path)); - } + entries = g_slist_sort(entries, (GCompareFunc) strcmp); - if (ret < 0) - return -1; + for (next = entries; next; next = next->next) { + int entry_ret; + const char *entry_input; + g_autofree char *entry_output = NULL; + const char *filename = next->data; - if (!entries) - return script_ret; + /* Get input from previous output */ + entry_input = (!script_ret && output && + !virStringIsEmpty(*output)) ? *output : input; + entry_ret = virRunScript(filename, id, opstr, + subopstr, extra, entry_input, + (output) ? &entry_output : NULL); + if (entry_ret < script_ret) + script_ret = entry_ret; - entries = g_slist_sort(entries, (GCompareFunc) strcmp); - - for (next = entries; next; next = next->next) { - int entry_ret; - const char *entry_input; - g_autofree char *entry_output = NULL; - const char *filename = next->data; - - /* Get input from previous output */ - entry_input = (!script_ret && output && - !virStringIsEmpty(*output)) ? *output : input; - entry_ret = virRunScript(filename, 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); + /* Replace output to new output from item */ + if (!entry_ret && output && !virStringIsEmpty(entry_output)) { + g_free(*output); + *output = g_steal_pointer(&entry_output); + } } } -- 2.56.0.rc0.29.g47ce80527c56

Add a new 'skip_system_hooks' boolean option to the daemon configuration file. When set to true, session (non-root) daemons will only search for hooks in the user directory (~/.config/libvirt/hooks), skipping the system-wide /etc/libvirt/hooks directory. The default is false, preserving backward compatibility by searching both system and user hook directories. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/remote/libvirtd.aug.in | 1 + src/remote/libvirtd.conf.in | 9 +++++++++ src/remote/remote_daemon.c | 1 + src/remote/remote_daemon_config.c | 3 +++ src/remote/remote_daemon_config.h | 2 ++ src/remote/test_libvirtd.aug.in | 1 + 6 files changed, 17 insertions(+) diff --git a/src/remote/libvirtd.aug.in b/src/remote/libvirtd.aug.in index 1f3bb5d0e212..85fb2fe7ca93 100644 --- a/src/remote/libvirtd.aug.in +++ b/src/remote/libvirtd.aug.in @@ -93,6 +93,7 @@ module @DAEMON_NAME_UC@ = let misc_entry = str_entry "host_uuid" | str_entry "host_uuid_source" | int_entry "ovs_timeout" + | bool_entry "skip_system_hooks" (* Each entry in the config is one of the following three ... *) let entry = sock_acl_entry diff --git a/src/remote/libvirtd.conf.in b/src/remote/libvirtd.conf.in index e4460e61ef1a..403adbefb475 100644 --- a/src/remote/libvirtd.conf.in +++ b/src/remote/libvirtd.conf.in @@ -558,3 +558,12 @@ # potential infinite waits blocking libvirt. # #ovs_timeout = 5 + +################################################################### +# Hooks: +# For non-root (session) daemons, hooks are searched in both the +# system directory (@SYSCONFDIR@/libvirt/hooks) and the user directory +# ($XDG_CONFIG_HOME/libvirt/hooks), with system hooks executed first. +# Set skip_system_hooks to 1 to only use user hooks. +# +#skip_system_hooks = 0 diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index ee3d10bc2312..b76e28c5db7c 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -1115,6 +1115,7 @@ int main(int argc, char **argv) { virAuditLog(config->audit_logging > 0); /* setup the hooks if any */ + virHookSetSkipSystemHooks(config->skip_system_hooks); if (virHookInitialize() < 0) { ret = VIR_DAEMON_ERR_HOOKS; goto cleanup; diff --git a/src/remote/remote_daemon_config.c b/src/remote/remote_daemon_config.c index bb6078967f96..9a4a6da4219d 100644 --- a/src/remote/remote_daemon_config.c +++ b/src/remote/remote_daemon_config.c @@ -396,6 +396,9 @@ daemonConfigLoadOptions(struct daemonConfig *data, if (virConfGetValueUInt(conf, "ovs_timeout", &data->ovs_timeout) < 0) return -1; + if (virConfGetValueBool(conf, "skip_system_hooks", &data->skip_system_hooks) < 0) + return -1; + return 0; } diff --git a/src/remote/remote_daemon_config.h b/src/remote/remote_daemon_config.h index e699889191cb..39357977b406 100644 --- a/src/remote/remote_daemon_config.h +++ b/src/remote/remote_daemon_config.h @@ -96,6 +96,8 @@ struct daemonConfig { unsigned int admin_keepalive_count; unsigned int ovs_timeout; + + bool skip_system_hooks; }; diff --git a/src/remote/test_libvirtd.aug.in b/src/remote/test_libvirtd.aug.in index a37b0daa55c9..bbd308d2be8a 100644 --- a/src/remote/test_libvirtd.aug.in +++ b/src/remote/test_libvirtd.aug.in @@ -75,3 +75,4 @@ module Test_@DAEMON_NAME@ = { "admin_keepalive_interval" = "5" } { "admin_keepalive_count" = "5" } { "ovs_timeout" = "5" } + { "skip_system_hooks" = "0" } -- 2.56.0.rc0.29.g47ce80527c56