Cta/cta584

ReleaseNotes.md

@@ -10,6 +10,7 @@
 - Upgraded eos-xrootd to 4.12.8 in CI versionlock.list file
 - cta/CTA#966 Unable to distinguish empty and wrong tape pool
 - cta/CTA#926 Improve MigrationReportPacker::ReportSkipped::execute() exception message
+- cta/CTA#584 Validate checksum when recalling from tape
 
 ### Bug fixes
 

tapeserver/castor/tape/tapeserver/daemon/DiskWriteTask.cpp

@@ -239,7 +239,7 @@ void DiskWriteTask::releaseAllBlock(){
         //errCode=mb->errorCode();
       }
       else{
-        errorMsg="Mismatch between expected and received filed or blockid";
+        errorMsg="Mismatch between expected and received fileid or blockid";
         //errCode=666;
       }
       lc.log(cta::log::ERR,errorMsg);

tapeserver/castor/tape/tapeserver/daemon/TapeReadTask.hpp

@@ -127,16 +127,12 @@ public:
           // end of file. append() also protects against reading too big tape blocks.
           while (mb->m_payload.append(*rf)) {
             tapeBlock++;
-            if(isVerifyOnly) {
-              // Normally, the checksum is calculated in DiskWriteTask::execute(). In verification-only mode, there is no
-              // write to disk. As we need to validate the checksum, we calculate it here.
-              checksum_adler32 = mb->m_payload.adler32(checksum_adler32);
-            }
           }
         } catch (const cta::exception::EndOfFile&) {
           // append() signaled the end of the file.
           stillReading = false;
         }
+        checksum_adler32 = mb->m_payload.adler32(checksum_adler32);
         localStats.readWriteTime += timer.secs(cta::utils::Timer::resetCounter);
         auto blockSize = mb->m_payload.size();
         localStats.dataVolume += blockSize;
@@ -144,12 +140,15 @@ public:
 	  localStats.repackBytesCount += blockSize;
         } else if(isVerifyOnly) {
           localStats.verifiedBytesCount += blockSize;
+          // Don't write the file to disk
+          mb->markAsVerifyOnly();
 	} else {
 	  localStats.userBytesCount += blockSize;
 	}
-        if(isVerifyOnly) {
-          // Don't write the file to disk
-          mb->markAsVerifyOnly();
+        // If we reached the end of the file, validate the checksum (throws an exception on bad checksum)
+        if(!stillReading) {
+          tapeReadChecksum.insert(cta::checksum::ADLER32, checksum_adler32);
+          m_retrieveJob->archiveFile.checksumBlob.validate(tapeReadChecksum);
         }
         // Pass the block to the disk write task
         m_fifo.pushDataBlock(mb);
@@ -157,11 +156,7 @@ public:
         watchdog.notify(blockSize);
         localStats.waitReportingTime += timer.secs(cta::utils::Timer::resetCounter);
       } //end of while(stillReading)
-      if(isVerifyOnly) {
-        tapeReadChecksum.insert(cta::checksum::ADLER32, checksum_adler32);
-        m_retrieveJob->archiveFile.checksumBlob.validate(tapeReadChecksum);
-      }
-      //  we have to signal the end of the tape read to the disk write task.
+      // We have to signal the end of the tape read to the disk write task.
       m_fifo.pushDataBlock(NULL);
       // Log the successful transfer
       localStats.totalTime = localTime.secs();
@@ -195,19 +190,18 @@ public:
 	    .add("userFilesCount",localStats.userFilesCount)
 	    .add("userBytesCount",localStats.userBytesCount)
 	    .add("verifiedFilesCount",localStats.verifiedFilesCount)
-	    .add("verifiedBytesCount",localStats.verifiedBytesCount);
-      if(isVerifyOnly) {
-        params.add("checksumType", "ADLER32")
-              .add("checksumValue", cta::checksum::ChecksumBlob::ByteArrayToHex(tapeReadChecksum.at(cta::checksum::ADLER32)));
-      }
+	    .add("verifiedBytesCount",localStats.verifiedBytesCount)
+            .add("checksumType", "ADLER32")
+            .add("checksumValue", cta::checksum::ChecksumBlob::ByteArrayToHex(tapeReadChecksum.at(cta::checksum::ADLER32)));
       lc.log(cta::log::INFO, "File successfully read from tape");
       // Add the local counts to the session's
       stats.add(localStats);
     } //end of try
     catch (const cta::exception::Exception & ex) {
-      //we end up there because :
-      //-- openReadFile brought us here (cant position to the file)
+      // We end up here because:
+      //-- openReadFile brought us here (can't position to the file)
       //-- m_payload.append brought us here (error while reading the file)
+      //-- checksum validation failed (after reading the last block from tape)
       // Record the error in the watchdog
       if (currentErrorToCount.size()) {
         watchdog.addToErrorCount(currentErrorToCount);