From 03c881a2fe933871487b0aea3a4eb289d45f70bf Mon Sep 17 00:00:00 2001 From: Andreas Rogge Date: Tue, 15 Jan 2019 11:29:48 +0100 Subject: [PATCH] stored: do not treat read error as EoT by default 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". --- .../stored/backends/generic_tape_device.cc | 2 +- core/src/stored/block.cc | 41 ++++++++------- core/src/stored/bls.cc | 31 ++++++----- core/src/stored/btape.cc | 52 ++++++++++--------- core/src/stored/dev.h | 14 ++++- core/src/stored/label.cc | 2 +- core/src/stored/read_record.cc | 38 ++++++++------ core/src/stored/stored_conf.cc | 1 + core/src/stored/stored_conf.h | 1 + 9 files changed, 103 insertions(+), 79 deletions(-) diff --git a/core/src/stored/backends/generic_tape_device.cc b/core/src/stored/backends/generic_tape_device.cc index 4768009cb36..63c5b41cafa 100644 --- a/core/src/stored/backends/generic_tape_device.cc +++ b/core/src/stored/backends/generic_tape_device.cc @@ -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()); diff --git a/core/src/stored/block.cc b/core/src/stored/block.cc index 680370211f2..6fed8e47106 100644 --- a/core/src/stored/block.cc +++ b/core/src/stored/block.cc @@ -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 { /* @@ -934,16 +934,16 @@ 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; } /** @@ -951,7 +951,7 @@ bool DeviceControlRecord::ReadBlockFromDevice(bool check_block_numbers) * 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; @@ -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", @@ -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: @@ -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; @@ -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, @@ -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; } /* @@ -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; @@ -1072,7 +1073,7 @@ bool DeviceControlRecord::ReadBlockFromDev(bool check_block_numbers) dev->file_size += block->read_len; goto reread; } - return false; + return Error; } /* @@ -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"); @@ -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(); @@ -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 */ diff --git a/core/src/stored/bls.cc b/core/src/stored/bls.cc index a6257662eed..fdd3101b9d7 100644 --- a/core/src/stored/bls.cc +++ b/core/src/stored/bls.cc @@ -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; @@ -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", diff --git a/core/src/stored/btape.cc b/core/src/stored/btape.cc index 97142da8e43..c9b09c14318 100644 --- a/core/src/stored/btape.cc +++ b/core/src/stored/btape.cc @@ -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; @@ -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")); @@ -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")); @@ -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); @@ -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); @@ -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) { @@ -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; } @@ -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; } @@ -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; } diff --git a/core/src/stored/dev.h b/core/src/stored/dev.h index e6a81337fd4..7eb62ec9a54 100644 --- a/core/src/stored/dev.h +++ b/core/src/stored/dev.h @@ -82,6 +82,16 @@ enum { W_WAKE }; +/** + * Return values for ReadBlockFromDevice() + */ +enum ReadStatus { + Error = 0, + Ok, + EndOfFile, + EndOfTape, +}; + /** * Arguments to open_dev() */ @@ -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 diff --git a/core/src/stored/label.cc b/core/src/stored/label.cc index e6315b10c2b..b5c0e6e00bb 100644 --- a/core/src/stored/label.cc +++ b/core/src/stored/label.cc @@ -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()); diff --git a/core/src/stored/read_record.cc b/core/src/stored/read_record.cc index 4c0492f96fb..e9cc45a871b 100644 --- a/core/src/stored/read_record.cc +++ b/core/src/stored/read_record.cc @@ -196,8 +196,11 @@ bool ReadNextBlockFromDevice(DeviceControlRecord *dcr, DeviceRecord *trec; while (1) { - if (!dcr->ReadBlockFromDevice(CHECK_BLOCK_NUMBERS)) { - if (dcr->dev->AtEot()) { + switch(dcr->ReadBlockFromDevice(CHECK_BLOCK_NUMBERS)) { + case Ok: + // no handling required if read was successful + break; + case EndOfTape: Jmsg(jcr, M_INFO, 0, _("End of Volume at file %u on device %s, Volume \"%s\"\n"), dcr->dev->file, dcr->dev->print_name(), dcr->VolumeName); @@ -244,26 +247,27 @@ bool ReadNextBlockFromDevice(DeviceControlRecord *dcr, * After reading label, we must read first data block */ continue; - } else if (dcr->dev->AtEof()) { + case EndOfFile: Dmsg3(200, "End of file %u on device %s, Volume \"%s\"\n", dcr->dev->file, dcr->dev->print_name(), dcr->VolumeName); continue; - } else if (dcr->dev->IsShortBlock()) { - Jmsg1(jcr, M_ERROR, 0, "%s", dcr->dev->errmsg); - continue; - } else { - /* - * I/O error or strange end of tape - */ - DisplayTapeErrorStatus(jcr, dcr->dev); - if (forge_on || jcr->ignore_label_errors) { - dcr->dev->fsr(1); /* try skipping bad record */ - Pmsg0(000, _("Did fsr in attemp to skip bad record.\n")); + default: + if (dcr->dev->IsShortBlock()) { + Jmsg1(jcr, M_ERROR, 0, "%s", dcr->dev->errmsg); continue; + } else { + /* + * I/O error or strange end of tape + */ + DisplayTapeErrorStatus(jcr, dcr->dev); + if (forge_on || jcr->ignore_label_errors) { + dcr->dev->fsr(1); /* try skipping bad record */ + Pmsg0(000, _("Did fsr in attemp to skip bad record.\n")); + continue; + } + *status = false; + return false; } - *status = false; - return false; - } } Dmsg2(debuglevel, "Read new block at pos=%u:%u\n", dcr->dev->file, dcr->dev->block_num); diff --git a/core/src/stored/stored_conf.cc b/core/src/stored/stored_conf.cc index 9d8cf133eee..1aefd323173 100644 --- a/core/src/stored/stored_conf.cc +++ b/core/src/stored/stored_conf.cc @@ -260,6 +260,7 @@ static ResourceItem dev_items[] = { NULL}, {"AutoInflate", CFG_TYPE_IODIRECTION, ITEM(res_dev.autoinflate), 0, 0, NULL, "13.4.0-", NULL}, {"CollectStatistics", CFG_TYPE_BOOL, ITEM(res_dev.collectstats), 0, CFG_ITEM_DEFAULT, "true", NULL, NULL}, + {"EofOnErrorIsEot", CFG_TYPE_BOOL, ITEM(res_dev.eof_on_error_is_eot), 0, CFG_ITEM_DEFAULT, NULL, NULL, NULL}, {NULL, 0, {0}, 0, 0, NULL, NULL, NULL}}; /** diff --git a/core/src/stored/stored_conf.h b/core/src/stored/stored_conf.h index 69a21371138..524c0efd833 100644 --- a/core/src/stored/stored_conf.h +++ b/core/src/stored/stored_conf.h @@ -148,6 +148,7 @@ class DeviceResource : public BareosResource { bool drive_crypto_enabled; /**< Enable hardware crypto */ bool query_crypto_status; /**< Query device for crypto status */ bool collectstats; /**< Set if statistics should be collected */ + bool eof_on_error_is_eot; /**< Interpret EOF during read error as EOT */ drive_number_t drive; /**< Autochanger logical drive number */ drive_number_t drive_index; /**< Autochanger physical drive index */ char cap_bits[CAP_BYTES]; /**< Capabilities of this device */