[libvirt PATCH] qemu: use stop reason as 'extra' field
This adds the reason the domain stopped to the 'extra' field, which will be sent to the 'qemu' hook script (if defined). Signed-off-by: Mike Pontillo <mpontillo@digitalocean.com> --- Our hook processor often needs to know the reason why a guest stopped. I am propsing this change in case other hook processing implementations could also benefit from this information. --- src/qemu/qemu_process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index be418ad8e6..425b73c37e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8432,7 +8432,7 @@ void qemuProcessStop(virQEMUDriver *driver, /* we can't stop the operation even if the script raised an error */ ignore_value(virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name, VIR_HOOK_QEMU_OP_STOPPED, VIR_HOOK_SUBOP_END, - NULL, xml, NULL)); + virDomainShutoffReasonTypeToString(reason), xml, NULL)); } /* Reset Security Labels unless caller don't want us to */ -- 2.34.1
On Sat, Mar 11, 2023 at 01:08:33AM +0000, Mike Pontillo wrote:
This adds the reason the domain stopped to the 'extra' field, which will be sent to the 'qemu' hook script (if defined).
Signed-off-by: Mike Pontillo <mpontillo@digitalocean.com>
---
Our hook processor often needs to know the reason why a guest stopped. I am propsing this change in case other hook processing implementations could also benefit from this information.
I think this might be beneficial for other as well. Just few things I noticed: 1) This should be captured in documentation as well so that libvirt.org/hooks.html mentions this 2) Maybe we could come up with some simple key=value format for the extra parameter so that it is extensible, although not many extending was done in these parts
--- src/qemu/qemu_process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index be418ad8e6..425b73c37e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8432,7 +8432,7 @@ void qemuProcessStop(virQEMUDriver *driver, /* we can't stop the operation even if the script raised an error */ ignore_value(virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name, VIR_HOOK_QEMU_OP_STOPPED, VIR_HOOK_SUBOP_END, - NULL, xml, NULL)); + virDomainShutoffReasonTypeToString(reason), xml, NULL)); }
/* Reset Security Labels unless caller don't want us to */ -- 2.34.1
On 3/13/23 14:19, Martin Kletzander wrote:
I think this might be beneficial for other as well. Just few things I noticed:
1) This should be captured in documentation as well so that libvirt.org/hooks.html mentions this
Good catch; I can look at updating the documentation as well.
2) Maybe we could come up with some simple key=value format for the extra parameter so that it is extensible, although not many extending was done in these parts
I think if an extensible format is desired, a JSON dictionary might be ideal. Then, hook processor authors can use standard tooling to parse and interpret the contents of 'extra'. Because even if we make it extensible using a simple key=value format, it starts become problematic even with a case like the enum value "from snapshot" for virDomainShutoffReason. After looking at existing usages of the 'extra' field in virHookCall(), I think only VIR_HOOK_DRIVER_DAEMON uses the 'extra' field. In that case, it is a single string: either "SIGHUP", "start", or "shutdown". (Although in this case, the 'extra' information may be redundant; it looks like the contents of 'extra' are already implied by the other arguments.) I'm not sure how much flexibility there is to change what 'extra' contains, since users' hook processors may be impacted. Maybe, in the future, an additional 'json' argument could be added after 'extra' if additional extension is needed. Regards, Mike
participants (2)
-
Martin Kletzander -
Mike Pontillo