Bug #6623
closediostat -exrn has missing comma (field separator) in header
100%
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
Updated by James Blachly over 6 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, ",");
Updated by Igor Kozhukhov over 6 years ago
- Status changed from New to In Progress
- Assignee set to James Blachly
Updated by Electric Monk about 6 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>