Add -p (physpath) and -s (serial#) to diskinfo
ClosedPublic
Actions

Authored by allanjude on Sep 8 2016, 4:21 AM.

Details

Reviewers

emaste
wblock
rpokala
bcr

Group Reviewers

manpages

Commits

rS320555: Add -s (serial) and -p (physpath) to diskinfo

Summary

Return the bare requested information, intended for scripting

The serial number of a SAS/SCSI device can be returned with 'camcontrol inquiery disk -S'
This instead provides a way to get this information from both SAS and SATA devices

the -p and -s flags are mutually exclusive, and cannot be used with any other flags.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

allanjude updated this revision to Diff 20168.Sep 8 2016, 4:21 AM

allanjude retitled this revision from to Add -i (ident) and -p (physpath) to diskinfo.

allanjude updated this object.

allanjude edited the test plan for this revision. (Show Details)

allanjude added reviewers: wblock, bcr, emaste.

Herald added a reviewer: manpages. · View Herald TranscriptSep 8 2016, 4:21 AM

Herald added a subscriber: imp. · View Herald Transcript

Make sure to s/inquiery/inquiry/ for the description before submitting. :-)

Other that, looks good.

This revision is now accepted and ready to land.Sep 8 2016, 4:59 AM

wblock added inline comments.Sep 9 2016, 1:19 PM

usr.sbin/diskinfo/diskinfo.8
61 ↗	(On Diff #20168)	s/Prints/Print/
63 ↗	(On Diff #20168)	s/Triggers/Trigger/ or possibly just "Make".
65 ↗	(On Diff #20168)	s/Triggers/Perform/ The way this is written, the "the disks" should be possessive. But it is simpler to just change it to just "disk": Perform a simple and rather naive benchmark of disk seek
68 ↗	(On Diff #20168)	Simplify: Return the disk ident, usually the serial number.
70 ↗	(On Diff #20168)	s/Returns/Return/ But this also needs to explain what the term "physical path" means.
71 ↗	(On Diff #20168)	This is a sentence fragment that can be merged into a full sentence explaining the "physical path" term.

diskinfo grew -i (iops benchmark) in the meantime

solve the merge conflicts and rename the serial number operation to -s

This revision now requires review to proceed.Oct 13 2016, 1:41 AM

allanjude retitled this revision from Add -i (ident) and -p (physpath) to diskinfo to Add -p (physpath) and -s (serial#) to diskinfo.Oct 13 2016, 1:41 AM

allanjude updated this object.

allanjude edited edge metadata.

wblock added inline comments.Oct 14 2016, 5:24 PM

usr.sbin/diskinfo/diskinfo.8
78 ↗	(On Diff #21334)	Isn't this a duplicate of lines 61-62? Both of them still have the second sentence fragment, which should be a separate sentence: Return the physical path of the disk. This is a (string? can we be more specific about format?) that identifies the physical location of the disk in an enclosure.
usr.sbin/diskinfo/diskinfo.c
111 ↗	(On Diff #21334)	This is vague. Can I use -p and -s together but without other options? That would be "on their own", maybe. How about -p or -s cannot be used with other options
140 ↗	(On Diff #21334)	It would be more useful to the user to tell them what they can be used on: -p and -s only operate on actual devices