[PATCH] qsd: Do not use error_report() before monitor_init

Hanna Reitz posted 1 patch 1 year, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220609122852.21140-1-hreitz@redhat.com
Maintainers: Kevin Wolf <kwolf@redhat.com>
storage-daemon/qemu-storage-daemon.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
[PATCH] qsd: Do not use error_report() before monitor_init
Posted by Hanna Reitz 1 year, 11 months ago
error_report() only works once monitor_init_globals_core() has been
called, which is not the case when parsing the --daemonize option.  Use
fprintf(stderr, ...) instead.

Fixes: 2525edd85fec53e23fda98974a15e3b3c8957596 ("qsd: Add --daemonize")
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
---
 storage-daemon/qemu-storage-daemon.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index c104817cdd..0890495c40 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -286,7 +286,11 @@ static void process_options(int argc, char *argv[], bool pre_init_pass)
             }
         case OPTION_DAEMONIZE:
             if (os_set_daemonize(true) < 0) {
-                error_report("--daemonize not supported in this build");
+                /*
+                 * --daemonize is parsed before monitor_init_globals_core(), so
+                 * error_report() does not work yet
+                 */
+                fprintf(stderr, "--daemonize not supported in this build\n");
                 exit(EXIT_FAILURE);
             }
             break;
-- 
2.35.3
Re: [PATCH] qsd: Do not use error_report() before monitor_init
Posted by Peter Maydell 1 year, 11 months ago
On Thu, 9 Jun 2022 at 15:26, Hanna Reitz <hreitz@redhat.com> wrote:
>
> error_report() only works once monitor_init_globals_core() has been
> called, which is not the case when parsing the --daemonize option.  Use
> fprintf(stderr, ...) instead.
>
> Fixes: 2525edd85fec53e23fda98974a15e3b3c8957596 ("qsd: Add --daemonize")
> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
> ---
>  storage-daemon/qemu-storage-daemon.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
> index c104817cdd..0890495c40 100644
> --- a/storage-daemon/qemu-storage-daemon.c
> +++ b/storage-daemon/qemu-storage-daemon.c
> @@ -286,7 +286,11 @@ static void process_options(int argc, char *argv[], bool pre_init_pass)
>              }
>          case OPTION_DAEMONIZE:
>              if (os_set_daemonize(true) < 0) {
> -                error_report("--daemonize not supported in this build");
> +                /*
> +                 * --daemonize is parsed before monitor_init_globals_core(), so
> +                 * error_report() does not work yet
> +                 */
> +                fprintf(stderr, "--daemonize not supported in this build\n");
>                  exit(EXIT_FAILURE);
>              }
>              break;

Is it worth making error_report() have code for "called in early startup
before monitor was initialized" that falls back to "just print to stderr"?
Having our standard error reporting function not be usable everywhere is
kind of surprising...

-- PMM
Re: [PATCH] qsd: Do not use error_report() before monitor_init
Posted by Philippe Mathieu-Daudé via 1 year, 11 months ago
On 9/6/22 14:28, Hanna Reitz wrote:
> error_report() only works once monitor_init_globals_core() has been
> called, which is not the case when parsing the --daemonize option.  Use
> fprintf(stderr, ...) instead.
> 
> Fixes: 2525edd85fec53e23fda98974a15e3b3c8957596 ("qsd: Add --daemonize")
> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
> ---
>   storage-daemon/qemu-storage-daemon.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>