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: automatically increment tape block size in case the buffer is too small #1496

Merged

Conversation

arogge
Copy link
Member

@arogge arogge commented Jun 28, 2023

This change allows Bareos to read tape volumes even if the configured Maximum Block Size is too small.
You will still see an error message like this to notify you that something is not right:

28-Jun 16:10 bareos-sd JobId 15: Error: stored/block.cc:1057 Block length 162132 is greater than buffer 64512. Attempting recovery.
28-Jun 16:10 bareos-sd JobId 15: stored/block.cc:1079 Setting block buffer size to 162132 bytes.

However, restore will not fail anymore.

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)

Make sure you check/merge the PR using devtools/pr-tool to have some simple automated checks run and a proper changelog record added.

General
  • Is the PR title usable as CHANGELOG entry?
  • Purpose of the PR is understood
  • Commit descriptions are understandable and well formatted
    Check backport line
    Required backport PRs have been created
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

@arogge arogge force-pushed the dev/arogge/master/tape-autodetect-blocksize branch from d5d1fa3 to ab01b56 Compare June 28, 2023 15:29
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

Looks good; Please check my comment.

core/src/stored/backends/unix_tape_device.cc Outdated Show resolved Hide resolved
core/src/stored/block.cc Outdated Show resolved Hide resolved
core/src/stored/backends/unix_tape_device.cc Outdated Show resolved Hide resolved
@pstorz pstorz self-assigned this Jun 29, 2023
@arogge
Copy link
Member Author

arogge commented Jul 4, 2023

@sebsura I implemented what I think the both of us settled on. Would you mind taking another look?
I also made sure we never allocate unless we run into a read error.

@arogge arogge requested review from sebsura and pstorz July 4, 2023 15:12
Copy link
Contributor

@sebsura sebsura left a comment

Choose a reason for hiding this comment

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

The code looks great. One small detail still needs work.
I think you should mention the build script fix in the PR text itself.
The title should probably prepended by something like stored: since this mostly effects the storage daemon.

core/src/stored/backends/unix_tape_device.cc Outdated Show resolved Hide resolved
core/src/stored/backends/unix_tape_device.cc Outdated Show resolved Hide resolved
@sebsura sebsura changed the title automatically increment tape block size in case the buffer is too small stored: automatically increment tape block size in case the buffer is too small Aug 10, 2023
Copy link
Contributor

@sebsura sebsura left a comment

Choose a reason for hiding this comment

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

Looks great!

@arogge arogge force-pushed the dev/arogge/master/tape-autodetect-blocksize branch from 93bd51e to 833b83b Compare August 17, 2023 12:28
arogge and others added 4 commits August 17, 2023 13:00
dist-tarball.sh was supposed to create "clean" tarballs, that is with
a path-prefix of bareos-x.y.z based on the version-number. This didn't
happen as rewriting in tar did not take place.
With this patch the paths provided to tar are now all prefixed with './'
which allows the substitution to work correctly.
when the unix_tape_device encounters a block that is too large for the
buffer to handle, it will now re-read with incrementing buffer sizes
until it can provide the first part of the block.
This allows Bareos to reconfigure the block buffer size for tape devices
like it already does for file devices.
When bsr() is called to initiare a re-read on a tape, it will decrement
ths block_num. As during the re-read the block_num was not incremented,
it shouldn't be decremented.
@BareosBot BareosBot force-pushed the dev/arogge/master/tape-autodetect-blocksize branch from e3fad2c to 5b21b2b Compare August 17, 2023 13:01
@BareosBot BareosBot merged commit bd2c4b4 into bareos:master Aug 17, 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

4 participants