Page MenuHomeFreeBSD

Add a new libiscsiutil library.
ClosedPublic

Authored by jhb on Dec 17 2021, 8:58 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 24, 5:25 AM
Unknown Object (File)
Tue, Apr 23, 11:51 AM
Unknown Object (File)
Tue, Apr 16, 6:49 PM
Unknown Object (File)
Tue, Apr 16, 6:49 PM
Unknown Object (File)
Tue, Apr 16, 6:49 PM
Unknown Object (File)
Tue, Apr 16, 6:49 PM
Unknown Object (File)
Tue, Apr 16, 6:49 PM
Unknown Object (File)
Tue, Apr 16, 6:49 PM

Details

Summary

Move some of the code duplicated between ctld(8) and iscsid(8) into a
libiscsiutil library.

Sharing the low-level PDU code did require having a
'struct connection' base class with a method table to permit separate
initiator vs target behavior (e.g. in handling proxy PDUs).

Sponsored by: Chelsio Communications

Diff Detail

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

Event Timeline

jhb requested review of this revision.Dec 17 2021, 8:58 PM

(I ended up not going back and rewriting parts of this to use C++)

Note that there is a net/libiscsi in ports, so perhaps this should use a different name, but I'm not sure what name to use in that case?

I've looked quite briefly, but it looks pretty mechanical change due to complete duplication. I agree that libiscsi name is too obvious. I am actually using SCSI test suite out of github.com/sahlberg/libiscsi sometimes. Not sure what is better though, thinking about: iscsiutil, utiliscsi, kiscsi, ...

This revision is now accepted and ready to land.Dec 18 2021, 3:44 PM

Heh, that’s why I didn’t make it a library in the first place I think; naming is hard :-)

FWIW, I really like the libiscsiutil name, I feel it really well describes what it really is.

libiscsiutil works for me. We could also perhaps make it a private or internal lib (no public .so in /usr/lib) so that only ctld and iscsid would use it. Tagging @emaste so he can render an opinion on if that makes sense (and if so, which one it should be)

jhb retitled this revision from Add a new libiscsi library. to Add a new libiscsiutil library..Dec 21 2021, 12:28 AM
jhb edited the summary of this revision. (Show Details)
  • Rename libiscsi to libiscsiutil.
This revision now requires review to proceed.Dec 21 2021, 12:32 AM
In D33544#758946, @jhb wrote:

We could also perhaps make it a private or internal lib (no public .so in /usr/lib) so that only ctld and iscsid would use it.

I think it would be fine if there is a nice way to do it. I don't really expect it to be reused by anything else, since it is quite specialized and pretty small.

if there is a nice way to do it

There is, just setting PRIVATELIB= in the Makefile will install it with a libprivate prefix, or INTERNALLIB= to create a .a to link against at build time, but not install it.

As to which to choose it comes down to the usual arguments between dynamic and static linking. If the library is large and used by multiple consumers PRIVATELIB seems the way to go. For this case I'd think INTERNALLIB.

  • Make libiscsutil an internal library.
share/mk/src.libnames.mk
560

LIBISCSIUTILDIR?

share/mk/src.libnames.mk
560

The default should work? (There are several settings here that are just the default and aren't necessary.) We seem to have a mix, e.g. LIBBE depends on the default. Oh, hmm, _LIB_OBJTOP vs _OBJTOP. Wonder when that matters.

735

Here is the loop that sets the default.

share/mk/src.libnames.mk
560

Oh, yeah - more magic :(
I'm fine with excluding it if the default works

share/mk/src.libnames.mk
560

I don't know that it does as the default uses _OBJTOP whereas all the other ones here use _LIB_OBJTOP. Probably best to be explicit to be safe, as I don't know the use cases when _LIB_OBJTOP != OBJTOP.

build bits look good to me

This revision is now accepted and ready to land.Dec 22 2021, 1:36 AM
This revision was automatically updated to reflect the committed changes.