stored: remove warning for maximum block size for tapes#1375
Merged
arogge merged 8 commits intobareos:masterfrom Mar 2, 2023
Merged
Conversation
9932601 to
086b941
Compare
alaaeddineelamri
approved these changes
Feb 17, 2023
Contributor
There was a problem hiding this comment.
good for me :)
PR-tool reports commit messages that are too long:
abfefd42f gtest: warn developers to disable their local bareos instance: headline too long
508701648 tests: Write tape with capital t like our docs are saying to do it: headline too long
as a reminder for how the PR-tool works, commit headline should be <=60 characters, and commit body should be <=72 characters
b7e0773 to
4c5fbf7
Compare
During pass 2 dummy resources are being created that end up not being really used. These dummy resources often do not get filled with the complete information; in particular device_type was never filled out since its std::string. Previously the dummy resource was being used to validate the resource configuration. This lead to spurious warnings since the validator expected the data to be actually correct, e.g. it checked to see if the device_resource was a tape or not. Since this info was not filled out it threw a warning. Now the resource created in pass 1 is used for validation.
Since the config parsing step does not always take care to lower case the device type, we need to check case insensitivly wether the device_type is tape or not!
Since other parts of the system are actually using equality comparision, we should ensure that any validated resource is actually usable by the rest of the system. In this case we do this by lower_case-ing the device type before we start the validation process.
Since the documentation uses capital letters our tests should reflect that so that we do not miss any bugs that get introduced by this discrepancy.
If you have a local bareos director running that is using a port that a test wants to start a director on it will fail. Normally this will not happen but one particular test checks the default config. In that case it might not be obvious to check wether you have a bareos director running in the background. This hint was added for this particular purpose.
Similar to the situation in stored, here the pass 2 resources are just dummies which do not contain every information. As such we only validate resources from pass 1.
538adc7 to
c4affb1
Compare
10 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
3 participants
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!
When the storage daemon encounters a device config that contains 'Maximum Block Size = ...' it was always throwing the warning "[...] Setting 'Maximum Block Size' is only supported on tape devices' even if the device was a tape device.
The reason for this is twofold. The config parser puts the device_type verbatim into the DeviceResource object without taking care to lower case it for the many
device_type = DeviceType::B_TAPE_DEVstring comparisions.Another bug amplified this: the validation of config objects included dummy object which do not contain every set option.
This included the device_type option, which was left blank; this caused the validation to never consider the device a tape drive.
Because of their small size this pr contains two other changes:
a hint was added to let the developer know of this possibility if the particular test fails.
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