During fixing some other issue found out a few minor bugs
related to CH driver.
First bug is related to monitor object ref count
in virCHStartEventHandler and virCHEventHandlerLoop, as
object was unrefed by parent while is still used by child
thread. Moved object unref to the correct place to the
correct thread.
Second bug is related to v object ref count in
virCHStartEventHandler, as vm object was unrefed,
while is still used later, move object unref to
after its last usage.
Third bug is related to error handling and
propagation. Last meaninful error was reset
by virCHProcessStop, which have led to
unknown error message in virsh. Add error
preservation at the begining of the function
and restoring it at the end to keep
consistent error output
Kirill Shchetiniuk (2):
ch: virCHProcessStop preserve last meaningful error
ch: ref count fix in virCHEventHandlerLoop and virCHStartEventHandler
src/ch/ch_events.c | 4 ++--
src/ch/ch_process.c | 5 +++++
2 files changed, 7 insertions(+), 2 deletions(-)
--
2.48.1
Show replies by date
The last meaningful error was being reset during
the execution of virCHProcessStop, which led to
inconsistent error output in case any error
occurred before the stop. To maintain consistent
error output and avoid unknown error messages,
last meaningful error preservation has been added.
Signed-off-by: Kirill Shchetiniuk <kshcheti(a)redhat.com>
---
src/ch/ch_process.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c
index 9a85f7869e..9388d5bd83 100644
--- a/src/ch/ch_process.c
+++ b/src/ch/ch_process.c
@@ -995,11 +995,14 @@ 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);
+ virErrorPreserveLast(&orig_err);
+
if (priv->monitor) {
g_clear_pointer(&priv->monitor, virCHMonitorClose);
}
@@ -1032,6 +1035,8 @@ virCHProcessStop(virCHDriver *driver,
virHostdevReAttachDomainDevices(driver->hostdevMgr, CH_DRIVER_NAME, def,
hostdev_flags);
+
+ virErrorRestore(&orig_err);
return 0;
}
--
2.48.1
Monitor Reference Counting Fix: The monitor was being unreferenced
inside virCHEventHandlerLoop, that has led to a segmentation fault.
The monitor was unreferenced after successful child thread creation
while it was still being used in the child thread. To maintain
correct reference counting, the monitor unreference has been moved
to virCHStartEventHandler (the child thread). Other monitor
unreference in virCHStartEventHandler are retained for cases where
new thread creation fails, as the reference in not used afterward.
VM Reference Counting Fix: The VM object was being unreferenced
in virCHEventHandlerLoop while it was still being used by the debug
print. To resolve this, the VM unreference has been moved to after
the debug print.
Signed-off-by: Kirill Shchetiniuk <kshcheti(a)redhat.com>
---
src/ch/ch_events.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c
index 1cce30836a..2dd3e7ecc2 100644
--- a/src/ch/ch_events.c
+++ b/src/ch/ch_events.c
@@ -287,8 +287,9 @@ virCHEventHandlerLoop(void *data)
}
g_clear_pointer(&mon->event_buffer.buffer, g_free);
- virObjectUnref(vm);
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