- improved error handling (bnc#718914)

diff --git a/package/snapper.changes b/package/snapper.changes
index 0cb777a81c6ec29a5ece7eae73a1a30cc59f636c..3c85048e74148e1c37072016cbad7f7cf6292e2e 100644 (file)
--- a/package/snapper.changes
+++ b/package/snapper.changes
@@ -1,3 +1,8 @@
+-------------------------------------------------------------------
+Tue Sep 20 11:05:47 CEST 2011 - aschnell@suse.de
+
+- improved error handling (bnc#718914)
+
 -------------------------------------------------------------------
 Thu Sep 15 16:44:26 CEST 2011 - aschnell@suse.de
 

diff --git a/snapper/Exception.h b/snapper/Exception.h
index e63ca97dc87e72385936f3186ac11c2d3c1ff448..14eb95c0ac61b71c133a42896ef5efed4e520b2a 100644 (file)
--- a/snapper/Exception.h
+++ b/snapper/Exception.h
@@ -30,26 +30,33 @@
 namespace snapper
 {
 
-    struct FileNotFoundException : public std::exception
+    struct SnapperException : public std::exception
+    {
+       explicit SnapperException() throw() {}
+       virtual const char* what() const throw() { return "generic snapper exception"; }
+    };
+
+
+    struct FileNotFoundException : public SnapperException
     {
        explicit FileNotFoundException() throw() {}
        virtual const char* what() const throw() { return "file not found"; }
     };
 
 
-    struct IllegalSnapshotException : public std::exception
+    struct IllegalSnapshotException : public SnapperException
     {
        explicit IllegalSnapshotException() throw() {}
        virtual const char* what() const throw() { return "illegal snapshot"; }
     };
 
-    struct LogicErrorException : public std::exception
+    struct LogicErrorException : public SnapperException
     {
        explicit LogicErrorException() throw() {}
        virtual const char* what() const throw() { return "logic error"; }
     };
 
-    struct IOErrorException : public std::exception
+    struct IOErrorException : public SnapperException
     {
        explicit IOErrorException() throw() {}
        virtual const char* what() const throw() { return "IO error"; }

diff --git a/snapper/Snapper.h b/snapper/Snapper.h
index 88a5e927dd1765160d30281ae31a50e727358412..91a44d4f00c86f340567d54fe2b68e3450675558 100644 (file)
--- a/snapper/Snapper.h
+++ b/snapper/Snapper.h
@@ -75,32 +75,32 @@ namespace snapper
     };
 
 
-    struct ConfigNotFoundException : public std::exception
+    struct ConfigNotFoundException : public SnapperException
     {
        explicit ConfigNotFoundException() throw() {}
        virtual const char* what() const throw() { return "config not found"; }
     };
 
-    struct InvalidConfigException : public std::exception
+    struct InvalidConfigException : public SnapperException
     {
        explicit InvalidConfigException() throw() {}
        virtual const char* what() const throw() { return "invalid config"; }
     };
 
-    struct InvalidUserdataException : public std::exception
+    struct InvalidUserdataException : public SnapperException
     {
        explicit InvalidUserdataException() throw() {}
        virtual const char* what() const throw() { return "invalid userdata"; }
     };
 
-    struct ListConfigsFailedException : public std::exception
+    struct ListConfigsFailedException : public SnapperException
     {
        explicit ListConfigsFailedException(const char* msg) throw() : msg(msg) {}
        virtual const char* what() const throw() { return msg; }
        const char* msg;
     };
 
-    struct AddConfigFailedException : public std::exception
+    struct AddConfigFailedException : public SnapperException
     {
        explicit AddConfigFailedException(const char* msg) throw() : msg(msg) {}
        virtual const char* what() const throw() { return msg; }

diff --git a/snapper/Snapshot.cc b/snapper/Snapshot.cc
index ffac62f220a05edf17972b310495310ce2aa14f8..dff35b077ee324e7f25cc96a9d556801640b3053 100644 (file)
--- a/snapper/Snapshot.cc
+++ b/snapper/Snapshot.cc
@@ -337,21 +337,18 @@ namespace snapper
     }
 
 
-    bool
+    void
     Snapshot::flushInfo()
     {
        if (!info_modified)
-           return true;
-
-       if (!writeInfo())
-           return false;
+           return;
 
+       writeInfo();
        info_modified = false;
-       return true;
     }
 
 
-    bool
+    void
     Snapshot::writeInfo() const
     {
        XmlFile xml;
@@ -380,9 +377,19 @@ namespace snapper
            setChildValue(userdata_node, "value", it->second);
        }
 
-       xml.save(infoDir() + "/info.xml");
+       if (!xml.save(infoDir() + "/info.xml.tmp"))
+       {
+           y2err("saving info.xml failed infoDir: " << infoDir() << " errno: << " << errno <<
+                 " (" << strerror(errno) << ")");
+           throw IOErrorException();
+       }
 
-       return true;
+       if (rename(string(infoDir() + "/info.xml.tmp").c_str(), string(infoDir() + "/info.xml").c_str()) != 0)
+       {
+           y2err("rename info.xml failed infoDir: " << infoDir() << " errno: << " << errno <<
+                 " (" << strerror(errno) << ")");
+           throw IOErrorException();
+       }
     }
 
 
@@ -434,9 +441,7 @@ namespace snapper
        snapshot.description = description;
        snapshot.info_modified = true;
 
-       snapshot.createFilesystemSnapshot();
-
-       return entries.insert(entries.end(), snapshot);
+       return createHelper(snapshot);
     }
 
 
@@ -447,9 +452,7 @@ namespace snapper
        snapshot.description = description;
        snapshot.info_modified = true;
 
-       snapshot.createFilesystemSnapshot();
-
-       return entries.insert(entries.end(), snapshot);
+       return createHelper(snapshot);
     }
 
 
@@ -463,7 +466,33 @@ namespace snapper
        snapshot.pre_num = pre->getNum();
        snapshot.info_modified = true;
 
-       snapshot.createFilesystemSnapshot();
+       return createHelper(snapshot);
+    }
+
+
+    Snapshots::iterator
+    Snapshots::createHelper(Snapshot& snapshot)
+    {
+       try
+       {
+           snapshot.createFilesystemSnapshot();
+       }
+       catch (const CreateSnapshotFailedException& e)
+       {
+           rmdir(snapshot.infoDir().c_str());
+           throw;
+       }
+
+       try
+       {
+           snapshot.flushInfo();
+       }
+       catch (const IOErrorException& e)
+       {
+           snapshot.deleteFilesystemSnapshot();
+           rmdir(snapshot.infoDir().c_str());
+           throw;
+       }
 
        return entries.insert(entries.end(), snapshot);
     }

diff --git a/snapper/Snapshot.h b/snapper/Snapshot.h
index b41fcfea19b998289d0ff54704fa2fdf996853bd..769c0bdc49ea598f1c8ef22bb16a6e22ca35fea1 100644 (file)
--- a/snapper/Snapshot.h
+++ b/snapper/Snapshot.h
@@ -29,6 +29,8 @@
 #include <list>
 #include <map>
 
+#include "snapper/Exception.h"
+
 
 namespace snapper
 {
@@ -43,32 +45,32 @@ namespace snapper
     enum SnapshotType { SINGLE, PRE, POST };
 
 
-    struct CreateSnapshotFailedException : public std::exception
+    struct CreateSnapshotFailedException : public SnapperException
     {
        explicit CreateSnapshotFailedException() throw() {}
        virtual const char* what() const throw() { return "create snapshot failed"; }
     };
 
-    struct DeleteSnapshotFailedException : public std::exception
+    struct DeleteSnapshotFailedException : public SnapperException
     {
        explicit DeleteSnapshotFailedException() throw() {}
        virtual const char* what() const throw() { return "delete snapshot failed"; }
     };
 
 
-    struct IsSnapshotMountedFailedException : public std::exception
+    struct IsSnapshotMountedFailedException : public SnapperException
     {
        explicit IsSnapshotMountedFailedException() throw() {}
        virtual const char* what() const throw() { return "is snapshot mounted failed"; }
     };
 
-    struct MountSnapshotFailedException : public std::exception
+    struct MountSnapshotFailedException : public SnapperException
     {
        explicit MountSnapshotFailedException() throw() {}
        virtual const char* what() const throw() { return "mount snapshot failed"; }
     };
 
-    struct UmountSnapshotFailedException : public std::exception
+    struct UmountSnapshotFailedException : public SnapperException
     {
        explicit UmountSnapshotFailedException() throw() {}
        virtual const char* what() const throw() { return "umount snapshot failed"; }
@@ -103,7 +105,7 @@ namespace snapper
        void setUserdata(const map<string, string>& userdata);
        map<string, string> getUserdata() const { return userdata; }
 
-       bool flushInfo();
+       void flushInfo();
 
        string infoDir() const;
        string snapshotDir() const;
@@ -133,7 +135,7 @@ namespace snapper
 
        bool info_modified;
 
-       bool writeInfo() const;
+       void writeInfo() const;
 
        void createFilesystemSnapshot() const;
        void deleteFilesystemSnapshot() const;
@@ -190,6 +192,8 @@ namespace snapper
        iterator createPreSnapshot(string description);
        iterator createPostSnapshot(const_iterator pre);
 
+       iterator createHelper(Snapshot& snapshot);
+
        void deleteSnapshot(iterator snapshot);
 
        unsigned int nextNumber();

diff --git a/snapper/XmlFile.h b/snapper/XmlFile.h
index c148952d45b41807d6441ccc6b8be081bb020697..3c8d9e867f58ffb57b7b26cc6d8c367bf1d8feba 100644 (file)
--- a/snapper/XmlFile.h
+++ b/snapper/XmlFile.h
@@ -48,7 +48,7 @@ namespace snapper
        ~XmlFile();
 
        bool save(const string& filename)
-           { return xmlSaveFormatFile(filename.c_str(), doc, 1); }
+           { return xmlSaveFormatFile(filename.c_str(), doc, 1) != -1; }
 
        void setRootElement(xmlNode* node)
            { xmlDocSetRootElement(doc, node); }

diff --git a/tools/snapper.cc b/tools/snapper.cc
index cba5b3faab3d5807da263dbd004e32f1ee6b3f27..547fb0f454c7d02e11fdc9a8acf6c258d9c0ce0d 100644 (file)
--- a/tools/snapper.cc
+++ b/tools/snapper.cc
@@ -1136,7 +1136,17 @@ main(int argc, char** argv)
            sh->setUndoCallback(&undo_callback_impl);
        }
 
-       (*cmd->second)();
+       try
+       {
+           (*cmd->second)();
+       }
+       catch (const SnapperException& e)
+       {
+           y2err("caught final exception");
+           cerr << sformat(_("Command failed (%s). See log for more information."),
+                           e.what()) << endl;
+           exit(EXIT_FAILURE);
+       }
 
        deleteSnapper(sh);
     }