Skip to content

Commit

Permalink
stored: do not treat read error as EoT by default
Browse files Browse the repository at this point in the history 
Fixes #1034: Read error on tape may be misinterpreted as end-of-tape
Previously stored treated a read-error immediately
following an end-of-file mark on a tape as end-of-tape
instead of an error.
This patch makes stored raise an error in this case,
but allows to switch back to the previous behaviour on
a per-device basis using the new configuration option
"Eof On Error Is Eot".
  • Loading branch information
@arogge
arogge committed Jan 15, 2019
1 parent dc63968 commit 03c881a
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 79 deletions.
2 changes: 1 addition & 1 deletion core/src/stored/backends/generic_tape_device.cc
View file
Expand Up @@ -1406,7 +1406,7 @@ bool generic_tape_device::Reposition(DeviceControlRecord *dcr, uint32_t rfile, u
return fsr(rblock-block_num);
} else {
while (rblock > block_num) {
if (!dcr->ReadBlockFromDev(NO_BLOCK_NUMBER_CHECK)) {
if (Ok != dcr->ReadBlockFromDev(NO_BLOCK_NUMBER_CHECK)) {
BErrNo be;
dev_errno = errno;
Dmsg2(30, "Failed to find requested block on %s: ERR=%s", prt_name, be.bstrerror());
Expand Down
41 changes: 21 additions & 20 deletions core/src/stored/block.cc
View file
Expand Up @@ -399,7 +399,7 @@ static void RereadLastBlock(DeviceControlRecord *dcr)
* Note, this can destroy dev->errmsg
*/
dcr->block = lblock;
if (!dcr->ReadBlockFromDev(NO_BLOCK_NUMBER_CHECK)) {
if (Ok != dcr->ReadBlockFromDev(NO_BLOCK_NUMBER_CHECK)) {
Jmsg(jcr, M_ERROR, 0, _("Re-read last block at EOT failed. ERR=%s"), dev->errmsg);
} else {
/*
Expand Down Expand Up @@ -934,24 +934,24 @@ bool DeviceControlRecord::WriteBlockToDevice()
/**
* Read block with locking
*/
bool DeviceControlRecord::ReadBlockFromDevice(bool check_block_numbers)
ReadStatus DeviceControlRecord::ReadBlockFromDevice(bool check_block_numbers)
{
bool ok;
ReadStatus status;

Dmsg0(250, "Enter ReadBlockFromDevice\n");
dev->rLock();
ok = ReadBlockFromDev(check_block_numbers);
status = ReadBlockFromDev(check_block_numbers);
dev->Unlock();
Dmsg0(250, "Leave ReadBlockFromDevice\n");
return ok;
return status;
}

/**
* Read the next block into the block structure and unserialize
* the block header. For a file, the block may be partially
* or completely in the current buffer.
*/
bool DeviceControlRecord::ReadBlockFromDev(bool check_block_numbers)
ReadStatus DeviceControlRecord::ReadBlockFromDev(bool check_block_numbers)
{
ssize_t status;
int looping;
Expand All @@ -961,13 +961,13 @@ bool DeviceControlRecord::ReadBlockFromDev(bool check_block_numbers)
if (JobCanceled(jcr)) {
Mmsg(dev->errmsg, _("Job failed or canceled.\n"));
block->read_len = 0;
return false;
return Error;
}

if (dev->AtEot()) {
Mmsg(dev->errmsg, _("Attempt to read past end of tape or file.\n"));
block->read_len = 0;
return false;
return EndOfTape;
}
looping = 0;
Dmsg1(250, "Full read in ReadBlockFromDevice() len=%d\n",
Expand All @@ -978,7 +978,7 @@ bool DeviceControlRecord::ReadBlockFromDev(bool check_block_numbers)
dev->fd(), dev->file, dev->block_num, dev->print_name());
Jmsg(dcr->jcr, M_WARNING, 0, "%s", dev->errmsg);
block->read_len = 0;
return false;
return Error;
}

reread:
Expand All @@ -988,7 +988,7 @@ bool DeviceControlRecord::ReadBlockFromDev(bool check_block_numbers)
dev->print_name());
Jmsg(jcr, M_ERROR, 0, "%s", dev->errmsg);
block->read_len = 0;
return false;
return Error;
}

retry = 0;
Expand Down Expand Up @@ -1019,10 +1019,11 @@ bool DeviceControlRecord::ReadBlockFromDev(bool check_block_numbers)
GeneratePluginEvent(jcr, bsdEventReadError, dcr);

Jmsg(jcr, M_ERROR, 0, "%s", dev->errmsg);
if (dev->AtEof()) { /* EOF just seen? */
dev->SetEot(); /* yes, error => EOT */
if (device->eof_on_error_is_eot && dev->AtEof()) { /* EOF just seen? */
dev->SetEot(); /* yes, error => EOT */
return EndOfTape;
}
return false;
return Error;
}

Dmsg3(250, "Read device got %d bytes at %u:%u\n", status,
Expand All @@ -1035,10 +1036,10 @@ bool DeviceControlRecord::ReadBlockFromDev(bool check_block_numbers)
dev->file, dev->block_num, dev->print_name());
if (dev->AtEof()) { /* EOF already read? */
dev->SetEot(); /* yes, 2 EOFs => EOT */
return 0;
return EndOfTape;
}
dev->SetAteof();
return false; /* return eof */
return EndOfFile;
}

/*
Expand All @@ -1062,7 +1063,7 @@ bool DeviceControlRecord::ReadBlockFromDev(bool check_block_numbers)
dev->SetShortBlock();
block->read_len = block->binbuf = 0;
Dmsg2(200, "set block=%p binbuf=%d\n", block, block->binbuf);
return false; /* return error */
return Error;
}

// BlockNumber = block->BlockNumber + 1;
Expand All @@ -1072,7 +1073,7 @@ bool DeviceControlRecord::ReadBlockFromDev(bool check_block_numbers)
dev->file_size += block->read_len;
goto reread;
}
return false;
return Error;
}

/*
Expand All @@ -1095,7 +1096,7 @@ bool DeviceControlRecord::ReadBlockFromDev(bool check_block_numbers)
Mmsg(dev->errmsg, "%s", dev->bstrerror());
Jmsg(jcr, M_ERROR, 0, "%s", dev->errmsg);
block->read_len = 0;
return false;
return Error;
}
} else {
Dmsg0(250, "Seek to beginning of block for reread.\n");
Expand Down Expand Up @@ -1126,7 +1127,7 @@ bool DeviceControlRecord::ReadBlockFromDev(bool check_block_numbers)
Jmsg(jcr, M_ERROR, 0, "%s", dev->errmsg);
dev->SetShortBlock();
block->read_len = block->binbuf = 0;
return false; /* return error */
return Error;
}

dev->ClearShortBlock();
Expand Down Expand Up @@ -1188,7 +1189,7 @@ bool DeviceControlRecord::ReadBlockFromDev(bool check_block_numbers)
Dmsg2(250, "Exit read_block read_len=%d block_len=%d\n",
block->read_len, block->block_len);
block->block_read = true;
return true;
return Ok;
}

} /* namespace storagedaemon */
31 changes: 18 additions & 13 deletions core/src/stored/bls.cc
View file
Expand Up @@ -306,13 +306,15 @@ static void DoBlocks(char *infname)
DeviceBlock *block = dcr->block;
char buf1[100], buf2[100];
for ( ;; ) {
if (!dcr->ReadBlockFromDevice(NO_BLOCK_NUMBER_CHECK)) {
Dmsg1(100, "!read_block(): ERR=%s\n", dev->bstrerror());
if (dev->AtEot()) {
switch(dcr->ReadBlockFromDevice(NO_BLOCK_NUMBER_CHECK)) {
case Ok:
// no special handling required
break;
case EndOfTape:
if (!MountNextReadVolume(dcr)) {
Jmsg(jcr, M_INFO, 0, _("Got EOM at file %u on device %s, Volume \"%s\"\n"),
dev->file, dev->print_name(), dcr->VolumeName);
break;
return;
}
/* Read and discard Volume label */
DeviceRecord *record;
Expand All @@ -322,19 +324,22 @@ static void DoBlocks(char *infname)
GetSessionRecord(dev, record, &sessrec);
FreeRecord(record);
Jmsg(jcr, M_INFO, 0, _("Mounted Volume \"%s\".\n"), dcr->VolumeName);
} else if (dev->AtEof()) {
break;
case EndOfFile:
Jmsg(jcr, M_INFO, 0, _("End of file %u on device %s, Volume \"%s\"\n"),
dev->file, dev->print_name(), dcr->VolumeName);
Dmsg0(20, "read_record got eof. try again\n");
continue;
} else if (dev->IsShortBlock()) {
Jmsg(jcr, M_INFO, 0, "%s", dev->errmsg);
continue;
} else {
/* I/O error */
DisplayTapeErrorStatus(jcr, dev);
break;
}
default:
Dmsg1(100, "!read_block(): ERR=%s\n", dev->bstrerror());
if (dev->IsShortBlock()) {
Jmsg(jcr, M_INFO, 0, "%s", dev->errmsg);
continue;
} else {
/* I/O error */
DisplayTapeErrorStatus(jcr, dev);
return;
}
}
if (!MatchBsrBlock(bsr, block)) {
Dmsg5(100, "reject Blk=%u blen=%u bVer=%d SessId=%u SessTim=%u\n",
Expand Down
52 changes: 27 additions & 25 deletions core/src/stored/btape.cc
View file
Expand Up @@ -878,7 +878,7 @@ static bool re_read_block_test()
goto bail_out;
}
Pmsg0(0, _("Backspace record OK.\n"));
if (!dcr->ReadBlockFromDev(NO_BLOCK_NUMBER_CHECK)) {
if (Ok != dcr->ReadBlockFromDev(NO_BLOCK_NUMBER_CHECK)) {
BErrNo be;
Pmsg1(0, _("Read block failed! ERR=%s\n"), be.bstrerror(dev->dev_errno));
goto bail_out;
Expand Down Expand Up @@ -1251,7 +1251,7 @@ static bool write_read_test()
*/
for (uint32_t i = 1; i <= 2 * num_recs; i++) {
read_again:
if (!dcr->ReadBlockFromDev(NO_BLOCK_NUMBER_CHECK)) {
if (Ok != dcr->ReadBlockFromDev(NO_BLOCK_NUMBER_CHECK)) {
BErrNo be;
if (dev->AtEof()) {
Pmsg0(-1, _("Got EOF on tape.\n"));
Expand Down Expand Up @@ -1369,7 +1369,7 @@ static bool position_test()
goto bail_out;
}
read_again:
if (!dcr->ReadBlockFromDev(NO_BLOCK_NUMBER_CHECK)) {
if (Ok != dcr->ReadBlockFromDev(NO_BLOCK_NUMBER_CHECK)) {
BErrNo be;
if (dev->AtEof()) {
Pmsg0(-1, _("Got EOF on tape.\n"));
Expand Down Expand Up @@ -2081,9 +2081,11 @@ static void scan_blocks()
dev->UpdatePos(dcr);
tot_files = dev->file;
for (;;) {
if (!dcr->ReadBlockFromDevice(NO_BLOCK_NUMBER_CHECK)) {
Dmsg1(100, "!read_block(): ERR=%s\n", dev->bstrerror());
if (dev->AtEot()) {
switch(dcr->ReadBlockFromDevice(NO_BLOCK_NUMBER_CHECK)) {
case Ok:
// no special handling required
break;
case EndOfTape:
if (blocks > 0) {
if (blocks==1) {
printf(_("1 block of %d bytes in file %d\n"), block_size, dev->file);
Expand All @@ -2094,8 +2096,7 @@ static void scan_blocks()
blocks = 0;
}
goto bail_out;
}
if (dev->AtEof()) {
case EndOfFile:
if (blocks > 0) {
if (blocks==1) {
printf(_("1 block of %d bytes in file %d\n"), block_size, dev->file);
Expand All @@ -2107,22 +2108,23 @@ static void scan_blocks()
}
printf(_("End of File mark.\n"));
continue;
}
if (BitIsSet(ST_SHORT, dev->state)) {
if (blocks > 0) {
if (blocks==1) {
printf(_("1 block of %d bytes in file %d\n"), block_size, dev->file);
default:
Dmsg1(100, "!read_block(): ERR=%s\n", dev->bstrerror());
if (BitIsSet(ST_SHORT, dev->state)) {
if (blocks > 0) {
if (blocks==1) {
printf(_("1 block of %d bytes in file %d\n"), block_size, dev->file);
}
else {
printf(_("%d blocks of %d bytes in file %d\n"), blocks, block_size, dev->file);
}
blocks = 0;
}
else {
printf(_("%d blocks of %d bytes in file %d\n"), blocks, block_size, dev->file);
}
blocks = 0;
printf(_("Short block read.\n"));
continue;
}
printf(_("Short block read.\n"));
continue;
}
printf(_("Error reading block. ERR=%s\n"), dev->bstrerror());
goto bail_out;
printf(_("Error reading block. ERR=%s\n"), dev->bstrerror());
goto bail_out;
}
if (block->block_len != block_size) {
if (blocks > 0) {
Expand Down Expand Up @@ -2615,7 +2617,7 @@ static bool do_unfill()
goto bail_out;
}
Pmsg1(-1, _("Reading block %u.\n"), last_block_num);
if (!dcr->ReadBlockFromDevice(NO_BLOCK_NUMBER_CHECK)) {
if (Ok != dcr->ReadBlockFromDevice(NO_BLOCK_NUMBER_CHECK)) {
Pmsg1(-1, _("Error reading block: ERR=%s\n"), dev->bstrerror());
goto bail_out;
}
Expand Down Expand Up @@ -2667,7 +2669,7 @@ static bool do_unfill()
goto bail_out;
}
Pmsg1(-1, _("Reading block %d.\n"), dev->block_num);
if (!dcr->ReadBlockFromDevice(NO_BLOCK_NUMBER_CHECK)) {
if (Ok != dcr->ReadBlockFromDevice(NO_BLOCK_NUMBER_CHECK)) {
Pmsg1(-1, _("Error reading block: ERR=%s\n"), dev->bstrerror());
goto bail_out;
}
Expand All @@ -2683,7 +2685,7 @@ static bool do_unfill()
goto bail_out;
}
Pmsg1(-1, _("Reading block %d.\n"), dev->block_num);
if (!dcr->ReadBlockFromDevice(NO_BLOCK_NUMBER_CHECK)) {
if (Ok != dcr->ReadBlockFromDevice(NO_BLOCK_NUMBER_CHECK)) {
Pmsg1(-1, _("Error reading block: ERR=%s\n"), dev->bstrerror());
goto bail_out;
}
Expand Down
14 changes: 12 additions & 2 deletions core/src/stored/dev.h
View file
Expand Up @@ -82,6 +82,16 @@ enum {
W_WAKE
};

/**
* Return values for ReadBlockFromDevice()
*/
enum ReadStatus {
Error = 0,
Ok,
EndOfFile,
EndOfTape,
};

/**
* Arguments to open_dev()
*/
Expand Down Expand Up @@ -755,8 +765,8 @@ class DeviceControlRecord : public SmartAlloc {
*/
bool WriteBlockToDevice();
bool WriteBlockToDev();
bool ReadBlockFromDevice(bool check_block_numbers);
bool ReadBlockFromDev(bool check_block_numbers);
ReadStatus ReadBlockFromDevice(bool check_block_numbers);
ReadStatus ReadBlockFromDev(bool check_block_numbers);

/*
* Methods in label.c
Expand Down
2 changes: 1 addition & 1 deletion core/src/stored/label.cc
View file
Expand Up @@ -148,7 +148,7 @@ int ReadDevVolumeLabel(DeviceControlRecord *dcr)
EmptyBlock(dcr->block);

Dmsg0(130, "Big if statement in ReadVolumeLabel\n");
if (!dcr->ReadBlockFromDev(NO_BLOCK_NUMBER_CHECK)) {
if (Ok != dcr->ReadBlockFromDev(NO_BLOCK_NUMBER_CHECK)) {
Mmsg(jcr->errmsg, _("Requested Volume \"%s\" on %s is not a Bareos "
"labeled Volume, because: ERR=%s"), NPRT(VolName),
dev->print_name(), dev->print_errmsg());
Expand Down

0 comments on commit 03c881a

Please sign in to comment.