[PATCH v2] qemu: add a monitor to /proc/$pid when killing times out

Boris Fiuczynski posted 1 patch 3 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20240719125823.110105-1-fiuczy@linux.ibm.com
There is a newer version of this series
src/qemu/qemu_domain.c  |   8 +++
src/qemu/qemu_domain.h  |   2 +
src/qemu/qemu_driver.c  |  18 +++++++
src/qemu/qemu_process.c | 115 ++++++++++++++++++++++++++++++++++++++--
src/qemu/qemu_process.h |   1 +
5 files changed, 139 insertions(+), 5 deletions(-)
[PATCH v2] qemu: add a monitor to /proc/$pid when killing times out
Posted by Boris Fiuczynski 3 months, 1 week ago
In cases when a QEMU process takes longer than the time sigterm and
sigkill are issued to kill the process do not simply fail and leave the
VM in state VIR_DOMAIN_SHUTDOWN until the daemon stops. Instead set up
an fd on /proc/$pid and get notified when the QEMU process finally has
terminated to cleanup the VM state.

Resolves: https://issues.redhat.com/browse/RHEL-28819
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
---
 src/qemu/qemu_domain.c  |   8 +++
 src/qemu/qemu_domain.h  |   2 +
 src/qemu/qemu_driver.c  |  18 +++++++
 src/qemu/qemu_process.c | 115 ++++++++++++++++++++++++++++++++++++++--
 src/qemu/qemu_process.h |   1 +
 5 files changed, 139 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2134b11038..8147ff02fd 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1889,6 +1889,11 @@ qemuDomainObjPrivateFree(void *data)
 
     virChrdevFree(priv->devs);
 
+    if (priv->pidMonitored >= 0) {
+        virEventRemoveHandle(priv->pidMonitored);
+        priv->pidMonitored = -1;
+    }
+
     /* This should never be non-NULL if we get here, but just in case... */
     if (priv->mon) {
         VIR_ERROR(_("Unexpected QEMU monitor still active during domain deletion"));
@@ -1934,6 +1939,8 @@ qemuDomainObjPrivateAlloc(void *opaque)
     priv->blockjobs = virHashNew(virObjectUnref);
     priv->fds = virHashNew(g_object_unref);
 
+    priv->pidMonitored = -1;
+
     /* agent commands block by default, user can choose different behavior */
     priv->agentTimeout = VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_BLOCK;
     priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX;
@@ -11680,6 +11687,7 @@ qemuProcessEventFree(struct qemuProcessEvent *event)
     case QEMU_PROCESS_EVENT_RESET:
     case QEMU_PROCESS_EVENT_NBDKIT_EXITED:
     case QEMU_PROCESS_EVENT_MONITOR_EOF:
+    case QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED:
     case QEMU_PROCESS_EVENT_LAST:
         break;
     }
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index d777559119..a5092dd7f0 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -119,6 +119,7 @@ struct _qemuDomainObjPrivate {
 
     bool beingDestroyed;
     char *pidfile;
+    int pidMonitored;
 
     virDomainPCIAddressSet *pciaddrs;
     virDomainUSBAddressSet *usbaddrs;
@@ -469,6 +470,7 @@ typedef enum {
     QEMU_PROCESS_EVENT_UNATTENDED_MIGRATION,
     QEMU_PROCESS_EVENT_RESET,
     QEMU_PROCESS_EVENT_NBDKIT_EXITED,
+    QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED,
 
     QEMU_PROCESS_EVENT_LAST
 } qemuProcessEventType;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9f3013e231..6b1e4084f6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4041,6 +4041,21 @@ processNbdkitExitedEvent(virDomainObj *vm,
 }
 
 
+static void
+processShutdownCompletedEvent(virQEMUDriver *driver,
+                              virDomainObj *vm)
+{
+    if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0)
+        return;
+
+    if (virDomainObjIsActive(vm))
+        qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_UNKNOWN,
+                        VIR_ASYNC_JOB_NONE, 0);
+
+    virDomainObjEndJob(vm);
+}
+
+
 static void qemuProcessEventHandler(void *data, void *opaque)
 {
     struct qemuProcessEvent *processEvent = data;
@@ -4101,6 +4116,9 @@ static void qemuProcessEventHandler(void *data, void *opaque)
     case QEMU_PROCESS_EVENT_NBDKIT_EXITED:
         processNbdkitExitedEvent(vm, processEvent->data);
         break;
+    case QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED:
+        processShutdownCompletedEvent(driver, vm);
+        break;
     case QEMU_PROCESS_EVENT_LAST:
         break;
     }
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 25dfd04272..86ca495a20 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -25,6 +25,7 @@
 #include <unistd.h>
 #include <signal.h>
 #include <sys/stat.h>
+#include <sys/syscall.h>
 #if defined(__linux__)
 # include <linux/capability.h>
 #elif defined(__FreeBSD__)
@@ -8387,9 +8388,107 @@ qemuProcessCreatePretendCmdBuild(virDomainObj *vm,
 }
 
 
+typedef struct {
+    virDomainObj *vm;
+    int pidfd;
+} qemuProcessInShutdownEventData;
+
+
+static qemuProcessInShutdownEventData*
+qemuProcessInShutdownEventDataNew(virDomainObj *vm, int pidfd)
+{
+    qemuProcessInShutdownEventData *d = g_new(qemuProcessInShutdownEventData, 1);
+    d->vm = virObjectRef(vm);
+    d->pidfd = pidfd;
+    return d;
+}
+
+
+static void
+qemuProcessInShutdownEventDataFree(qemuProcessInShutdownEventData *d)
+{
+    virObjectUnref(d->vm);
+    VIR_FORCE_CLOSE(d->pidfd);
+    g_free(d);
+}
+
+
+static void
+qemuProcessInShutdownPidfdCb(int watch,
+                             int fd,
+                             int events G_GNUC_UNUSED,
+                             void *opaque)
+{
+    qemuProcessInShutdownEventData *data = opaque;
+    virDomainObj *vm = data->vm;
+
+    VIR_DEBUG("vm=%p name=%s pid=%lld fd=%d",
+              vm, vm->def->name, (long long)vm->pid, fd);
+
+    virEventRemoveHandle(watch);
+
+    virObjectLock(vm);
+
+    VIR_INFO("QEMU process %lld finally completed termination",
+             (long long)vm->pid);
+
+    QEMU_DOMAIN_PRIVATE(vm)->pidMonitored = -1;
+    qemuProcessEventSubmit(vm, QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED,
+                           0, 0, NULL);
+
+    virObjectUnlock(vm);
+}
+
+
+static int
+qemuProcessInShutdownStartMonitor(virDomainObj *vm)
+{
+    qemuDomainObjPrivate *priv = vm->privateData;
+    qemuProcessInShutdownEventData *data;
+    int pidfd;
+    int ret = -1;
+
+    VIR_DEBUG("vm=%p name=%s pid=%lld pidMonitored=%d",
+              vm, vm->def->name, (long long)vm->pid,
+              priv->pidMonitored);
+
+    if (priv->pidMonitored >= 0) {
+        VIR_DEBUG("Monitoring qemu in-shutdown process %i already set up", vm->pid);
+        goto cleanup;
+    }
+
+    pidfd = syscall(SYS_pidfd_open, vm->pid, 0);
+    if (pidfd < 0) {
+        if (errno == ESRCH) /* process has already terminated */
+            ret = 1;
+        goto cleanup;
+    }
+
+    data = qemuProcessInShutdownEventDataNew(vm, pidfd);
+    if ((priv->pidMonitored = virEventAddHandle(pidfd,
+                                                VIR_EVENT_HANDLE_READABLE,
+                                                qemuProcessInShutdownPidfdCb,
+                                                data,
+                                                (virFreeCallback)qemuProcessInShutdownEventDataFree)) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                    _("failed to monitor qemu in-shutdown process %1$i"),
+                    vm->pid);
+        qemuProcessInShutdownEventDataFree(data);
+        goto cleanup;
+    }
+    VIR_DEBUG("Monitoring qemu in-shutdown process %i for termination", vm->pid);
+    ret = 0;
+
+ cleanup:
+    return ret;
+}
+
+
 int
 qemuProcessKill(virDomainObj *vm, unsigned int flags)
 {
+    int ret = -1;
+
     VIR_DEBUG("vm=%p name=%s pid=%lld flags=0x%x",
               vm, vm->def->name,
               (long long)vm->pid, flags);
@@ -8410,10 +8509,16 @@ qemuProcessKill(virDomainObj *vm, unsigned int flags)
 
     /* Request an extra delay of two seconds per current nhostdevs
      * to be safe against stalls by the kernel freeing up the resources */
-    return virProcessKillPainfullyDelay(vm->pid,
-                                        !!(flags & VIR_QEMU_PROCESS_KILL_FORCE),
-                                        vm->def->nhostdevs * 2,
-                                        false);
+    ret = virProcessKillPainfullyDelay(vm->pid,
+                                       !!(flags & VIR_QEMU_PROCESS_KILL_FORCE),
+                                       vm->def->nhostdevs * 2,
+                                       false);
+
+    if (ret < 0 && (flags & VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR))
+        if (qemuProcessInShutdownStartMonitor(vm) == 1)
+            ret = 0; /* process termination detected */
+
+    return ret;
 }
 
 
@@ -8438,7 +8543,7 @@ qemuProcessBeginStopJob(virDomainObj *vm,
      * cleared inside qemuProcessStop */
     priv->beingDestroyed = true;
 
-    if (qemuProcessKill(vm, killFlags) < 0)
+    if (qemuProcessKill(vm, killFlags|VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR) < 0)
         goto error;
 
     /* Wake up anything waiting on domain condition */
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index cb67bfcd2d..2324aeb7bd 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -180,6 +180,7 @@ typedef enum {
    VIR_QEMU_PROCESS_KILL_FORCE  = 1 << 0,
    VIR_QEMU_PROCESS_KILL_NOWAIT = 1 << 1,
    VIR_QEMU_PROCESS_KILL_NOCHECK = 1 << 2, /* bypass the running vm check */
+   VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR = 1 << 3, /* on error enable process monitor */
 } virQemuProcessKillMode;
 
 int qemuProcessKill(virDomainObj *vm, unsigned int flags);
-- 
2.45.0
Re: [PATCH v2] qemu: add a monitor to /proc/$pid when killing times out
Posted by Daniel P. Berrangé 3 months, 1 week ago
On Fri, Jul 19, 2024 at 02:58:23PM +0200, Boris Fiuczynski wrote:
> In cases when a QEMU process takes longer than the time sigterm and
> sigkill are issued to kill the process do not simply fail and leave the
> VM in state VIR_DOMAIN_SHUTDOWN until the daemon stops. Instead set up
> an fd on /proc/$pid and get notified when the QEMU process finally has
> terminated to cleanup the VM state.
> 
> Resolves: https://issues.redhat.com/browse/RHEL-28819
> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
> ---
>  src/qemu/qemu_domain.c  |   8 +++
>  src/qemu/qemu_domain.h  |   2 +
>  src/qemu/qemu_driver.c  |  18 +++++++
>  src/qemu/qemu_process.c | 115 ++++++++++++++++++++++++++++++++++++++--
>  src/qemu/qemu_process.h |   1 +
>  5 files changed, 139 insertions(+), 5 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 2134b11038..8147ff02fd 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1889,6 +1889,11 @@ qemuDomainObjPrivateFree(void *data)
>  
>      virChrdevFree(priv->devs);
>  
> +    if (priv->pidMonitored >= 0) {
> +        virEventRemoveHandle(priv->pidMonitored);
> +        priv->pidMonitored = -1;
> +    }
> +
>      /* This should never be non-NULL if we get here, but just in case... */
>      if (priv->mon) {
>          VIR_ERROR(_("Unexpected QEMU monitor still active during domain deletion"));
> @@ -1934,6 +1939,8 @@ qemuDomainObjPrivateAlloc(void *opaque)
>      priv->blockjobs = virHashNew(virObjectUnref);
>      priv->fds = virHashNew(g_object_unref);
>  
> +    priv->pidMonitored = -1;
> +
>      /* agent commands block by default, user can choose different behavior */
>      priv->agentTimeout = VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_BLOCK;
>      priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX;
> @@ -11680,6 +11687,7 @@ qemuProcessEventFree(struct qemuProcessEvent *event)
>      case QEMU_PROCESS_EVENT_RESET:
>      case QEMU_PROCESS_EVENT_NBDKIT_EXITED:
>      case QEMU_PROCESS_EVENT_MONITOR_EOF:
> +    case QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED:
>      case QEMU_PROCESS_EVENT_LAST:
>          break;
>      }
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index d777559119..a5092dd7f0 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -119,6 +119,7 @@ struct _qemuDomainObjPrivate {
>  
>      bool beingDestroyed;
>      char *pidfile;
> +    int pidMonitored;
>  
>      virDomainPCIAddressSet *pciaddrs;
>      virDomainUSBAddressSet *usbaddrs;
> @@ -469,6 +470,7 @@ typedef enum {
>      QEMU_PROCESS_EVENT_UNATTENDED_MIGRATION,
>      QEMU_PROCESS_EVENT_RESET,
>      QEMU_PROCESS_EVENT_NBDKIT_EXITED,
> +    QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED,
>  
>      QEMU_PROCESS_EVENT_LAST
>  } qemuProcessEventType;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 9f3013e231..6b1e4084f6 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4041,6 +4041,21 @@ processNbdkitExitedEvent(virDomainObj *vm,
>  }
>  
>  
> +static void
> +processShutdownCompletedEvent(virQEMUDriver *driver,
> +                              virDomainObj *vm)
> +{
> +    if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0)
> +        return;
> +
> +    if (virDomainObjIsActive(vm))
> +        qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_UNKNOWN,
> +                        VIR_ASYNC_JOB_NONE, 0);
> +
> +    virDomainObjEndJob(vm);
> +}
> +
> +
>  static void qemuProcessEventHandler(void *data, void *opaque)
>  {
>      struct qemuProcessEvent *processEvent = data;
> @@ -4101,6 +4116,9 @@ static void qemuProcessEventHandler(void *data, void *opaque)
>      case QEMU_PROCESS_EVENT_NBDKIT_EXITED:
>          processNbdkitExitedEvent(vm, processEvent->data);
>          break;
> +    case QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED:
> +        processShutdownCompletedEvent(driver, vm);
> +        break;
>      case QEMU_PROCESS_EVENT_LAST:
>          break;
>      }
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 25dfd04272..86ca495a20 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -25,6 +25,7 @@
>  #include <unistd.h>
>  #include <signal.h>
>  #include <sys/stat.h>
> +#include <sys/syscall.h>
>  #if defined(__linux__)
>  # include <linux/capability.h>
>  #elif defined(__FreeBSD__)
> @@ -8387,9 +8388,107 @@ qemuProcessCreatePretendCmdBuild(virDomainObj *vm,
>  }
>  
>  
> +typedef struct {
> +    virDomainObj *vm;
> +    int pidfd;
> +} qemuProcessInShutdownEventData;
> +
> +
> +static qemuProcessInShutdownEventData*
> +qemuProcessInShutdownEventDataNew(virDomainObj *vm, int pidfd)
> +{
> +    qemuProcessInShutdownEventData *d = g_new(qemuProcessInShutdownEventData, 1);
> +    d->vm = virObjectRef(vm);
> +    d->pidfd = pidfd;
> +    return d;
> +}
> +
> +
> +static void
> +qemuProcessInShutdownEventDataFree(qemuProcessInShutdownEventData *d)
> +{
> +    virObjectUnref(d->vm);
> +    VIR_FORCE_CLOSE(d->pidfd);
> +    g_free(d);
> +}
> +
> +
> +static void
> +qemuProcessInShutdownPidfdCb(int watch,
> +                             int fd,
> +                             int events G_GNUC_UNUSED,
> +                             void *opaque)
> +{
> +    qemuProcessInShutdownEventData *data = opaque;
> +    virDomainObj *vm = data->vm;
> +
> +    VIR_DEBUG("vm=%p name=%s pid=%lld fd=%d",
> +              vm, vm->def->name, (long long)vm->pid, fd);
> +
> +    virEventRemoveHandle(watch);
> +
> +    virObjectLock(vm);
> +
> +    VIR_INFO("QEMU process %lld finally completed termination",
> +             (long long)vm->pid);
> +
> +    QEMU_DOMAIN_PRIVATE(vm)->pidMonitored = -1;
> +    qemuProcessEventSubmit(vm, QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED,
> +                           0, 0, NULL);
> +
> +    virObjectUnlock(vm);
> +}
> +
> +
> +static int
> +qemuProcessInShutdownStartMonitor(virDomainObj *vm)
> +{
> +    qemuDomainObjPrivate *priv = vm->privateData;
> +    qemuProcessInShutdownEventData *data;
> +    int pidfd;
> +    int ret = -1;
> +
> +    VIR_DEBUG("vm=%p name=%s pid=%lld pidMonitored=%d",
> +              vm, vm->def->name, (long long)vm->pid,
> +              priv->pidMonitored);
> +
> +    if (priv->pidMonitored >= 0) {
> +        VIR_DEBUG("Monitoring qemu in-shutdown process %i already set up", vm->pid);
> +        goto cleanup;
> +    }
> +
> +    pidfd = syscall(SYS_pidfd_open, vm->pid, 0);
> +    if (pidfd < 0) {
> +        if (errno == ESRCH) /* process has already terminated */
> +            ret = 1;
> +        goto cleanup;
> +    }
> +
> +    data = qemuProcessInShutdownEventDataNew(vm, pidfd);
> +    if ((priv->pidMonitored = virEventAddHandle(pidfd,
> +                                                VIR_EVENT_HANDLE_READABLE,
> +                                                qemuProcessInShutdownPidfdCb,
> +                                                data,
> +                                                (virFreeCallback)qemuProcessInShutdownEventDataFree)) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                    _("failed to monitor qemu in-shutdown process %1$i"),
> +                    vm->pid);
> +        qemuProcessInShutdownEventDataFree(data);
> +        goto cleanup;
> +    }
> +    VIR_DEBUG("Monitoring qemu in-shutdown process %i for termination", vm->pid);
> +    ret = 0;
> +
> + cleanup:
> +    return ret;
> +}

This will need some #ifdef-ery because I believe SYS_pidfd_open won't
exist on all Linux versions we need, and certainly won't exist on
non-Linux, and nor will syscall() fro that matter.

> +
> +
>  int
>  qemuProcessKill(virDomainObj *vm, unsigned int flags)
>  {
> +    int ret = -1;
> +
>      VIR_DEBUG("vm=%p name=%s pid=%lld flags=0x%x",
>                vm, vm->def->name,
>                (long long)vm->pid, flags);
> @@ -8410,10 +8509,16 @@ qemuProcessKill(virDomainObj *vm, unsigned int flags)
>  
>      /* Request an extra delay of two seconds per current nhostdevs
>       * to be safe against stalls by the kernel freeing up the resources */
> -    return virProcessKillPainfullyDelay(vm->pid,
> -                                        !!(flags & VIR_QEMU_PROCESS_KILL_FORCE),
> -                                        vm->def->nhostdevs * 2,
> -                                        false);
> +    ret = virProcessKillPainfullyDelay(vm->pid,
> +                                       !!(flags & VIR_QEMU_PROCESS_KILL_FORCE),
> +                                       vm->def->nhostdevs * 2,
> +                                       false);
> +
> +    if (ret < 0 && (flags & VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR))
> +        if (qemuProcessInShutdownStartMonitor(vm) == 1)
> +            ret = 0; /* process termination detected */
> +
> +    return ret;
>  }
>  
>  
> @@ -8438,7 +8543,7 @@ qemuProcessBeginStopJob(virDomainObj *vm,
>       * cleared inside qemuProcessStop */
>      priv->beingDestroyed = true;
>  
> -    if (qemuProcessKill(vm, killFlags) < 0)
> +    if (qemuProcessKill(vm, killFlags|VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR) < 0)
>          goto error;
>  
>      /* Wake up anything waiting on domain condition */
> diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
> index cb67bfcd2d..2324aeb7bd 100644
> --- a/src/qemu/qemu_process.h
> +++ b/src/qemu/qemu_process.h
> @@ -180,6 +180,7 @@ typedef enum {
>     VIR_QEMU_PROCESS_KILL_FORCE  = 1 << 0,
>     VIR_QEMU_PROCESS_KILL_NOWAIT = 1 << 1,
>     VIR_QEMU_PROCESS_KILL_NOCHECK = 1 << 2, /* bypass the running vm check */
> +   VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR = 1 << 3, /* on error enable process monitor */
>  } virQemuProcessKillMode;
>  
>  int qemuProcessKill(virDomainObj *vm, unsigned int flags);
> -- 
> 2.45.0
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH v2] qemu: add a monitor to /proc/$pid when killing times out
Posted by Boris Fiuczynski 3 months, 1 week ago
On 7/19/24 3:29 PM, Daniel P. Berrangé wrote:
>> +static int
>> +qemuProcessInShutdownStartMonitor(virDomainObj *vm)
>> +{
>> +    qemuDomainObjPrivate *priv = vm->privateData;
>> +    qemuProcessInShutdownEventData *data;
>> +    int pidfd;
>> +    int ret = -1;
>> +
>> +    VIR_DEBUG("vm=%p name=%s pid=%lld pidMonitored=%d",
>> +              vm, vm->def->name, (long long)vm->pid,
>> +              priv->pidMonitored);
>> +
>> +    if (priv->pidMonitored >= 0) {
>> +        VIR_DEBUG("Monitoring qemu in-shutdown process %i already set up", vm->pid);
>> +        goto cleanup;
>> +    }
>> +
>> +    pidfd = syscall(SYS_pidfd_open, vm->pid, 0);
>> +    if (pidfd < 0) {
>> +        if (errno == ESRCH) /* process has already terminated */
>> +            ret = 1;
>> +        goto cleanup;
>> +    }
>> +
>> +    data = qemuProcessInShutdownEventDataNew(vm, pidfd);
>> +    if ((priv->pidMonitored = virEventAddHandle(pidfd,
>> +                                                VIR_EVENT_HANDLE_READABLE,
>> +                                                qemuProcessInShutdownPidfdCb,
>> +                                                data,
>> +                                                (virFreeCallback)qemuProcessInShutdownEventDataFree)) < 0) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                    _("failed to monitor qemu in-shutdown process %1$i"),
>> +                    vm->pid);
>> +        qemuProcessInShutdownEventDataFree(data);
>> +        goto cleanup;
>> +    }
>> +    VIR_DEBUG("Monitoring qemu in-shutdown process %i for termination", vm->pid);
>> +    ret = 0;
>> +
>> + cleanup:
>> +    return ret;
>> +}
> This will need some #ifdef-ery because I believe SYS_pidfd_open won't
> exist on all Linux versions we need, and certainly won't exist on
> non-Linux, and nor will syscall() fro that matter.

Ups, I forgot about that. Sending a v3 out.

-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294