[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] rework daemonizing logic in qemu-nbd
From: |
Michael Tokarev |
Subject: |
Re: [Qemu-devel] [PATCH] rework daemonizing logic in qemu-nbd |
Date: |
Mon, 16 Jan 2012 11:22:34 +0400 |
User-agent: |
Mozilla/5.0 (X11; Linux i686 on x86_64; rv:5.0) Gecko/20110805 Icedove/5.0 |
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 'address@hidden'
--cc "Paolo Bonzini <address@hidden>" --cc address@hidden 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
- [Qemu-devel] [PATCH] rework daemonizing logic in qemu-nbd, Michael Tokarev, 2012/01/14
- Re: [Qemu-devel] [PATCH] rework daemonizing logic in qemu-nbd, Paolo Bonzini, 2012/01/15
- Re: [Qemu-devel] [PATCH] rework daemonizing logic in qemu-nbd, Michael Tokarev, 2012/01/15
- Re: [Qemu-devel] [PATCH] rework daemonizing logic in qemu-nbd, Paolo Bonzini, 2012/01/15
- Re: [Qemu-devel] [PATCH] rework daemonizing logic in qemu-nbd, Michael Tokarev, 2012/01/15
- Re: [Qemu-devel] [PATCH] rework daemonizing logic in qemu-nbd, Paolo Bonzini, 2012/01/15
- Re: [Qemu-devel] [PATCH] rework daemonizing logic in qemu-nbd, Paolo Bonzini, 2012/01/15
- Re: [Qemu-devel] [PATCH] rework daemonizing logic in qemu-nbd,
Michael Tokarev <=
- Re: [Qemu-devel] [PATCH] rework daemonizing logic in qemu-nbd, Paolo Bonzini, 2012/01/16