[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC][PATCH v2]Add timestamp to error message
From: |
Seiji Aguchi |
Subject: |
Re: [Qemu-devel] [RFC][PATCH v2]Add timestamp to error message |
Date: |
Thu, 2 May 2013 20:39:56 +0000 |
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
> > :|
>