-
Notifications
You must be signed in to change notification settings - Fork 23
WIP: Lustre plugin for gufi_dir2index #170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
967eb16
to
9a43865
Compare
Can you also add a simple test plugin + tests? |
contrib/lustre_plugin.c
Outdated
int rc = llapi_file_get_stripe(path, layout_info); | ||
|
||
if (rc) { | ||
printf("lustre plugin: error getting stripe info for %s: %s\n", path, strerror(errno)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stderr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
errno
should be copied before being used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
errno
should be copied before being used
What's the reasoning for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While in this case you're probably fine, people tend to call other functions that they don't realize change errno
, and end up getting prints that say that there are no errors even though they entered an error block. The way to prevent this is to copy errno
so that side effects don't mess up the call to strerror
. I forget what I was doing, but I've seen this happen recently.
src/utils.c
Outdated
return 0; | ||
} | ||
|
||
void *lib = dlopen(in->plugin_name.data, RTLD_NOW); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does dlclose
need to be called?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is unloading shared libraries normal practice? Valgrind, at least, doesn't seem to perceive it as a resource leak...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I have a habit of closing things I open.
This is needed for dynamically loading plugins that use sqlite3 functions. GUFI binaries are built with the sqlite3 object file statically linked. When a plugin is loaded that calls sqlite3 functons, it must refer to the definitions of the sqlite3 symbols that live in the GUFI binary that the plugin is loaded into. The `-export-dynamic` linker option makes the GUFI binaries export these symbols so that the plugin is able to resolve them correctly.
This enables the user to run custom code during database generation. The user can populate the database with new tables, modify entries, etc.
9a43865
to
7c2f11e
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #170 +/- ##
==========================================
- Coverage 91.32% 91.06% -0.26%
==========================================
Files 57 57
Lines 8376 8398 +22
Branches 1109 1116 +7
==========================================
- Hits 7649 7648 -1
- Misses 451 470 +19
- Partials 276 280 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Yep, I'll work on that next. |
* | ||
* $ cd contrib | ||
* | ||
* $ gcc -g -O0 -c -fPIC -I../build/deps/sqlite3/include -I../include -I$LUSTRE_INCLUDE_DIR -I$LUSTRE_INCLUDE_DIR/uapi lustre_plugin.c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
../build/deps/sqlite3/include
-> "${DEP_INSTALL_PREFIX}/sqlite3/include"
* | ||
* Returns 0 on success or 1 on failure. | ||
*/ | ||
int load_plugin_library(struct input *in, char *plugin_name) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static
IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY | ||
OF SUCH DAMAGE. | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 empty lines
* $ gcc -g -O0 -c -fPIC -I../build/deps/sqlite3/include -I../include -I$LUSTRE_INCLUDE_DIR -I$LUSTRE_INCLUDE_DIR/uapi lustre_plugin.c | ||
* | ||
* $ gcc -shared -o liblustre_plugin.so lustre_plugin.o -L$LUSTRE_LIBRARY_DIR -llustreapi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add to CMake
fd865c3
to
5ab578d
Compare
This is a work-in-progress change that adds the ability to pass a plugin library to run user-specific code during gufi_dir2index.
The plugin included here does some simple gathering of Lustre stripe information and stores it into the sqlite3 database.