[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC][PATCH] 3/3] [TOOLS][XENTRACE] Various tidyups to xentrace tools.
> - use err/errx/warn, instead of PERROR, perror and fprintf Good stuff! I like this. > - Match () place ment consistent in this files Thanks for doing this. While you're at it, would you like to fix the bracing? Some of it is in K&R style: if ( aoeuaoeu ) { Whereas other Xen code uses: if ( aoeuaoeu ) { Braces always starting on a new line, for all types of block. > - Use consistent tabs and whitespacing. Thanks for doing this. Cheers, Mark > > Signed-off-by: Tony Breeds <tony@xxxxxxxxxxxxxxxxxx> > --- > > tools/xentrace/xentrace.c | 137 > ++++++++++++++--------------------------- tools/xentrace/xentrace_format | > 51 +++++++-------- > 2 files changed, 73 insertions(+), 115 deletions(-) > > Index: xen-unstable.hg-mainline.xentrace/tools/xentrace/xentrace.c > =================================================================== > --- xen-unstable.hg-mainline.xentrace.orig/tools/xentrace/xentrace.c > +++ xen-unstable.hg-mainline.xentrace/tools/xentrace/xentrace.c > @@ -22,6 +22,7 @@ > #include <signal.h> > #include <inttypes.h> > #include <string.h> > +#include <err.h> > > #include <xen/xen.h> > #include <xen/trace.h> > @@ -42,16 +43,6 @@ > > #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) > > -#define PERROR(_m, _a...) \ > -do { \ > - int __saved_errno = errno; \ > - fprintf(stderr, "ERROR: " _m " (%d = %s)\n" , ## _a , \ > - __saved_errno, strerror(__saved_errno)); \ > - errno = __saved_errno; \ > -} while (0) > - > -extern FILE *stderr; > - > /***** Compile time configuration of defaults > ********************************/ > > /* when we've got more records than this waiting, we log it to the output > */ @@ -88,7 +79,7 @@ void close_handler(int signal) > struct timespec millis_to_timespec(unsigned long millis) > { > struct timespec spec; > - > + > spec.tv_sec = millis / 1000; > spec.tv_nsec = (millis % 1000) * 1000; > > @@ -128,10 +119,7 @@ void write_rec(uint32_t cpu, struct t_re > } > > if ( written != 8 ) > - { > - PERROR("Failed to write trace record"); > - exit(EXIT_FAILURE); > - } > + err(EXIT_FAILURE, "Failed to write trace record"); > } > > static void get_tbufs(unsigned long *mfn, unsigned long *size) > @@ -139,21 +127,16 @@ static void get_tbufs(unsigned long *mfn > int xc_handle = xc_interface_open(); > int ret; > > - if ( xc_handle < 0 ) > - { > - exit(EXIT_FAILURE); > - } > + if ( xc_handle < 0 ) > + errx(EXIT_FAILURE, "Unable to open the xc interface"); > > - if(!opts.tbuf_size) > - opts.tbuf_size = DEFAULT_TBUF_SIZE; > + if ( !opts.tbuf_size ) > + opts.tbuf_size = DEFAULT_TBUF_SIZE; > > ret = xc_tbuf_enable(xc_handle, opts.tbuf_size, mfn, size); > > if ( ret != 0 ) > - { > - perror("Couldn't enable trace buffers"); > - exit(1); > - } > + err(EXIT_FAILURE, "Couldn't enable trace buffers"); > > xc_interface_close(xc_handle); > } > @@ -174,10 +157,8 @@ struct t_buf *map_tbufs(unsigned long tb > > xc_handle = xc_interface_open(); > > - if ( xc_handle < 0 ) > - { > - exit(EXIT_FAILURE); > - } > + if ( xc_handle < 0 ) > + errx(EXIT_FAILURE, "Unable to open the xc interface"); > > /* On PPC (At least) the DOMID arg is ignored in dom0 */ > tbufs_mapped = xc_map_foreign_range(xc_handle, DOMID_XEN, > @@ -186,18 +167,15 @@ struct t_buf *map_tbufs(unsigned long tb > > xc_interface_close(xc_handle); > > - if ( tbufs_mapped == 0 ) > - { > - PERROR("Failed to mmap trace buffers"); > - exit(EXIT_FAILURE); > - } > + if ( tbufs_mapped == 0 ) > + err(EXIT_FAILURE, "Failed to mmap trace buffers"); > > return tbufs_mapped; > } > > /** > * set_mask - set the cpu/event mask in HV > - * @mask: the new mask > + * @mask: the new mask > * @type: the new mask type,0-event mask, 1-cpu mask > * > */ > @@ -206,21 +184,19 @@ void set_mask(uint32_t mask, int type) > int ret = 0; > int xc_handle = xc_interface_open(); /* for accessing control > interface */ > > - if (type == 1) { > + if ( type == 1 ) { > ret = xc_tbuf_set_cpu_mask(xc_handle, mask); > - fprintf(stderr, "change cpumask to 0x%x\n", mask); > - } else if (type == 0) { > + warnx("change cpumask to 0x%x\n", mask); > + } else if ( type == 0 ) { > ret = xc_tbuf_set_evt_mask(xc_handle, mask); > - fprintf(stderr, "change evtmask to 0x%x\n", mask); > + warnx("change evtmask to 0x%x\n", mask); > } > > xc_interface_close(xc_handle); > > if ( ret != 0 ) > - { > - PERROR("Failure to get trace buffer pointer from Xen and set the > new mask"); - exit(EXIT_FAILURE); > - } > + err(EXIT_FAILURE, "Failure to get trace buffer pointer from " > + "Xen and set the new mask"); > } > > /** > @@ -240,11 +216,8 @@ struct t_buf **init_bufs_ptrs(void *bufs > > user_ptrs = (struct t_buf **)calloc(num, sizeof(struct t_buf *)); > if ( user_ptrs == NULL ) > - { > - PERROR( "Failed to allocate memory for buffer pointers\n"); > - exit(EXIT_FAILURE); > - } > - > + err(EXIT_FAILURE, "Failed to allocate memory for buffer > pointers"); + > /* initialise pointers to the trace buffers - given the size of a > trace * buffer and the value of bufs_maped, we can easily calculate these > */ for ( i = 0; i<num; i++ ) > @@ -269,13 +242,10 @@ struct t_rec **init_rec_ptrs(struct t_bu > { > int i; > struct t_rec **data; > - > + > data = calloc(num, sizeof(struct t_rec *)); > if ( data == NULL ) > - { > - PERROR("Failed to allocate memory for data pointers\n"); > - exit(EXIT_FAILURE); > - } > + err(EXIT_FAILURE, "Failed to allocate memory for data pointers"); > > for ( i = 0; i < num; i++ ) > data[i] = (struct t_rec *)(meta[i] + 1); > @@ -291,14 +261,11 @@ uint32_t get_num_cpus(void) > xc_physinfo_t physinfo; > int xc_handle = xc_interface_open(); > int ret; > - > + > ret = xc_physinfo(xc_handle, &physinfo); > - > + > if ( ret != 0 ) > - { > - PERROR("Failure to get logical CPU count from Xen"); > - exit(EXIT_FAILURE); > - } > + errx(EXIT_FAILURE, "Failure to get logical CPU count from Xen"); > > xc_interface_close(xc_handle); > > @@ -377,17 +344,17 @@ int parse_evtmask(char *arg, struct argp > char *inval; > > /* search filtering class */ > - if (strcmp(arg, "gen") == 0){ > + if ( strcmp(arg, "gen") == 0 ) > setup->evt_mask |= TRC_GEN; > - } else if(strcmp(arg, "sched") == 0){ > + else if ( strcmp(arg, "sched") == 0 ) > setup->evt_mask |= TRC_SCHED; > - } else if(strcmp(arg, "dom0op") == 0){ > + else if ( strcmp(arg, "dom0op") == 0 ) > setup->evt_mask |= TRC_DOM0OP; > - } else if(strcmp(arg, "vmx") == 0){ > + else if ( strcmp(arg, "vmx") == 0 ) > setup->evt_mask |= TRC_VMX; > - } else if(strcmp(arg, "all") == 0){ > + else if ( strcmp(arg, "all") == 0 ) > setup->evt_mask |= TRC_ALL; > - } else { > + else { > setup->evt_mask = strtol(arg, &inval, 0); > if ( inval == arg ) > argp_usage(state); > @@ -430,13 +397,13 @@ error_t cmd_parser(int key, char *arg, s > argp_usage(state); > } > break; > - > + > case 'e': /* set new event mask for filtering*/ > { > parse_evtmask(arg, state); > } > break; > - > + > case 'S': /* set tbuf size (given in pages) */ > { > char *inval; > @@ -454,7 +421,7 @@ error_t cmd_parser(int key, char *arg, s > argp_usage(state); > } > break; > - > + > default: > return ARGP_ERR_UNKNOWN; > } > @@ -473,16 +440,16 @@ const struct argp_option cmd_opts[] = > "(default " xstr(NEW_DATA_THRESH) ")." }, > > { .name = "poll-sleep", .key='s', .arg="p", > - .doc = > + .doc = > "Set sleep time, p, in milliseconds between polling the trace buffer > " "for new data (default " xstr(POLL_SLEEP_MILLIS) ")." }, > > { .name = "cpu-mask", .key='c', .arg="c", > - .doc = > + .doc = > "set cpu-mask " }, > > { .name = "evt-mask", .key='e', .arg="e", > - .doc = > + .doc = > "set evt-mask " }, > > { .name = "trace-buf-size", .key='S', .arg="N", > @@ -504,7 +471,7 @@ const struct argp parser_def = > "\v" > "This tool is used to capture trace buffer data from Xen. The data is > " "output in a binary format, in the following order:\n\n" > - " CPU(uint) TSC(uint64_t) EVENT(uint32_t) D1 D2 D3 D4 D5 " > + " CPU(uint32_t) TSC(uint64_t) EVENT(uint32_t) D1 D2 D3 D4 D5 " > "(all uint32_t)\n\n" > "The output should be parsed using the tool xentrace_format, which can > " "produce human-readable output in ASCII format." > @@ -513,8 +480,8 @@ const struct argp parser_def = > > const char *argp_program_version = "xentrace v1.1"; > const char *argp_program_bug_address = "<mark.a.williamson@xxxxxxxxx>"; > - > - > + > + > int main(int argc, char **argv) > { > int outfd = 1, ret; > @@ -529,31 +496,23 @@ int main(int argc, char **argv) > > argp_parse(&parser_def, argc, argv, 0, 0, &opts); > > - if (opts.evt_mask != 0) { > + if ( opts.evt_mask != 0 ) > set_mask(opts.evt_mask, 0); > - } > > - if (opts.cpu_mask != 0) { > + if ( opts.cpu_mask != 0 ) > set_mask(opts.cpu_mask, 1); > - } > > if ( opts.outfile ) > outfd = open(opts.outfile, O_WRONLY | O_CREAT | O_LARGEFILE, > 0644); > > - if(outfd < 0) > - { > - perror("Could not open output file"); > - exit(EXIT_FAILURE); > - } > + if ( outfd < 0 ) > + err(EXIT_FAILURE, "Could not open output file"); > > - if(isatty(outfd)) > - { > - fprintf(stderr, "Cannot output to a TTY, specify a log file.\n"); > - exit(EXIT_FAILURE); > - } > + if ( isatty(outfd) ) > + errx(EXIT_FAILURE, "Cannot output to a TTY, specify a log > file.\n"); > > logfile = fdopen(outfd, "w"); > - > + > /* ensure that if we get a signal, we'll do cleanup, then exit */ > act.sa_handler = close_handler; > act.sa_flags = 0; > Index: xen-unstable.hg-mainline.xentrace/tools/xentrace/xentrace_format > =================================================================== > --- xen-unstable.hg-mainline.xentrace.orig/tools/xentrace/xentrace_format > +++ xen-unstable.hg-mainline.xentrace/tools/xentrace/xentrace_format > @@ -24,7 +24,7 @@ def usage(): > Which correspond to the CPU number, event ID, timestamp counter > and the 5 data fields from the trace record. There should be one such rule > for each type of event. > - > + > Depending on your system and the volume of trace buffer data, > 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 > @@ -34,7 +34,7 @@ def usage(): > > def read_defs(defs_file): > defs = {} > - > + > fd = open(defs_file) > > reg = re.compile('(\S+)\s+(\S.*)') > @@ -43,14 +43,14 @@ def read_defs(defs_file): > line = fd.readline() > if not line: > break > - > - if line[0] == '#' or line[0] == '\n': > - continue > - > + > + if line[0] == '#' or line[0] == '\n': > + continue > + > m = reg.match(line) > > if not m: print >> sys.stderr, "Bad format file" ; sys.exit(1) > - > + > defs[str(eval(m.group(1)))] = m.group(2) > > return defs > @@ -70,7 +70,7 @@ try: > opts, arg = getopt.getopt(sys.argv[1:], "c:" ) > > for opt in opts: > - if opt[0] == '-c' : mhz = int(opt[1]) > + if opt[0] == '-c' : mhz = int(opt[1]) > > except getopt.GetoptError: > usage() > @@ -96,7 +96,7 @@ i=0 > > while not interrupted: > try: > - i=i+1 > + i=i+1 > line = sys.stdin.read(struct.calcsize(CPUREC)) > if not line: > break > @@ -108,19 +108,20 @@ while not interrupted: > > (tsc, event, d1, d2, d3, d4, d5) = struct.unpack(TRCREC, line) > > - #tsc = (tscH<<32) | tscL > + #tsc = (tscH<<32) | tscL > > - #print i, tsc > + #print i, tsc > > if cpu >= len(last_tsc): > last_tsc += [0] * (cpu - len(last_tsc) + 1) > - elif tsc < last_tsc[cpu]: > - print "TSC stepped backward cpu %d ! %d %d" % > (cpu,tsc,last_tsc[cpu]) + elif tsc < last_tsc[cpu]: > + print "TSC stepped backward cpu %d ! %d %d" % (cpu, tsc, > + last_tsc[cpu]) > > - last_tsc[cpu] = tsc > + last_tsc[cpu] = tsc > > - if mhz: > - tsc = tsc / (mhz*1000000.0) > + if mhz: > + tsc = tsc / (mhz*1000000.0) > > args = {'cpu' : cpu, > 'tsc' : tsc, > @@ -131,15 +132,13 @@ while not interrupted: > '4' : d4, > '5' : d5 } > > - try: > - > - if defs.has_key(str(event)): > - print defs[str(event)] % args > - else: > - if defs.has_key(str(0)): print defs[str(0)] % args > - except TypeError: > - print defs[str(event)] > - print args > - > + try: > + if defs.has_key(str(event)): > + print defs[str(event)] % args > + else: > + if defs.has_key(str(0)): print defs[str(0)] % args > + except TypeError: > + print defs[str(event)] > + print args > > except IOError, struct.error: sys.exit() > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@xxxxxxxxxxxxxxxxxxx > http://lists.xensource.com/xen-devel -- Dave: Just a question. What use is a unicyle with no seat? And no pedals! Mark: To answer a question with a question: What use is a skateboard? Dave: Skateboards have wheels. Mark: My wheel has a wheel! _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |