diff mbox series

[3/5] qemu-nbd: properly report error on error in dup2() after qemu_daemon()

Message ID 20230717145544.194786-4-den@openvz.org (mailing list archive)
State New, archived
Headers show
Series qemu-nbd: fix regression with qemu-nbd --fork run over ssh | expand

Commit Message

Denis V. Lunev July 17, 2023, 2:55 p.m. UTC
We are trying to temporary redirect stderr of daemonized process to
a pipe to report a error and get failed. In that case we could not
use error_report() helper, but should write the message directly into
the problematic pipe.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Eric Blake <eblake@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 qemu-nbd.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Eric Blake July 18, 2023, 5:45 p.m. UTC | #1
On Mon, Jul 17, 2023 at 04:55:42PM +0200, Denis V. Lunev wrote:
> We are trying to temporary redirect stderr of daemonized process to

temporarily

> a pipe to report a error and get failed. In that case we could not
> use error_report() helper, but should write the message directly into
> the problematic pipe.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Eric Blake <eblake@redhat.com>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  qemu-nbd.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 186ce9474c..4450cc826b 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -937,7 +937,20 @@ int main(int argc, char **argv)
>              ret = qemu_daemon(1, 0);
>

errno from qemu_daemon...

>              /* Temporarily redirect stderr to the parent's pipe...  */
> -            dup2(stderr_fd[1], STDERR_FILENO);
> +            if (dup2(stderr_fd[1], STDERR_FILENO) < 0) {

...is still potentially lost here...

> +                char str[256];
> +                snprintf(str, sizeof(str),
> +                         "%s: Failed to link stderr to the pipe: %s\n",
> +                         g_get_prgname(), strerror(errno));
> +                /*
> +                 * We are unable to use error_report() here as we need to get
> +                 * stderr pointed to the parent's pipe. Write to that pipe
> +                 * manually.
> +                 */
> +                ret = write(stderr_fd[1], str, strlen(str));
> +                exit(EXIT_FAILURE);
> +            }
> +
>              if (ret < 0) {
>                  error_report("Failed to daemonize: %s", strerror(errno));

...before use here.

Patch 4/5 addresses that, but I'm inclined to just swap the order of
the two patches when applying the series in my NBD tree.

Reviewed-by: Eric Blake <eblake@redhat.com>
diff mbox series

Patch

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 186ce9474c..4450cc826b 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -937,7 +937,20 @@  int main(int argc, char **argv)
             ret = qemu_daemon(1, 0);
 
             /* Temporarily redirect stderr to the parent's pipe...  */
-            dup2(stderr_fd[1], STDERR_FILENO);
+            if (dup2(stderr_fd[1], STDERR_FILENO) < 0) {
+                char str[256];
+                snprintf(str, sizeof(str),
+                         "%s: Failed to link stderr to the pipe: %s\n",
+                         g_get_prgname(), strerror(errno));
+                /*
+                 * We are unable to use error_report() here as we need to get
+                 * stderr pointed to the parent's pipe. Write to that pipe
+                 * manually.
+                 */
+                ret = write(stderr_fd[1], str, strlen(str));
+                exit(EXIT_FAILURE);
+            }
+
             if (ret < 0) {
                 error_report("Failed to daemonize: %s", strerror(errno));
                 exit(EXIT_FAILURE);