Page MenuHomeFreeBSD

Dynamically load .so modules to expand functionality
ClosedPublic

Authored by imp on Dec 6 2018, 5:15 PM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 5 2024, 3:44 AM
Unknown Object (File)
Jan 2 2024, 7:57 PM
Unknown Object (File)
Dec 24 2023, 1:14 PM
Unknown Object (File)
Dec 22 2023, 9:53 PM
Unknown Object (File)
Dec 13 2023, 3:59 AM
Unknown Object (File)
Nov 30 2023, 7:01 AM
Unknown Object (File)
Nov 26 2023, 5:48 PM
Unknown Object (File)
Oct 19 2023, 8:57 PM
Subscribers
None

Details

Summary

o Dynamically load all the .so files found in /libexec/nvmecontrol and

/usr/local/libexec/nvmecontrol.

o Link nvmecontrol -rdynamic so that its symbols are visible to the

libraries we load.

o Create mirrored linker sets that we dynamically expand.
o Add the linked-in top and logpage linker sets to the mirrors for them

and add those sets to the mirrors when we load a new .so.

o Add some macros to help hide the names of the linker sets.
o Update the man page.

Test Plan

run through all the commands before / after the change and make sure they work.
Use my experimental builds of intel.so and wdc.so to proof the concept.
A future commit will create the modules.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

imp added a reviewer: cem.

Mostly LGTM, thanks.

sbin/nvmecontrol/nvmecontrol.c
89 ↗(On Diff #51665)

style nit: separate line for static void

151–159 ↗(On Diff #51665)

Seems like this could be simplified somewhat with realloc:

m->begin = reallocarray(m->begin, cur_n + add_n, sizeof(void *));
if (m->begin == NULL)
    err(1, "expanding mirror set");
memcpy(m->begin + cur_n, bp, add_n * sizeof(void *));

(err can be used, malloc/realloc/etc set errno on failure.)

324 ↗(On Diff #51665)

extra space between NULL and ;

325 ↗(On Diff #51665)

Could use dent->d_namlen in place of strlen(dent->d_name).

327–329 ↗(On Diff #51665)

I'd err() this failure — if this tiny allocation failed, we're not going to be able to make much progress anyway.

351 ↗(On Diff #51665)

closedir(dir); ?

360–361 ↗(On Diff #51665)

I'm not sure if libexec is the right place for these. They might just be 'lib'. libexec is generally for helper programs, not modules. There is some precedence here in, e.g., /lib/geom. (Either way, it should be documented in hier.7.)

sbin/nvmecontrol/nvmecontrol.h
92 ↗(On Diff #51665)

How did you land on the name "mirror?" It's confusing to me. Something like "contiguous_set" would make more sense to me, since the goal is to contain multiple .so's set tables in a single contiguous VA for easy searching.

imp marked 7 inline comments as done.Dec 6 2018, 9:21 PM
imp added inline comments.
sbin/nvmecontrol/nvmecontrol.c
89 ↗(On Diff #51665)

used in one place, and less gross than before, so expanded inline.

151–159 ↗(On Diff #51665)

Good idea.

324 ↗(On Diff #51665)

thanks

325 ↗(On Diff #51665)

true. Don't really need to optimize here, but it's easy.

327–329 ↗(On Diff #51665)

OK.

351 ↗(On Diff #51665)

nice catch.

360–361 ↗(On Diff #51665)

I Was all set to argue, but we have /lib/geom, /lib/casper (annd libexec/casper which just has executables) and /usr/lib/clang. So I'm sold.

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

updates per cem@

LGTM. I still find the 'mirror' name confusing, but maybe that's a personal problem ;-).

This revision is now accepted and ready to land.Dec 6 2018, 10:09 PM
In D18455#393317, @cem wrote:

LGTM. I still find the 'mirror' name confusing, but maybe that's a personal problem ;-).

I'll change it to set_concat which is less confusing.

This revision was automatically updated to reflect the committed changes.