All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Jiri Pirko <jiri@resnulli.us>,
	qemu-block@nongnu.org, Juan Quintela <quintela@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	Jason Wang <jasowang@redhat.com>,
	qemu-devel@nongnu.org, Laurent Vivier <laurent@vivier.eu>,
	virtio-fs@redhat.com, Gerd Hoffmann <kraxel@redhat.com>,
	Max Reitz <mreitz@redhat.com>, Brad Smith <brad@comstyle.com>
Subject: Re: [Virtio-fs] [PATCH 7/7] virtiofsd: use GDateTime for formatting timestamp for debug messages
Date: Thu, 6 May 2021 18:45:19 +0100	[thread overview]
Message-ID: <YJQrL/TWrmHJoqy7@work-vm> (raw)
In-Reply-To: <YJQSnyAIkww0ApJM@work-vm>

* Dr. David Alan Gilbert (dgilbert@redhat.com) wrote:
> * Dr. David Alan Gilbert (dgilbert@redhat.com) wrote:
> > * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > > The GDateTime APIs provided by GLib avoid portability pitfalls, such
> > > as some platforms where 'struct timeval.tv_sec' field is still 'long'
> > > instead of 'time_t'. When combined with automatic cleanup, GDateTime
> > > often results in simpler code too.
> > > 
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > 
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> Queued this 7/7 via virtiofsd

Hi Dan,
  I've had to drop this because it triggers seccomp:

#0  0x00007f0afec4d21b in readlink () from /lib64/libc.so.6
#1  0x00007f0afeecff24 in g_file_read_link () from /lib64/libglib-2.0.so.0
#2  0x00007f0afef16390 in g_time_zone_new_identifier () from /lib64/libglib-2.0.so.0
#3  0x00007f0afef16fe8 in g_time_zone_new_local () from /lib64/libglib-2.0.so.0
#4  0x00007f0afeec9520 in g_date_time_new_now_local () from /lib64/libglib-2.0.so.0
#5  0x000055a6bd6bc27a in log_func (level=FUSE_LOG_INFO, fmt=0x55a6bd6e3cda "%s: Entry\n", ap=0x7ffece4b2f50) at ../tools/virtiofsd/passthrough_ll.c:3578
#6  0x000055a6bd6b3dd5 in fuse_log (level=level@entry=FUSE_LOG_INFO, fmt=fmt@entry=0x55a6bd6e3cda "%s: Entry\n") at ../tools/virtiofsd/fuse_log.c:37
#7  0x000055a6bd6ba9fd in virtio_loop (se=se@entry=0x55a6bee29a30) at ../tools/virtiofsd/fuse_virtio.c:878
#8  0x000055a6bd6b2017 in main (argc=<optimized out>, argv=<optimized out>) at ../tools/virtiofsd/passthrough_ll.c:3868


so you either need to add seccomp rules and/or persuade it not to moan;
maybe dropping down to UTC would work.

Dave

> > > ---
> > >  tools/virtiofsd/passthrough_ll.c | 25 ++++---------------------
> > >  1 file changed, 4 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> > > index 1553d2ef45..cdd224918c 100644
> > > --- a/tools/virtiofsd/passthrough_ll.c
> > > +++ b/tools/virtiofsd/passthrough_ll.c
> > > @@ -3558,10 +3558,6 @@ static void setup_nofile_rlimit(unsigned long rlimit_nofile)
> > >  static void log_func(enum fuse_log_level level, const char *fmt, va_list ap)
> > >  {
> > >      g_autofree char *localfmt = NULL;
> > > -    struct timespec ts;
> > > -    struct tm tm;
> > > -    char sec_fmt[sizeof "2020-12-07 18:17:54"];
> > > -    char zone_fmt[sizeof "+0100"];
> > >  
> > >      if (current_log_level < level) {
> > >          return;
> > > @@ -3573,23 +3569,10 @@ static void log_func(enum fuse_log_level level, const char *fmt, va_list ap)
> > >              localfmt = g_strdup_printf("[ID: %08ld] %s", syscall(__NR_gettid),
> > >                                         fmt);
> > >          } else {
> > > -            /* try formatting a broken-down timestamp */
> > > -            if (clock_gettime(CLOCK_REALTIME, &ts) != -1 &&
> > > -                localtime_r(&ts.tv_sec, &tm) != NULL &&
> > > -                strftime(sec_fmt, sizeof sec_fmt, "%Y-%m-%d %H:%M:%S",
> > > -                         &tm) != 0 &&
> > > -                strftime(zone_fmt, sizeof zone_fmt, "%z", &tm) != 0) {
> > > -                localfmt = g_strdup_printf("[%s.%02ld%s] [ID: %08ld] %s",
> > > -                                           sec_fmt,
> > > -                                           ts.tv_nsec / (10L * 1000 * 1000),
> > > -                                           zone_fmt, syscall(__NR_gettid),
> > > -                                           fmt);
> > > -            } else {
> > > -                /* fall back to a flat timestamp */
> > > -                localfmt = g_strdup_printf("[%" PRId64 "] [ID: %08ld] %s",
> > > -                                           get_clock(), syscall(__NR_gettid),
> > > -                                           fmt);
> > > -            }
> > > +            g_autoptr(GDateTime) now = g_date_time_new_now_local();
> > > +            g_autofree char *nowstr = g_date_time_format(now, "%Y-%m-%d %H:%M:%S.%f%z");
> > > +            localfmt = g_strdup_printf("[%s] [ID: %08ld] %s",
> > > +                                       nowstr, syscall(__NR_gettid), fmt);
> > >          }
> > >          fmt = localfmt;
> > >      }
> > > -- 
> > > 2.31.1
> > > 
> > -- 
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
> > 
> -- 
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


WARNING: multiple messages have this Message-ID (diff)
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Jiri Pirko <jiri@resnulli.us>,
	qemu-block@nongnu.org, Juan Quintela <quintela@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	Jason Wang <jasowang@redhat.com>,
	qemu-devel@nongnu.org, Laurent Vivier <laurent@vivier.eu>,
	virtio-fs@redhat.com, Gerd Hoffmann <kraxel@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Max Reitz <mreitz@redhat.com>, Brad Smith <brad@comstyle.com>
Subject: Re: [PATCH 7/7] virtiofsd: use GDateTime for formatting timestamp for debug messages
Date: Thu, 6 May 2021 18:45:19 +0100	[thread overview]
Message-ID: <YJQrL/TWrmHJoqy7@work-vm> (raw)
In-Reply-To: <YJQSnyAIkww0ApJM@work-vm>

* Dr. David Alan Gilbert (dgilbert@redhat.com) wrote:
> * Dr. David Alan Gilbert (dgilbert@redhat.com) wrote:
> > * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > > The GDateTime APIs provided by GLib avoid portability pitfalls, such
> > > as some platforms where 'struct timeval.tv_sec' field is still 'long'
> > > instead of 'time_t'. When combined with automatic cleanup, GDateTime
> > > often results in simpler code too.
> > > 
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > 
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> Queued this 7/7 via virtiofsd

Hi Dan,
  I've had to drop this because it triggers seccomp:

#0  0x00007f0afec4d21b in readlink () from /lib64/libc.so.6
#1  0x00007f0afeecff24 in g_file_read_link () from /lib64/libglib-2.0.so.0
#2  0x00007f0afef16390 in g_time_zone_new_identifier () from /lib64/libglib-2.0.so.0
#3  0x00007f0afef16fe8 in g_time_zone_new_local () from /lib64/libglib-2.0.so.0
#4  0x00007f0afeec9520 in g_date_time_new_now_local () from /lib64/libglib-2.0.so.0
#5  0x000055a6bd6bc27a in log_func (level=FUSE_LOG_INFO, fmt=0x55a6bd6e3cda "%s: Entry\n", ap=0x7ffece4b2f50) at ../tools/virtiofsd/passthrough_ll.c:3578
#6  0x000055a6bd6b3dd5 in fuse_log (level=level@entry=FUSE_LOG_INFO, fmt=fmt@entry=0x55a6bd6e3cda "%s: Entry\n") at ../tools/virtiofsd/fuse_log.c:37
#7  0x000055a6bd6ba9fd in virtio_loop (se=se@entry=0x55a6bee29a30) at ../tools/virtiofsd/fuse_virtio.c:878
#8  0x000055a6bd6b2017 in main (argc=<optimized out>, argv=<optimized out>) at ../tools/virtiofsd/passthrough_ll.c:3868


so you either need to add seccomp rules and/or persuade it not to moan;
maybe dropping down to UTC would work.

Dave

> > > ---
> > >  tools/virtiofsd/passthrough_ll.c | 25 ++++---------------------
> > >  1 file changed, 4 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> > > index 1553d2ef45..cdd224918c 100644
> > > --- a/tools/virtiofsd/passthrough_ll.c
> > > +++ b/tools/virtiofsd/passthrough_ll.c
> > > @@ -3558,10 +3558,6 @@ static void setup_nofile_rlimit(unsigned long rlimit_nofile)
> > >  static void log_func(enum fuse_log_level level, const char *fmt, va_list ap)
> > >  {
> > >      g_autofree char *localfmt = NULL;
> > > -    struct timespec ts;
> > > -    struct tm tm;
> > > -    char sec_fmt[sizeof "2020-12-07 18:17:54"];
> > > -    char zone_fmt[sizeof "+0100"];
> > >  
> > >      if (current_log_level < level) {
> > >          return;
> > > @@ -3573,23 +3569,10 @@ static void log_func(enum fuse_log_level level, const char *fmt, va_list ap)
> > >              localfmt = g_strdup_printf("[ID: %08ld] %s", syscall(__NR_gettid),
> > >                                         fmt);
> > >          } else {
> > > -            /* try formatting a broken-down timestamp */
> > > -            if (clock_gettime(CLOCK_REALTIME, &ts) != -1 &&
> > > -                localtime_r(&ts.tv_sec, &tm) != NULL &&
> > > -                strftime(sec_fmt, sizeof sec_fmt, "%Y-%m-%d %H:%M:%S",
> > > -                         &tm) != 0 &&
> > > -                strftime(zone_fmt, sizeof zone_fmt, "%z", &tm) != 0) {
> > > -                localfmt = g_strdup_printf("[%s.%02ld%s] [ID: %08ld] %s",
> > > -                                           sec_fmt,
> > > -                                           ts.tv_nsec / (10L * 1000 * 1000),
> > > -                                           zone_fmt, syscall(__NR_gettid),
> > > -                                           fmt);
> > > -            } else {
> > > -                /* fall back to a flat timestamp */
> > > -                localfmt = g_strdup_printf("[%" PRId64 "] [ID: %08ld] %s",
> > > -                                           get_clock(), syscall(__NR_gettid),
> > > -                                           fmt);
> > > -            }
> > > +            g_autoptr(GDateTime) now = g_date_time_new_now_local();
> > > +            g_autofree char *nowstr = g_date_time_format(now, "%Y-%m-%d %H:%M:%S.%f%z");
> > > +            localfmt = g_strdup_printf("[%s] [ID: %08ld] %s",
> > > +                                       nowstr, syscall(__NR_gettid), fmt);
> > >          }
> > >          fmt = localfmt;
> > >      }
> > > -- 
> > > 2.31.1
> > > 
> > -- 
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
> > 
> -- 
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



  reply	other threads:[~2021-05-06 17:45 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-05 10:36 [Virtio-fs] [PATCH 0/7] replace all use of strftime() with g_date_time_format() Daniel P. Berrangé
2021-05-05 10:36 ` Daniel P. Berrangé
2021-05-05 10:36 ` [Virtio-fs] [PATCH 1/7] migration: use GDateTime for formatting timestamp in snapshot names Daniel P. Berrangé
2021-05-05 10:36   ` Daniel P. Berrangé
2021-05-05 14:42   ` [Virtio-fs] " Dr. David Alan Gilbert
2021-05-05 14:42     ` Dr. David Alan Gilbert
2021-05-06  3:14   ` [Virtio-fs] " Brad Smith
2021-05-06  3:14     ` Brad Smith
2021-05-05 10:36 ` [Virtio-fs] [PATCH 2/7] block: use GDateTime for formatting timestamp when dumping snapshot info Daniel P. Berrangé
2021-05-05 10:36   ` Daniel P. Berrangé
2021-05-07 17:05   ` [Virtio-fs] " Max Reitz
2021-05-07 17:05     ` Max Reitz
2021-05-05 10:36 ` [Virtio-fs] [PATCH 3/7] net/rocker: use GDateTime for formatting timestamp in debug messages Daniel P. Berrangé
2021-05-05 10:36   ` Daniel P. Berrangé
2021-06-11 16:28   ` [Virtio-fs] " Daniel P. Berrangé
2021-06-11 16:28     ` Daniel P. Berrangé
2021-06-14 16:43     ` [Virtio-fs] " Juan Quintela
2021-06-14 16:43       ` Juan Quintela
2021-05-05 10:36 ` [Virtio-fs] [PATCH 4/7] usb/dev-mtp: use GDateTime for formatting timestamp for objects Daniel P. Berrangé
2021-05-05 10:36   ` Daniel P. Berrangé
2021-05-05 13:08   ` [Virtio-fs] " Gerd Hoffmann
2021-05-05 13:08     ` Gerd Hoffmann
2021-05-05 10:37 ` [Virtio-fs] [PATCH 5/7] io: use GDateTime for formatting timestamp for websock headers Daniel P. Berrangé
2021-05-05 10:37   ` Daniel P. Berrangé
2021-06-11 16:29   ` [Virtio-fs] " Daniel P. Berrangé
2021-06-11 16:29     ` Daniel P. Berrangé
2021-06-14 15:41     ` [Virtio-fs] " Dr. David Alan Gilbert
2021-06-14 15:41       ` Dr. David Alan Gilbert
2021-05-05 10:37 ` [Virtio-fs] [PATCH 6/7] linux-user: use GDateTime for formatting timestamp for core file Daniel P. Berrangé
2021-05-05 10:37   ` Daniel P. Berrangé
2021-05-05 11:18   ` [Virtio-fs] " Laurent Vivier
2021-05-05 11:18     ` Laurent Vivier
2021-05-15 19:51   ` [Virtio-fs] " Laurent Vivier
2021-05-15 19:51     ` Laurent Vivier
2021-05-05 10:37 ` [Virtio-fs] [PATCH 7/7] virtiofsd: use GDateTime for formatting timestamp for debug messages Daniel P. Berrangé
2021-05-05 10:37   ` Daniel P. Berrangé
2021-05-05 14:51   ` [Virtio-fs] " Dr. David Alan Gilbert
2021-05-05 14:51     ` Dr. David Alan Gilbert
2021-05-06 16:00     ` [Virtio-fs] " Dr. David Alan Gilbert
2021-05-06 16:00       ` Dr. David Alan Gilbert
2021-05-06 17:45       ` Dr. David Alan Gilbert [this message]
2021-05-06 17:45         ` Dr. David Alan Gilbert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YJQrL/TWrmHJoqy7@work-vm \
    --to=dgilbert@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=brad@comstyle.com \
    --cc=jasowang@redhat.com \
    --cc=jiri@resnulli.us \
    --cc=kraxel@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=laurent@vivier.eu \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=virtio-fs@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.