[libvirt] [PATCH RFC] virhook: adding virHookCheck() inside virHookCall().

[Daniel P. Berrange]

On Mon, Jul 11, 2016 at 03:22:44PM -0300, Julio Faracco wrote:
> This commit introduces the virHookCheck() before running the command (hook).
> The virHookCheck() before virCommandRun() will avoid errors with changes
> (removal and other permissions changes) in the hook file, while the libvirt
> daemon is running.
> 
> Now, when you remove the hook file while libvirtd is running you get the
> following error:
> 
> virsh # start WINDOWS_7
> error: Failed to start domain WINDOWS_7
> error: Hook script execution failed: internal error: Child process (LC_ALL=C
> PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
> HOME=/home/julio USER=root LOGNAME=root /etc/libvirt/hooks/qemu WINDOWS_7
> prepare begin -) unexpected exit status 127: libvirt:  error : cannot execute
> binary /etc/libvirt/hooks/qemu: No such file or directory
> 
> Cc: Carlos Castilho <ccast at br.ibm.com>
> Signed-off-by: Julio Faracco <jcfaracco at gmail.com>
> ---
>  src/util/virhook.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/util/virhook.c b/src/util/virhook.c
> index d37d6da..f741b30 100644
> --- a/src/util/virhook.c
> +++ b/src/util/virhook.c
> @@ -294,7 +294,12 @@ virHookCall(int driver,
>      if (output)
>          virCommandSetOutputBuffer(cmd, output);
>  
> -    ret = virCommandRun(cmd, NULL);
> +    ret = virHookCheck(driver, virHookDriverTypeToString(driver));
> +
> +    if (ret > 0) {
> +        ret = virCommandRun(cmd, NULL);
> +    }

This only fixes half of the problem - you're addressing the case where
a  hook disappears while libvirtd is running, but not the case where
one appears. It also doesn't update the cache of which hooks are present
so we end up repeatedly checking for the hook.

I can't help thinking that we should do something better than that. Perhaps
virHookInitialize should open an inotify handle and register it with the
main loop so it can automatically update its cache of which hooks exist.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|