Project

General

Profile

Actions

Bug #6623

closed

iostat -exrn has missing comma (field separator) in header

Added by James Blachly over 5 years ago. Updated about 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
cmd - userland programs
Start date:
2016-02-07
Due date:
% Done:

100%

Estimated time:
1.00 h
Difficulty:
Bite-size
Tags:
needs-triage
Gerrit CR:

Description

Reminder: -r flag to iostat is "raw" output - actually machine parseable CSV.

This issue whereby a comma is missing between the penultimate header field "tot" and the final header field "device" only occurs with the specific combination of -e + -x + -n (when in -r mode)

Interestingly while making this bug report I also noticed a mistake in -rx mode whereby there is an EXTRA terminal comma after %b in the header row which will screw up parsers that might expect a final unnamed field to appear in every row.

Demo (extensive list of devices truncated):

# iostat -re
errors
device,s/w,h/w,trn,tot
cmdk0,0,0,0,0
lofi1,0,0,0,0
ramdisk1,0,0,0,0
sd0,0,0,0,0
sd1,0,0,0,0

# iostat -rx
extended device statistics
device,r/s,w/s,kr/s,kw/s,wait,actv,svc_t,%w,%b,
cmdk0,0.1,10.9,3.7,121.8,0.0,0.0,0.4,0,0
lofi1,0.1,0.0,0.4,0.0,0.0,0.0,1.1,0,0
ramdisk1,0.1,23.5,0.9,92.2,0.0,0.0,0.0,0,0
sd0,0.0,0.0,0.0,0.0,0.0,0.0,0.8,0,0
sd1,150.4,8.6,17337.0,626.6,0.0,0.5,3.2,0,24

# iostat -rn
tty,c2d0,lofi1,ramdisk1,c1t0d0,cpu
tin,tout,kps,tps,serv,kps,tps,serv,kps,tps,serv,kps,tps,serv,us,sy,dt,id
0,244,125,11,0,0,0,1,93,24,0,0,0,1,0,2,0,98

Now, combinations:

# iostat -rex
extended device statistics,errors
device,r/s,w/s,kr/s,kw/s,wait,actv,svc_t,%w,%b,s/w,h/w,trn,tot
cmdk0,0.1,10.9,3.7,121.7,0.0,0.0,0.4,0,0,0,0,0,0
lofi1,0.1,0.0,0.4,0.0,0.0,0.0,1.1,0,0,0,0,0,0
ramdisk1,0.1,23.5,0.9,92.2,0.0,0.0,0.0,0,0,0,0,0,0
sd0,0.0,0.0,0.0,0.0,0.0,0.0,0.8,0,0,0,0,0,0
sd1,150.3,8.6,17323.9,628.6,0.0,0.5,3.2,0,24,0,0,0,0

# iostat -ren
errors
s/w,h/w,trn,tot,device
0,0,0,0,c2d0
0,0,0,0,lofi1
0,0,0,0,ramdisk1
0,0,0,0,c1t0d0
0,0,0,0,c0t9d1

# iostat -rxn
extended device statistics
r/s,w/s,kr/s,kw/s,wait,actv,wsvc_t,asvc_t,%w,%b,device
0.1,10.9,3.7,121.7,0.0,0.0,0.1,0.3,0,0,c2d0
0.1,0.0,0.4,0.0,0.0,0.0,0.4,0.7,0,0,lofi1
0.1,23.5,0.9,92.2,0.0,0.0,0.0,0.0,0,0,ramdisk1
0.0,0.0,0.0,0.0,0.0,0.0,0.0,0.8,0,0,c1t0d0
150.2,8.6,17317.1,629.4,0.0,0.5,0.0,3.2,0,24,c0t9d1

# iostat -rexn
extended device statistics,errors
r/s,w/s,kr/s,kw/s,wait,actv,wsvc_t,asvc_t,%w,%b,s/w,h/w,trn,totdevice
0.1,10.9,3.7,121.7,0.0,0.0,0.1,0.3,0,0,0,0,0,0,c2d0
0.1,0.0,0.4,0.0,0.0,0.0,0.4,0.7,0,0,0,0,0,0,lofi1
0.1,23.5,0.9,92.2,0.0,0.0,0.0,0.0,0,0,0,0,0,0,ramdisk1
0.0,0.0,0.0,0.0,0.0,0.0,0.0,0.8,0,0,0,0,0,0,c1t0d0
150.2,8.6,17314.5,629.6,0.0,0.5,0.0,3.2,0,24,0,0,0,0,c0t9d1

Note in the very last report when combining all three (e, x, n) in -r mode there is no field sep between "tot" and "device".
r/s,w/s,kr/s,kw/s,wait,actv,wsvc_t,asvc_t,%w,%b,s/w,h/w,trn,*totdevice*

I believe this error is in:
illumos-gate/usr/src/cmd/stat/iostat/iostat.c
in function do_format (line 1258)

I believe the missing comma is in line 1323
[[https://github.com/illumos/illumos-gate/blob/139510fb6efa97dbe5f5479594b308d940cab8d1/usr/src/cmd/stat/iostat/iostat.c#L1323]]

As for the extra comma in -rx mode, that's line 1311 [[https://github.com/illumos/illumos-gate/blob/139510fb6efa97dbe5f5479594b308d940cab8d1/usr/src/cmd/stat/iostat/iostat.c#L1311]] with *header = NULL (form line 1274)

Now why did I make an extensive report and not just fix it myself when I know enough C to be dangerous (barely)? To be honest the prospect building the source tree is daunting and I haven't a clue where to start. There are a lot of little rough edges that I think novices could tackle if there were a nice getting started document (how to set up a dev environment, how to build only the userspace tools, how to submit a patch and to whom, etc.)

Hope this is helpful

Kind regards

Actions #1

Updated by James Blachly about 5 years ago

$ diff ~/iostat.c iostat.c
1325a1326,1332
>                                       /* if no -e flag, header == NULL    */
>                                       /* and there will be trailing comma */
>                                       /* in the full disk_header          */
>                                       if (*header == NULL) {
>                                               /* chop trailing comma */
>                                               disk_header[strlen(disk_header) - 1] = 0;
>                                       }
1336a1344,1352
>                                       /* if -rnxe, "tot" (from -e) and */
>                                       /* "device" are run together     */
>                                       /* due to lack of trailing comma */
>                                       /* in 'header'. However, adding  */
>                                       /* trailing comma to header at   */
>                                       /* its definition leads to prob- */
>                                       /* lems elsewhere so it's added  */
>                                       /* here in this edge case -rnxe  */
>                                       if (*header != NULL) strcat(header, ",");
Actions #2

Updated by Igor Kozhukhov about 5 years ago

  • Status changed from New to In Progress
  • Assignee set to James Blachly
Actions #3

Updated by Electric Monk about 5 years ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

git commit 7e61e14d83b646ca8dbac7c2abdecf23066d7ce5

commit  7e61e14d83b646ca8dbac7c2abdecf23066d7ce5
Author: James Blachly <james.blachly@gmail.com>
Date:   2016-04-05T15:21:46.000Z

    6623 iostat -exrn has missing comma (field separator) in header
    Reviewed by: Garrett D'Amore <garrett@damore.org>
    Reviewed by: Yuri Pankov <yuri.pankov@nexenta.com>
    Approved by: Robert Mustacchi <rm@joyent.com>

Actions

Also available in: Atom PDF