Introduce phys2disk.
AbandonedPublic
Actions

Authored by oshogbo on Nov 4 2017, 4:04 PM.

Details

Reviewers

allanjude
markj
cem
imp
wblock
mav
ken

Group Reviewers

cam

Summary

This extensions allows us to map connected disk to static disk number.
For the CAM devices we use bus, target and lun to distinguish disk number.
For the mfi we use enclosure and slot.

This extension allow us to enumerate disk for example that the left top disk will be named disk0.

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

oshogbo created this revision.Nov 4 2017, 4:04 PM

markj added reviewers: mav, ken.Nov 4 2017, 4:10 PM

Why this way, instead of adding handling all that inside disk(9) instead? That would "just work" for all the periph drivers, without changing them.

Also, have you considered using disk_add_alias(9) instead? If you added an optional mechanism to add aliases based on physical location, it would "just work" without the need to configure the maps by hand.

Regenerate with -U99999.

In D12945#268728, @trasz wrote:

Why this way, instead of adding handling all that inside disk(9) instead? That would "just work" for all the periph drivers, without changing them.

We set the name after doing disk_alloc(9), or do you have different idea?

In D12945#268732, @oshogbo wrote:

In D12945#268728, @trasz wrote:

Why this way, instead of adding handling all that inside disk(9) instead? That would "just work" for all the periph drivers, without changing them.

We set the name after doing disk_alloc(9), or do you have different idea?

We have alias support for geom, but there may be some issues with it (I have it to look at). It would need to be expanded a little.

Also, you didn't do nda nor mda. Nor the dozen or so other non-CAM storage devices.

Ideally, this would be done 100% in geom.

In addition to the man page changes I listed, you also need to bump the .Dd to the date of the commit. Thanks!

sbin/geom/class/part/gpart.8
1150	adds a map
1154	s/get/queried/
1159	s/means/mean/
1160	kill the first "the".
1167	s/get/queried/
1173	s/disk/disks/
1174	Remove the extra space before "disk".
1176	Put a comma after ada

I agree with above commenters that this is too narrow and specific. For example, for mps/mpr HBAs BTL addresses are tied not to physical location, but to disk SAS addresses. It means in case of disk replacement it will likely change, making any numbering based on top unstable.

wblock added inline comments.Nov 17 2017, 7:09 PM

sbin/geom/class/part/gpart.8
1150	Even then... not sure what this is saying. Maybe: Add a map for how the disk is to be renamed.
1154	All these values can be queried with
1155	.Xr camcontrol 8 .
1156	Delete this.
1157	For example,``` (add comma, no "the")
1159	means that the disk on bus 0, target 0 and lun 0 should be renamed to disk12.
1160	Unclear here, mfi(4)? If the disk is connected to an .Xr mfi 4 controller, the
1167	Values can be queried with .Xr mfiutil 8 .
1169	Delete this line.
1170	Separate disk mappings, not disks. Use spaces to separate multiple disk mappings.
1172	This is not a path, so not .Pa. Delete the "For". (Untested, but I think this is the right idea.) .Bd -literal -offset indent b0t1l0=0 b0t2l0=1 .Ed shows two disk renaming parameters.
1173	Delete this line.
1174	Use the serial comma. The disk on bus 0, target 1, lun 0 will be renamed to disk0. The disk on bus 0, target 2, lun 0 will be renamed to disk 1.
1176	More commas, including a serial comma. When this sysctl is in use, the ad, ada, or mfisyspd devices will not be created
1177	if the device is on this list. Why sysctl and which device? Not sure what this is trying to say. Maybe Add .Xr ad 4 , .Xr ada 4 , .Xr mfisyspd ? devices to this list to prevent device creation by those drivers. But it is not clear. Please explain in detail.
1179	Not sure what "No 1" means.
1180	Automatic resize behavior. When this variable is enabled and a new provider size is detected, the schema metadata is resized. Run .Cm gpart commit to commit changes to disk.