Skip to content
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

Issue 6243 - dsctl bdb2mdb should cleanly fail if bundled libdb is not available #6351

Merged
merged 6 commits into from
Oct 10, 2024

Conversation

progier389
Copy link
Contributor

@progier389 progier389 commented Oct 1, 2024

Determine what kind of Berkeley Database library is available
and display a clear error message if it is not available or not usable

Issue: #6243

Reviewed by: @tbordaz , @vashirov , @droideck (Thanks!)

src/lib389/lib389/cli_ctl/dblib.py Outdated Show resolved Hide resolved
return BDB_IMPL_STATUS.READ_ONLY
if pkgs[BDB_RPM] is not False and symbs[BDB_SYMBOL] is True:
return BDB_IMPL_STATUS.RPM
if symbs[BDB_SYMBOL] is False and pkgs[BDB_BUNDLED_RPM] is False:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think at this step pkgs[BDB_BUNDLED_RPM] is false so we may not test it.

Why if we have not the bdb symbols should we fallback to NONE. Having BDB_RPM is not enough to decide it is RPM ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. FYI: the values have a 3 way logic: True,False or None (if we are not able to determine if
the rpm is installed or not ==> not True does not mean False

No if ds389 is build to use Bundle RPM or Read-Obly bdb, even if someone somehow install libdb it will not be used because the backend is not linked with libdb ...

src/lib389/lib389/_constants.py Outdated Show resolved Hide resolved
BDB_SYMBOL = 'bdb_start'
TESTED_SYMBOLS = ( BDB_SYMBOL, BDBRO_SYMBOL )

OBJDUMP_TOOL = '/usr/bin/objdump'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

objdump is part of binutils. In a proper prod environment it should not be present on the system.

for ppath in glob.glob(f'{pdir}/libback-ldbm.*'):
plugin = ppath
if plugin and os.path.isfile(OBJDUMP_TOOL):
rc = subprocess.run(['objdump', '-R', plugin], text=True, capture_output=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of testing for symbols, can we make a decision about type of libdb based on /proc/$(pidof ns-slapd)/maps contents?

For example:

  • bundled libdb
grep libdb /proc/$(pidof ns-slapd)/maps
7fdd4d9eb000-7fdd4da0f000 r--p 00000000 fd:02 4623526                    /usr/lib64/dirsrv/libdb-5.3-389ds.so
7fdd4da0f000-7fdd4db61000 r-xp 00024000 fd:02 4623526                    /usr/lib64/dirsrv/libdb-5.3-389ds.so
7fdd4db61000-7fdd4dba7000 r--p 00176000 fd:02 4623526                    /usr/lib64/dirsrv/libdb-5.3-389ds.so
7fdd4dba7000-7fdd4dbb1000 r--p 001bb000 fd:02 4623526                    /usr/lib64/dirsrv/libdb-5.3-389ds.so
7fdd4dbb1000-7fdd4dbb2000 rw-p 001c5000 fd:02 4623526                    /usr/lib64/dirsrv/libdb-5.3-389ds.so
  • system libdb
grep libdb /proc/$(pidof ns-slapd)/maps
ffffa5470000-ffffa549b000 r--p 00000000 fd:00 1181522                    /usr/lib64/libdb-5.3.so
ffffa549b000-ffffa54a0000 ---p 0002b000 fd:00 1181522                    /usr/lib64/libdb-5.3.so
ffffa54a0000-ffffa55d4000 r-xp 00030000 fd:00 1181522                    /usr/lib64/libdb-5.3.so
ffffa55d4000-ffffa55e0000 ---p 00164000 fd:00 1181522                    /usr/lib64/libdb-5.3.so
ffffa55e0000-ffffa562a000 r--p 00170000 fd:00 1181522                    /usr/lib64/libdb-5.3.so
ffffa562a000-ffffa5636000 ---p 001ba000 fd:00 1181522                    /usr/lib64/libdb-5.3.so
ffffa5636000-ffffa5640000 r--p 001c6000 fd:00 1181522                    /usr/lib64/libdb-5.3.so
ffffa5640000-ffffa5641000 rw-p 001d0000 fd:00 1181522                    /usr/lib64/libdb-5.3.so

And with no libdb available (or if it's already an instance with mdb), there wouldn't be any output.
The only caveat is that the instance needs to be running...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting idea,. IMHO we can even keep the benefit of both solutions:
Keep current code and if status is UNKNOWN then try to get the process maps

Another solution would be to use ldd on libback-ldbm.so since ldd is provided by glibc-common that should always be installed

@progier389
Copy link
Contributor Author

Here is a version that directly for strings in libback-ldbm.so binary (without needing any tools)

@progier389 progier389 added the work in progress Work in Progress - can be reviewed, but not ready for merge. label Oct 7, 2024
@progier389
Copy link
Contributor Author

progier389 commented Oct 9, 2024

Fixed two issues:

  • wrong bundle bdb plugin name (No idea why I choose to name the plugin containing bundle-bdb glue: libback-lmdb 😉 )
  • using glob.glob anywhere because mixing some modules using import glob and other using from glob import glob cause trouble. Since there was far less file using the second form, I moved them to the first form.

@progier389 progier389 removed the work in progress Work in Progress - can be reviewed, but not ready for merge. label Oct 9, 2024
Copy link
Member

@droideck droideck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides the minor issue, looks good!

return BDB_IMPL_STATUS.READ_ONLY
if plgstrs[libdb] is True:
# standard bdb package build
return BDB_IMPL_STATUS.STABDARD
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a type (STANDARD?).
Also, it's not in src/lib389/lib389/_constants.py, I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Good catch! i fixed the typo and renamed RPM into STANDARD in the Enum definition.

@progier389 progier389 merged commit 92eadd1 into 389ds:main Oct 10, 2024
9 checks passed
progier389 added a commit that referenced this pull request Oct 10, 2024
…t available (#6351)

Issue 6243 - dsctl bdb2mdb should cleanly fail if bundled libdb is not avaiable

Determine what kind of Berkeley Database library is available
and display a clear error message if it is not available or not usable

Issue: #6243

Reviewed by: @tbordaz , @vashirov , @droideck (Thanks!)

(cherry picked from commit 92eadd1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants