[PATCH 0/3] ch: monitor daemonization, sync after reboot, and shutdown fixes
1. Run CH monitor as a daemon Made the monitor process daemonized to prevent VM termination if the CH driver crashes. Added pidfile for daemon's pid aquiring and tracking as well as its init. 2. Update domain info after reboot Fixed an issue where domain properties (e.g., serial console path) were not updated after VM reboot. Added VIR_CH_EVENT_VM_REBOOTED handling to keep the transient domain definition consistent. 3. Update VM shutdown event handler VM monitor was still up even if VM was shut off, which led to an inability to start the domain again. virsh # shutdown ch-test Domain 'ch-test' is being shutdown virsh # list Id Name State ------------------------------ 722117 ch-test shut off Ensured the CH monitor process terminates along with the VM shutdown (e.g., executed using virsh). Updated virCHEventStopProcess to have proper job type. Kirill Shchetiniuk (3): ch: virCHMonitorNew() run new CH monitor daemonized ch: virCHProcessEvent() update domain info after reboot ch: virCHProcessEvent() vm shutdown event handler fix src/ch/ch_domain.c | 1 + src/ch/ch_domain.h | 1 + src/ch/ch_events.c | 8 ++++---- src/ch/ch_monitor.c | 24 ++++++++++++++++++++++-- src/ch/ch_process.c | 18 +++++++++++++++++- src/ch/ch_process.h | 2 ++ 6 files changed, 47 insertions(+), 7 deletions(-) -- 2.48.1
When the new CH monitor was started, it ran as a non-daemonized process and was a child of the CH driver process. This led to a situation where if the CH driver died, the monitor process were killed too, terminating the running VM under the monitor. This led to termination of all VM started under the libvirt. Make new monitor running daemonized to avoid VMs shutdown when driver dies. Also added a pidfile its preparetion to be able to aquire daemon's PID. Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com> --- src/ch/ch_domain.c | 1 + src/ch/ch_domain.h | 1 + src/ch/ch_monitor.c | 24 ++++++++++++++++++++++-- src/ch/ch_process.c | 16 ++++++++++++++++ 4 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c index a08b18c5b9..c0c9acd85b 100644 --- a/src/ch/ch_domain.c +++ b/src/ch/ch_domain.c @@ -68,6 +68,7 @@ virCHDomainObjPrivateFree(void *data) virBitmapFree(priv->autoCpuset); virBitmapFree(priv->autoNodeset); virCgroupFree(priv->cgroup); + g_free(priv->pidfile); g_free(priv); } diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h index 8dea2b2123..69a657f6af 100644 --- a/src/ch/ch_domain.h +++ b/src/ch/ch_domain.h @@ -36,6 +36,7 @@ struct _virCHDomainObjPrivate { virBitmap *autoCpuset; virBitmap *autoNodeset; virCgroup *cgroup; + char *pidfile; }; #define CH_DOMAIN_PRIVATE(vm) \ diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 0ba927a194..1dc085a755 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -27,6 +27,7 @@ #include "datatypes.h" #include "ch_conf.h" +#include "ch_domain.h" #include "ch_events.h" #include "ch_interface.h" #include "ch_monitor.h" @@ -37,6 +38,7 @@ #include "virfile.h" #include "virjson.h" #include "virlog.h" +#include "virpidfile.h" #include "virstring.h" #define VIR_FROM_THIS VIR_FROM_CH @@ -584,6 +586,8 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg, int logfile) { g_autoptr(virCHMonitor) mon = NULL; g_autoptr(virCommand) cmd = NULL; + virCHDomainObjPrivate *priv = vm->privateData; + int rv; int socket_fd = 0; int event_monitor_fd; @@ -644,6 +648,7 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg, int logfile) virCommandSetErrorFD(cmd, &logfile); virCommandNonblockingFDs(cmd); virCommandSetUmask(cmd, 0x002); + socket_fd = chMonitorCreateSocket(mon->socketpath); if (socket_fd < 0) { virReportSystemError(errno, @@ -655,13 +660,28 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg, int logfile) virCommandAddArg(cmd, "--api-socket"); virCommandAddArgFormat(cmd, "fd=%d", socket_fd); virCommandPassFD(cmd, socket_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); - virCommandAddArg(cmd, "--event-monitor"); virCommandAddArgFormat(cmd, "path=%s", mon->eventmonitorpath); + virCommandSetPidFile(cmd, priv->pidfile); + virCommandDaemonize(cmd); /* launch Cloud-Hypervisor socket */ - if (virCommandRunAsync(cmd, &mon->pid) < 0) + rv = virCommandRun(cmd, NULL); + + if (rv == 0) { + if ((rv = virPidFileReadPath(priv->pidfile, &mon->pid)) < 0) { + virReportSystemError(-rv, + _("Domain %1$s didn't show up"), + vm->def->name); + return NULL; + } + VIR_DEBUG("CH vm=%p name=%s running with pid=%lld", + vm, vm->def->name, (long long)vm->pid); + } else { + VIR_DEBUG("CH vm=%p name=%s failed to spawn", + vm, vm->def->name); return NULL; + } /* open the reader end of fifo before start Event Handler */ while ((event_monitor_fd = open(mon->eventmonitorpath, O_RDONLY)) < 0) { diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c index b3eddd61e8..0954de6180 100644 --- a/src/ch/ch_process.c +++ b/src/ch/ch_process.c @@ -36,6 +36,7 @@ #include "virjson.h" #include "virlog.h" #include "virnuma.h" +#include "virpidfile.h" #include "virstring.h" #include "ch_interface.h" #include "ch_hostdev.h" @@ -850,6 +851,21 @@ virCHProcessPrepareHost(virCHDriver *driver, virDomainObj *vm) if (virCHHostdevPrepareDomainDevices(driver, vm->def, hostdev_flags) < 0) return -1; + VIR_FREE(priv->pidfile); + if (!(priv->pidfile = virPidFileBuildPath(cfg->stateDir, vm->def->name))) { + virReportSystemError(errno, + "%s", _("Failed to build pidfile path.")); + return -1; + } + + if (unlink(priv->pidfile) < 0 && + errno != ENOENT) { + virReportSystemError(errno, + _("Cannot remove stale PID file %1$s"), + priv->pidfile); + return -1; + } + /* Ensure no historical cgroup for this VM is lying around */ VIR_DEBUG("Ensuring no historical cgroup is lying around"); virDomainCgroupRemoveCgroup(vm, priv->cgroup, priv->machineName); -- 2.48.1
On 3/25/25 15:11, Kirill Shchetiniuk via Devel wrote:
When the new CH monitor was started, it ran as a non-daemonized process and was a child of the CH driver process. This led to a situation where if the CH driver died, the monitor process were killed too, terminating the running VM under the monitor. This led to termination of all VM started under the libvirt.
Make new monitor running daemonized to avoid VMs shutdown when driver dies. Also added a pidfile its preparetion to be able to aquire daemon's PID.
Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com> --- src/ch/ch_domain.c | 1 + src/ch/ch_domain.h | 1 + src/ch/ch_monitor.c | 24 ++++++++++++++++++++++-- src/ch/ch_process.c | 16 ++++++++++++++++ 4 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c index a08b18c5b9..c0c9acd85b 100644 --- a/src/ch/ch_domain.c +++ b/src/ch/ch_domain.c @@ -68,6 +68,7 @@ virCHDomainObjPrivateFree(void *data) virBitmapFree(priv->autoCpuset); virBitmapFree(priv->autoNodeset); virCgroupFree(priv->cgroup); + g_free(priv->pidfile); g_free(priv); }
diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h index 8dea2b2123..69a657f6af 100644 --- a/src/ch/ch_domain.h +++ b/src/ch/ch_domain.h @@ -36,6 +36,7 @@ struct _virCHDomainObjPrivate { virBitmap *autoCpuset; virBitmap *autoNodeset; virCgroup *cgroup; + char *pidfile; };
#define CH_DOMAIN_PRIVATE(vm) \ diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 0ba927a194..1dc085a755 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -27,6 +27,7 @@
#include "datatypes.h" #include "ch_conf.h" +#include "ch_domain.h" #include "ch_events.h" #include "ch_interface.h" #include "ch_monitor.h" @@ -37,6 +38,7 @@ #include "virfile.h" #include "virjson.h" #include "virlog.h" +#include "virpidfile.h" #include "virstring.h"
#define VIR_FROM_THIS VIR_FROM_CH @@ -584,6 +586,8 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg, int logfile) { g_autoptr(virCHMonitor) mon = NULL; g_autoptr(virCommand) cmd = NULL; + virCHDomainObjPrivate *priv = vm->privateData; + int rv; int socket_fd = 0; int event_monitor_fd;
@@ -644,6 +648,7 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg, int logfile) virCommandSetErrorFD(cmd, &logfile); virCommandNonblockingFDs(cmd); virCommandSetUmask(cmd, 0x002); + socket_fd = chMonitorCreateSocket(mon->socketpath); if (socket_fd < 0) { virReportSystemError(errno, @@ -655,13 +660,28 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg, int logfile) virCommandAddArg(cmd, "--api-socket"); virCommandAddArgFormat(cmd, "fd=%d", socket_fd); virCommandPassFD(cmd, socket_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); - virCommandAddArg(cmd, "--event-monitor"); virCommandAddArgFormat(cmd, "path=%s", mon->eventmonitorpath); + virCommandSetPidFile(cmd, priv->pidfile); + virCommandDaemonize(cmd);
/* launch Cloud-Hypervisor socket */ - if (virCommandRunAsync(cmd, &mon->pid) < 0) + rv = virCommandRun(cmd, NULL); + + if (rv == 0) { + if ((rv = virPidFileReadPath(priv->pidfile, &mon->pid)) < 0) { + virReportSystemError(-rv, + _("Domain %1$s didn't show up"), + vm->def->name); + return NULL; + } + VIR_DEBUG("CH vm=%p name=%s running with pid=%lld", + vm, vm->def->name, (long long)vm->pid);
This needs to be mon->pid because vm->pid is set elsewhere (in the caller virCHProcessStartRestore() after virCHProcessConnectMonitor() returns).
+ } else { + VIR_DEBUG("CH vm=%p name=%s failed to spawn", + vm, vm->def->name); return NULL; + }
Alternatively, this can be simplified to: if (virCommandRun() < 0) { VIR_DEBUG(...) return NULL; } if ((rv = virPidFileReadPath()) < 0) { virReportSystemError(-rv, ...); return NULL; }
/* open the reader end of fifo before start Event Handler */ while ((event_monitor_fd = open(mon->eventmonitorpath, O_RDONLY)) < 0) { diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c index b3eddd61e8..0954de6180 100644 --- a/src/ch/ch_process.c +++ b/src/ch/ch_process.c @@ -36,6 +36,7 @@ #include "virjson.h" #include "virlog.h" #include "virnuma.h" +#include "virpidfile.h" #include "virstring.h" #include "ch_interface.h" #include "ch_hostdev.h" @@ -850,6 +851,21 @@ virCHProcessPrepareHost(virCHDriver *driver, virDomainObj *vm) if (virCHHostdevPrepareDomainDevices(driver, vm->def, hostdev_flags) < 0) return -1;
+ VIR_FREE(priv->pidfile); + if (!(priv->pidfile = virPidFileBuildPath(cfg->stateDir, vm->def->name))) { + virReportSystemError(errno, + "%s", _("Failed to build pidfile path.")); + return -1; + } + + if (unlink(priv->pidfile) < 0 && + errno != ENOENT) { + virReportSystemError(errno, + _("Cannot remove stale PID file %1$s"), + priv->pidfile); + return -1; + } +
This works, but something similar should be in virCHProcessStop().
/* Ensure no historical cgroup for this VM is lying around */ VIR_DEBUG("Ensuring no historical cgroup is lying around"); virDomainCgroupRemoveCgroup(vm, priv->cgroup, priv->machineName);
Michal
On a Tuesday in 2025, Kirill Shchetiniuk via Devel wrote:
When the new CH monitor was started, it ran as a non-daemonized process and was a child of the CH driver process. This led to a situation where if the CH driver died, the monitor process were killed too, terminating the running VM under the monitor. This led to termination of all VM started under the libvirt.
Make new monitor running daemonized to avoid VMs shutdown when driver dies. Also added a pidfile its preparetion to be able to aquire daemon's PID.
Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com> --- src/ch/ch_domain.c | 1 + src/ch/ch_domain.h | 1 + src/ch/ch_monitor.c | 24 ++++++++++++++++++++++-- src/ch/ch_process.c | 16 ++++++++++++++++ 4 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 0ba927a194..1dc085a755 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -644,6 +648,7 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg, int logfile) virCommandSetErrorFD(cmd, &logfile); virCommandNonblockingFDs(cmd); virCommandSetUmask(cmd, 0x002); +
Nitpick: unrelated whitespace change. In some projects, these should be squashed together with the patch changing the function as you've done here, but in libvirt we separate them from the functional changes (it's easier to review a patch that changes just whitespace, then the actual functional changes are smaller)
socket_fd = chMonitorCreateSocket(mon->socketpath); if (socket_fd < 0) { virReportSystemError(errno, @@ -655,13 +660,28 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg, int logfile) virCommandAddArg(cmd, "--api-socket"); virCommandAddArgFormat(cmd, "fd=%d", socket_fd); virCommandPassFD(cmd, socket_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); -
This newline could have stayed - it seprated the arguments.
virCommandAddArg(cmd, "--event-monitor"); virCommandAddArgFormat(cmd, "path=%s", mon->eventmonitorpath); + virCommandSetPidFile(cmd, priv->pidfile); + virCommandDaemonize(cmd);
/* launch Cloud-Hypervisor socket */ - if (virCommandRunAsync(cmd, &mon->pid) < 0) + rv = virCommandRun(cmd, NULL); + + if (rv == 0) { + if ((rv = virPidFileReadPath(priv->pidfile, &mon->pid)) < 0) { + virReportSystemError(-rv, + _("Domain %1$s didn't show up"),
Not a nitpick - double spaces in the error message. This is user-visible. Jano
+ vm->def->name); + return NULL; + } + VIR_DEBUG("CH vm=%p name=%s running with pid=%lld", + vm, vm->def->name, (long long)vm->pid); + } else { + VIR_DEBUG("CH vm=%p name=%s failed to spawn", + vm, vm->def->name); return NULL; + }
/* open the reader end of fifo before start Event Handler */ while ((event_monitor_fd = open(mon->eventmonitorpath, O_RDONLY)) < 0) {
Thanks Jan, Didn’t noticed the issue with the white symbols when submitted a patch, next time will keep it in mind and be careful. Btw, do we have some tools to check and warn such possible issues? Kirill
2. 4. 2025 v 18:36, Ján Tomko <jtomko@redhat.com>:
On a Tuesday in 2025, Kirill Shchetiniuk via Devel wrote:
When the new CH monitor was started, it ran as a non-daemonized process and was a child of the CH driver process. This led to a situation where if the CH driver died, the monitor process were killed too, terminating the running VM under the monitor. This led to termination of all VM started under the libvirt.
Make new monitor running daemonized to avoid VMs shutdown when driver dies. Also added a pidfile its preparetion to be able to aquire daemon's PID.
Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com> --- src/ch/ch_domain.c | 1 + src/ch/ch_domain.h | 1 + src/ch/ch_monitor.c | 24 ++++++++++++++++++++++-- src/ch/ch_process.c | 16 ++++++++++++++++ 4 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 0ba927a194..1dc085a755 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -644,6 +648,7 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg, int logfile) virCommandSetErrorFD(cmd, &logfile); virCommandNonblockingFDs(cmd); virCommandSetUmask(cmd, 0x002); +
Nitpick: unrelated whitespace change. In some projects, these should be squashed together with the patch changing the function as you've done here, but in libvirt we separate them from the functional changes (it's easier to review a patch that changes just whitespace, then the actual functional changes are smaller)
socket_fd = chMonitorCreateSocket(mon->socketpath); if (socket_fd < 0) { virReportSystemError(errno, @@ -655,13 +660,28 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg, int logfile) virCommandAddArg(cmd, "--api-socket"); virCommandAddArgFormat(cmd, "fd=%d", socket_fd); virCommandPassFD(cmd, socket_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); -
This newline could have stayed - it seprated the arguments.
virCommandAddArg(cmd, "--event-monitor"); virCommandAddArgFormat(cmd, "path=%s", mon->eventmonitorpath); + virCommandSetPidFile(cmd, priv->pidfile); + virCommandDaemonize(cmd);
/* launch Cloud-Hypervisor socket */ - if (virCommandRunAsync(cmd, &mon->pid) < 0) + rv = virCommandRun(cmd, NULL); + + if (rv == 0) { + if ((rv = virPidFileReadPath(priv->pidfile, &mon->pid)) < 0) { + virReportSystemError(-rv, + _("Domain %1$s didn't show up"),
Not a nitpick - double spaces in the error message. This is user-visible.
Jano
+ vm->def->name); + return NULL; + } + VIR_DEBUG("CH vm=%p name=%s running with pid=%lld", + vm, vm->def->name, (long long)vm->pid); + } else { + VIR_DEBUG("CH vm=%p name=%s failed to spawn", + vm, vm->def->name); return NULL; + }
/* open the reader end of fifo before start Event Handler */ while ((event_monitor_fd = open(mon->eventmonitorpath, O_RDONLY)) < 0) { <signature.asc>
On a Wednesday in 2025, Kirill Shchetiniuk wrote:
Thanks Jan,
Didn’t noticed the issue with the white symbols when submitted a patch, next time will keep it in mind and be careful. Btw, do we have some tools to check and warn such possible issues?
For the extra/removed lines, git diff/git show. For the translatable error message? I'm afraid we cannot do that automatically - there are places like --help output where we use multiple spaces to align columns in the output. If you left the error message untouched, I assume eventually some translator would show up and report it / send a fix. Jano
Kirill
2. 4. 2025 v 18:36, Ján Tomko <jtomko@redhat.com>:
On a Tuesday in 2025, Kirill Shchetiniuk via Devel wrote:
When the new CH monitor was started, it ran as a non-daemonized process and was a child of the CH driver process. This led to a situation where if the CH driver died, the monitor process were killed too, terminating the running VM under the monitor. This led to termination of all VM started under the libvirt.
Make new monitor running daemonized to avoid VMs shutdown when driver dies. Also added a pidfile its preparetion to be able to aquire daemon's PID.
Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com> --- src/ch/ch_domain.c | 1 + src/ch/ch_domain.h | 1 + src/ch/ch_monitor.c | 24 ++++++++++++++++++++++-- src/ch/ch_process.c | 16 ++++++++++++++++ 4 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 0ba927a194..1dc085a755 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -644,6 +648,7 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg, int logfile) virCommandSetErrorFD(cmd, &logfile); virCommandNonblockingFDs(cmd); virCommandSetUmask(cmd, 0x002); +
Nitpick: unrelated whitespace change. In some projects, these should be squashed together with the patch changing the function as you've done here, but in libvirt we separate them from the functional changes (it's easier to review a patch that changes just whitespace, then the actual functional changes are smaller)
socket_fd = chMonitorCreateSocket(mon->socketpath); if (socket_fd < 0) { virReportSystemError(errno, @@ -655,13 +660,28 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg, int logfile) virCommandAddArg(cmd, "--api-socket"); virCommandAddArgFormat(cmd, "fd=%d", socket_fd); virCommandPassFD(cmd, socket_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); -
This newline could have stayed - it seprated the arguments.
virCommandAddArg(cmd, "--event-monitor"); virCommandAddArgFormat(cmd, "path=%s", mon->eventmonitorpath); + virCommandSetPidFile(cmd, priv->pidfile); + virCommandDaemonize(cmd);
/* launch Cloud-Hypervisor socket */ - if (virCommandRunAsync(cmd, &mon->pid) < 0) + rv = virCommandRun(cmd, NULL); + + if (rv == 0) { + if ((rv = virPidFileReadPath(priv->pidfile, &mon->pid)) < 0) { + virReportSystemError(-rv, + _("Domain %1$s didn't show up"),
Not a nitpick - double spaces in the error message. This is user-visible.
Jano
+ vm->def->name); + return NULL; + } + VIR_DEBUG("CH vm=%p name=%s running with pid=%lld", + vm, vm->def->name, (long long)vm->pid); + } else { + VIR_DEBUG("CH vm=%p name=%s failed to spawn", + vm, vm->def->name); return NULL; + }
/* open the reader end of fifo before start Event Handler */ while ((event_monitor_fd = open(mon->eventmonitorpath, O_RDONLY)) < 0) { <signature.asc>
When the domain was rebooted, some of its properties were changed but not updated in the transient domain definition. This led to the inability to connect to the serial console as its path had changed during the reboot but was not updated in the domain definition. Added VIR_CH_EVENT_VM_REBOOTED event handling to update the information in transient domain definition after domain's reboot is completed to maintain it in consistent state. Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com> --- src/ch/ch_events.c | 6 +++++- src/ch/ch_process.c | 2 +- src/ch/ch_process.h | 2 ++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c index 2dd3e7ecc2..32846ba4e5 100644 --- a/src/ch/ch_events.c +++ b/src/ch/ch_events.c @@ -97,7 +97,6 @@ virCHProcessEvent(virCHMonitor *mon, case VIR_CH_EVENT_VM_BOOTING: case VIR_CH_EVENT_VM_BOOTED: case VIR_CH_EVENT_VM_REBOOTING: - case VIR_CH_EVENT_VM_REBOOTED: case VIR_CH_EVENT_VM_PAUSING: case VIR_CH_EVENT_VM_PAUSED: case VIR_CH_EVENT_VM_RESUMING: @@ -120,6 +119,11 @@ virCHProcessEvent(virCHMonitor *mon, virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_SHUTDOWN); virObjectUnlock(vm); break; + case VIR_CH_EVENT_VM_REBOOTED: + virObjectLock(vm); + virCHProcessUpdateInfo(vm); + virObjectUnlock(vm); + break; case VIR_CH_EVENT_LAST: default: VIR_WARN("%s: Unknown event: %s", vm->def->name, full_event); diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c index 0954de6180..31aa49b3a5 100644 --- a/src/ch/ch_process.c +++ b/src/ch/ch_process.c @@ -131,7 +131,7 @@ virCHProcessUpdateConsole(virDomainObj *vm, virCHProcessUpdateConsoleDevice(vm, config, "serial"); } -static int +int virCHProcessUpdateInfo(virDomainObj *vm) { g_autoptr(virJSONValue) info = NULL; diff --git a/src/ch/ch_process.h b/src/ch/ch_process.h index 38bfce3b7f..7a6995b7cf 100644 --- a/src/ch/ch_process.h +++ b/src/ch/ch_process.h @@ -36,3 +36,5 @@ int virCHProcessSetupVcpu(virDomainObj *vm, int virCHProcessStartRestore(virCHDriver *driver, virDomainObj *vm, const char *from); + +int virCHProcessUpdateInfo(virDomainObj *vm); -- 2.48.1
When the domain shutdown was executed from virsh, only the VM process (a child of the CH monitor) was terminated. Since we assume only one VM per monitor, the monitor process should also be terminated. Modified the VM shutdown event handler to match the VMM shutdown behavior, ensuring the VM monitor stops along with the VM. Also updated the virCHEventStopProcess job type, as it only destroys the domain rather than modifying anything. Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com> --- src/ch/ch_events.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c index 32846ba4e5..f1dc5c6f4c 100644 --- a/src/ch/ch_events.c +++ b/src/ch/ch_events.c @@ -57,7 +57,7 @@ virCHEventStopProcess(virDomainObj *vm, virCHDriver *driver = ((virCHDomainObjPrivate *)vm->privateData)->driver; virObjectLock(vm); - if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY)) + if (virDomainObjBeginJob(vm, VIR_JOB_DESTROY)) return -1; virCHProcessStop(driver, vm, reason); virDomainObjEndJob(vm); @@ -108,17 +108,13 @@ virCHProcessEvent(virCHMonitor *mon, case VIR_CH_EVENT_VM_DELETED: break; case VIR_CH_EVENT_VMM_SHUTDOWN: + case VIR_CH_EVENT_VM_SHUTDOWN: if (virCHEventStopProcess(vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN)) { VIR_WARN("Failed to mark the VM(%s) as SHUTDOWN!", vm->def->name); ret = -1; } break; - case VIR_CH_EVENT_VM_SHUTDOWN: - virObjectLock(vm); - virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_SHUTDOWN); - virObjectUnlock(vm); - break; case VIR_CH_EVENT_VM_REBOOTED: virObjectLock(vm); virCHProcessUpdateInfo(vm); -- 2.48.1
On 3/25/25 15:11, Kirill Shchetiniuk via Devel wrote:
1. Run CH monitor as a daemon Made the monitor process daemonized to prevent VM termination if the CH driver crashes. Added pidfile for daemon's pid aquiring and tracking as well as its init.
2. Update domain info after reboot Fixed an issue where domain properties (e.g., serial console path) were not updated after VM reboot. Added VIR_CH_EVENT_VM_REBOOTED handling to keep the transient domain definition consistent.
3. Update VM shutdown event handler VM monitor was still up even if VM was shut off, which led to an inability to start the domain again.
virsh # shutdown ch-test Domain 'ch-test' is being shutdown
virsh # list Id Name State ------------------------------ 722117 ch-test shut off
Ensured the CH monitor process terminates along with the VM shutdown (e.g., executed using virsh). Updated virCHEventStopProcess to have proper job type.
Kirill Shchetiniuk (3): ch: virCHMonitorNew() run new CH monitor daemonized ch: virCHProcessEvent() update domain info after reboot ch: virCHProcessEvent() vm shutdown event handler fix
src/ch/ch_domain.c | 1 + src/ch/ch_domain.h | 1 + src/ch/ch_events.c | 8 ++++---- src/ch/ch_monitor.c | 24 ++++++++++++++++++++++-- src/ch/ch_process.c | 18 +++++++++++++++++- src/ch/ch_process.h | 2 ++ 6 files changed, 47 insertions(+), 7 deletions(-)
-- 2.48.1
I'm fixing 1/3 and merging. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (3)
-
Ján Tomko -
Kirill Shchetiniuk -
Michal Prívozník