Skip to content

Commit

Permalink
stored: fix unintended decrement of block_num
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
arogge authored and BareosBot committed Aug 17, 2023
1 parent decfa5f commit 7dfe7a2
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 26 deletions.
56 changes: 32 additions & 24 deletions core/src/stored/backends/unix_tape_device.cc
Expand Up @@ -52,20 +52,16 @@
#include "generic_tape_device.h"
#include "unix_tape_device.h"

namespace {
static constexpr size_t size_increment = 1024 * 1024;
static constexpr size_t max_size_increment = 16 * size_increment;
static constexpr size_t next_size_increment(size_t count)
namespace storagedaemon {

static std::vector<std::size_t> bufsizes(std::size_t count)
{
if (count < size_increment) {
return size_increment;
} else {
return (count / size_increment) * 2 * size_increment;
}
constexpr std::size_t mb = 1024 * 1024;
std::vector<std::size_t> sizes{1 * mb, 2 * mb, 4 * mb, 8 * mb, 16 * mb};
sizes.erase(sizes.begin(),
std::upper_bound(sizes.begin(), sizes.end(), count));
return sizes;
}
} // namespace

namespace storagedaemon {

int unix_tape_device::d_ioctl(int fd, ioctl_req_t request, char* op)
{
Expand All @@ -80,21 +76,33 @@ unix_tape_device::unix_tape_device()
ssize_t unix_tape_device::d_read(int fd, void* buffer, size_t count)
{
ssize_t ret = ::read(fd, buffer, count);
/* when the driver fails to `read()` with `ENOMEM`, then the provided buffer
* was too small by re-reading with a temporary buffer that is enlarged
* step-by-step, we can read the block and returns its first `count` bytes.
/* If the driver fails to `read()` with `ENOMEM`, then the provided buffer
* was too small. By re-reading with a temporary buffer that is enlarged
* step-by-step, we can read the block and return the first `count` bytes.
* This allows the calling code to read the block header and resize its buffer
* according to the size recorded in that header.
*/
for (size_t bufsize = next_size_increment(count);
ret == -1 && errno == ENOMEM && bufsize <= max_size_increment;
bufsize = next_size_increment(bufsize)) {
std::vector<char> tmpbuf(bufsize);
bsr(1); // go back one block so we can re-read
block_num++; // re-increment the block counter that bsr() just incremented
if (auto tmpret = ::read(fd, tmpbuf.data(), tmpbuf.size()); tmpret != -1) {
memcpy(buffer, tmpbuf.data(), count);
ret = std::min(tmpret, static_cast<ssize_t>(count));
if (ret == -1 && errno == ENOMEM && HasCap(CAP_BSR)) {
for (auto bufsize : bufsizes(count)) {
// first go back one block so we can re-read
if (!bsr(1)) {
/* when backward-spacing fails for some reason, there's not much we
* can do, so we just return the original ENOMEM and hope that the
* caller knows the device is in a non well-defined state.
*/
errno = ENOMEM;
return -1;
}
block_num++; // re-increment the block counter bsr() just decremented
std::vector<char> tmpbuf(bufsize);
if (auto tmpret = ::read(fd, tmpbuf.data(), tmpbuf.size());
tmpret != -1) {
memcpy(buffer, tmpbuf.data(), count);
ret = std::min(tmpret, static_cast<ssize_t>(count));
break; // successful read
} else if (errno != ENOMEM) {
break; // some other error occured
}
}
}
return ret;
Expand Down
2 changes: 1 addition & 1 deletion core/src/stored/backends/unix_tape_device.h
Expand Up @@ -36,7 +36,7 @@ class unix_tape_device : public generic_tape_device {
~unix_tape_device() { close(nullptr); };

int d_ioctl(int fd, ioctl_req_t request, char* op) override;
virtual ssize_t d_read(int fd, void* buffer, size_t count) override;
ssize_t d_read(int fd, void* buffer, size_t count) override;
};

} /* namespace storagedaemon */
Expand Down
4 changes: 3 additions & 1 deletion core/src/stored/block.cc
Expand Up @@ -1076,7 +1076,9 @@ DeviceControlRecord::ReadStatus DeviceControlRecord::ReadBlockFromDev(bool)
// Attempt to Reposition to re-read the block
if (dev->IsTape()) {
Dmsg0(250, "BootStrapRecord for reread; block too big for buffer.\n");
if (!dev->bsr(1)) {
if (dev->bsr(1)) {
dev->block_num++; // re-increment what bsr() decremented
} else {
Mmsg(dev->errmsg, "%s", dev->bstrerror());
Jmsg(jcr, M_ERROR, 0, "%s", dev->errmsg);
block->read_len = 0;
Expand Down

0 comments on commit 7dfe7a2

Please sign in to comment.