在 2019/1/9 上午12:18, fei 写道:
在 2019年1月8日,01:50,Markus Armbruster <arm...@redhat.com> 写道:
Fei Li <f...@suse.com> writes:
For iothread_complete: utilize the existed errp to propagate the
error and do the corresponding cleanup to replace the temporary
&error_abort.
For qemu_signalfd_compat: add a local_err to hold the error, and
return the corresponding error code to replace the temporary
&error_abort.
I'd split the patch.
Ok.
Cc: Markus Armbruster <arm...@redhat.com>
Cc: Eric Blake <ebl...@redhat.com>
Signed-off-by: Fei Li <f...@suse.com>
---
iothread.c | 17 +++++++++++------
util/compatfd.c | 11 ++++++++---
2 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/iothread.c b/iothread.c
index 8e8aa01999..7335dacf0b 100644
--- a/iothread.c
+++ b/iothread.c
@@ -164,9 +164,7 @@ static void iothread_complete(UserCreatable *obj, Error
**errp)
&local_error);
if (local_error) {
error_propagate(errp, local_error);
- aio_context_unref(iothread->ctx);
- iothread->ctx = NULL;
- return;
+ goto fail;
}
qemu_mutex_init(&iothread->init_done_lock);
@@ -178,9 +176,12 @@ static void iothread_complete(UserCreatable *obj, Error
**errp)
*/
name = object_get_canonical_path_component(OBJECT(obj));
thread_name = g_strdup_printf("IO %s", name);
- /* TODO: let the further caller handle the error instead of abort() here */
- qemu_thread_create(&iothread->thread, thread_name, iothread_run,
- iothread, QEMU_THREAD_JOINABLE, &error_abort);
+ if (!qemu_thread_create(&iothread->thread, thread_name, iothread_run,
+ iothread, QEMU_THREAD_JOINABLE, errp)) {
+ g_free(thread_name);
+ g_free(name);
I suspect you're missing cleanup here:
qemu_cond_destroy(&iothread->init_done_cond);
qemu_mutex_destroy(&iothread->init_done_lock);
I remember I checked the code, when ucc->complete() fails, there’s a finalize()
function to do the destroy.
To be specific, the qemu_xxx_destroy() is called by
object_unref() => object_finalize() => object_deinit() =>
type->instance_finalize(obj); (that is, iothread_instance_finalize).
For the iothread_complete(), it is only called in
user_creatable_complete() as ucc->complete().
I checked the code, when callers of user_creatable_complete() fails, all
of them will call
object_unref() to call the qemu_xxx_destroy(), except one &error_abort
case (e.i. desugar_shm()).
But did not test all the callers, so let’s wait for Stefan’s feedback. :)
But again, I did not do all the test. Correct me if I am wrong. :)
But I'm not 100% sure, to be honest. Stefan, can you help?
+ goto fail;
+ }
g_free(thread_name);
g_free(name);
I'd avoid the code duplication like this:
thread_ok = qemu_thread_create(&iothread->thread, thread_name,
iothread_run, iothread,
QEMU_THREAD_JOINABLE, errp);
g_free(thread_name);
g_free(name);
if (!thread_ok) {
qemu_cond_destroy(&iothread->init_done_cond);
qemu_mutex_destroy(&iothread->init_done_lock);
goto fail;
}
Ok, thanks.
Have a nice day
Fei
Matter of taste.
Hmm, iothread.c has no maintainer. Stefan, you created it, would you be
willing to serve as maintainer?