Merged
Conversation
c5c1b77 to
e01db20
Compare
2128739 to
f1937de
Compare
Setting the limit to #devices + 10% should always be enough and much better than a static 19.
5e89468 to
133c022
Compare
133c022 to
393b440
Compare
florian-at-bareos
approved these changes
May 14, 2025
Contributor
florian-at-bareos
left a comment
There was a problem hiding this comment.
Nice work!
I like that you return error info from OpenDatabase!
The only thing which is a little weird now is that
db->OpenDatabase(nullptr) evaluates to false if successfull and to true if not.
But that's something we can overlook for now.
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thank you for contributing to the Bareos Project!
This pr fixes a long standing issue with bareos where one storage daemon can sometimes only request up to 19 volumes from the same pool/mediatype combination concurrently.
This is because the sd has a hard limit of 19 requests to get a suitable volume. As the director does not know which volume is currently in use, its possible to that it returns 19 volumes that are already in use by other devices.
In this pr we upped the limit to (number of devices)+10%. In the future we should make sure that the sd already tells the director immediately which volumes the sd is already using., reducing the number of round trips required.
This also contains a fix for a director crash that happens when it cannot connect to the database. This crash occurs because its not safe to lock the db in such cases (as the lock may not exist yet), but we still want to access the error string (which may be protected by that lock). I added a workaround, so that dbs that failed to open are considered "private" (as they can not have been shared yet!), in which case accessing error string is allowed even without the lock.
Thirdly, this pr contains some cmake improvements which should make it much faster to (re-)configure a bareos build directory. Unit tests are now not parsed via a cmake regex parser to find out which tests exist, but they are executed in a post build to gather that information.
Please check
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)
Make sure you check/merge the PR using
devtools/pr-toolto have some simple automated checks run and a proper changelog record added.General
Source code quality
Tests