From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:37683) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rmgtq-0000uj-Bm for qemu-devel@nongnu.org; Mon, 16 Jan 2012 02:22:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Rmgtp-0001uJ-0N for qemu-devel@nongnu.org; Mon, 16 Jan 2012 02:22:38 -0500 Received: from isrv.corpit.ru ([86.62.121.231]:60966) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rmgto-0001uC-O5 for qemu-devel@nongnu.org; Mon, 16 Jan 2012 02:22:36 -0500 Message-ID: <4F13D03A.7020303@msgid.tls.msk.ru> Date: Mon, 16 Jan 2012 11:22:34 +0400 From: Michael Tokarev MIME-Version: 1.0 References: <20120114124210.E43483DE@gandalf.tls.msk.ru> <4F12CB7C.308@msgid.tls.msk.ru> <4F12FAA7.5000103@redhat.com> <4F130261.4010205@msgid.tls.msk.ru> <4F130D78.1070903@redhat.com> In-Reply-To: <4F130D78.1070903@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] rework daemonizing logic in qemu-nbd List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org On 15.01.2012 21:31, Paolo Bonzini wrote: > On 01/15/2012 05:44 PM, Michael Tokarev wrote: >>>>>> + * stdout (temporarily) to the pipe to parent, >>>>> >>>>> This is a bit of a hack. >>>> >>>> There's another way -- to keep the writing pipe end in some >>>> local variable and use that one instead of STDOUT_FILENO. >>>> I can do it that way for sure, just thought it's already >>>> using too much local variables. >>> >>> Yes, that would be better. >> >> Done in a v2 version I sent you. > > Please stay on the list. Sorry? I sent it to you and to the list, here's the command line from my .bash_history: git format-patch --subject-prefix="PATCH v2" --stdout --to 'qemu-devel@nongnu.org' --cc "Paolo Bonzini " --cc mjt@tls.msk.ru HEAD^ | /usr/sbin/sendmail -t -i On which list I shoult stay? [] >> Um, I missed that "half of this" part. Indeed, nbd_client_thread() >> does dup2(STDOUT_FILENO, STDERR_FILENO) which should go away, but >> it is harmless for now, and can be addressed in a separate patch. > > Again, _the client thread_ is the right place to do this! See below. [] >> We're doomed anyway, and it is even good >> we've a small remote chance for our error message to >> be seen. Currently it just goes to /dev/null. > > No, currently it is sent from the daemon to the parent through the pipe, the parent prints it and exits with status code 1. With your patch, if the dup2 wins the race you exit with status code 0; if the client thread wins the race it is the same as master. Aha. I finally see what you mean. I still disagree, -- all the operations done in the client thread can be done before forking a new thread, syncronously, and _that_ will be the easiest and cleanest solution here. >> That's not a bad intention. I'm fixing existing logic without >> introducing new logical changes. If you want to fix other >> stuff, it is better be done in a separate commit/change. > > AFAIK the only known bug (besides the devfd/sockfd mixup) is the missing chdir, and that should be fixed first. It all looked so ugly to me that I didn't even want to think about just adding a chdir() instead of getting rid of daemon(). But ok, I can go that ugly route too. Thanks, /mjt