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

[Julio Faracco]

Hi Daniel,

2016-07-12 4:59 GMT-03:00 Daniel P. Berrange <berrange at redhat.com>:
> 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.

Thanks for your suggestions.
Yes, we reproduce it here to confirm what we already known.
If you create a hook manually, with libvirtd running the hook won't be applied.

Unless, if you restart the daemon as we are doing when the hook is removed.

> 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.

Yes, we can thing here in a good way to avoid that.
Lets try and resend some suggestion.
In any case, we have the solution when the hook is removed

Thanks.

> 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 :|