Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[inputs/diskio] Fix how major and minor identifiers of block devices are read. #5566

Merged
merged 3 commits into from
Mar 22, 2019

Conversation

Soulou
Copy link
Contributor

@Soulou Soulou commented Mar 11, 2019

The current implementation assure that the major and the minor are
coded on one byte. But they are not:

brw-rw----  1 root disk    252, 290 Feb 25 11:36 dm-290

290 as minor in this example is over 1 byte.

So after wondering why all my devices iops weren't correctly stored,
I found out that several points were added for some disks. For dm-290
it was overriding 252:34, instead of getting udev stats for 252:290.

The solution is here:
https://sites.uclouvain.be/SystInfo/usr/include/sys/sysmacros.h.html

The implementation is directly taken from this, fixing my bug.

@Soulou Soulou changed the title Fix how major and minor identifiers of block devices are read. [inputs/diskio] Fix how major and minor identifiers of block devices are read. Mar 11, 2019
The current implementation assure that the major and the minor are
coded on one byte. But they are not:

```
brw-rw----  1 root disk    252, 290 Feb 25 11:36 dm-290
```

290 as minor in this example is over 1 byte.

So after wondering why all my devices iops weren't correctly stored,
I found out that several points were added for some disks. For `dm-290`
it was overriding `252:34`, instead of getting udev stats for `252:290`.

The solution is here:
https://sites.uclouvain.be/SystInfo/usr/include/sys/sysmacros.h.html

The implementation is directly taken from this, fixing my bug.
@Soulou Soulou force-pushed the fix/diskio/major-minor branch from 513a392 to 92b01ab Compare March 11, 2019 00:31
@danielnelson
Copy link
Contributor

Could we use these functions to extract the major/minor numbers?

https://github.com/golang/sys/blob/c8c8c57fd1e1a977c10821a559dda983a9a9a3b5/unix/dev_linux.go#L21-L32

@danielnelson danielnelson added this to the 1.10.1 milestone Mar 11, 2019
@danielnelson danielnelson added the fix pr to fix corresponding bug label Mar 11, 2019
@Soulou
Copy link
Contributor Author

Soulou commented Mar 15, 2019

Definitely, I'm doing it now

@Soulou
Copy link
Contributor Author

Soulou commented Mar 15, 2019

done

@danielnelson
Copy link
Contributor

@Soulou could you look into this build error on mipsle?

[ERROR] run: Command 'GOOS=linux GOARCH=mipsle go build -o /go/src/github.com/influxdata/telegraf/build/linux/mipsel/telegraf  -ldflags="-w -s -X main.branch=pull/5566 -X main.commit=2118fbe7" ./cmd/telegraf' failed with error: # github.com/influxdata/telegraf/plugins/inputs/diskio
plugins/inputs/diskio/diskio_linux.go:38:26: cannot use stat.Rdev (type uint32) as type uint64 in argument to unix.Major
plugins/inputs/diskio/diskio_linux.go:39:26: cannot use stat.Rdev (type uint32) as type uint64 in argument to unix.Minor

@danielnelson danielnelson modified the milestones: 1.10.1, 1.10.2 Mar 19, 2019
For most platforms, stat.Rdev is already a uint64 so this is without any effect
for linux,mipsle, unix.Stat_t.Rdev is a uint32, but the way to compute major and minor doesn't change, casting the uint32 has no impact either
@Soulou
Copy link
Contributor Author

Soulou commented Mar 21, 2019

done :)

@danielnelson danielnelson merged commit 9ba023f into influxdata:master Mar 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix pr to fix corresponding bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants