Re: [Qemu-devel] [PATCH v2 2/4] ui/vnc.c: polish vnc_init_func

2018-09-13 Thread Fei Li 




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 
---
  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(>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(>mutex);

-vnc_start_worker_thread();
+if (!vnc_start_worker_thread(errp)) {
+return;
+}
  
  vd->dcl.ops = _ops;

  register_displaychangelistener(>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, _err);
+if (local_err) {
+error_reportf_err(local_err, "Failed to init VNC server: ");
+exit(1);
+}
  vnc_display_open(id, _err);
  if (local_err != NULL) {
  error_reportf_err(local_err, "Failed to start VNC server: ");
--
2.13.7


Fam

Re: [Qemu-devel] [PATCH v2 2/4] ui/vnc.c: polish vnc_init_func

2018-09-12 Thread Fam Zheng 
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 
> ---
>  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(>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(>mutex);
> -vnc_start_worker_thread();
> +if (!vnc_start_worker_thread(errp)) {
> +return;
> +}
>  
>  vd->dcl.ops = _ops;
>  register_displaychangelistener(>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, _err);
> +if (local_err) {
> +error_reportf_err(local_err, "Failed to init VNC server: ");
> +exit(1);
> +}
>  vnc_display_open(id, _err);
>  if (local_err != NULL) {
>  error_reportf_err(local_err, "Failed to start VNC server: ");
> -- 
> 2.13.7
> 

Fam

[Qemu-devel] [PATCH v2 2/4] ui/vnc.c: polish vnc_init_func

2018-09-07 Thread Fei Li 
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 
---
 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(>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(>mutex);
-vnc_start_worker_thread();
+if (!vnc_start_worker_thread(errp)) {
+return;
+}
 
 vd->dcl.ops = _ops;
 register_displaychangelistener(>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, _err);
+if (local_err) {
+error_reportf_err(local_err, "Failed to init VNC server: ");
+exit(1);
+}
 vnc_display_open(id, _err);
 if (local_err != NULL) {
 error_reportf_err(local_err, "Failed to start VNC server: ");
-- 
2.13.7