| Message ID | 20181225140449.15786-9-fli@suse.com |
|---|---|
| State | New |
| Headers | show |
| Series | [for-4.0,v9,01/16] Fix segmentation fault when qemu_signal_init fails | expand |
Fei Li <fli@suse.com> writes: > Utilize the existed errp to propagate the error instead of the > temporary &error_abort. > > Cc: Markus Armbruster <armbru@redhat.com> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com> > Signed-off-by: Fei Li <fli@suse.com> > --- > dump.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/dump.c b/dump.c > index c35d6ddd22..ef5ea324fa 100644 > --- a/dump.c > +++ b/dump.c > @@ -2020,9 +2020,10 @@ void qmp_dump_guest_memory(bool paging, const char *file, > if (detach_p) { > /* detached dump */ > s->detached = true; > - /* TODO: let the further caller handle the error instead of abort() */ > - qemu_thread_create(&s->dump_thread, "dump_thread", dump_thread, > - s, QEMU_THREAD_DETACHED, &error_abort); > + if (!qemu_thread_create(&s->dump_thread, "dump_thread", dump_thread, > + s, QEMU_THREAD_DETACHED, errp)) { > + /* keep 'if' here in case there is further error handling logic */ > + } I don't think keeping the conditional "just in case" is worthwhile. Plain qemu_thread_create(&s->dump_thread, "dump_thread", dump_thread, s, QEMU_THREAD_DETACHED, errp); should do fine. > } else { > /* sync dump */ > dump_process(s, errp);
> 在 2019年1月8日，01:21，Markus Armbruster <armbru@redhat.com> 写道： > > Fei Li <fli@suse.com> writes: > >> Utilize the existed errp to propagate the error instead of the >> temporary &error_abort. >> >> Cc: Markus Armbruster <armbru@redhat.com> >> Cc: Marc-André Lureau <marcandre.lureau@redhat.com> >> Signed-off-by: Fei Li <fli@suse.com> >> --- >> dump.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/dump.c b/dump.c >> index c35d6ddd22..ef5ea324fa 100644 >> --- a/dump.c >> +++ b/dump.c >> @@ -2020,9 +2020,10 @@ void qmp_dump_guest_memory(bool paging, const char *file, >> if (detach_p) { >> /* detached dump */ >> s->detached = true; >> - /* TODO: let the further caller handle the error instead of abort() */ >> - qemu_thread_create(&s->dump_thread, "dump_thread", dump_thread, >> - s, QEMU_THREAD_DETACHED, &error_abort); >> + if (!qemu_thread_create(&s->dump_thread, "dump_thread", dump_thread, >> + s, QEMU_THREAD_DETACHED, errp)) { >> + /* keep 'if' here in case there is further error handling logic */ >> + } > > I don't think keeping the conditional "just in case" is worthwhile. > Plain > > qemu_thread_create(&s->dump_thread, "dump_thread", dump_thread, > s, QEMU_THREAD_DETACHED, errp); > > should do fine. Ok, will simplify this in next version. Have a nice day, thanks Fei > >> } else { >> /* sync dump */ >> dump_process(s, errp);
diff --git a/dump.c b/dump.c index c35d6ddd22..ef5ea324fa 100644 --- a/dump.c +++ b/dump.c @@ -2020,9 +2020,10 @@ void qmp_dump_guest_memory(bool paging, const char *file, if (detach_p) { /* detached dump */ s->detached = true; - /* TODO: let the further caller handle the error instead of abort() */ - qemu_thread_create(&s->dump_thread, "dump_thread", dump_thread, - s, QEMU_THREAD_DETACHED, &error_abort); + if (!qemu_thread_create(&s->dump_thread, "dump_thread", dump_thread, + s, QEMU_THREAD_DETACHED, errp)) { + /* keep 'if' here in case there is further error handling logic */ + } } else { /* sync dump */ dump_process(s, errp);
Utilize the existed errp to propagate the error instead of the temporary &error_abort. Cc: Markus Armbruster <armbru@redhat.com> Cc: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Fei Li <fli@suse.com> --- dump.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)