From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47338) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UXXzh-0001Lw-Vb for qemu-devel@nongnu.org; Wed, 01 May 2013 10:26:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UXXzg-0002yd-M3 for qemu-devel@nongnu.org; Wed, 01 May 2013 10:26:53 -0400 Received: from e24smtp01.br.ibm.com ([32.104.18.85]:50875) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UXXzg-0002yS-9v for qemu-devel@nongnu.org; Wed, 01 May 2013 10:26:52 -0400 Received: from /spool/local by e24smtp01.br.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 1 May 2013 11:26:49 -0300 Received: from d24relay01.br.ibm.com (d24relay01.br.ibm.com [9.8.31.16]) by d24dlp02.br.ibm.com (Postfix) with ESMTP id 853891DC006A for ; Wed, 1 May 2013 10:26:46 -0400 (EDT) Received: from d24av03.br.ibm.com (d24av03.br.ibm.com [9.8.31.95]) by d24relay01.br.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r41ENwwD1327142 for ; Wed, 1 May 2013 11:23:58 -0300 Received: from d24av03.br.ibm.com (loopback [127.0.0.1]) by d24av03.br.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r41CR7aA021213 for ; Wed, 1 May 2013 09:27:08 -0300 From: Anthony Liguori In-Reply-To: <20130501122500.GG13875@redhat.com> References: <1367265445-27365-1-git-send-email-saguchi@redhat.com> <20130501120550.GC28932@stefanha-thinkpad.redhat.com> <518107A1.7050906@redhat.com> <20130501122500.GG13875@redhat.com> Date: Wed, 01 May 2013 09:26:38 -0500 Message-ID: <87ppxan5cx.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [RFC][PATCH v2]Add timestamp to error message List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , Eric Blake Cc: kwolf@redhat.com, dle-develop@lists.sourceforge.net, Seiji Aguchi , stefanha@redhat.com, mst@redhat.com, Stefan Hajnoczi , mtosatti@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, av1474@comtv.ru, tomoki.sekiyama@hds.com, pbonzini@redhat.com, lcapitulino@redhat.com, Seiji Aguchi "Daniel P. Berrange" writes: > On Wed, May 01, 2013 at 06:16:33AM -0600, Eric Blake wrote: >> On 05/01/2013 06:05 AM, Stefan Hajnoczi wrote: >> >> >> + error_printf( >> >> + "%4d-%02d-%02d %02d:%02d:%02d.%03lld+0000 ", >> >> + fields.tm_year + 1900, fields.tm_mon + 1, fields.tm_mday, >> >> + fields.tm_hour, fields.tm_min, fields.tm_sec, >> >> + now % 1000); >> > >> > Can strftime(3) be used instead of copying code from glibc? >> >> No, because glibc's strftime() is not async-signal safe, and therefore >> is not safe to call between fork() and exec() (libvirt hit actual >> deadlocks[1] where a child was blocked on a mutex associated with >> time-related functions that happened to be held by another parent >> thread, but where the fork nuked that other thread so the mutex would >> never clear). This code is copied from libvirt, which copied the >> no-lock-needed safe portions of glibc for a reason. >> >> [1] https://www.redhat.com/archives/libvir-list/2011-November/msg01609.html > > NB the original libvirt code had this & other related functions in > a standalone file, along with a significant test suite. I think it > is better from a maintenence POV to keep this functionality in a > file that's separate from the qemu error file, so it can be reused > elsewhere in QEMU if needed. It should also copy the test suite, > since it is bad practice to throw away tests which already exist. I also don't buy that we can't use strftime. There are very few places where we use fork() in QEMU (it doesn't exist on Windows so it can't be used without protection). None of those places use the error reporting infrastructure. This code is also extremely naive. It doesn't take into account leap seconds and makes bad assumptions about leap years. Regards, Anthony Liguori > > Daniel > -- > |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- http://virt-manager.org :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| > |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|