Thank you for giving the comments. I will separate this functionality from the qemu error file.
> 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. OK. I will use strftime(). Seiji > -----Original Message----- > From: qemu-devel-bounces+seiji.aguchi=hds....@nongnu.org > [mailto:qemu-devel-bounces+seiji.aguchi=hds....@nongnu.org] > On Behalf Of Anthony Liguori > Sent: Wednesday, May 01, 2013 10:27 AM > To: Daniel P. Berrange; Eric Blake > Cc: kw...@redhat.com; dle-deve...@lists.sourceforge.net; Seiji Aguchi; > stefa...@redhat.com; m...@redhat.com; Stefan Hajnoczi; > mtosa...@redhat.com; qemu-devel@nongnu.org; arm...@redhat.com; > av1...@comtv.ru; Tomoki Sekiyama; pbonz...@redhat.com; > lcapitul...@redhat.com; Seiji Aguchi > Subject: Re: [Qemu-devel] [RFC][PATCH v2]Add timestamp to error message > > "Daniel P. Berrange" <berra...@redhat.com> 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 > > :| >
