| Message ID | 20230906131616.7681-1-javi.merino@cloud.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1] tools/xentrace/xentrace_format: Add python 3 compatibility | expand |
On 06/09/2023 2:14 pm, Javi Merino wrote: > diff --git a/tools/xentrace/xentrace_format b/tools/xentrace/xentrace_format > index 5ff85ae2e8..166ebae325 100644 > --- a/tools/xentrace/xentrace_format > +++ b/tools/xentrace/xentrace_format > @@ -4,11 +4,15 @@ > > # Program for reformatting trace buffer output according to user-supplied rules > > +from __future__ import division > +from __future__ import print_function > +from __future__ import unicode_literals > +from builtins import str > +from past.utils import old_div This adds a new dependency on a package we (upstream Xen) don't currently use. AFAICT, it's only for... > @@ -223,7 +232,7 @@ while not interrupted: > last_tsc[cpu] = tsc > > if mhz: > - tsc = tsc / (mhz*1000000.0) > + tsc = old_div(tsc, (mhz*1000000.0)) ... this, which is always int / float and doesn't fall into Py2's int/int behaviour in the first place. I'm pretty sure the code can just stay as it is, without needing to use old_div(). ~Andrew
On Mon, Sep 11, 2023 at 11:52:43AM +0100, Andrew Cooper wrote: > On 06/09/2023 2:14 pm, Javi Merino wrote: > > diff --git a/tools/xentrace/xentrace_format b/tools/xentrace/xentrace_format > > index 5ff85ae2e8..166ebae325 100644 > > --- a/tools/xentrace/xentrace_format > > +++ b/tools/xentrace/xentrace_format > > @@ -4,11 +4,15 @@ > > > > # Program for reformatting trace buffer output according to user-supplied rules > > > > +from __future__ import division > > +from __future__ import print_function > > +from __future__ import unicode_literals > > +from builtins import str > > +from past.utils import old_div > > This adds a new dependency on a package we (upstream Xen) don't > currently use. AFAICT, it's only for... > > > @@ -223,7 +232,7 @@ while not interrupted: > > last_tsc[cpu] = tsc > > > > if mhz: > > - tsc = tsc / (mhz*1000000.0) > > + tsc = old_div(tsc, (mhz*1000000.0)) > > ... this, which is always int / float and doesn't fall into Py2's > int/int behaviour in the first place. > > I'm pretty sure the code can just stay as it is, without needing to use > old_div(). Ok, I will the dependency on python3 future and the old_div() for v2. Cheers, Javi
On 11/09/2023 2:44 pm, Javi Merino wrote: > On Mon, Sep 11, 2023 at 11:52:43AM +0100, Andrew Cooper wrote: >> On 06/09/2023 2:14 pm, Javi Merino wrote: >>> diff --git a/tools/xentrace/xentrace_format b/tools/xentrace/xentrace_format >>> index 5ff85ae2e8..166ebae325 100644 >>> --- a/tools/xentrace/xentrace_format >>> +++ b/tools/xentrace/xentrace_format >>> @@ -4,11 +4,15 @@ >>> >>> # Program for reformatting trace buffer output according to user-supplied rules >>> >>> +from __future__ import division >>> +from __future__ import print_function >>> +from __future__ import unicode_literals >>> +from builtins import str >>> +from past.utils import old_div >> This adds a new dependency on a package we (upstream Xen) don't >> currently use. AFAICT, it's only for... >> >>> @@ -223,7 +232,7 @@ while not interrupted: >>> last_tsc[cpu] = tsc >>> >>> if mhz: >>> - tsc = tsc / (mhz*1000000.0) >>> + tsc = old_div(tsc, (mhz*1000000.0)) >> ... this, which is always int / float and doesn't fall into Py2's >> int/int behaviour in the first place. >> >> I'm pretty sure the code can just stay as it is, without needing to use >> old_div(). > Ok, I will the dependency on python3 future and the old_div() for v2. My point is that I don't think we need this dependency at all, and I don't think you need to change this line at all. ~Andrew
On Mon, Sep 11, 2023 at 02:45:56PM +0100, Andrew Cooper wrote: > On 11/09/2023 2:44 pm, Javi Merino wrote: > > On Mon, Sep 11, 2023 at 11:52:43AM +0100, Andrew Cooper wrote: > >> On 06/09/2023 2:14 pm, Javi Merino wrote: > >>> diff --git a/tools/xentrace/xentrace_format b/tools/xentrace/xentrace_format > >>> index 5ff85ae2e8..166ebae325 100644 > >>> --- a/tools/xentrace/xentrace_format > >>> +++ b/tools/xentrace/xentrace_format > >>> @@ -4,11 +4,15 @@ > >>> > >>> # Program for reformatting trace buffer output according to user-supplied rules > >>> > >>> +from __future__ import division > >>> +from __future__ import print_function > >>> +from __future__ import unicode_literals > >>> +from builtins import str > >>> +from past.utils import old_div > >> This adds a new dependency on a package we (upstream Xen) don't > >> currently use. AFAICT, it's only for... > >> > >>> @@ -223,7 +232,7 @@ while not interrupted: > >>> last_tsc[cpu] = tsc > >>> > >>> if mhz: > >>> - tsc = tsc / (mhz*1000000.0) > >>> + tsc = old_div(tsc, (mhz*1000000.0)) > >> ... this, which is always int / float and doesn't fall into Py2's > >> int/int behaviour in the first place. > >> > >> I'm pretty sure the code can just stay as it is, without needing to use > >> old_div(). > > Ok, I will the dependency on python3 future and the old_div() for v2. > > My point is that I don't think we need this dependency at all, and I > don't think you need to change this line at all. Yes. And I forgot to add "remove" and the sentence made no sense. What I meant was: Ok, I will remove the dependency on python3 future and the old_div() for v2. Cheers, Javi
diff --git a/tools/xentrace/xentrace_format b/tools/xentrace/xentrace_format index 5ff85ae2e8..166ebae325 100644 --- a/tools/xentrace/xentrace_format +++ b/tools/xentrace/xentrace_format @@ -4,11 +4,15 @@ # Program for reformatting trace buffer output according to user-supplied rules +from __future__ import division +from __future__ import print_function +from __future__ import unicode_literals +from builtins import str +from past.utils import old_div import re, sys, string, signal, struct, os, getopt def usage(): - print >> sys.stderr, \ - "Usage: " + sys.argv[0] + """ defs-file + print("Usage: " + sys.argv[0] + """ defs-file Parses trace data in binary format, as output by Xentrace and reformats it according to the rules in a file of definitions. The rules in this file should have the format ({ and } show grouping @@ -29,7 +33,7 @@ def usage(): this script may not be able to keep up with the output of xentrace if it is piped directly. In these circumstances you should have xentrace output to a file for processing off-line. - """ + """, file=sys.stderr) sys.exit(1) def read_defs(defs_file): @@ -49,7 +53,7 @@ def read_defs(defs_file): m = reg.match(line) - if not m: print >> sys.stderr, "Bad format file" ; sys.exit(1) + if not m: print("Bad format file", file=sys.stderr) ; sys.exit(1) defs[str(eval(m.group(1)))] = m.group(2) @@ -83,8 +87,8 @@ interrupted = 0 try: defs = read_defs(arg[0]) -except IOError, exn: - print exn +except IOError as exn: + print(exn) sys.exit(1) # structure of trace record (as output by xentrace): @@ -117,12 +121,17 @@ TRC_PV_HYPERCALL_SUBCALL = 0x20100e NR_VECTORS = 256 irq_measure = [{'count':0, 'tot_cycles':0, 'max_cycles':0}] * NR_VECTORS +if sys.version_info >= (3, 0): + stdin_bytes = sys.stdin.buffer +else: + stdin_bytes = sys.stdin + i=0 while not interrupted: try: i=i+1 - line = sys.stdin.read(struct.calcsize(HDRREC)) + line = stdin_bytes.read(struct.calcsize(HDRREC)) if not line: break event = struct.unpack(HDRREC, line)[0] @@ -140,43 +149,43 @@ while not interrupted: tsc = 0 if tsc_in == 1: - line = sys.stdin.read(struct.calcsize(TSCREC)) + line = stdin_bytes.read(struct.calcsize(TSCREC)) if not line: break tsc = struct.unpack(TSCREC, line)[0] if n_data == 1: - line = sys.stdin.read(struct.calcsize(D1REC)) + line = stdin_bytes.read(struct.calcsize(D1REC)) if not line: break d1 = struct.unpack(D1REC, line)[0] if n_data == 2: - line = sys.stdin.read(struct.calcsize(D2REC)) + line = stdin_bytes.read(struct.calcsize(D2REC)) if not line: break (d1, d2) = struct.unpack(D2REC, line) if n_data == 3: - line = sys.stdin.read(struct.calcsize(D3REC)) + line = stdin_bytes.read(struct.calcsize(D3REC)) if not line: break (d1, d2, d3) = struct.unpack(D3REC, line) if n_data == 4: - line = sys.stdin.read(struct.calcsize(D4REC)) + line = stdin_bytes.read(struct.calcsize(D4REC)) if not line: break (d1, d2, d3, d4) = struct.unpack(D4REC, line) if n_data == 5: - line = sys.stdin.read(struct.calcsize(D5REC)) + line = stdin_bytes.read(struct.calcsize(D5REC)) if not line: break (d1, d2, d3, d4, d5) = struct.unpack(D5REC, line) if n_data == 6: - line = sys.stdin.read(struct.calcsize(D6REC)) + line = stdin_bytes.read(struct.calcsize(D6REC)) if not line: break (d1, d2, d3, d4, d5, d6) = struct.unpack(D6REC, line) if n_data == 7: - line = sys.stdin.read(struct.calcsize(D7REC)) + line = stdin_bytes.read(struct.calcsize(D7REC)) if not line: break (d1, d2, d3, d4, d5, d6, d7) = struct.unpack(D7REC, line) @@ -211,7 +220,7 @@ while not interrupted: if cpu >= len(last_tsc): last_tsc += [0] * (cpu - len(last_tsc) + 1) elif tsc < last_tsc[cpu] and tsc_in == 1: - print "TSC stepped backward cpu %d ! %d %d" % (cpu,tsc,last_tsc[cpu]) + print("TSC stepped backward cpu %d ! %d %d" % (cpu,tsc,last_tsc[cpu])) # provide relative TSC if last_tsc[cpu] > 0 and tsc_in == 1: @@ -223,7 +232,7 @@ while not interrupted: last_tsc[cpu] = tsc if mhz: - tsc = tsc / (mhz*1000000.0) + tsc = old_div(tsc, (mhz*1000000.0)) args = {'cpu' : cpu, 'tsc' : tsc, @@ -239,18 +248,18 @@ while not interrupted: try: - if defs.has_key(str(event)): - print defs[str(event)] % args + if str(event) in defs: + print(defs[str(event)] % args) else: - if defs.has_key(str(0)): print defs[str(0)] % args + if str(0) in defs: print(defs[str(0)] % args) except TypeError: - if defs.has_key(str(event)): - print defs[str(event)] - print args + if str(event) in defs: + print(defs[str(event)]) + print(args) else: - if defs.has_key(str(0)): - print defs[str(0)] - print args + if str(0) in defs: + print(defs[str(0)]) + print(args) - except IOError, struct.error: sys.exit() + except (IOError, struct.error): sys.exit()
Closes issue #155 Signed-off-by: Javi Merino <javi.merino@cloud.com> --- Tested using the format file in tools/xentrace/formats. With this patch, both python2 and python3 produce the same output. tools/xentrace/xentrace_format | 63 +++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 27 deletions(-)