* [PATCH 2/2] scripts/tracetool: Add plainlog backend @ 2020-06-17 13:36 BALATON Zoltan 2020-06-17 15:41 ` Alex Bennée 2020-06-18 7:31 ` Stefan Hajnoczi 0 siblings, 2 replies; 9+ messages in thread From: BALATON Zoltan @ 2020-06-17 13:36 UTC (permalink / raw) To: qemu-devel; +Cc: qemu-trivial, Stefan Hajnoczi [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain, Size: 1913 bytes --] Add a backend that is the same as the log backend but omits the process id and timestamp so logs are easier to read and diff-able. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- scripts/tracetool/backend/plainlog.py | 48 +++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 scripts/tracetool/backend/plainlog.py diff --git a/scripts/tracetool/backend/plainlog.py b/scripts/tracetool/backend/plainlog.py new file mode 100644 index 0000000000..40bbfa6d76 --- /dev/null +++ b/scripts/tracetool/backend/plainlog.py @@ -0,0 +1,48 @@ +# -*- coding: utf-8 -*- + +""" +Stderr built-in backend, plain log without proc ID and time. +""" + +__author__ = "Lluís Vilanova <vilanova@ac.upc.edu>" +__copyright__ = "Copyright 2012-2017, Lluís Vilanova <vilanova@ac.upc.edu>" +__license__ = "GPL version 2 or (at your option) any later version" + +__maintainer__ = "Stefan Hajnoczi" +__email__ = "stefanha@linux.vnet.ibm.com" + + +from tracetool import out + + +PUBLIC = True + + +def generate_h_begin(events, group): + out('#include "qemu/log-for-trace.h"', + '') + + +def generate_h(event, group): + argnames = ", ".join(event.args.names()) + if len(event.args) > 0: + argnames = ", " + argnames + + if "vcpu" in event.properties: + # already checked on the generic format code + cond = "true" + else: + cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper()) + + out(' if (%(cond)s && qemu_loglevel_mask(LOG_TRACE)) {', + ' qemu_log("%(name)s " %(fmt)s "\\n" %(argnames)s);', + ' }', + cond=cond, + name=event.name, + fmt=event.fmt.rstrip("\n"), + argnames=argnames) + + +def generate_h_backend_dstate(event, group): + out(' trace_event_get_state_dynamic_by_id(%(event_id)s) || \\', + event_id="TRACE_" + event.name.upper()) -- 2.21.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] scripts/tracetool: Add plainlog backend 2020-06-17 13:36 [PATCH 2/2] scripts/tracetool: Add plainlog backend BALATON Zoltan @ 2020-06-17 15:41 ` Alex Bennée 2020-06-17 17:53 ` BALATON Zoltan 2020-06-18 7:31 ` Stefan Hajnoczi 1 sibling, 1 reply; 9+ messages in thread From: Alex Bennée @ 2020-06-17 15:41 UTC (permalink / raw) To: BALATON Zoltan; +Cc: qemu-trivial, qemu-devel, Stefan Hajnoczi BALATON Zoltan <balaton@eik.bme.hu> writes: Did this patch get separated from a larger series (2/2)? > Add a backend that is the same as the log backend but omits the > process id and timestamp so logs are easier to read and diff-able. I'd argue for this to be the behaviour of plain log (given it's mixing with other log output). If not then maybe plainlog would be the default log type if nothing is passed to configure? > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > --- > scripts/tracetool/backend/plainlog.py | 48 +++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > create mode 100644 scripts/tracetool/backend/plainlog.py > > diff --git a/scripts/tracetool/backend/plainlog.py b/scripts/tracetool/backend/plainlog.py > new file mode 100644 > index 0000000000..40bbfa6d76 > --- /dev/null > +++ b/scripts/tracetool/backend/plainlog.py > @@ -0,0 +1,48 @@ > +# -*- coding: utf-8 -*- > + > +""" > +Stderr built-in backend, plain log without proc ID and time. > +""" > + > +__author__ = "Lluís Vilanova <vilanova@ac.upc.edu>" > +__copyright__ = "Copyright 2012-2017, Lluís Vilanova <vilanova@ac.upc.edu>" > +__license__ = "GPL version 2 or (at your option) any later version" > + > +__maintainer__ = "Stefan Hajnoczi" > +__email__ = "stefanha@linux.vnet.ibm.com" > + > + > +from tracetool import out > + > + > +PUBLIC = True > + > + > +def generate_h_begin(events, group): > + out('#include "qemu/log-for-trace.h"', > + '') > + > + > +def generate_h(event, group): > + argnames = ", ".join(event.args.names()) > + if len(event.args) > 0: > + argnames = ", " + argnames > + > + if "vcpu" in event.properties: > + # already checked on the generic format code > + cond = "true" > + else: > + cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper()) > + > + out(' if (%(cond)s && qemu_loglevel_mask(LOG_TRACE)) {', > + ' qemu_log("%(name)s " %(fmt)s "\\n" %(argnames)s);', > + ' }', > + cond=cond, > + name=event.name, > + fmt=event.fmt.rstrip("\n"), > + argnames=argnames) > + > + > +def generate_h_backend_dstate(event, group): > + out(' trace_event_get_state_dynamic_by_id(%(event_id)s) || \\', > + event_id="TRACE_" + event.name.upper()) -- Alex Bennée ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] scripts/tracetool: Add plainlog backend 2020-06-17 15:41 ` Alex Bennée @ 2020-06-17 17:53 ` BALATON Zoltan 0 siblings, 0 replies; 9+ messages in thread From: BALATON Zoltan @ 2020-06-17 17:53 UTC (permalink / raw) To: Alex Bennée; +Cc: qemu-trivial, qemu-devel, Stefan Hajnoczi [-- Attachment #1: Type: text/plain, Size: 3100 bytes --] On Wed, 17 Jun 2020, Alex Bennée wrote: > Did this patch get separated from a larger series (2/2)? No sorry, just used format-patch for two unrelated patches and forgot to remove this. This patch is standalone and Philippe pointed out the other one labelled 1/2 is not needed as there's already a similar patch queued. >> Add a backend that is the same as the log backend but omits the >> process id and timestamp so logs are easier to read and diff-able. > > I'd argue for this to be the behaviour of plain log (given it's mixing > with other log output). If not then maybe plainlog would be the default > log type if nothing is passed to configure? I'd like if the default log backend was not adding timestamps and random numbers to log messages for easier digesting and diffing but the current log backend does that and maybe there are people who like that default so instead of making them need to change their ways I'm proposing this backend that those who like plain logs can use instead. That's a less disruptive change than changing the default log backend but if others want that I'm fine with that too. Regards, BALATON Zoltan >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >> --- >> scripts/tracetool/backend/plainlog.py | 48 +++++++++++++++++++++++++++ >> 1 file changed, 48 insertions(+) >> create mode 100644 scripts/tracetool/backend/plainlog.py >> >> diff --git a/scripts/tracetool/backend/plainlog.py b/scripts/tracetool/backend/plainlog.py >> new file mode 100644 >> index 0000000000..40bbfa6d76 >> --- /dev/null >> +++ b/scripts/tracetool/backend/plainlog.py >> @@ -0,0 +1,48 @@ >> +# -*- coding: utf-8 -*- >> + >> +""" >> +Stderr built-in backend, plain log without proc ID and time. >> +""" >> + >> +__author__ = "Lluís Vilanova <vilanova@ac.upc.edu>" >> +__copyright__ = "Copyright 2012-2017, Lluís Vilanova <vilanova@ac.upc.edu>" >> +__license__ = "GPL version 2 or (at your option) any later version" >> + >> +__maintainer__ = "Stefan Hajnoczi" >> +__email__ = "stefanha@linux.vnet.ibm.com" >> + >> + >> +from tracetool import out >> + >> + >> +PUBLIC = True >> + >> + >> +def generate_h_begin(events, group): >> + out('#include "qemu/log-for-trace.h"', >> + '') >> + >> + >> +def generate_h(event, group): >> + argnames = ", ".join(event.args.names()) >> + if len(event.args) > 0: >> + argnames = ", " + argnames >> + >> + if "vcpu" in event.properties: >> + # already checked on the generic format code >> + cond = "true" >> + else: >> + cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper()) >> + >> + out(' if (%(cond)s && qemu_loglevel_mask(LOG_TRACE)) {', >> + ' qemu_log("%(name)s " %(fmt)s "\\n" %(argnames)s);', >> + ' }', >> + cond=cond, >> + name=event.name, >> + fmt=event.fmt.rstrip("\n"), >> + argnames=argnames) >> + >> + >> +def generate_h_backend_dstate(event, group): >> + out(' trace_event_get_state_dynamic_by_id(%(event_id)s) || \\', >> + event_id="TRACE_" + event.name.upper()) > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] scripts/tracetool: Add plainlog backend 2020-06-17 13:36 [PATCH 2/2] scripts/tracetool: Add plainlog backend BALATON Zoltan 2020-06-17 15:41 ` Alex Bennée @ 2020-06-18 7:31 ` Stefan Hajnoczi 2020-06-18 9:07 ` Daniel P. Berrangé 1 sibling, 1 reply; 9+ messages in thread From: Stefan Hajnoczi @ 2020-06-18 7:31 UTC (permalink / raw) To: BALATON Zoltan; +Cc: qemu-trivial, qemu-devel [-- Attachment #1: Type: text/plain, Size: 2361 bytes --] On Wed, Jun 17, 2020 at 03:36:29PM +0200, BALATON Zoltan wrote: > Add a backend that is the same as the log backend but omits the > process id and timestamp so logs are easier to read and diff-able. > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > --- > scripts/tracetool/backend/plainlog.py | 48 +++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > create mode 100644 scripts/tracetool/backend/plainlog.py > > diff --git a/scripts/tracetool/backend/plainlog.py b/scripts/tracetool/backend/plainlog.py > new file mode 100644 > index 0000000000..40bbfa6d76 > --- /dev/null > +++ b/scripts/tracetool/backend/plainlog.py > @@ -0,0 +1,48 @@ > +# -*- coding: utf-8 -*- > + > +""" > +Stderr built-in backend, plain log without proc ID and time. > +""" > + > +__author__ = "Llu????s Vilanova <vilanova@ac.upc.edu>" > +__copyright__ = "Copyright 2012-2017, Llu????s Vilanova <vilanova@ac.upc.edu>" There is a Unicode issue here, Lluís' name is not printed correctly. > +__license__ = "GPL version 2 or (at your option) any later version" > + > +__maintainer__ = "Stefan Hajnoczi" > +__email__ = "stefanha@linux.vnet.ibm.com" > + > + > +from tracetool import out > + > + > +PUBLIC = True > + > + > +def generate_h_begin(events, group): > + out('#include "qemu/log-for-trace.h"', > + '') > + > + > +def generate_h(event, group): > + argnames = ", ".join(event.args.names()) > + if len(event.args) > 0: > + argnames = ", " + argnames > + > + if "vcpu" in event.properties: > + # already checked on the generic format code > + cond = "true" > + else: > + cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper()) > + > + out(' if (%(cond)s && qemu_loglevel_mask(LOG_TRACE)) {', > + ' qemu_log("%(name)s " %(fmt)s "\\n" %(argnames)s);', > + ' }', > + cond=cond, > + name=event.name, > + fmt=event.fmt.rstrip("\n"), > + argnames=argnames) It is not necessary to introduce a new backend. There could be an option that controls whether or not the timestamp/tid is printed. For example, -trace timestamp=off or maybe the timestmap/tid can be integrated into qemu_log() itself so that it's used more consistently and a -d timestamp option enables it. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] scripts/tracetool: Add plainlog backend 2020-06-18 7:31 ` Stefan Hajnoczi @ 2020-06-18 9:07 ` Daniel P. Berrangé 2020-06-18 10:30 ` BALATON Zoltan 2020-06-18 15:35 ` Stefan Hajnoczi 0 siblings, 2 replies; 9+ messages in thread From: Daniel P. Berrangé @ 2020-06-18 9:07 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: qemu-trivial, qemu-devel On Thu, Jun 18, 2020 at 08:31:24AM +0100, Stefan Hajnoczi wrote: > On Wed, Jun 17, 2020 at 03:36:29PM +0200, BALATON Zoltan wrote: > > Add a backend that is the same as the log backend but omits the > > process id and timestamp so logs are easier to read and diff-able. > > > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > > --- > > scripts/tracetool/backend/plainlog.py | 48 +++++++++++++++++++++++++++ > > 1 file changed, 48 insertions(+) > > create mode 100644 scripts/tracetool/backend/plainlog.py > > > > diff --git a/scripts/tracetool/backend/plainlog.py b/scripts/tracetool/backend/plainlog.py > > new file mode 100644 > > index 0000000000..40bbfa6d76 > > --- /dev/null > > +++ b/scripts/tracetool/backend/plainlog.py > > @@ -0,0 +1,48 @@ > > +# -*- coding: utf-8 -*- > > + > > +""" > > +Stderr built-in backend, plain log without proc ID and time. > > +""" > > + > > +__author__ = "Llu????s Vilanova <vilanova@ac.upc.edu>" > > +__copyright__ = "Copyright 2012-2017, Llu????s Vilanova <vilanova@ac.upc.edu>" > > There is a Unicode issue here, Lluís' name is not printed correctly. > > > +__license__ = "GPL version 2 or (at your option) any later version" > > + > > +__maintainer__ = "Stefan Hajnoczi" > > +__email__ = "stefanha@linux.vnet.ibm.com" > > + > > + > > +from tracetool import out > > + > > + > > +PUBLIC = True > > + > > + > > +def generate_h_begin(events, group): > > + out('#include "qemu/log-for-trace.h"', > > + '') > > + > > + > > +def generate_h(event, group): > > + argnames = ", ".join(event.args.names()) > > + if len(event.args) > 0: > > + argnames = ", " + argnames > > + > > + if "vcpu" in event.properties: > > + # already checked on the generic format code > > + cond = "true" > > + else: > > + cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper()) > > + > > + out(' if (%(cond)s && qemu_loglevel_mask(LOG_TRACE)) {', > > + ' qemu_log("%(name)s " %(fmt)s "\\n" %(argnames)s);', > > + ' }', > > + cond=cond, > > + name=event.name, > > + fmt=event.fmt.rstrip("\n"), > > + argnames=argnames) > > It is not necessary to introduce a new backend. There could be an option > that controls whether or not the timestamp/tid is printed. For example, > -trace timestamp=off or maybe the timestmap/tid can be integrated into > qemu_log() itself so that it's used more consistently and a -d timestamp > option enables it. QEMU already has a "-msg timestamp=on|off" option that controls whether error reports on stderr get a timestamp. I think it is probably reasonable for this existing option to apply to anything QEMU prints to stdout/err, and thus we could wire it up for qemu_log(). Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] scripts/tracetool: Add plainlog backend 2020-06-18 9:07 ` Daniel P. Berrangé @ 2020-06-18 10:30 ` BALATON Zoltan 2020-06-18 15:35 ` Stefan Hajnoczi 1 sibling, 0 replies; 9+ messages in thread From: BALATON Zoltan @ 2020-06-18 10:30 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: qemu-trivial, qemu-devel, Stefan Hajnoczi [-- Attachment #1: Type: text/plain, Size: 3460 bytes --] On Thu, 18 Jun 2020, Daniel P. Berrangé wrote: > On Thu, Jun 18, 2020 at 08:31:24AM +0100, Stefan Hajnoczi wrote: >> On Wed, Jun 17, 2020 at 03:36:29PM +0200, BALATON Zoltan wrote: >>> Add a backend that is the same as the log backend but omits the >>> process id and timestamp so logs are easier to read and diff-able. >>> >>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >>> --- >>> scripts/tracetool/backend/plainlog.py | 48 +++++++++++++++++++++++++++ >>> 1 file changed, 48 insertions(+) >>> create mode 100644 scripts/tracetool/backend/plainlog.py >>> >>> diff --git a/scripts/tracetool/backend/plainlog.py b/scripts/tracetool/backend/plainlog.py >>> new file mode 100644 >>> index 0000000000..40bbfa6d76 >>> --- /dev/null >>> +++ b/scripts/tracetool/backend/plainlog.py >>> @@ -0,0 +1,48 @@ >>> +# -*- coding: utf-8 -*- >>> + >>> +""" >>> +Stderr built-in backend, plain log without proc ID and time. >>> +""" >>> + >>> +__author__ = "Llu????s Vilanova <vilanova@ac.upc.edu>" >>> +__copyright__ = "Copyright 2012-2017, Llu????s Vilanova <vilanova@ac.upc.edu>" >> >> There is a Unicode issue here, Lluís' name is not printed correctly. It's just an issue with the email formatting, should be OK if you read it in UTF8 which is the default on most machines nowaday just forgot to add corresponding email header. >>> +__license__ = "GPL version 2 or (at your option) any later version" >>> + >>> +__maintainer__ = "Stefan Hajnoczi" >>> +__email__ = "stefanha@linux.vnet.ibm.com" >>> + >>> + >>> +from tracetool import out >>> + >>> + >>> +PUBLIC = True >>> + >>> + >>> +def generate_h_begin(events, group): >>> + out('#include "qemu/log-for-trace.h"', >>> + '') >>> + >>> + >>> +def generate_h(event, group): >>> + argnames = ", ".join(event.args.names()) >>> + if len(event.args) > 0: >>> + argnames = ", " + argnames >>> + >>> + if "vcpu" in event.properties: >>> + # already checked on the generic format code >>> + cond = "true" >>> + else: >>> + cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper()) >>> + >>> + out(' if (%(cond)s && qemu_loglevel_mask(LOG_TRACE)) {', >>> + ' qemu_log("%(name)s " %(fmt)s "\\n" %(argnames)s);', >>> + ' }', >>> + cond=cond, >>> + name=event.name, >>> + fmt=event.fmt.rstrip("\n"), >>> + argnames=argnames) >> >> It is not necessary to introduce a new backend. There could be an option >> that controls whether or not the timestamp/tid is printed. For example, >> -trace timestamp=off or maybe the timestmap/tid can be integrated into >> qemu_log() itself so that it's used more consistently and a -d timestamp >> option enables it. > > QEMU already has a "-msg timestamp=on|off" option that controls whether > error reports on stderr get a timestamp. I think it is probably reasonable > for this existing option to apply to anything QEMU prints to stdout/err, > and thus we could wire it up for qemu_log(). I don't know how to do that so can you submit a patch please? It would be preferable if an additional command line option wasn't necessary to print logs without timestamps (so changing the defailt to that if you don't mind changing current trace behaviour) or the default could be set at configure time like with this patch --enable-trace-backends=plainlog sets that (although this does not have option to change it at runtime). Regards, BALATON Zoltan ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] scripts/tracetool: Add plainlog backend 2020-06-18 9:07 ` Daniel P. Berrangé 2020-06-18 10:30 ` BALATON Zoltan @ 2020-06-18 15:35 ` Stefan Hajnoczi 2020-06-18 15:43 ` Daniel P. Berrangé 1 sibling, 1 reply; 9+ messages in thread From: Stefan Hajnoczi @ 2020-06-18 15:35 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: qemu-trivial, qemu-devel [-- Attachment #1: Type: text/plain, Size: 3235 bytes --] On Thu, Jun 18, 2020 at 10:07:41AM +0100, Daniel P. Berrangé wrote: > On Thu, Jun 18, 2020 at 08:31:24AM +0100, Stefan Hajnoczi wrote: > > On Wed, Jun 17, 2020 at 03:36:29PM +0200, BALATON Zoltan wrote: > > > Add a backend that is the same as the log backend but omits the > > > process id and timestamp so logs are easier to read and diff-able. > > > > > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > > > --- > > > scripts/tracetool/backend/plainlog.py | 48 +++++++++++++++++++++++++++ > > > 1 file changed, 48 insertions(+) > > > create mode 100644 scripts/tracetool/backend/plainlog.py > > > > > > diff --git a/scripts/tracetool/backend/plainlog.py b/scripts/tracetool/backend/plainlog.py > > > new file mode 100644 > > > index 0000000000..40bbfa6d76 > > > --- /dev/null > > > +++ b/scripts/tracetool/backend/plainlog.py > > > @@ -0,0 +1,48 @@ > > > +# -*- coding: utf-8 -*- > > > + > > > +""" > > > +Stderr built-in backend, plain log without proc ID and time. > > > +""" > > > + > > > +__author__ = "Llu????s Vilanova <vilanova@ac.upc.edu>" > > > +__copyright__ = "Copyright 2012-2017, Llu????s Vilanova <vilanova@ac.upc.edu>" > > > > There is a Unicode issue here, Lluís' name is not printed correctly. > > > > > +__license__ = "GPL version 2 or (at your option) any later version" > > > + > > > +__maintainer__ = "Stefan Hajnoczi" > > > +__email__ = "stefanha@linux.vnet.ibm.com" > > > + > > > + > > > +from tracetool import out > > > + > > > + > > > +PUBLIC = True > > > + > > > + > > > +def generate_h_begin(events, group): > > > + out('#include "qemu/log-for-trace.h"', > > > + '') > > > + > > > + > > > +def generate_h(event, group): > > > + argnames = ", ".join(event.args.names()) > > > + if len(event.args) > 0: > > > + argnames = ", " + argnames > > > + > > > + if "vcpu" in event.properties: > > > + # already checked on the generic format code > > > + cond = "true" > > > + else: > > > + cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper()) > > > + > > > + out(' if (%(cond)s && qemu_loglevel_mask(LOG_TRACE)) {', > > > + ' qemu_log("%(name)s " %(fmt)s "\\n" %(argnames)s);', > > > + ' }', > > > + cond=cond, > > > + name=event.name, > > > + fmt=event.fmt.rstrip("\n"), > > > + argnames=argnames) > > > > It is not necessary to introduce a new backend. There could be an option > > that controls whether or not the timestamp/tid is printed. For example, > > -trace timestamp=off or maybe the timestmap/tid can be integrated into > > qemu_log() itself so that it's used more consistently and a -d timestamp > > option enables it. > > QEMU already has a "-msg timestamp=on|off" option that controls whether > error reports on stderr get a timestamp. I think it is probably reasonable > for this existing option to apply to anything QEMU prints to stdout/err, > and thus we could wire it up for qemu_log(). I thought about that but the features are somewhat unrelated. If we unify them, how about making the timestamp/tid apply to *all* qemu_log() output, not just tracing? Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] scripts/tracetool: Add plainlog backend 2020-06-18 15:35 ` Stefan Hajnoczi @ 2020-06-18 15:43 ` Daniel P. Berrangé 2020-06-23 14:47 ` Stefan Hajnoczi 0 siblings, 1 reply; 9+ messages in thread From: Daniel P. Berrangé @ 2020-06-18 15:43 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: qemu-trivial, qemu-devel On Thu, Jun 18, 2020 at 04:35:16PM +0100, Stefan Hajnoczi wrote: > On Thu, Jun 18, 2020 at 10:07:41AM +0100, Daniel P. Berrangé wrote: > > On Thu, Jun 18, 2020 at 08:31:24AM +0100, Stefan Hajnoczi wrote: > > > On Wed, Jun 17, 2020 at 03:36:29PM +0200, BALATON Zoltan wrote: > > > > Add a backend that is the same as the log backend but omits the > > > > process id and timestamp so logs are easier to read and diff-able. > > > > > > > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > > > > --- > > > > scripts/tracetool/backend/plainlog.py | 48 +++++++++++++++++++++++++++ > > > > 1 file changed, 48 insertions(+) > > > > create mode 100644 scripts/tracetool/backend/plainlog.py > > > > > > > > diff --git a/scripts/tracetool/backend/plainlog.py b/scripts/tracetool/backend/plainlog.py > > > > new file mode 100644 > > > > index 0000000000..40bbfa6d76 > > > > --- /dev/null > > > > +++ b/scripts/tracetool/backend/plainlog.py > > > > @@ -0,0 +1,48 @@ > > > > +# -*- coding: utf-8 -*- > > > > + > > > > +""" > > > > +Stderr built-in backend, plain log without proc ID and time. > > > > +""" > > > > + > > > > +__author__ = "Llu????s Vilanova <vilanova@ac.upc.edu>" > > > > +__copyright__ = "Copyright 2012-2017, Llu????s Vilanova <vilanova@ac.upc.edu>" > > > > > > There is a Unicode issue here, Lluís' name is not printed correctly. > > > > > > > +__license__ = "GPL version 2 or (at your option) any later version" > > > > + > > > > +__maintainer__ = "Stefan Hajnoczi" > > > > +__email__ = "stefanha@linux.vnet.ibm.com" > > > > + > > > > + > > > > +from tracetool import out > > > > + > > > > + > > > > +PUBLIC = True > > > > + > > > > + > > > > +def generate_h_begin(events, group): > > > > + out('#include "qemu/log-for-trace.h"', > > > > + '') > > > > + > > > > + > > > > +def generate_h(event, group): > > > > + argnames = ", ".join(event.args.names()) > > > > + if len(event.args) > 0: > > > > + argnames = ", " + argnames > > > > + > > > > + if "vcpu" in event.properties: > > > > + # already checked on the generic format code > > > > + cond = "true" > > > > + else: > > > > + cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper()) > > > > + > > > > + out(' if (%(cond)s && qemu_loglevel_mask(LOG_TRACE)) {', > > > > + ' qemu_log("%(name)s " %(fmt)s "\\n" %(argnames)s);', > > > > + ' }', > > > > + cond=cond, > > > > + name=event.name, > > > > + fmt=event.fmt.rstrip("\n"), > > > > + argnames=argnames) > > > > > > It is not necessary to introduce a new backend. There could be an option > > > that controls whether or not the timestamp/tid is printed. For example, > > > -trace timestamp=off or maybe the timestmap/tid can be integrated into > > > qemu_log() itself so that it's used more consistently and a -d timestamp > > > option enables it. > > > > QEMU already has a "-msg timestamp=on|off" option that controls whether > > error reports on stderr get a timestamp. I think it is probably reasonable > > for this existing option to apply to anything QEMU prints to stdout/err, > > and thus we could wire it up for qemu_log(). > > I thought about that but the features are somewhat unrelated. > > If we unify them, how about making the timestamp/tid apply to *all* > qemu_log() output, not just tracing? That's exactly what I intended. Essentially if QEMU is going to add timestamps to things it writes to stdout/err, then it should do that universally for all parts of the code base that use stdio. This means error_report(), qemu_log(), and any other places that are relevant wrt stdio. Having separate timestamp on/off switches for each feature is not desirable. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] scripts/tracetool: Add plainlog backend 2020-06-18 15:43 ` Daniel P. Berrangé @ 2020-06-23 14:47 ` Stefan Hajnoczi 0 siblings, 0 replies; 9+ messages in thread From: Stefan Hajnoczi @ 2020-06-23 14:47 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: qemu-trivial, qemu-devel, Stefan Hajnoczi [-- Attachment #1: Type: text/plain, Size: 4142 bytes --] On Thu, Jun 18, 2020 at 04:43:23PM +0100, Daniel P. Berrangé wrote: > On Thu, Jun 18, 2020 at 04:35:16PM +0100, Stefan Hajnoczi wrote: > > On Thu, Jun 18, 2020 at 10:07:41AM +0100, Daniel P. Berrangé wrote: > > > On Thu, Jun 18, 2020 at 08:31:24AM +0100, Stefan Hajnoczi wrote: > > > > On Wed, Jun 17, 2020 at 03:36:29PM +0200, BALATON Zoltan wrote: > > > > > Add a backend that is the same as the log backend but omits the > > > > > process id and timestamp so logs are easier to read and diff-able. > > > > > > > > > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > > > > > --- > > > > > scripts/tracetool/backend/plainlog.py | 48 +++++++++++++++++++++++++++ > > > > > 1 file changed, 48 insertions(+) > > > > > create mode 100644 scripts/tracetool/backend/plainlog.py > > > > > > > > > > diff --git a/scripts/tracetool/backend/plainlog.py b/scripts/tracetool/backend/plainlog.py > > > > > new file mode 100644 > > > > > index 0000000000..40bbfa6d76 > > > > > --- /dev/null > > > > > +++ b/scripts/tracetool/backend/plainlog.py > > > > > @@ -0,0 +1,48 @@ > > > > > +# -*- coding: utf-8 -*- > > > > > + > > > > > +""" > > > > > +Stderr built-in backend, plain log without proc ID and time. > > > > > +""" > > > > > + > > > > > +__author__ = "Llu????s Vilanova <vilanova@ac.upc.edu>" > > > > > +__copyright__ = "Copyright 2012-2017, Llu????s Vilanova <vilanova@ac.upc.edu>" > > > > > > > > There is a Unicode issue here, Lluís' name is not printed correctly. > > > > > > > > > +__license__ = "GPL version 2 or (at your option) any later version" > > > > > + > > > > > +__maintainer__ = "Stefan Hajnoczi" > > > > > +__email__ = "stefanha@linux.vnet.ibm.com" > > > > > + > > > > > + > > > > > +from tracetool import out > > > > > + > > > > > + > > > > > +PUBLIC = True > > > > > + > > > > > + > > > > > +def generate_h_begin(events, group): > > > > > + out('#include "qemu/log-for-trace.h"', > > > > > + '') > > > > > + > > > > > + > > > > > +def generate_h(event, group): > > > > > + argnames = ", ".join(event.args.names()) > > > > > + if len(event.args) > 0: > > > > > + argnames = ", " + argnames > > > > > + > > > > > + if "vcpu" in event.properties: > > > > > + # already checked on the generic format code > > > > > + cond = "true" > > > > > + else: > > > > > + cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper()) > > > > > + > > > > > + out(' if (%(cond)s && qemu_loglevel_mask(LOG_TRACE)) {', > > > > > + ' qemu_log("%(name)s " %(fmt)s "\\n" %(argnames)s);', > > > > > + ' }', > > > > > + cond=cond, > > > > > + name=event.name, > > > > > + fmt=event.fmt.rstrip("\n"), > > > > > + argnames=argnames) > > > > > > > > It is not necessary to introduce a new backend. There could be an option > > > > that controls whether or not the timestamp/tid is printed. For example, > > > > -trace timestamp=off or maybe the timestmap/tid can be integrated into > > > > qemu_log() itself so that it's used more consistently and a -d timestamp > > > > option enables it. > > > > > > QEMU already has a "-msg timestamp=on|off" option that controls whether > > > error reports on stderr get a timestamp. I think it is probably reasonable > > > for this existing option to apply to anything QEMU prints to stdout/err, > > > and thus we could wire it up for qemu_log(). > > > > I thought about that but the features are somewhat unrelated. > > > > If we unify them, how about making the timestamp/tid apply to *all* > > qemu_log() output, not just tracing? > > That's exactly what I intended. > > Essentially if QEMU is going to add timestamps to things it writes to > stdout/err, then it should do that universally for all parts of the code > base that use stdio. This means error_report(), qemu_log(), and any > other places that are relevant wrt stdio. > > Having separate timestamp on/off switches for each feature is not > desirable. Okay. I'll take a look at this tomorrow. Thanks! Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-06-23 15:12 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-17 13:36 [PATCH 2/2] scripts/tracetool: Add plainlog backend BALATON Zoltan 2020-06-17 15:41 ` Alex Bennée 2020-06-17 17:53 ` BALATON Zoltan 2020-06-18 7:31 ` Stefan Hajnoczi 2020-06-18 9:07 ` Daniel P. Berrangé 2020-06-18 10:30 ` BALATON Zoltan 2020-06-18 15:35 ` Stefan Hajnoczi 2020-06-18 15:43 ` Daniel P. Berrangé 2020-06-23 14:47 ` Stefan Hajnoczi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).