Re: [Qemu-devel] [PATCH 3/4] pidfile: stop making pidfile error a special case

[Gonglei]

On 2014/10/30 23:07, Michael Tokarev wrote:

> In case of -daemonize, we write non-zero to the daemon
> pipe only if pidfile creation failed, so the parent will
> report error about pidfile problem.  There's no need to
> make special case for this, since all other errors are
> reported by the child just fine.  Let the parent report
> error and simplify logic in os_daemonize().
> 
> This way, we don't need os_pidfile_error() function, since
> it only prints error now, so put the error reporting printf
> into the only place where qemu_create_pidfile() is called,
> in vl.c.
> 
> While at it, fix wrong identation in os_daemonize().

s/identation/identification/

> 
> Signed-off-by: Michael Tokarev <address@hidden>
> ---
>  include/qemu-common.h |    1 -
>  os-posix.c            |   29 ++++++-----------------------
>  os-win32.c            |    5 -----
>  vl.c                  |    2 +-
>  4 files changed, 7 insertions(+), 30 deletions(-)
> 
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index b87e9c2..f862214 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -357,7 +357,6 @@ char *qemu_find_file(int type, const char *name);
>  void os_setup_early_signal_handling(void);
>  char *os_find_datadir(void);
>  void os_parse_cmd_args(int index, const char *optarg);
> -void os_pidfile_error(void);
>  
>  /* Convert a byte between binary and BCD.  */
>  static inline uint8_t to_bcd(uint8_t val)
> diff --git a/os-posix.c b/os-posix.c
> index eada8d4..a3b96d9 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -221,18 +221,12 @@ void os_daemonize(void)
>              do {
>                  len = read(fds[0], &status, 1);
>              } while (len < 0 && errno == EINTR);
> -            if (len != 1) {
> -                exit(1);
> -            }

Does this check need to be removed?

> -            else if (status == 1) {
> -                fprintf(stderr, "Could not acquire pidfile\n");
> -                exit(1);
> -            } else {
> -                exit(0);
> -            }
> -            } else if (pid < 0) {
> -                exit(1);
> -            }
> +
> +            exit(len == 1 && status == 0 ? 0 : 1);
> +
> +        } else if (pid < 0) {
> +            exit(1);
> +        }
>  
>          close(fds[0]);
>          daemon_pipe = fds[1];
> @@ -290,17 +284,6 @@ void os_setup_post(void)
>      }
>  }
>  
> -void os_pidfile_error(void)
> -{
> -    if (daemonize) {
> -        uint8_t status = 1;
> -        if (write(daemon_pipe, &status, 1) != 1) {
> -            perror("daemonize. Writing to pipe\n");
> -        }
> -    } else
> -        fprintf(stderr, "Could not acquire pid file: %s\n", strerror(errno));
> -}
> -
>  void os_set_line_buffering(void)
>  {
>      setvbuf(stdout, NULL, _IOLBF, 0);
> diff --git a/os-win32.c b/os-win32.c
> index 5f95caa..c0daf8e 100644
> --- a/os-win32.c
> +++ b/os-win32.c
> @@ -104,11 +104,6 @@ void os_parse_cmd_args(int index, const char *optarg)
>      return;
>  }
>  
> -void os_pidfile_error(void)
> -{
> -    fprintf(stderr, "Could not acquire pid file: %s\n", strerror(errno));
> -}
> -
>  int qemu_create_pidfile(const char *filename)
>  {
>      char buffer[128];
> diff --git a/vl.c b/vl.c
> index f6b3546..150524c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3999,7 +3999,7 @@ int main(int argc, char **argv, char **envp)
>  #endif
>  
>      if (pid_file && qemu_create_pidfile(pid_file) != 0) {
> -        os_pidfile_error();
> +        fprintf(stderr, "Could not acquire pid file: %s\n", strerror(errno));
>          exit(1);
>      }
>