[libvirt] [PATCH] qemu: Report shutdown event details

Martin Kletzander posted 1 patch 6 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/57d4f890881275431f664fe0688ecfd96e2d8210.1492008339.git.mkletzan@redhat.com
There is a newer version of this series
examples/object-events/event-test.c |  9 +++++++++
include/libvirt/libvirt-domain.h    |  3 +++
src/qemu/qemu_monitor.c             |  6 +++---
src/qemu/qemu_monitor.h             |  3 ++-
src/qemu/qemu_monitor_json.c        |  4 ++--
src/qemu/qemu_process.c             | 20 ++++++++++++++++++++
tools/virsh-domain.c                |  6 +++++-
7 files changed, 44 insertions(+), 7 deletions(-)
[libvirt] [PATCH] qemu: Report shutdown event details
Posted by Martin Kletzander 6 years, 11 months ago
QEMU will likely report the details of it shutting down, particularly
the system it received in case it was killed.  We should forward that
information along.  Since the only way we can extend information
provided to the user is adding event details, we might as well emit
multiple events (one with the reason for the shutdown and keep the one
for the shutdown being finished for clarity and compatibility).

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1384007

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
This is waiting for a patch in QEMU [1] that I tested this with.

[1] https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg01098.html


 examples/object-events/event-test.c |  9 +++++++++
 include/libvirt/libvirt-domain.h    |  3 +++
 src/qemu/qemu_monitor.c             |  6 +++---
 src/qemu/qemu_monitor.h             |  3 ++-
 src/qemu/qemu_monitor_json.c        |  4 ++--
 src/qemu/qemu_process.c             | 20 ++++++++++++++++++++
 tools/virsh-domain.c                |  6 +++++-
 7 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/examples/object-events/event-test.c b/examples/object-events/event-test.c
index 12690cac09ce..b4e1dd7970d9 100644
--- a/examples/object-events/event-test.c
+++ b/examples/object-events/event-test.c
@@ -240,6 +240,15 @@ eventDetailToString(int event,
             case VIR_DOMAIN_EVENT_SHUTDOWN_FINISHED:
                 return "Finished";

+            case VIR_DOMAIN_EVENT_SHUTDOWN_SIGINT:
+                return "Killed with SIGINT";
+
+            case VIR_DOMAIN_EVENT_SHUTDOWN_SIGHUP:
+                return "Killed with SIGHUP";
+
+            case VIR_DOMAIN_EVENT_SHUTDOWN_SIGTERM:
+                return "Killed with SIGTERM";
+
             case VIR_DOMAIN_EVENT_SHUTDOWN_LAST:
                 break;
             }
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 501996bc84f1..b3619c4baae7 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -2984,6 +2984,9 @@ typedef enum {
  */
 typedef enum {
     VIR_DOMAIN_EVENT_SHUTDOWN_FINISHED = 0, /* Guest finished shutdown sequence */
+    VIR_DOMAIN_EVENT_SHUTDOWN_SIGINT = 1,   /* Guest is shutting down due to SIGINT */
+    VIR_DOMAIN_EVENT_SHUTDOWN_SIGHUP = 2,   /* Guest is shutting down due to SIGHUP */
+    VIR_DOMAIN_EVENT_SHUTDOWN_SIGTERM = 3,  /* Guest is shutting down due to SIGKILL */

 # ifdef VIR_ENUM_SENTINELS
     VIR_DOMAIN_EVENT_SHUTDOWN_LAST
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 1d40d521a9b3..044b58ff090d 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1323,12 +1323,12 @@ qemuMonitorEmitEvent(qemuMonitorPtr mon, const char *event,


 int
-qemuMonitorEmitShutdown(qemuMonitorPtr mon)
+qemuMonitorEmitShutdown(qemuMonitorPtr mon, const char *signal)
 {
     int ret = -1;
-    VIR_DEBUG("mon=%p", mon);
+    VIR_DEBUG("mon=%p signal=%s", mon, NULLSTR(signal));

-    QEMU_MONITOR_CALLBACK(mon, ret, domainShutdown, mon->vm);
+    QEMU_MONITOR_CALLBACK(mon, ret, domainShutdown, mon->vm, signal);
     return ret;
 }

diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 12f98beba763..c6ecee66a140 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -130,6 +130,7 @@ typedef int (*qemuMonitorDomainEventCallback)(qemuMonitorPtr mon,
                                               void *opaque);
 typedef int (*qemuMonitorDomainShutdownCallback)(qemuMonitorPtr mon,
                                                  virDomainObjPtr vm,
+                                                 const char *signal,
                                                  void *opaque);
 typedef int (*qemuMonitorDomainResetCallback)(qemuMonitorPtr mon,
                                               virDomainObjPtr vm,
@@ -344,7 +345,7 @@ int qemuMonitorGetDiskSecret(qemuMonitorPtr mon,
 int qemuMonitorEmitEvent(qemuMonitorPtr mon, const char *event,
                          long long seconds, unsigned int micros,
                          const char *details);
-int qemuMonitorEmitShutdown(qemuMonitorPtr mon);
+int qemuMonitorEmitShutdown(qemuMonitorPtr mon, const char *signal);
 int qemuMonitorEmitReset(qemuMonitorPtr mon);
 int qemuMonitorEmitPowerdown(qemuMonitorPtr mon);
 int qemuMonitorEmitStop(qemuMonitorPtr mon);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index aeb777d37c12..4f67b4496b43 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -523,9 +523,9 @@ qemuMonitorJSONKeywordStringToJSON(const char *str, const char *firstkeyword)
 }


-static void qemuMonitorJSONHandleShutdown(qemuMonitorPtr mon, virJSONValuePtr data ATTRIBUTE_UNUSED)
+static void qemuMonitorJSONHandleShutdown(qemuMonitorPtr mon, virJSONValuePtr data)
 {
-    qemuMonitorEmitShutdown(mon);
+    qemuMonitorEmitShutdown(mon, virJSONValueObjectGetString(data, "signal"));
 }

 static void qemuMonitorJSONHandleReset(qemuMonitorPtr mon, virJSONValuePtr data ATTRIBUTE_UNUSED)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 5e119a237fba..3265db8f1590 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -634,10 +634,12 @@ qemuProcessHandleEvent(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
 static int
 qemuProcessHandleShutdown(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
                           virDomainObjPtr vm,
+                          const char *signal,
                           void *opaque)
 {
     virQEMUDriverPtr driver = opaque;
     qemuDomainObjPrivatePtr priv;
+    virObjectEventPtr pre_event = NULL;
     virObjectEventPtr event = NULL;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);

@@ -662,6 +664,23 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
     virDomainObjSetState(vm,
                          VIR_DOMAIN_SHUTDOWN,
                          VIR_DOMAIN_SHUTDOWN_UNKNOWN);
+
+    if (signal) {
+        virDomainEventShutdownDetailType detail = 0;
+
+        if (STREQ(signal, "int"))
+            detail = VIR_DOMAIN_EVENT_SHUTDOWN_SIGINT;
+        else if (STREQ(signal, "hup"))
+            detail = VIR_DOMAIN_EVENT_SHUTDOWN_SIGHUP;
+        else if (STREQ(signal, "term"))
+            detail = VIR_DOMAIN_EVENT_SHUTDOWN_SIGTERM;
+
+        if (detail)
+            pre_event = virDomainEventLifecycleNewFromObj(vm,
+                                                          VIR_DOMAIN_EVENT_SHUTDOWN,
+                                                          detail);
+    }
+
     event = virDomainEventLifecycleNewFromObj(vm,
                                      VIR_DOMAIN_EVENT_SHUTDOWN,
                                      VIR_DOMAIN_EVENT_SHUTDOWN_FINISHED);
@@ -678,6 +697,7 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon ATTRIBUTE_UNUSED,

  unlock:
     virObjectUnlock(vm);
+    qemuDomainEventQueue(driver, pre_event);
     qemuDomainEventQueue(driver, event);
     virObjectUnref(cfg);

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index db8accfe436b..12c1908c44f9 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -12220,7 +12220,11 @@ VIR_ENUM_IMPL(virshDomainEventStopped,
 VIR_ENUM_DECL(virshDomainEventShutdown)
 VIR_ENUM_IMPL(virshDomainEventShutdown,
               VIR_DOMAIN_EVENT_SHUTDOWN_LAST,
-              N_("Finished"))
+              N_("Finished"),
+              N_("Killed with SIGINT"),
+              N_("Killed with SIGHUP"),
+              N_("Killed with SIGTERM"),
+    )

 VIR_ENUM_DECL(virshDomainEventPMSuspended)
 VIR_ENUM_IMPL(virshDomainEventPMSuspended,
--
2.12.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Report shutdown event details
Posted by Eric Blake 6 years, 11 months ago
On 04/12/2017 09:48 AM, Martin Kletzander wrote:
> QEMU will likely report the details of it shutting down, particularly
> the system it received in case it was killed.  We should forward that
> information along.  Since the only way we can extend information
> provided to the user is adding event details, we might as well emit
> multiple events (one with the reason for the shutdown and keep the one
> for the shutdown being finished for clarity and compatibility).
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1384007
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
> This is waiting for a patch in QEMU [1] that I tested this with.
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg01098.html

The discussion on the qemu patch says it will probably not expose signal
names, but merely a boolean of guest- vs. host-initiated.  So you'll
have to respin on top of whatever qemu settles on, but the idea is sane.
 You are correct that since we can't expand the existing event, we have
to add a second one, and wire up both events to fire at once (we've done
it before, so there's precedence).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Report shutdown event details
Posted by Peter Krempa 6 years, 11 months ago
On Wed, Apr 12, 2017 at 09:55:23 -0500, Eric Blake wrote:
> On 04/12/2017 09:48 AM, Martin Kletzander wrote:
> > QEMU will likely report the details of it shutting down, particularly
> > the system it received in case it was killed.  We should forward that
> > information along.  Since the only way we can extend information
> > provided to the user is adding event details, we might as well emit
> > multiple events (one with the reason for the shutdown and keep the one
> > for the shutdown being finished for clarity and compatibility).
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1384007
> > 
> > Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> > ---
> > This is waiting for a patch in QEMU [1] that I tested this with.
> > 
> > [1] https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg01098.html
> 
> The discussion on the qemu patch says it will probably not expose signal
> names, but merely a boolean of guest- vs. host-initiated.  So you'll
> have to respin on top of whatever qemu settles on, but the idea is sane.
>  You are correct that since we can't expand the existing event, we have
> to add a second one, and wire up both events to fire at once (we've done
> it before, so there's precedence).

Umm, yes, the signal names are kind of qemu/linux specific so we really
should only expose the fact whether the host or guest initiated the
shutdown.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Report shutdown event details
Posted by Martin Kletzander 6 years, 11 months ago
On Wed, Apr 12, 2017 at 05:04:37PM +0200, Peter Krempa wrote:
>On Wed, Apr 12, 2017 at 09:55:23 -0500, Eric Blake wrote:
>> On 04/12/2017 09:48 AM, Martin Kletzander wrote:
>> > QEMU will likely report the details of it shutting down, particularly
>> > the system it received in case it was killed.  We should forward that
>> > information along.  Since the only way we can extend information
>> > provided to the user is adding event details, we might as well emit
>> > multiple events (one with the reason for the shutdown and keep the one
>> > for the shutdown being finished for clarity and compatibility).
>> >
>> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1384007
>> >
>> > Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> > ---
>> > This is waiting for a patch in QEMU [1] that I tested this with.
>> >
>> > [1] https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg01098.html
>>
>> The discussion on the qemu patch says it will probably not expose signal
>> names, but merely a boolean of guest- vs. host-initiated.  So you'll
>> have to respin on top of whatever qemu settles on, but the idea is sane.
>>  You are correct that since we can't expand the existing event, we have
>> to add a second one, and wire up both events to fire at once (we've done
>> it before, so there's precedence).
>

Is it OK to have just another shutdown event detail or should I wire up
a new lifecycle event?

>Umm, yes, the signal names are kind of qemu/linux specific so we really

Not really, we have more hypervisors where VMs are processes that we
see, and even though the notion of "signals" doesn't exist in Windows
per se, it's not like you cannot be killed there.  Plus it's just one of
multiple platforms that we run on, all the other have signals ;)

>should only expose the fact whether the host or guest initiated the
>shutdown.

But I agree here.  If we draw a clear line between guest/host
initiation, which is not *that* clear [1] IMHO, then we're fine.
Especially when we leave the differentiation decision on the
hypervisor itself ;)

Have a nice day,
Martin

[1] For example what is it when you send an ACPI event to the guest from
    the host using virsh?  And what if it ignores that event, but then
    shuts down itself due to different reason right after that?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list