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

stored: fix crashes of storage tools when autoxflate plugin is loaded #1348

Merged
merged 11 commits into from
Feb 3, 2023

Conversation

pstorz
Copy link
Member

@pstorz pstorz commented Jan 3, 2023

The low level storage tools bcopy, bextract, bls, bscan and btape load sd plugins if they are configured.

Unfortunately, the plugin event bSdEventSetupRecordTranslation was not emitted before, so that the autoxflate plugin did not setup the compression buffers.

Now, all these tools do emit the plugin event, and the autoxflate-sd plugin also checks for the buffer before using it and emits an message otherwise.

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

General
  • PR name is meaningful
  • Purpose of the PR is understood
  • Commit descriptions are understandable and well formatted
  • Check backport line
  • Is the PR title usable as CHANGELOG entry?
  • Separate commit for CHANGELOG.md ("update CHANGELOG.md"). The PR number is correct.
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)
  • Required documentation changes are present and part of the PR
  • bareos-check-sources --since-merge does not report any problems
Tests
  • Decision taken that a test is required (if not, then remove this paragraph)
  • The choice of the type of test (unit test or systemtest) is reasonable
  • Testname matches exactly what is being tested
  • On a fail, output of the test leads quickly to the origin of the fault

Copy link
Member

@arogge arogge left a comment

Choose a reason for hiding this comment

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

AFAIR we have autoxflate in the "basic" bareos test.
We should add more sub-tests that run the tools (at least bextract, bls and bscan) on the volumes created by the basic backup/restore test.

@pstorz pstorz requested a review from arogge January 20, 2023 12:04
Copy link
Member

@arogge arogge left a comment

Choose a reason for hiding this comment

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

The tests look promising, but apparently we have memory leaks in the tools when autoxflate is enabled, thus the tests fail in the sanitizer stage.
I think there is also a fixture missing, as with enough parallelism (i.e. -j 100) the tests failed on my machine, too.

@pstorz pstorz force-pushed the dev/pstorz/master/fix-bls-crash branch from 5bdb0d8 to 0b112db Compare January 27, 2023 13:38
@pstorz pstorz requested a review from arogge February 2, 2023 09:11
@arogge arogge force-pushed the dev/pstorz/master/fix-bls-crash branch from 2698bbf to fa08600 Compare February 2, 2023 14:09
@arogge arogge added the bugfix label Feb 2, 2023
@arogge arogge force-pushed the dev/pstorz/master/fix-bls-crash branch from fa08600 to 10ddce9 Compare February 2, 2023 14:29
@pstorz pstorz merged commit 334ec7e into master Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants