From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:37558)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <den@openvz.org>) id 1abUCB-0005kj-D6
	for qemu-devel@nongnu.org; Thu, 03 Mar 2016 09:25:40 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <den@openvz.org>) id 1abUC5-0007gh-Q6
	for qemu-devel@nongnu.org; Thu, 03 Mar 2016 09:25:39 -0500
Received: from mx2.parallels.com ([199.115.105.18]:50556)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <den@openvz.org>) id 1abUC5-0007gd-CT
	for qemu-devel@nongnu.org; Thu, 03 Mar 2016 09:25:33 -0500
References: <1457012886-7626-1-git-send-email-den@openvz.org>
	<56D840D4.4010109@redhat.com> <56D841C6.1060303@openvz.org>
	<56D84487.5070009@redhat.com> <56D84555.7090003@openvz.org>
	<56D8470C.5000201@redhat.com>
From: "Denis V. Lunev" <den@openvz.org>
Message-ID: <56D84955.6070508@openvz.org>
Date: Thu, 3 Mar 2016 17:25:25 +0300
MIME-Version: 1.0
In-Reply-To: <56D8470C.5000201@redhat.com>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 1/1] log: fix hanged connect from
	virt-manager to libvirt
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>, qemu-devel@nongnu.org, Dimitris Aragiorgis <dimara@arrikto.com>

On 03/03/2016 05:15 PM, Paolo Bonzini wrote:
>
> On 03/03/2016 15:08, Denis V. Lunev wrote:
>> On 03/03/2016 05:04 PM, Paolo Bonzini wrote:
>>> On 03/03/2016 14:53, Denis V. Lunev wrote:
>>>>> A patch has been posted already, and I'll send a pull request tomorrow.
>>>> I have not seen it. Can you post a link?
>>> [PATCH] log: do not log if QEMU is daemonized but without -D
>>>
>>> but I found a missing case that your patch handles, so I'll post a v2
>>> now.
>>>
>>> Paolo
>> that is fine :) let's see. Though I think that resulted code is more
>> readable in my approach.
> Yes, I was going for the smallest change.  Cleanups can be done on top.
>
> Actually, the patch in v1 is fine.  My worry after looking at your patch
> was that I didn't have the dup2(stdout, stderr) case.  However, with my
> change you can never call qemu_log_close if is_daemonized(), because
> even the monitor command "logfile" cannot set logfilename to NULL.
>
> Paolo
IMHO you are wrong.

void qemu_set_log_filename(const char *filename)
{
     g_free(logfilename);
     logfilename = g_strdup(filename);
     qemu_log_close();
     qemu_set_log(qemu_loglevel);
}

static void hmp_logfile(Monitor *mon, const QDict *qdict)
{
     qemu_set_log_filename(qdict_get_str(qdict, "filename"));
}

This means that we will have qemu_log_close()
called in ANY case, even daemonized.

 From my point of view stderr will continue to be mapped
to the old file if we request to stop logging either by
zero mask or by setting empty filename.

Den