Message ID | 20181225140449.15786-15-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: > Supplement the error handling for vnc_thread_worker_thread: add > an Error parameter for it to propagate the error to its caller to > handle in case it fails, and make it return a Boolean to indicate > whether it succeeds. > > Cc: Markus Armbruster <armbru@redhat.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Signed-off-by: Fei Li <fli@suse.com> > --- > ui/vnc-jobs.c | 17 +++++++++++------ > ui/vnc-jobs.h | 2 +- > ui/vnc.c | 4 +++- > 3 files changed, 15 insertions(+), 8 deletions(-) > > diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c > index 5712f1f501..35a652d1fd 100644 > --- a/ui/vnc-jobs.c > +++ b/ui/vnc-jobs.c > @@ -332,16 +332,21 @@ static bool vnc_worker_thread_running(void) > return queue; /* Check global queue */ > } > > -void vnc_start_worker_thread(void) > +bool vnc_start_worker_thread(Error **errp) > { > VncJobQueue *q; > > - if (vnc_worker_thread_running()) > - return ; > + if (vnc_worker_thread_running()) { > + goto out; Why not simply return true? > + } > > q = vnc_queue_init(); > - /* TODO: let the further caller handle the error instead of abort() here */ > - qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread, > - q, QEMU_THREAD_DETACHED, &error_abort); > + if (!qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread, > + q, QEMU_THREAD_DETACHED, errp)) { > + vnc_queue_clear(q); > + return false; > + } > queue = q; /* Set global queue */ > +out: > + return true; > } > diff --git a/ui/vnc-jobs.h b/ui/vnc-jobs.h > index 59f66bcc35..14640593db 100644 > --- a/ui/vnc-jobs.h > +++ b/ui/vnc-jobs.h > @@ -37,7 +37,7 @@ void vnc_job_push(VncJob *job); > void vnc_jobs_join(VncState *vs); > > void vnc_jobs_consume_buffer(VncState *vs); > -void vnc_start_worker_thread(void); > +bool vnc_start_worker_thread(Error **errp); > > /* Locks */ > static inline int vnc_trylock_display(VncDisplay *vd) > diff --git a/ui/vnc.c b/ui/vnc.c > index 0c1b477425..0ffe9e6a5d 100644 > --- a/ui/vnc.c > +++ b/ui/vnc.c > @@ -3236,7 +3236,9 @@ void vnc_display_init(const char *id, Error **errp) > vd->connections_limit = 32; > > qemu_mutex_init(&vd->mutex); > - vnc_start_worker_thread(); > + if (!vnc_start_worker_thread(errp)) { > + return; > + } > > vd->dcl.ops = &dcl_ops; > register_displaychangelistener(&vd->dcl);
> 在 2019年1月8日,01:54,Markus Armbruster <armbru@redhat.com> 写道: > > Fei Li <fli@suse.com> writes: > >> Supplement the error handling for vnc_thread_worker_thread: add >> an Error parameter for it to propagate the error to its caller to >> handle in case it fails, and make it return a Boolean to indicate >> whether it succeeds. >> >> Cc: Markus Armbruster <armbru@redhat.com> >> Cc: Gerd Hoffmann <kraxel@redhat.com> >> Signed-off-by: Fei Li <fli@suse.com> >> --- >> ui/vnc-jobs.c | 17 +++++++++++------ >> ui/vnc-jobs.h | 2 +- >> ui/vnc.c | 4 +++- >> 3 files changed, 15 insertions(+), 8 deletions(-) >> >> diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c >> index 5712f1f501..35a652d1fd 100644 >> --- a/ui/vnc-jobs.c >> +++ b/ui/vnc-jobs.c >> @@ -332,16 +332,21 @@ static bool vnc_worker_thread_running(void) >> return queue; /* Check global queue */ >> } >> >> -void vnc_start_worker_thread(void) >> +bool vnc_start_worker_thread(Error **errp) >> { >> VncJobQueue *q; >> >> - if (vnc_worker_thread_running()) >> - return ; >> + if (vnc_worker_thread_running()) { >> + goto out; > > Why not simply return true? Sounds right.. Will remove the below “out:” too. Have a nice day, thanks Fei > >> + } >> >> q = vnc_queue_init(); >> - /* TODO: let the further caller handle the error instead of abort() here */ >> - qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread, >> - q, QEMU_THREAD_DETACHED, &error_abort); >> + if (!qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread, >> + q, QEMU_THREAD_DETACHED, errp)) { >> + vnc_queue_clear(q); >> + return false; >> + } >> queue = q; /* Set global queue */ >> +out: >> + return true; >> } >> diff --git a/ui/vnc-jobs.h b/ui/vnc-jobs.h >> index 59f66bcc35..14640593db 100644 >> --- a/ui/vnc-jobs.h >> +++ b/ui/vnc-jobs.h >> @@ -37,7 +37,7 @@ void vnc_job_push(VncJob *job); >> void vnc_jobs_join(VncState *vs); >> >> void vnc_jobs_consume_buffer(VncState *vs); >> -void vnc_start_worker_thread(void); >> +bool vnc_start_worker_thread(Error **errp); >> >> /* Locks */ >> static inline int vnc_trylock_display(VncDisplay *vd) >> diff --git a/ui/vnc.c b/ui/vnc.c >> index 0c1b477425..0ffe9e6a5d 100644 >> --- a/ui/vnc.c >> +++ b/ui/vnc.c >> @@ -3236,7 +3236,9 @@ void vnc_display_init(const char *id, Error **errp) >> vd->connections_limit = 32; >> >> qemu_mutex_init(&vd->mutex); >> - vnc_start_worker_thread(); >> + if (!vnc_start_worker_thread(errp)) { >> + return; >> + } >> >> vd->dcl.ops = &dcl_ops; >> register_displaychangelistener(&vd->dcl);
diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c index 5712f1f501..35a652d1fd 100644 --- a/ui/vnc-jobs.c +++ b/ui/vnc-jobs.c @@ -332,16 +332,21 @@ static bool vnc_worker_thread_running(void) return queue; /* Check global queue */ } -void vnc_start_worker_thread(void) +bool vnc_start_worker_thread(Error **errp) { VncJobQueue *q; - if (vnc_worker_thread_running()) - return ; + if (vnc_worker_thread_running()) { + goto out; + } q = vnc_queue_init(); - /* TODO: let the further caller handle the error instead of abort() here */ - qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread, - q, QEMU_THREAD_DETACHED, &error_abort); + if (!qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread, + q, QEMU_THREAD_DETACHED, errp)) { + vnc_queue_clear(q); + return false; + } queue = q; /* Set global queue */ +out: + return true; } diff --git a/ui/vnc-jobs.h b/ui/vnc-jobs.h index 59f66bcc35..14640593db 100644 --- a/ui/vnc-jobs.h +++ b/ui/vnc-jobs.h @@ -37,7 +37,7 @@ void vnc_job_push(VncJob *job); void vnc_jobs_join(VncState *vs); void vnc_jobs_consume_buffer(VncState *vs); -void vnc_start_worker_thread(void); +bool vnc_start_worker_thread(Error **errp); /* Locks */ static inline int vnc_trylock_display(VncDisplay *vd) diff --git a/ui/vnc.c b/ui/vnc.c index 0c1b477425..0ffe9e6a5d 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -3236,7 +3236,9 @@ void vnc_display_init(const char *id, Error **errp) vd->connections_limit = 32; qemu_mutex_init(&vd->mutex); - vnc_start_worker_thread(); + if (!vnc_start_worker_thread(errp)) { + return; + } vd->dcl.ops = &dcl_ops; register_displaychangelistener(&vd->dcl);
Supplement the error handling for vnc_thread_worker_thread: add an Error parameter for it to propagate the error to its caller to handle in case it fails, and make it return a Boolean to indicate whether it succeeds. Cc: Markus Armbruster <armbru@redhat.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Fei Li <fli@suse.com> --- ui/vnc-jobs.c | 17 +++++++++++------ ui/vnc-jobs.h | 2 +- ui/vnc.c | 4 +++- 3 files changed, 15 insertions(+), 8 deletions(-)