qemu-stable
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH vOther2 1/1] qemu-nbd: Restore "qemu-nbd -v --fork" output


From: Eric Blake
Subject: Re: [PATCH vOther2 1/1] qemu-nbd: Restore "qemu-nbd -v --fork" output
Date: Mon, 11 Sep 2023 09:48:57 -0500
User-agent: NeoMutt/20230517

On Sun, Sep 10, 2023 at 11:40:25AM +0300, Michael Tokarev wrote:
> 25.08.2023 23:08, Denis V. Lunev wrote:
> > +            /* Remember parent's stderr if we will be restoring it. */
> > +            if (verbose /* fork_process is set */) {
> > +                opts.stderr = dup(STDERR_FILENO);
> > +                if (opts.stderr < 0) {
> > +                    error_report("Could not dup stdedd: %s", 
> > strerror(errno));
> > +                    exit(EXIT_FAILURE);
> > +                }
> > +            }
> > +
> >               ret = qemu_daemon(1, 0);
> 
> I haven't looked closely to this development.
> 
> To me it all looks.. backwards.
> 
> Instead of saving stderr around qemu_daemon() call, it might be more
> productive to tell qemu_daemon() to stop redirecting stderr (and maybe
> instrumenting it to do so).

I tried to do that in an earlier revision of this patch, but it
changed iotest output.  We've already had a lot of churn on patches
that were supposed to fix a regression but in turn caused another
regression.

> 
> Besides, qemu has 2 daemon implementations, one is qemu_daemon()
> in util/oslib-posix.c and another is os_daemonize() in os-posix.c.
> Note os_daemonize() does the right thing wrt logging already, -
> 
>         /* In case -D is given do not redirect stderr to /dev/null */
>         if (!qemu_log_enabled()) {
>             dup2(fd, 2);
>         }
> 
> but I guess nbd does not use qemu_log_enabled() et al.
> 
> Also, qemu-nbd can benefit from using -runas/-chroot too.
> 
> Ideally this whole thing should be consolidated.  I already took a
> step towards this by moving softmmu-specific stuff from os-posix.c
> to softmmu/, this work should continue. When it's done, we can
> revert this band-aid change for a real solution.

Indeed, there may still be further cleanups to do once the os-posix.c
cleanups are in.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org




reply via email to

[Prev in Thread] Current Thread [Next in Thread]