Re: [libvirt] [PATCH] [4/6] Add the script hook support to the libvirt daemon
On Fri, Mar 26, 2010 at 11:00:54AM -0600, Eric Blake wrote:
On 03/26/2010 09:44 AM, Daniel Veillard wrote:
>
> + /* setup the hooks */
> + virHookInitialize();
Shouldn't this check for a return of -1?
Somehow, yes, as -1 should only be returned in case of serious errors,
not just missing scripts or directories. So yes and I'm adding a
corresponding virDaemonErr.
> +
> /* Disable error func, now logging is setup */
> virSetErrorFunc(NULL, virshErrorHandler);
>
> + /*
> + * Call the daemon startup hook
> + */
> + virHookCall(VIR_HOOK_DRIVER_DAEMON, "-", VIR_HOOK_DAEMON_OP_START,
> + 0, "start", NULL);
For that matter, virHookCall calls virHookInitialize under the hood, do
we need to repeat that ourselves? And if we don't need to repeat it,
does virHookInitialize still need to be exported?
I prefer to have a separate initialization routine, since we are
caching the state, this routine allow to force a scan of the directory
if needed. So not needed right now, but I prefer to keep it.
Should we check for virHookCall failure?
On daemon exit that would be useless since threads are already
stopped, for reload I don't see that to be cancelleable. There is only
at daemon start that the script could potentially return an error and
prevent the daemon from starting. Weighting the pros and cons of being
able to avoid the daemon versus the risk of breaking the daemon startup
due to some system error, I initially decided to let the daemon start
anyway. It's not a clear cut, I'm adding a comment to keep the issue
open.
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/