[libvirt] [PATCH RFC] virhook: adding virHookCheck() inside virHookCall().
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@br.ibm.com> Signed-off-by: Julio Faracco <jcfaracco@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); + } + if (ret < 0) { /* Convert INTERNAL_ERROR into known error. */ virReportError(VIR_ERR_HOOK_SCRIPT_FAILED, "%s", -- 1.9.1
Hi guys, I sent this patch because we are having some troubles to introduce/remove a new hook. We are creating a package here (RPM, DEB, etc) that configures a VDI and introduces a hook for customization. When the package is removed, the hook is too. After that, all VMs stop to working saying this message. So, we need to restart libvirt to take the effect and it is annoying. -- Julio Cesar Faracco 2016-07-11 15:22 GMT-03:00 Julio Faracco <jcfaracco@gmail.com>:
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@br.ibm.com> Signed-off-by: Julio Faracco <jcfaracco@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); + } + if (ret < 0) { /* Convert INTERNAL_ERROR into known error. */ virReportError(VIR_ERR_HOOK_SCRIPT_FAILED, "%s", -- 1.9.1
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@br.ibm.com> Signed-off-by: Julio Faracco <jcfaracco@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 :|
Hi Daniel, 2016-07-12 4:59 GMT-03:00 Daniel P. Berrange <berrange@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@br.ibm.com> Signed-off-by: Julio Faracco <jcfaracco@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 :|
participants (2)
-
Daniel P. Berrange -
Julio Faracco