LabelSession now treats user errors correctly

castor/messages/LabelError.proto

@@ -22,9 +22,6 @@ message LabelError {
   // The unit name of the drive
   required string unitname = 1;
 
-  // The error code
-  required uint32 code = 2;
-
   // The error message
-  required string message = 3;
+  required string message = 2;
 }

castor/messages/TapeserverProxy.hpp

@@ -153,10 +153,10 @@ public:
    * specified error.
    *
    * @param unitName The unit name of the tape drive.
-   * @param labelEx The error encountered by the label session.
+   * @param message The error message.
    */
   virtual void labelError(const std::string &unitName,
-    const castor::exception::Exception &labelEx) = 0;
+    const std::string &message) = 0;
 
 }; // class TapeserverProxy
 

castor/messages/TapeserverProxyDummy.cpp

@@ -106,5 +106,5 @@ deleteLogParams(const std::string &unitName,
 // labelError
 //------------------------------------------------------------------------------
 void castor::messages::TapeserverProxyDummy::labelError(
-  const std::string &unitName, const castor::exception::Exception &labelEx) {
+  const std::string &unitName, const std::string &message) {
 }

castor/messages/TapeserverProxyDummy.hpp

@@ -103,7 +103,6 @@ public:
   void tapeUnmountStarted(const std::string &vid,
     const std::string &unitName);
 
-
   /**
    * Notifies the tapeserverd daemon that the specified tape has been unmounted.
    *
@@ -143,10 +142,10 @@ public:
    * specified error.
    *
    * @param unitName The unit name of the tape drive.
-   * @param labelEx The error encountered by the label session.
+   * @param message The error message.
    */
   void labelError(const std::string &unitName,
-    const castor::exception::Exception &labelEx);
+    const std::string &message);
 
 }; // class TapeserverProxyDummy
 

castor/messages/TapeserverProxyZmq.cpp

@@ -680,11 +680,11 @@ castor::messages::Frame castor::messages::TapeserverProxyZmq::
 // labelError
 //------------------------------------------------------------------------------
 void castor::messages::TapeserverProxyZmq::labelError(
-  const std::string &unitName, const castor::exception::Exception &labelEx) {
+  const std::string &unitName, const std::string &message) {
   server::MutexLocker lock(&m_mutex);
 
   try {
-    const Frame rqst = createLabelErrorFrame(unitName, labelEx);
+    const Frame rqst = createLabelErrorFrame(unitName, message);
     sendFrame(m_serverSocket, rqst);
 
     ReturnValue reply;
@@ -711,7 +711,7 @@ void castor::messages::TapeserverProxyZmq::labelError(
 //------------------------------------------------------------------------------
 castor::messages::Frame castor::messages::TapeserverProxyZmq::
   createLabelErrorFrame(const std::string &unitName,
-  const castor::exception::Exception &labelEx) {
+  const std::string &message) {
   try {
     Frame frame;
 
@@ -721,8 +721,7 @@ castor::messages::Frame castor::messages::TapeserverProxyZmq::
 
     LabelError body;
     body.set_unitname(unitName);
-    body.set_code(labelEx.code());
-    body.set_message(labelEx.getMessage().str());
+    body.set_message(message);
     frame.serializeProtocolBufferIntoBody(body);
 
     return frame;

castor/messages/TapeserverProxyZmq.hpp

@@ -158,10 +158,9 @@ public:
    * specified error.
    *
    * @param unitName The unit name of the tape drive.
-   * @param LabelEx The error encountered by the label session.
+   * @param message The error message.
    */
-  void labelError(const std::string &unitName,
-    const castor::exception::Exception &labelEx);
+  void labelError(const std::string &unitName, const std::string &message);
 
 private:
 
@@ -307,11 +306,11 @@ private:
    * Creates a frame containing a LabelError message.
    *
    * @param unitName The unit name of the tape drive.
-   * @param LabelEx The error encountered by the label session.
+   * @param message The error message.
    * @return The frame.
    */
   Frame createLabelErrorFrame(const std::string &unitName,
-    const castor::exception::Exception &labelEx);
+    const std::string &message);
 
 }; // class TapeserverProxyZmq
 

castor/tape/tapeserver/daemon/CatalogueLabelSession.cpp

@@ -126,25 +126,25 @@ bool castor::tape::tapeserver::daemon::CatalogueLabelSession::handleTick() {
 //------------------------------------------------------------------------------
 void castor::tape::tapeserver::daemon::CatalogueLabelSession::
   sessionSucceeded() {
-  try {
-    legacymsg::writeTapeReplyMsg(m_netTimeout, m_labelCmdConnection, 0, "");
-  } catch(castor::exception::Exception &we) {
-    log::Param params[] = {log::Param("message", we.getMessage().str())};
-    m_log(LOG_ERR, "Failed to send success reply-message to label command",
-      params);
+  const bool thereWasAUserError = !m_labelErrors.empty();
+
+  if(thereWasAUserError) {
+    sendFailureReplyToLabelCommand();
+  } else {
+    sendSuccessReplyToLabelCommand();
   }
 }
 
 //------------------------------------------------------------------------------
-// sessionFailed
+// sendFailureReplyToLabelCommand
 //------------------------------------------------------------------------------
 void castor::tape::tapeserver::daemon::CatalogueLabelSession::
-  sessionFailed() {
+  sendFailureReplyToLabelCommand() {
   try {
-    if(!m_labelErrors.empty()) {
-      const castor::exception::Exception &labelEx = m_labelErrors.front();
+    const std::string labelErrors = concatLabelErrors(" ,");
+    if(!labelErrors.empty()) {
       legacymsg::writeTapeReplyMsg(m_netTimeout, m_labelCmdConnection,
-        labelEx.code(), labelEx.getMessage().str());
+        SEINTERNAL, labelErrors);
     } else {
       legacymsg::writeTapeReplyMsg(m_netTimeout, m_labelCmdConnection,
         SEINTERNAL, "Unknown error");
@@ -156,6 +156,44 @@ void castor::tape::tapeserver::daemon::CatalogueLabelSession::
   }
 }
 
+//------------------------------------------------------------------------------
+// sendSuccessReplyToLabelCommand
+//------------------------------------------------------------------------------
+void castor::tape::tapeserver::daemon::CatalogueLabelSession::
+  sendSuccessReplyToLabelCommand() {
+  try {
+    legacymsg::writeTapeReplyMsg(m_netTimeout, m_labelCmdConnection, 0, "");
+  } catch(castor::exception::Exception &we) { 
+    log::Param params[] = {log::Param("message", we.getMessage().str())};
+    m_log(LOG_ERR, "Failed to send success reply-message to label command",
+      params);
+  }
+}
+
+//------------------------------------------------------------------------------
+// sessionFailed
+//------------------------------------------------------------------------------
+void castor::tape::tapeserver::daemon::CatalogueLabelSession::
+  sessionFailed() {
+  sendFailureReplyToLabelCommand();
+}
+
+//------------------------------------------------------------------------------
+// concatLabelErrors
+//------------------------------------------------------------------------------
+std::string castor::tape::tapeserver::daemon::CatalogueLabelSession::
+  concatLabelErrors(const std::string &separator) {
+  std::ostringstream oss;
+  for(std::list<std::string>::const_iterator itor = m_labelErrors.begin();
+    itor != m_labelErrors.end(); itor++) {
+    if(itor != m_labelErrors.begin()) {
+      oss << separator;
+    }
+    oss << *itor;
+  }
+  return oss.str();
+}
+
 //------------------------------------------------------------------------------
 // getAssignmentTime
 //------------------------------------------------------------------------------
@@ -208,6 +246,6 @@ bool castor::tape::tapeserver::daemon::CatalogueLabelSession::
 // receivedLabelError
 //-----------------------------------------------------------------------------
 void castor::tape::tapeserver::daemon::CatalogueLabelSession::
-  receivedLabelError(const castor::exception::Exception &labelEx) {
-  m_labelErrors.push_back(labelEx);
+  receivedLabelError(const std::string &message) {
+  m_labelErrors.push_back(message);
 }

castor/tape/tapeserver/daemon/CatalogueLabelSession.hpp

@@ -143,11 +143,11 @@ public:
   bool tapeIsBeingMounted() const throw();
 
   /**
-   * To be called if the label session has sent tapserverd a label error.
+   * To be called when the label session has sent tapeserverd a label error.
    *
-   * @param labelEx The label error.
+   * @param message The error message.
    */
-  void receivedLabelError(const castor::exception::Exception &labelEx);
+  void receivedLabelError(const std::string &message);
 
 protected:
 
@@ -198,7 +198,7 @@ private:
   /**
    * List of label errors receved from the label session.
    */
-  std::list<castor::exception::Exception> m_labelErrors;
+  std::list<std::string> m_labelErrors;
 
   /**
    * Determines whether or not the user of the label session has the access
@@ -221,6 +221,27 @@ private:
     const legacymsg::TapeLabelRqstMsgBody &labelJob,
     const int labelCmdConnection);
 
+  /**
+   * Concatenates all of the errors received from the label session into a
+   * single string.
+   *
+   * @param separator The separator to be placed between each error message.
+   * @return The result.
+   */
+  std::string concatLabelErrors(const std::string &separator);
+
+  /**
+   * Sends a failure reply to the tape labeling command-line tool.  The error
+   * message sent is the concatenation of all of the errors received from the
+   * label session.
+   */
+  void sendFailureReplyToLabelCommand();
+
+  /**
+   * Sends a success reply to the tape labeling command-line tool.
+   */
+  void sendSuccessReplyToLabelCommand();
+
 }; // class CatalogueLabelSession
 
 } // namespace daemon

castor/tape/tapeserver/daemon/CleanerSession.cpp

@@ -47,7 +47,7 @@ castor::tape::tapeserver::daemon::CleanerSession::CleanerSession(
 // execute
 //------------------------------------------------------------------------------
 castor::tape::tapeserver::daemon::Session::EndOfSessionAction
-  castor::tape::tapeserver::daemon::CleanerSession::execute() {
+  castor::tape::tapeserver::daemon::CleanerSession::execute() throw() {
   std::string errorMessage;
 
   try {
@@ -80,22 +80,23 @@ castor::tape::tapeserver::daemon::Session::EndOfSessionAction
 
   setProcessCapabilities("cap_sys_rawio+ep");
 
-  std::auto_ptr<tapeserver::drive::DriveInterface> drive(createDrive());
+  std::auto_ptr<drive::DriveInterface> drivePtr = createDrive();
+  drive::DriveInterface &drive = *drivePtr.get();
 
-  waitUntilDriveIsReady(drive.get());
+  waitUntilDriveIsReady(drive);
 
-  if(!drive->hasTapeInPlace()) {
+  if(!drive.hasTapeInPlace()) {
     m_log(LOG_INFO, "Cleaner found tape drive empty", params);
     return MARK_DRIVE_AS_UP;
   }
 
-  rewindDrive(drive.get());
+  rewindDrive(drive);
 
-  checkTapeContainsData(drive.get());
+  checkTapeContainsData(drive);
 
-  const std::string volumeLabelVSN = checkVolumeLabel(drive.get());
+  const std::string volumeLabelVSN = checkVolumeLabel(drive);
 
-  unloadTape(volumeLabelVSN, drive.get());
+  unloadTape(volumeLabelVSN, drive);
 
   dismountTape(volumeLabelVSN);
 
@@ -119,20 +120,21 @@ void castor::tape::tapeserver::daemon::CleanerSession::setProcessCapabilities(
 //------------------------------------------------------------------------------
 // createDrive
 //------------------------------------------------------------------------------
-castor::tape::tapeserver::drive::DriveInterface *
+std::auto_ptr<castor::tape::tapeserver::drive::DriveInterface>
   castor::tape::tapeserver::daemon::CleanerSession::createDrive() {
-  SCSI::DeviceVector dv(m_sysWrapper);    
+  SCSI::DeviceVector dv(m_sysWrapper);
   SCSI::DeviceInfo driveInfo = dv.findBySymlink(m_driveConfig.devFilename);
-  drive::DriveInterface *const drive = drive::createDrive(driveInfo,
-    m_sysWrapper);
+  
+  // Instantiate the drive object
+  std::auto_ptr<castor::tape::tapeserver::drive::DriveInterface>
+    drive(drive::createDrive(driveInfo, m_sysWrapper));
 
-  if(NULL == drive) {
+  if(NULL == drive.get()) {
     castor::exception::Exception ex;
-    ex.getMessage() <<
-      "Failed to instantiate drive object";
+    ex.getMessage() << "Failed to instantiate drive object";
     throw ex;
-  }
-
+  } 
+    
   return drive;
 }
 
@@ -140,7 +142,7 @@ castor::tape::tapeserver::drive::DriveInterface *
 // waitUntilDriveIsReady
 //------------------------------------------------------------------------------
 void castor::tape::tapeserver::daemon::CleanerSession::waitUntilDriveIsReady(
-  drive::DriveInterface *const drive) {
+  drive::DriveInterface &drive) {
   if(0 != m_driveReadyDelayInSeconds) {
     std::list<log::Param> params;
     params.push_back(log::Param("TPVID", m_vid));
@@ -150,7 +152,7 @@ void castor::tape::tapeserver::daemon::CleanerSession::waitUntilDriveIsReady(
 
     try {
       m_log(LOG_INFO, "Cleaner waiting for drive to be ready", params);
-      drive->waitUntilReady(m_driveReadyDelayInSeconds);
+      drive.waitUntilReady(m_driveReadyDelayInSeconds);
       m_log(LOG_INFO, "Cleaner detected drive is ready", params);
     } catch (castor::exception::Exception &ex) {
       params.push_back(log::Param("message", ex.getMessage().str()));
@@ -164,13 +166,13 @@ void castor::tape::tapeserver::daemon::CleanerSession::waitUntilDriveIsReady(
 // rewindDrive
 //------------------------------------------------------------------------------
 void castor::tape::tapeserver::daemon::CleanerSession::rewindDrive(
-  drive::DriveInterface *const drive) {
+  drive::DriveInterface &drive) {
   std::list<log::Param> params;
   params.push_back(log::Param("TPVID", m_vid));
   params.push_back(log::Param("unitName", m_driveConfig.unitName));
 
   m_log(LOG_INFO, "Cleaner rewinding tape", params);
-  drive->rewind();
+  drive.rewind();
   m_log(LOG_INFO, "Cleaner successfully rewound tape", params);
 }
 
@@ -178,13 +180,13 @@ void castor::tape::tapeserver::daemon::CleanerSession::rewindDrive(
 // checkTapeContainsData
 //------------------------------------------------------------------------------
 void castor::tape::tapeserver::daemon::CleanerSession::checkTapeContainsData(
-  drive::DriveInterface *const drive) {
+  drive::DriveInterface &drive) {
   std::list<log::Param> params;
   params.push_back(log::Param("TPVID", m_vid));
   params.push_back(log::Param("unitName", m_driveConfig.unitName));
 
   m_log(LOG_INFO, "Cleaner checking tape contains data", params);
-  if(drive->isTapeBlank()) {
+  if(drive.isTapeBlank()) {
     castor::exception::Exception ex;
     ex.getMessage() << "Tape is completely blank when it should be labeled";
     throw ex;
@@ -196,14 +198,14 @@ void castor::tape::tapeserver::daemon::CleanerSession::checkTapeContainsData(
 // checkVolumeLabel
 //------------------------------------------------------------------------------
 std::string castor::tape::tapeserver::daemon::CleanerSession::checkVolumeLabel(
-  drive::DriveInterface *const drive) {
+  drive::DriveInterface &drive) {
   tapeFile::VOL1 vol1;
   std::list<log::Param> params;
   params.push_back(log::Param("TPVID", m_vid));
   params.push_back(log::Param("unitName", m_driveConfig.unitName));
   
   try {
-    drive->readExactBlock((void * )&vol1, sizeof(vol1),
+    drive.readExactBlock((void * )&vol1, sizeof(vol1),
       "[CleanerSession::clean()] - Reading header VOL1");
     vol1.verify();
 
@@ -235,7 +237,7 @@ std::string castor::tape::tapeserver::daemon::CleanerSession::checkVolumeLabel(
 // unloadTape
 //------------------------------------------------------------------------------
 void castor::tape::tapeserver::daemon::CleanerSession::unloadTape(
-  const std::string &vid, drive::DriveInterface *const drive) {
+  const std::string &vid, drive::DriveInterface &drive) {
   std::list<log::Param> params;
   params.push_back(log::Param("TPVID", vid));
   params.push_back(log::Param("unitName", m_driveConfig.unitName));
@@ -251,7 +253,7 @@ void castor::tape::tapeserver::daemon::CleanerSession::unloadTape(
 
   try {
     m_log(LOG_INFO, "Cleaner unloading tape", params);
-    drive->unloadTape();
+    drive.unloadTape();
     m_log(LOG_INFO, "Cleaner unloaded tape", params);
   } catch (castor::exception::Exception &ne) {
     castor::exception::Exception ex;

castor/tape/tapeserver/daemon/CleanerSession.hpp

@@ -70,24 +70,11 @@ namespace daemon {
     /** 
      * Execute the session and return the type of action to be performed
      * immediately after the session has completed.
-     *  
-     * The session is responsible for mounting a tape into the tape drive,
-     * working with that tape, unloading the tape from the drive and then
-     * dismounting the tape from the drive and storing it back in its home slot
-     * within the tape library.
-     *
-     * If this method throws an exception and the session is not a cleaner
-     * session then it assumed that the post session action is
-     * EndOfSessionAction::CLEAN_DRIVE.
-     *
-     * If this method throws an exception and the session is a cleaner
-     * session then it assumed that the post session action is
-     * EndOfSessionAction::MARK_DRIVE_AS_DOWN.
      *
      * @return Returns the type of action to be performed after the session has
      * completed.
      */
-    EndOfSessionAction execute();
+    EndOfSessionAction execute() throw();
     
   private:
 
@@ -128,6 +115,13 @@ namespace daemon {
      */
     const uint32_t m_driveReadyDelayInSeconds;
 
+    /** 
+     * Execute the session and return the type of action to be performed
+     * immediately after the session has completed.
+     *
+     * @return Returns the type of action to be performed after the session has
+     * completed.
+     */
     EndOfSessionAction exceptionThrowingExecute();
 
     /**
@@ -143,21 +137,21 @@ namespace daemon {
      *
      * @return The tape drive.
      */
-    drive::DriveInterface *createDrive();
+    std::auto_ptr<drive::DriveInterface> createDrive();
 
     /**
      * Waits for the specified drive to be ready.
      *
      * @param drive The tape drive.
      */
-    void waitUntilDriveIsReady(drive::DriveInterface *const drive);
+    void waitUntilDriveIsReady(drive::DriveInterface &drive);
 
     /**
      * Rewinds the specified tape drive.
      *
      * @param drive The tape drive.
      */
-    void rewindDrive(drive::DriveInterface *const drive);
+    void rewindDrive(drive::DriveInterface &drive);
 
     /**
      * Checks the tape in the specified tape drive contains some data where no
@@ -165,7 +159,7 @@ namespace daemon {
      *
      * @param drive The tape drive.
      */
-    void checkTapeContainsData(drive::DriveInterface *const drive);
+    void checkTapeContainsData(drive::DriveInterface &drive);
 
     /**
      * Checks that the tape in the specified drive contains a valid volume
@@ -175,7 +169,7 @@ namespace daemon {
      * tested is present and rewound to the beginning.
      * @return The VSN stored within the colue label.
      */
-    std::string checkVolumeLabel(drive::DriveInterface *const drive);
+    std::string checkVolumeLabel(drive::DriveInterface &drive);
 
     /**
      * Unloads the specified tape from the specified tape drive.
@@ -184,7 +178,7 @@ namespace daemon {
      * that the value of this field is only used for logging purposes.
      * @param drive The tape drive.
      */
-    void unloadTape(const std::string &vid, drive::DriveInterface *const drive);
+    void unloadTape(const std::string &vid, drive::DriveInterface &drive);
 
     /**
      * Dismounts the specified tape.

castor/tape/tapeserver/daemon/DataTransferSessionTest.cpp

@@ -117,7 +117,7 @@ TEST(tapeServer, DataTransferSessionGooddayRecall) {
   {
     // Label the tape
     castor::tape::tapeFile::LabelSession ls(*mockSys.fake.m_pathToDrive["/dev/nst0"], 
-        "V12345", true);
+        "V12345");
     mockSys.fake.m_pathToDrive["/dev/nst0"]->rewind();
     // And write to it
     castor::tape::tapeserver::client::ClientInterface::VolumeInfo volInfo;
@@ -221,7 +221,7 @@ TEST(tapeServer, DataTransferSessionWrongRecall) {
   {
     // Label the tape
     castor::tape::tapeFile::LabelSession ls(*mockSys.fake.m_pathToDrive["/dev/nst0"], 
-        "V12345", true);
+        "V12345");
     mockSys.fake.m_pathToDrive["/dev/nst0"]->rewind();
     // And write to it
     castor::tape::tapeserver::client::ClientInterface::VolumeInfo volInfo;
@@ -384,7 +384,7 @@ TEST(tapeServer, DataTransferSessionFailtoMount) {
   {
     // Label the tape
     castor::tape::tapeFile::LabelSession ls(*mockSys.fake.m_pathToDrive["/dev/nst0"], 
-        "V12345", true);
+        "V12345");
     mockSys.fake.m_pathToDrive["/dev/nst0"]->rewind();
     // And write to it
     castor::tape::tapeserver::client::ClientInterface::VolumeInfo volInfo;
@@ -596,7 +596,7 @@ TEST(tapeServer, DataTransferSessionGooddayMigration) {
   
   // Just label the tape
   castor::tape::tapeFile::LabelSession ls(*mockSys.fake.m_pathToDrive["/dev/nst0"], 
-      "V12345", true);
+      "V12345");
   mockSys.fake.m_pathToDrive["/dev/nst0"]->rewind();
   
   tempFileVector tempFiles;
@@ -690,7 +690,7 @@ TEST(tapeServer, DataTransferSessionMissingFilesMigration) {
   
   // Just label the tape
   castor::tape::tapeFile::LabelSession ls(*mockSys.fake.m_pathToDrive["/dev/nst0"], 
-      "V12345", true);
+      "V12345");
   mockSys.fake.m_pathToDrive["/dev/nst0"]->rewind();
   
   // Prepare the files, but delete them immediately. The migration will fail.
@@ -778,7 +778,7 @@ TEST(tapeServer, DataTransferSessionTapeFullMigration) {
   
   // Just label the tape
   castor::tape::tapeFile::LabelSession ls(*mockSys.fake.m_pathToDrive["/dev/nst0"], 
-      "V12345", true);
+      "V12345");
   mockSys.fake.m_pathToDrive["/dev/nst0"]->rewind();
   
   tempFileVector tempFiles;
@@ -885,7 +885,7 @@ TEST(tapeServer, DataTransferSessionTapeFullOnFlushMigration) {
   
   // Just label the tape
   castor::tape::tapeFile::LabelSession ls(*mockSys.fake.m_pathToDrive["/dev/nst0"], 
-      "V12345", true);
+      "V12345");
   mockSys.fake.m_pathToDrive["/dev/nst0"]->rewind();
   
   tempFileVector tempFiles;

castor/tape/tapeserver/daemon/LabelSession.cpp

@@ -26,6 +26,7 @@
 #include "castor/legacymsg/MessageHeader.hpp"
 #include "castor/log/LogContext.hpp"
 #include "castor/tape/tapeserver/daemon/LabelSession.hpp"
+#include "castor/tape/tapeserver/file/Structures.hpp"
 #include "castor/exception/Exception.hpp"
 #include "castor/tape/utils/utils.hpp"
 #include "castor/System.hpp"
@@ -65,77 +66,109 @@ castor::tape::tapeserver::daemon::LabelSession::LabelSession(
 // execute
 //------------------------------------------------------------------------------
 castor::tape::tapeserver::daemon::Session::EndOfSessionAction
-  castor::tape::tapeserver::daemon::LabelSession::execute() {
+  castor::tape::tapeserver::daemon::LabelSession::execute() throw() {
+  std::string errorMessage;
+
   try {
-    m_capUtils.setProcText("cap_sys_rawio+ep");
-    {
-      log::Param params[] = {
-        log::Param("capabilities", m_capUtils.getProcText())};
-      m_log(LOG_INFO, "Label session set process capabilities for using tape",
-        params);
-    }
+    return exceptionThrowingExecute();
+  } catch(castor::exception::Exception &ex) {
+    errorMessage = ex.getMessage().str();
+  } catch(std::exception &se) {
+    errorMessage = se.what();
+  } catch(...) {
+    errorMessage = "Caught an unknown exception";
+  }