tools: error handling on journal import/export

Actually propagate nonzero returns codes!  Also
add checks on return values of I/O functions so
that someone doesn't think they've successfully
exported a journal if they haven't, and some
validation of the header pointers during import
so that people find out with a nice error
instead of an assertion if something is up.

Signed-off-by: John Spray <john.spray@redhat.com>
Fix compile issue in the position value cout.

Signed-off-by: Greg Farnum <gfarnum@redhat.com>

diff --git a/src/tools/cephfs/Dumper.cc b/src/tools/cephfs/Dumper.cc
index af3e19c57b35b9e1045b4f04702985cbed5cf0de..c8e7f8468528fba5903060dd072a862e73ac1376 100644 (file)
--- a/src/tools/cephfs/Dumper.cc
+++ b/src/tools/cephfs/Dumper.cc
@@ -69,7 +69,7 @@ int Dumper::recover_journal(Journaler *journaler)
 }
 
 
-void Dumper::dump(const char *dump_file)
+int Dumper::dump(const char *dump_file)
 {
   int r = 0;
 
@@ -77,7 +77,7 @@ void Dumper::dump(const char *dump_file)
                                        objecter, 0, 0, &timer, &finisher);
   r = recover_journal(&journaler);
   if (r) {
-    return;
+    return r;
   }
   uint64_t start = journaler.get_read_pos();
   uint64_t end = journaler.get_write_pos();
@@ -110,34 +110,56 @@ void Dumper::dump(const char *dump_file)
            (unsigned long long)journaler.last_committed.stream_format,
            (unsigned long long)journaler.last_committed.trimmed_pos, (unsigned long long)journaler.last_committed.trimmed_pos,
            4);
-    int r = safe_write(fd, buf, sizeof(buf));
-    if (r)
-      ceph_abort();
+    r = safe_write(fd, buf, sizeof(buf));
+    if (r) {
+      derr << "Error " << r << " (" << cpp_strerror(r) << ") writing journal file header" << dendl;
+      return r;
+    }
 
     // write the data
-    ::lseek64(fd, start, SEEK_SET);
-    bl.write_fd(fd);
-    ::close(fd);
+    off64_t seeked = ::lseek64(fd, start, SEEK_SET);
+    if (seeked == (off64_t)-1) {
+      r = errno;
+      derr << "Error " << r << " (" << cpp_strerror(r) << ") seeking to 0x" << std::hex << start << std::dec << dendl;
+      ::close(fd);
+      return r;
+    }
+    r = bl.write_fd(fd);
+    if (r) {
+      derr << "Error " << r << " (" << cpp_strerror(r) << ") writing journal file" << dendl;
+      ::close(fd);
+      return r;
+    }
+
+    r = ::close(fd);
+    if (r) {
+      r = errno;
+      derr << "Error " << r << " (" << cpp_strerror(r) << ") closing journal file" << dendl;
+      return r;
+    }
 
     cout << "wrote " << bl.length() << " bytes at offset " << start << " to " << dump_file << "\n"
         << "NOTE: this is a _sparse_ file; you can\n"
         << "\t$ tar cSzf " << dump_file << ".tgz " << dump_file << "\n"
         << "      to efficiently compress it while preserving sparseness." << std::endl;
+    return 0;
   } else {
     int err = errno;
     derr << "unable to open " << dump_file << ": " << cpp_strerror(err) << dendl;
+    return err;
   }
 }
 
-void Dumper::undump(const char *dump_file)
+int Dumper::undump(const char *dump_file)
 {
-
   cout << "undump " << dump_file << std::endl;
   
+  int r = 0;
   int fd = ::open(dump_file, O_RDONLY);
   if (fd < 0) {
-    derr << "couldn't open " << dump_file << ": " << cpp_strerror(errno) << dendl;
-    return;
+    r = errno;
+    derr << "couldn't open " << dump_file << ": " << cpp_strerror(r) << dendl;
+    return r;
   }
 
   // Ceph mds0 journal dump
@@ -145,10 +167,10 @@ void Dumper::undump(const char *dump_file)
   //        length 1097504 (0x10bf20)
 
   char buf[200];
-  int r = safe_read(fd, buf, sizeof(buf));
+  r = safe_read(fd, buf, sizeof(buf));
   if (r < 0) {
     VOID_TEMP_FAILURE_RETRY(::close(fd));
-    return;
+    return r;
   }
 
   long long unsigned start, len, write_pos, format, trimmed_pos;
@@ -164,8 +186,23 @@ void Dumper::undump(const char *dump_file)
     trimmed_pos = start - (start % g_default_file_layout.fl_object_size);
   }
 
+  if (trimmed_pos > start) {
+    derr << std::hex << "Invalid header (trimmed 0x" << trimmed_pos
+      << " > expire 0x" << start << std::dec << dendl;
+    return -EINVAL;
+  }
 
-  cout << "start " << start << " len " << len << std::endl;
+  if (start > write_pos) {
+    derr << std::hex << "Invalid header (expire 0x" << start
+      << " > write 0x" << write_pos << std::dec << dendl;
+    return -EINVAL;
+  }
+
+  cout << "start " << start <<
+    " len " << len <<
+    " write_pos " << write_pos <<
+    " format " << format <<
+    " trimmed_pos " << trimmed_pos << std::endl;
   
   Journaler::Header h;
   h.trimmed_pos = trimmed_pos;
@@ -194,7 +231,7 @@ void Dumper::undump(const char *dump_file)
   r = header_cond.wait();
   if (r != 0) {
     derr << "Failed to write header: " << cpp_strerror(r) << dendl;
-    return;
+    return r;
   }
 
   Filer filer(objecter);
@@ -237,7 +274,7 @@ void Dumper::undump(const char *dump_file)
     r = write_cond.wait();
     if (r != 0) {
       derr << "Failed to write header: " << cpp_strerror(r) << dendl;
-      return;
+      return r;
     }
       
     // Advance
@@ -247,5 +284,6 @@ void Dumper::undump(const char *dump_file)
 
   VOID_TEMP_FAILURE_RETRY(::close(fd));
   cout << "done." << std::endl;
+  return 0;
 }
 

diff --git a/src/tools/cephfs/Dumper.h b/src/tools/cephfs/Dumper.h
index 1986eca34feb75c58a1b48b614ac547221efbe8d..e94c5960eb1034f6f78abe76bebd3a123ed50598 100644 (file)
--- a/src/tools/cephfs/Dumper.h
+++ b/src/tools/cephfs/Dumper.h
@@ -40,8 +40,8 @@ public:
 
   int init(int rank);
   int recover_journal(Journaler *journaler);
-  void dump(const char *dumpfile);
-  void undump(const char *dumpfile);
+  int dump(const char *dumpfile);
+  int undump(const char *dumpfile);
 };
 
 #endif /* JOURNAL_DUMPER_H_ */

diff --git a/src/tools/cephfs/JournalTool.cc b/src/tools/cephfs/JournalTool.cc
index 976376c78ba51e3d724047917c3a8d0e98566934..058c7e08ffb18cafc8a9ba7e2165d236000ccd34 100644 (file)
--- a/src/tools/cephfs/JournalTool.cc
+++ b/src/tools/cephfs/JournalTool.cc
@@ -462,9 +462,9 @@ int JournalTool::journal_export(std::string const &path, bool import)
       return r;
     }
     if (import) {
-      dumper.undump(path.c_str());
+      r = dumper.undump(path.c_str());
     } else {
-      dumper.dump(path.c_str());
+      r = dumper.dump(path.c_str());
     }
     dumper.shutdown();
   }