4:27 a.m.
*** BLURB HERE ***
Kirill Shchetiniuk (2):
ch: memory (segmentation fault) fix
ch: preserve last error before stop fix
src/ch/ch_events.c | 6 +++---
src/ch/ch_process.c | 6 ++++++
2 files changed, 9 insertions(+), 3 deletions(-)
--
2.48.1
4:27 a.m.
New subject: [PATCH 1/2] ch: memory (segmentation fault) fix
Move monitor object unreference from virCHStartEventHandler
to virCHEventHandlerLoop. Put VM unreference after debug
print in virCHEventHandlerLoop.
Signed-off-by: Kirill Shchetiniuk <kshcheti(a)redhat.com>
---
src/ch/ch_events.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c
index 1cce30836a..20e7fbd705 100644
--- a/src/ch/ch_events.c
+++ b/src/ch/ch_events.c
@@ -286,9 +286,10 @@ virCHEventHandlerLoop(void *data)
}
}
- g_clear_pointer(&mon->event_buffer.buffer, g_free);
- virObjectUnref(vm);
+ g_clear_pointer(&(mon->event_buffer.buffer), g_free);
VIR_DEBUG("%s: Event handler loop thread exiting", vm->def->name);
+ virObjectUnref(vm);
+ virObjectUnref(mon);
return;
}
@@ -308,7 +309,6 @@ virCHStartEventHandler(virCHMonitor *mon)
virObjectUnref(mon);
return -1;
}
- virObjectUnref(mon);
g_atomic_int_set(&mon->event_handler_stop, 0);
return 0;
--
2.48.1
5:36 a.m.
New subject: [PATCH 1/2] ch: memory (segmentation fault) fix
On Thu, Mar 06, 2025 at 11:27:07 +0100, Kirill Shchetiniuk wrote:
Move monitor object unreference from virCHStartEventHandler
to virCHEventHandlerLoop. Put VM unreference after debug
print in virCHEventHandlerLoop.
Please use the commit message mainly to describe the problem you're
fixiing. You only described what you changed; not why.
Signed-off-by: Kirill Shchetiniuk <kshcheti(a)redhat.com>
---
src/ch/ch_events.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c
index 1cce30836a..20e7fbd705 100644
--- a/src/ch/ch_events.c
+++ b/src/ch/ch_events.c
@@ -286,9 +286,10 @@ virCHEventHandlerLoop(void *data)
}
}
- g_clear_pointer(&mon->event_buffer.buffer, g_free);
Okay so ...
- virObjectUnref(vm);
+ g_clear_pointer(&(mon->event_buffer.buffer), g_free);
... this was taking reference of 'mon' instead of the buffer.
VIR_DEBUG("%s: Event handler loop thread exiting",
vm->def->name);
and this debug technically accesses 'vm' which you no longer have
reference for ...
+ virObjectUnref(vm);
+ virObjectUnref(mon);
But that doesn't explain why this is needed.
return;
}
@@ -308,7 +309,6 @@ virCHStartEventHandler(virCHMonitor *mon)
virObjectUnref(mon);
and it is weird because you do unref it here ...
return -1;
}
- virObjectUnref(mon);
... but remove it from here.
g_atomic_int_set(&mon->event_handler_stop, 0);
return 0;
7:44 a.m.
New subject: [PATCH 1/2] ch: memory (segmentation fault) fix
Please use the commit message mainly to describe the problem you're
fixiing. You only described what you changed; not why.
Thank you for your feedback, next time will keep it mind.
and it is weird because you do unref it here ...
This unref executes in case of any failure during new thread start up, as it wont
be used later, otherwise this reference is used by virCHEventHandlerLoop thread and should
be unrefed when virCHEventHandlerLoop is finished.
... but remove it from here.
In case of successful startup we do not want to unref an object cause it will be
used in other thread.
4:27 a.m.
New subject: [PATCH 2/2] ch: preserve last error before stop fix
Add last meaningful error preservation to virCHProcessStop.
Signed-off-by: Kirill Shchetiniuk <kshcheti(a)redhat.com>
---
src/ch/ch_process.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c
index 9a85f7869e..38d466eaeb 100644
--- a/src/ch/ch_process.c
+++ b/src/ch/ch_process.c
@@ -995,11 +995,15 @@ virCHProcessStop(virCHDriver *driver,
virCHDomainObjPrivate *priv = vm->privateData;
virCHDriverConfig *cfg = virCHDriverGetConfig(driver);
virDomainDef *def = vm->def;
+ virErrorPtr orig_err = NULL;
size_t i;
VIR_DEBUG("Stopping VM name=%s pid=%d reason=%d",
vm->def->name, (int)vm->pid, (int)reason);
+ /* Preserve origin error */
+ virErrorPreserveLast(&orig_err);
+
if (priv->monitor) {
g_clear_pointer(&priv->monitor, virCHMonitorClose);
}
@@ -1032,6 +1036,8 @@ virCHProcessStop(virCHDriver *driver,
virHostdevReAttachDomainDevices(driver->hostdevMgr, CH_DRIVER_NAME, def,
hostdev_flags);
+
+ virErrorRestore(&orig_err);
return 0;
}
--
2.48.1
5:42 a.m.
New subject: [PATCH 2/2] ch: preserve last error before stop fix
On Thu, Mar 06, 2025 at 11:27:08 +0100, Kirill Shchetiniuk wrote:
Add last meaningful error preservation to virCHProcessStop.
Signed-off-by: Kirill Shchetiniuk <kshcheti(a)redhat.com>
---
src/ch/ch_process.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c
index 9a85f7869e..38d466eaeb 100644
--- a/src/ch/ch_process.c
+++ b/src/ch/ch_process.c
@@ -995,11 +995,15 @@ virCHProcessStop(virCHDriver *driver,
virCHDomainObjPrivate *priv = vm->privateData;
virCHDriverConfig *cfg = virCHDriverGetConfig(driver);
virDomainDef *def = vm->def;
+ virErrorPtr orig_err = NULL;
size_t i;
VIR_DEBUG("Stopping VM name=%s pid=%d reason=%d",
vm->def->name, (int)vm->pid, (int)reason);
+ /* Preserve origin error */
It's quite self-evident from the function called ;)
+ virErrorPreserveLast(&orig_err);
+
if (priv->monitor) {
g_clear_pointer(&priv->monitor, virCHMonitorClose);
}
@@ -1032,6 +1036,8 @@ virCHProcessStop(virCHDriver *driver,
virHostdevReAttachDomainDevices(driver->hostdevMgr, CH_DRIVER_NAME, def,
hostdev_flags);
+
+ virErrorRestore(&orig_err);
return 0;
}
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
7
days inactive
7
days old
5 comments
3 participants
participants (3)
-
Kirill Shchetiniuk -
kshcheti＠redhat.com
-
Peter Krempa