camcontrol: depop command
ClosedPublic
Actions

Authored by imp on Mar 2 2021, 7:41 PM.

Details

Reviewers

ken
scottl
mav
gbe

Group Reviewers

manpages

Commits

rG53cce2e744c1: camcontrol: depop command

Summary

Document the new depop command. This comand manages drive elements for drives
that support it. Storage elements are typically heads. Element status can be
discovered. Elements may be removed or restored. And the status of any current
depop operation can be assessed.

depop -d elm will remove elemement elm and truncate available capacity.
depop -l will list the current drive elmements and their current status.
depop -r elm will try to restore all retired elements and rebuild capacity.

Changing storage elements may reinitialize the drive. This opereation will lose
data and may take hours to complete. Use the drive provided timeout for
operations by default.

Test Plan

camcontrol depop da1 -l
<see the list of commands>
camcontrol depop da1 -d -e 1
see that it works...

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 37511
Build 34400: arc lint + arc unit

Event Timeline

imp created this revision.Mar 2 2021, 7:41 PM

Herald added a reviewer: manpages. · View Herald TranscriptMar 2 2021, 7:41 PM

imp requested review of this revision.Mar 2 2021, 7:41 PM

Harbormaster completed remote builds in B37511: Diff 84996.Mar 2 2021, 7:41 PM

imp edited the test plan for this revision. (Show Details)Mar 2 2021, 7:46 PM

imp added reviewers: ken, scottl, mav.

Noticed two nits after submitting. I'll look at fixing those, but they won't affect the rest of the structure of this code.

sbin/camcontrol/depop.c
128	this is bogus. There's a vpd that contains the timeout value we should use. This code should be updated to use that value as the default. Here and in depop_restore().
169	same.

A couple of small nits but overall this looks good.

sbin/camcontrol/camcontrol.c
9951	This should include [-a elm] [-d elm], etc.
sbin/camcontrol/depop.c
128	Is the VPD page mandatory? It isn't clear at least from SBC4r16. You may want to have a hard-coded default to fall back on if you can't get the VPD value and if the user hasn't specified one. I don't think you should use it here, but there are timeout descriptors that are optionally supported with REPORT SUPPORTED OPERATION CODES. But a lot of devices don't support that. (camcontrol opcodes device -T)

Document the new depop command.

Implement and document the new "depop" command.

sbin/camcontrol/camcontrol.8
2606	Is it usually a head though? Can't it just as easily be a surface of a platter which has gone bad (i.e. a defect taking out lots of sectors?) Or, in the case of an SSD, the failure of an entire NAND chip? I'd just leave off the `(usually heads)` entirely.
sbin/camcontrol/camcontrol.c
10216	Did you mean `-r`, not `-a`? That's what's implied by the manpage...?
10219	`-s` is not documented in the manpage.
sbin/camcontrol/depop.c
45	Please specify whether or not I/O can be done after a power-cycle which interrupts depop, and if/how the depop needs to be resumed or restarted.

Thanks for the feedback. I need to go through and fold it in.

sbin/camcontrol/camcontrol.8
2606	Don't know of any SSDs that support this :) However, I'll add 'in hard drives' instead since this appears to be most commonly implemented in hard drives.
sbin/camcontrol/camcontrol.c
9951	Gotcha. I put this here and then forgot to update it once I settled on the interface. Nice catch.
10216	last minute change.
10219	It should be removed. I'd planned on having it and dropped it at the last minute and didn't update this.
sbin/camcontrol/depop.c
45	The device is format corrupt. No I/O is possible until you do the depop command again.
128	It's in the block device characteristics VPD page (page 0xB1). I could check the supported VPD pages, but I'd planned on just fetching the VPD and silently defaulting to 1 day if I can't or it's zero for some reason. It is technically optional after all.