Hi,
On 11/05/2018 09:32 PM, Juan Quintela wrote:
Fei Li <f...@suse.com> wrote:
When qemu_signal_init() fails in qemu_init_main_loop(), we return
without setting an error. Its callers crash then when they try to
report the error with error_report_err().
To avoid such segmentation fault, add a new Error parameter to make
the call trace to propagate the err to the final caller.
Hi
I agree that there is a bug that exist here. But I think that the patch
is not 100% correct. What is the warrantee that when we call
qemu_signal_init() errp is not *already* assigned.
I think that we need to use here the same code that in the call to
aio_context_new() ...
i.e.
intsead of this
init_clocks(qemu_timer_notify_cb);
- ret = qemu_signal_init();
+ ret = qemu_signal_init(errp);
if (ret) {
return ret;
}
init_clocks(qemu_timer_notify_cb);
ret = qemu_signal_init();
ret = qemu_signal_init(&local_error);
if (ret) {
error_propagate(errp, local_error);
return ret;
}
This way it works correctly if errp is NULL, errp is already assigned,
etc, etc,
Or I am missing something?
Later, Juan.
We have discussed this in the first round of this patch series, just as
Daniel
and Fam said, we only need the local_err & error_propagate() when functions
like object_new_with_propv() returns void, in that way we need the
&local_err to
check whether that function succeeds.
But in qemu_signal_init, we have the "if (ret) {...}" to judge whether
it succeeds.
For more details, the following threads can be referred:
09/04/2018 07:26 PM
Re: [Qemu-devel] [PATCH 1/5] Fix segmentation fault when
qemu_signal_init fails
BTW, if qemu_signalfd() fails, we use an "error_setg_errno()" to handle:
- for NULL errp, we just set the error message to errp;
- for not-NULL errp, besides the error_setv() we have the
error_handle_fatal(errp, err).
If the passed errp is &error_fatal/&error_abort, qemu will exit(1)
right here.
Have a nice day, thanks :)
Fei
