Re: [Qemu-devel] [RFC][PATCH v2]Add timestamp to error message

[Seiji Aguchi]

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: address@hidden [mailto:address@hidden
> On Behalf Of Anthony Liguori
> Sent: Wednesday, May 01, 2013 10:27 AM
> To: Daniel P. Berrange; Eric Blake
> Cc: address@hidden; address@hidden; Seiji Aguchi; address@hidden; 
> address@hidden; Stefan Hajnoczi;
> address@hidden; address@hidden; address@hidden; address@hidden; Tomoki 
> Sekiyama; address@hidden;
> address@hidden; Seiji Aguchi
> Subject: Re: [Qemu-devel] [RFC][PATCH v2]Add timestamp to error message
> 
> "Daniel P. Berrange" <address@hidden> 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 
> > :|
>