Add a new Error parameter for vnc_display_init() to handle errors
in its caller: vnc_init_func(), just like vnc_display_open() does.
And let the call trace propagate the Error.

Besides, make vnc_start_worker_thread() return a bool to indicate
whether it succeeds instead of returning nothing.

Signed-off-by: Fei Li <fli@suse.com>
---
 include/ui/console.h |  2 +-
 ui/vnc-jobs.c        |  9 ++++++---
 ui/vnc-jobs.h        |  2 +-
 ui/vnc.c             | 12 +++++++++---
 4 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index fb969caf70..c17803c530 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -453,7 +453,7 @@ void qemu_display_early_init(DisplayOptions *opts);
 void qemu_display_init(DisplayState *ds, DisplayOptions *opts);
 
 /* vnc.c */
-void vnc_display_init(const char *id);
+void vnc_display_init(const char *id, Error **errp);
 void vnc_display_open(const char *id, Error **errp);
 void vnc_display_add_client(const char *id, int csock, bool skipauth);
 int vnc_display_password(const char *id, const char *password);
diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 929391f85d..8807d7217c 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -331,15 +331,18 @@ 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();
     qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread, q,
                        QEMU_THREAD_DETACHED);
     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 ccb1335d86..f632635ea4 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3206,7 +3206,7 @@ static const DisplayChangeListenerOps dcl_ops = {
     .dpy_cursor_define    = vnc_dpy_cursor_define,
 };
 
-void vnc_display_init(const char *id)
+void vnc_display_init(const char *id, Error **errp)
 {
     VncDisplay *vd;
 
@@ -3236,7 +3236,9 @@ void vnc_display_init(const char *id)
     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);
@@ -4079,7 +4081,11 @@ int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
     char *id = (char *)qemu_opts_id(opts);
 
     assert(id);
-    vnc_display_init(id);
+    vnc_display_init(id, &local_err);
+    if (local_err) {
+        error_reportf_err(local_err, "Failed to init VNC server: ");
+        exit(1);
+    }
     vnc_display_open(id, &local_err);
     if (local_err != NULL) {
         error_reportf_err(local_err, "Failed to start VNC server: ");
-- 
2.13.7

On Fri, 09/07 21:39, Fei Li wrote:
> Add a new Error parameter for vnc_display_init() to handle errors
> in its caller: vnc_init_func(), just like vnc_display_open() does.
> And let the call trace propagate the Error.
> 
> Besides, make vnc_start_worker_thread() return a bool to indicate
> whether it succeeds instead of returning nothing.
> 
> Signed-off-by: Fei Li <fli@suse.com>
> ---
>  include/ui/console.h |  2 +-
>  ui/vnc-jobs.c        |  9 ++++++---
>  ui/vnc-jobs.h        |  2 +-
>  ui/vnc.c             | 12 +++++++++---
>  4 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/include/ui/console.h b/include/ui/console.h
> index fb969caf70..c17803c530 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -453,7 +453,7 @@ void qemu_display_early_init(DisplayOptions *opts);
>  void qemu_display_init(DisplayState *ds, DisplayOptions *opts);
>  
>  /* vnc.c */
> -void vnc_display_init(const char *id);
> +void vnc_display_init(const char *id, Error **errp);
>  void vnc_display_open(const char *id, Error **errp);
>  void vnc_display_add_client(const char *id, int csock, bool skipauth);
>  int vnc_display_password(const char *id, const char *password);
> diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
> index 929391f85d..8807d7217c 100644
> --- a/ui/vnc-jobs.c
> +++ b/ui/vnc-jobs.c
> @@ -331,15 +331,18 @@ 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();
>      qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread, q,
>                         QEMU_THREAD_DETACHED);
>      queue = q; /* Set global queue */
> +out:
> +    return true;

This function always returns 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 ccb1335d86..f632635ea4 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -3206,7 +3206,7 @@ static const DisplayChangeListenerOps dcl_ops = {
>      .dpy_cursor_define    = vnc_dpy_cursor_define,
>  };
>  
> -void vnc_display_init(const char *id)
> +void vnc_display_init(const char *id, Error **errp)
>  {
>      VncDisplay *vd;
>  
> @@ -3236,7 +3236,9 @@ void vnc_display_init(const char *id)
>      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);
> @@ -4079,7 +4081,11 @@ int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
>      char *id = (char *)qemu_opts_id(opts);
>  
>      assert(id);
> -    vnc_display_init(id);
> +    vnc_display_init(id, &local_err);
> +    if (local_err) {
> +        error_reportf_err(local_err, "Failed to init VNC server: ");
> +        exit(1);
> +    }
>      vnc_display_open(id, &local_err);
>      if (local_err != NULL) {
>          error_reportf_err(local_err, "Failed to start VNC server: ");
> -- 
> 2.13.7
> 

Fam

On 09/12/2018 03:57 PM, Fam Zheng wrote:
> On Fri, 09/07 21:39, Fei Li wrote:
>> Add a new Error parameter for vnc_display_init() to handle errors
>> in its caller: vnc_init_func(), just like vnc_display_open() does.
>> And let the call trace propagate the Error.
>>
>> Besides, make vnc_start_worker_thread() return a bool to indicate
>> whether it succeeds instead of returning nothing.
>>
>> Signed-off-by: Fei Li <fli@suse.com>
>> ---
>>   include/ui/console.h |  2 +-
>>   ui/vnc-jobs.c        |  9 ++++++---
>>   ui/vnc-jobs.h        |  2 +-
>>   ui/vnc.c             | 12 +++++++++---
>>   4 files changed, 17 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/ui/console.h b/include/ui/console.h
>> index fb969caf70..c17803c530 100644
>> --- a/include/ui/console.h
>> +++ b/include/ui/console.h
>> @@ -453,7 +453,7 @@ void qemu_display_early_init(DisplayOptions *opts);
>>   void qemu_display_init(DisplayState *ds, DisplayOptions *opts);
>>   
>>   /* vnc.c */
>> -void vnc_display_init(const char *id);
>> +void vnc_display_init(const char *id, Error **errp);
>>   void vnc_display_open(const char *id, Error **errp);
>>   void vnc_display_add_client(const char *id, int csock, bool skipauth);
>>   int vnc_display_password(const char *id, const char *password);
>> diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
>> index 929391f85d..8807d7217c 100644
>> --- a/ui/vnc-jobs.c
>> +++ b/ui/vnc-jobs.c
>> @@ -331,15 +331,18 @@ 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();
>>       qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread, q,
>>                          QEMU_THREAD_DETACHED);
>>       queue = q; /* Set global queue */
>> +out:
>> +    return true;
> This function always returns true?
Yes, until checking qemu_thread_create()'s return value. Before applying
the checking code, qemu_thread_create still keeps abort() on failure.

Have a nice day, thanks
Fei
>
>>   }
>> 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 ccb1335d86..f632635ea4 100644
>> --- a/ui/vnc.c
>> +++ b/ui/vnc.c
>> @@ -3206,7 +3206,7 @@ static const DisplayChangeListenerOps dcl_ops = {
>>       .dpy_cursor_define    = vnc_dpy_cursor_define,
>>   };
>>   
>> -void vnc_display_init(const char *id)
>> +void vnc_display_init(const char *id, Error **errp)
>>   {
>>       VncDisplay *vd;
>>   
>> @@ -3236,7 +3236,9 @@ void vnc_display_init(const char *id)
>>       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);
>> @@ -4079,7 +4081,11 @@ int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
>>       char *id = (char *)qemu_opts_id(opts);
>>   
>>       assert(id);
>> -    vnc_display_init(id);
>> +    vnc_display_init(id, &local_err);
>> +    if (local_err) {
>> +        error_reportf_err(local_err, "Failed to init VNC server: ");
>> +        exit(1);
>> +    }
>>       vnc_display_open(id, &local_err);
>>       if (local_err != NULL) {
>>           error_reportf_err(local_err, "Failed to start VNC server: ");
>> -- 
>> 2.13.7
>>
> Fam
>
>