Page MenuHomeFreeBSD

libcam: Define depop structures and introduce scsi_wrap
ClosedPublic

Authored by imp on Mar 2 2021, 7:41 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Oct 25, 12:14 AM
Unknown Object (File)
Thu, Oct 24, 2:56 AM
Unknown Object (File)
Oct 6 2024, 4:28 AM
Unknown Object (File)
Oct 6 2024, 1:53 AM
Unknown Object (File)
Oct 5 2024, 8:22 PM
Unknown Object (File)
Oct 3 2024, 1:03 AM
Unknown Object (File)
Sep 30 2024, 2:14 PM
Unknown Object (File)
Sep 30 2024, 1:13 PM
Subscribers
None

Details

Summary

Define structures related to the depop set of commands (GET PHYSICAL ELEMENT
STATUS, REMOVE ELEMENT AND TRUNCATE, and RESTORE ELEMENT AND REBUILD) as
well as the CDB construction routines.

Also create scsi_wrap.c. This will have convenience routines that will do all
the elements of allocating the ccb, generating the CDB, sending the command
(looping as necessary for cases where data is returned, but it's size isn't
known up front), etc. As this functionality is fleshed out, calling many
camcontrol commands programatically gets much easier.

Test Plan

camcontrol depop uses these functions successfully today.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

imp requested review of this revision.Mar 2 2021, 7:41 PM
imp added reviewers: ken, scottl, mav.

The CDBs and filling functions look good. We should figure out what our user interface idiom is going to be for the scsi_wrap_* functions. Do we assume we have stdout/stderr, or do we provide those strings for the user to print in a way that works for the calling application?

lib/libcam/scsi_wrap.c
95

So, what do we want our interface to be, or our "contract", when we're wrapping commands in libcam?

Do we want to print to stdout/stderr by default, or provide a mechanism to get those strings back to the caller so that they can send them out via their logging interface or user interface?

I'm just thinking here about some library (really Ruby Gem) code I've written for Spectra that does some things that are similar to camcontrol, but the errors are handled in a way that is more friendly to the calling Ruby management application.

In D29017#649782, @ken wrote:

The CDBs and filling functions look good. We should figure out what our user interface idiom is going to be for the scsi_wrap_* functions. Do we assume we have stdout/stderr, or do we provide those strings for the user to print in a way that works for the calling application?

I was thinking that the higher level programs should report the error, but I haven't figured out the proper way to do this yet. I was leaning towards the latter, but hadn't come up with a good way to do that. It just occurred to me, though, having a buffer in 'cam_device' would be the right way to handle the errors? The current cam_print_error interface, though, requires a ccb. We could use cam_error_string instead, I think and just buffer that and have the user print that if they want to use this convenience interface?

lib/libcam/scsi_wrap.c
70

What number of 32-byte descriptors do you expect there be? If it is about heads, I'd guess about dozen. Wouldn't it be better to guess some size first to save one request in most cases?

78

Many tools were switched to calloc() to not call bzero explicitly.

lib/libcam/scsi_wrap.c
70

chuck had the same feedback. I just did 4k since that's big enough to return lots of elements, and small enough to not matter.

78

Sure. That's not a bad idea.

95

I'm thinking of adding an error sbuf to cam_device that accumulates errors that the calling process can use to report errors and then cleanup...

Use calloc, per mav. Use 4k allocation per chs and mav.

One minor comment.

lib/libcam/scsi_wrap.c
95

So what about threaded processes with multiple threads accessing the same device? I guess we'll expect the caller to lock that as needed, just as they would have to if they were accessing any other fields in there.

That's probably a reasonable way to handle the static cam_errbuf issue with multiple threads/devices potentially putting error messages in it.

imp marked an inline comment as done.Jul 12 2021, 11:10 PM
lib/libcam/scsi_wrap.c
95

I think that's a sensible strategy.

imp marked 2 inline comments as done.Sep 17 2021, 10:32 PM
This revision was not accepted when it landed; it landed in state Needs Review.Sep 20 2021, 10:40 PM
This revision was automatically updated to reflect the committed changes.