- implemented use-counter for mounts of snapshots

diff --git a/client/commands.cc b/client/commands.cc
index 5d6678713e06f77dcef17d6acb761d732c2887c9..127880a8b2889279304a31cd8e937ceb6a540576 100644 (file)
--- a/client/commands.cc
+++ b/client/commands.cc
@@ -217,12 +217,12 @@ command_delete_xsnapshots(DBus::Connection& conn, const string& config_name,
 
 string
 command_mount_xsnapshots(DBus::Connection& conn, const string& config_name,
-                        unsigned int num)
+                        unsigned int num, bool user_request)
 {
     DBus::MessageMethodCall call(SERVICE, OBJECT, INTERFACE, "MountSnapshot");
 
     DBus::Hoho hoho(call);
-    hoho << config_name << num;
+    hoho << config_name << num << user_request;
 
     DBus::Message reply = conn.send_with_reply_and_block(call);
 
@@ -237,12 +237,12 @@ command_mount_xsnapshots(DBus::Connection& conn, const string& config_name,
 
 void
 command_umount_xsnapshots(DBus::Connection& conn, const string& config_name,
-                         unsigned int num)
+                         unsigned int num, bool user_request)
 {
     DBus::MessageMethodCall call(SERVICE, OBJECT, INTERFACE, "UmountSnapshot");
 
     DBus::Hoho hoho(call);
-    hoho << config_name << num;
+    hoho << config_name << num << user_request;
 
     conn.send_with_reply_and_block(call);
 }

diff --git a/client/commands.h b/client/commands.h
index a8c840277e0a74dbfdd9a10a25d9ab652a1b151e..7fe966471ef03cabd46c6f2c140720742db83cc0 100644 (file)
--- a/client/commands.h
+++ b/client/commands.h
@@ -77,11 +77,11 @@ command_delete_xsnapshots(DBus::Connection& conn, const string& config_name,
 
 string
 command_mount_xsnapshots(DBus::Connection& conn, const string& config_name,
-                        unsigned int num);
+                        unsigned int num, bool user_request);
 
 void
 command_umount_xsnapshots(DBus::Connection& conn, const string& config_name,
-                         unsigned int num);
+                         unsigned int num, bool user_request);
 
 string
 command_get_xmount_point(DBus::Connection& conn, const string& config_name,

diff --git a/client/snapper.cc b/client/snapper.cc
index efd5005666cbff4f686404f6dc5f0e306890018c..407308274d712eff86b352b26f29e18a4d5ecbdd 100644 (file)
--- a/client/snapper.cc
+++ b/client/snapper.cc
@@ -174,12 +174,12 @@ struct MyComparison
        if (mount)
        {
            if (nums.first != 0)
-               file_paths.pre_path = command_mount_xsnapshots(conn, config_name, nums.first);
+               file_paths.pre_path = command_mount_xsnapshots(conn, config_name, nums.first, false);
            else
                file_paths.pre_path = file_paths.system_path;
 
            if (nums.second != 0)
-               file_paths.post_path = command_mount_xsnapshots(conn, config_name, nums.second);
+               file_paths.post_path = command_mount_xsnapshots(conn, config_name, nums.second, false);
            else
                file_paths.post_path = file_paths.system_path;
        }
@@ -729,7 +729,7 @@ command_mount(DBus::Connection& conn)
     {
        unsigned int num = read_num(getopts.popArg());
 
-       command_mount_xsnapshots(conn, config_name, num);
+       command_mount_xsnapshots(conn, config_name, num, true);
     }
 }
 
@@ -757,7 +757,7 @@ command_umount(DBus::Connection& conn)
     {
        unsigned int num = read_num(getopts.popArg());
 
-       command_umount_xsnapshots(conn, config_name, num);
+       command_umount_xsnapshots(conn, config_name, num, true);
     }
 }
 
@@ -1278,6 +1278,8 @@ main(int argc, char** argv)
            cerr << _("Config is locked.") << endl;
        else if (name == "error.config_in_use")
            cerr << _("Config is in use.") << endl;
+       else if (name == "error.snapshot_in_use")
+           cerr << _("Snapshot is in use.") << endl;
        else if (name == "error.unknown_file_use")
            cerr << _("Unknown file.") << endl;
        else if (name == "error.io_error")

diff --git a/doc/dbus-protocol.txt b/doc/dbus-protocol.txt
index e013416a6cb08ec0e05773c7dc7c5c992f03575f..f015d40771a9da6958ac9bf71170aeec277266ff 100644 (file)
--- a/doc/dbus-protocol.txt
+++ b/doc/dbus-protocol.txt
@@ -3,7 +3,7 @@ method ListConfigs
 method GetConfig config-name
 
 
-method CreateConfig config-name subvolume fstype template_name
+method CreateConfig config-name subvolume fstype template-name
 method DeleteConfig config-name
 
 signal ConfigCreated config-name
@@ -32,10 +32,13 @@ signal SnapshotCreated config-name number
 signal SnapshotModified config-name number
 signal SnapshotsDeleted config-name list(numbers)
 
-method MountSnapshot config-name number
-method UmountSnapshot config-name number
+method MountSnapshot config-name number user-request
+method UmountSnapshot config-name number user-request
 method GetMountPoint config-name number
 
+Snapshots mounted with user-request set to false will be unmounted (delayed)
+after the client disconnects.
+
 
 method CreateComparison config-name number1 number2
 method DeleteComparison config-name number1 number2

diff --git a/package/snapper.changes b/package/snapper.changes
index 24f9df45b04fa59e0dcb2edf756f3fb0bc65663b..527c40b85b3a3b1397ef53a46410aae18766f790 100644 (file)
--- a/package/snapper.changes
+++ b/package/snapper.changes
@@ -1,3 +1,8 @@
+-------------------------------------------------------------------
+Mon Nov 26 12:40:54 CET 2012 - aschnell@suse.de
+
+- implemented use-counter for mounts of snapshots
+
 -------------------------------------------------------------------
 Wed Nov 21 11:55:28 CET 2012 - aschnell@suse.de
 

diff --git a/server/Client.cc b/server/Client.cc
index 12bbf71c333fb35e484420c17927f71dc5f20f1b..78f95e2520fc4dd7531cbe317d9081869612cb9c 100644 (file)
--- a/server/Client.cc
+++ b/server/Client.cc
@@ -52,6 +52,26 @@ Client::~Client()
     {
        delete_comparison(it);
     }
+
+    for (map<pair<string, unsigned int>, unsigned int>::iterator it1 = mounts.begin();
+        it1 != mounts.end(); ++it1)
+    {
+       string config_name = it1->first.first;
+       unsigned int number = it1->first.second;
+       unsigned int use_count = it1->second;
+
+       MetaSnappers::iterator it2 = meta_snappers.find(config_name);
+
+       Snapper* snapper = it2->getSnapper();
+       Snapshots& snapshots = snapper->getSnapshots();
+
+       Snapshots::iterator snap = snapshots.find(number);
+       if (snap == snapshots.end())
+           throw IllegalSnapshotException();
+
+       for (unsigned int i = 0; i < use_count; ++i)
+           snap->umountFilesystemSnapshot(false);
+    }
 }
 
 
@@ -117,6 +137,28 @@ Client::has_lock(const string& config_name) const
 }
 
 
+void
+Client::add_mount(const string& config_name, unsigned int number)
+{
+    mounts[make_pair(config_name, number)]++;
+}
+
+
+void
+Client::remove_mount(const string& config_name, unsigned int number)
+{
+    map<pair<string, unsigned int>, unsigned int>::iterator it =
+       mounts.find(make_pair(config_name, number));
+    if (it != mounts.end())
+    {
+       if (it->second > 1)
+           it->second--;
+       else
+           mounts.erase(it);
+    }
+}
+
+
 void
 Client::introspect(DBus::Connection& conn, DBus::Message& msg)
 {
@@ -240,11 +282,13 @@ Client::introspect(DBus::Connection& conn, DBus::Message& msg)
        "    <method name='MountSnapshot'>\n"
        "      <arg name='config-name' type='s' direction='in'/>\n"
        "      <arg name='number' type='u' direction='in'/>\n"
+       "      <arg name='user-request' type='b' direction='in'/>\n"
        "    </method>\n"
 
        "    <method name='UmountSnapshot'>\n"
        "      <arg name='config-name' type='s' direction='in'/>\n"
        "      <arg name='number' type='u' direction='in'/>\n"
+       "      <arg name='user-request' type='b' direction='in'/>\n"
        "    </method>\n"
 
        "    <method name='GetMountPoint'>\n"
@@ -338,18 +382,39 @@ Client::check_lock(DBus::Connection& conn, DBus::Message& msg, const string& con
 }
 
 
-struct InUse : public std::exception
+struct ConfigInUse : public std::exception
 {
-    explicit InUse() throw() {}
-    virtual const char* what() const throw() { return "in use"; }
+    explicit ConfigInUse() throw() {}
+    virtual const char* what() const throw() { return "config in use"; }
+};
+
+
+struct SnapshotInUse : public std::exception
+{
+    explicit SnapshotInUse() throw() {}
+    virtual const char* what() const throw() { return "snapshot in use"; }
 };
 
 
 void
-Client::check_in_use(const MetaSnapper& meta_snapper) const
+Client::check_config_in_use(const MetaSnapper& meta_snapper) const
 {
     if (meta_snapper.use_count() != 0)
-       throw InUse();
+       throw ConfigInUse();
+}
+
+
+void
+Client::check_snapshot_in_use(const MetaSnapper& meta_snapper, unsigned int number) const
+{
+    for (Clients::const_iterator it1 = clients.begin(); it1 != clients.end(); ++it1)
+    {
+       map<pair<string, unsigned int>, unsigned int>::const_iterator it2 =
+           it1->mounts.find(make_pair(meta_snapper.configName(), number));
+
+       if (it2 != it1->mounts.end())
+           throw SnapshotInUse();
+    }
 }
 
 
@@ -511,7 +576,7 @@ Client::delete_config(DBus::Connection& conn, DBus::Message& msg)
 
     check_permission(conn, msg);
     check_lock(conn, msg, config_name);
-    check_in_use(*it);
+    check_config_in_use(*it);
 
     meta_snappers.deleteConfig(it);
 
@@ -847,18 +912,20 @@ Client::delete_snapshots(DBus::Connection& conn, DBus::Message& msg)
 
     boost::unique_lock<boost::shared_mutex> lock(big_mutex);
 
-    MetaSnappers::iterator it = meta_snappers.find(config_name);
+    MetaSnappers::iterator it1 = meta_snappers.find(config_name);
 
-    check_permission(conn, msg, *it);
+    check_permission(conn, msg, *it1);
     check_lock(conn, msg, config_name);
-    check_in_use(*it);
+    check_config_in_use(*it1);
 
-    Snapper* snapper = it->getSnapper();
+    Snapper* snapper = it1->getSnapper();
     Snapshots& snapshots = snapper->getSnapshots();
 
-    for (list<unsigned int>::const_iterator it = nums.begin(); it != nums.end(); ++it)
+    for (list<unsigned int>::const_iterator it2 = nums.begin(); it2 != nums.end(); ++it2)
     {
-       Snapshots::iterator snap = snapshots.find(*it);
+       check_snapshot_in_use(*it1, *it2);
+
+       Snapshots::iterator snap = snapshots.find(*it2);
 
        snapper->deleteSnapshot(snap);
     }
@@ -876,11 +943,13 @@ Client::mount_snapshot(DBus::Connection& conn, DBus::Message& msg)
 {
     string config_name;
     dbus_uint32_t num;
+    bool user_request;
 
     DBus::Hihi hihi(msg);
-    hihi >> config_name >> num;
+    hihi >> config_name >> num >> user_request;
 
-    y2deb("MountSnapshot config_name:" << config_name << " num:" << num);
+    y2deb("MountSnapshot config_name:" << config_name << " num:" << num <<
+         " user_request:" << user_request);
 
     boost::unique_lock<boost::shared_mutex> lock(big_mutex);
 
@@ -895,7 +964,10 @@ Client::mount_snapshot(DBus::Connection& conn, DBus::Message& msg)
     if (snap == snapshots.end())
        throw IllegalSnapshotException();
 
-    snap->mountFilesystemSnapshot();
+    snap->mountFilesystemSnapshot(user_request);
+
+    if (!user_request)
+       add_mount(config_name, num);
 
     string mount_point = snap->snapshotDir();
 
@@ -913,11 +985,13 @@ Client::umount_snapshot(DBus::Connection& conn, DBus::Message& msg)
 {
     string config_name;
     dbus_uint32_t num;
+    bool user_request;
 
     DBus::Hihi hihi(msg);
-    hihi >> config_name >> num;
+    hihi >> config_name >> num >> user_request;
 
-    y2deb("UmountSnapshot config_name:" << config_name << " num:" << num);
+    y2deb("UmountSnapshot config_name:" << config_name << " num:" << num <<
+         " user_request:" << user_request);
 
     boost::unique_lock<boost::shared_mutex> lock(big_mutex);
 
@@ -932,7 +1006,10 @@ Client::umount_snapshot(DBus::Connection& conn, DBus::Message& msg)
     if (snap == snapshots.end())
        throw IllegalSnapshotException();
 
-    snap->umountFilesystemSnapshot();
+    snap->umountFilesystemSnapshot(user_request);
+
+    if (!user_request)
+       remove_mount(config_name, num);
 
     DBus::MessageMethodReturn reply(msg);
 
@@ -1225,11 +1302,16 @@ Client::dispatch(DBus::Connection& conn, DBus::Message& msg)
        DBus::MessageError reply(msg, "error.config_locked", DBUS_ERROR_FAILED);
        conn.send(reply);
     }
-    catch (const InUse& e)
+    catch (const ConfigInUse& e)
     {
        DBus::MessageError reply(msg, "error.config_in_use", DBUS_ERROR_FAILED);
        conn.send(reply);
     }
+    catch (const SnapshotInUse& e)
+    {
+       DBus::MessageError reply(msg, "error.snapshot_in_use", DBUS_ERROR_FAILED);
+       conn.send(reply);
+    }
     catch (const NoComparison& e)
     {
        DBus::MessageError reply(msg, "error.no_comparisons", DBUS_ERROR_FAILED);

diff --git a/server/Client.h b/server/Client.h
index 3c1054b907ebd6e80f5dfa2002cd9952e48be2de..3ee8b9c8155a3e4f477cf0fb8bc18e153b46e0f3 100644 (file)
--- a/server/Client.h
+++ b/server/Client.h
@@ -69,7 +69,8 @@ public:
     void check_permission(DBus::Connection& conn, DBus::Message& msg,
                          const MetaSnapper& meta_snapper) const;
     void check_lock(DBus::Connection& conn, DBus::Message& msg, const string& config_name) const;
-    void check_in_use(const MetaSnapper& meta_snapper) const;
+    void check_config_in_use(const MetaSnapper& meta_snapper) const;
+    void check_snapshot_in_use(const MetaSnapper& meta_snapper, unsigned int number) const;
 
     void signal_config_created(DBus::Connection& conn, const string& config_name);
     void signal_config_deleted(DBus::Connection& conn, const string& config_name);
@@ -120,12 +121,17 @@ public:
     void remove_lock(const string& config_name);
     bool has_lock(const string& config_name) const;
 
+    void add_mount(const string& config_name, unsigned int number);
+    void remove_mount(const string& config_name, unsigned int number);
+
     const string name;
 
     list<Comparison*> comparisons;
 
     set<string> locks;
 
+    map<pair<string, unsigned int>, unsigned int> mounts;
+
     struct Task
     {
        Task(DBus::Connection& conn, DBus::Message& msg) : conn(conn), msg(msg) {}

diff --git a/server/MetaSnapper.cc b/server/MetaSnapper.cc
index 6c7bfe22341d8c597d84d156ae87394c3ba2267d..91ef14354f255a82188f11b63fb544e848f6009f 100644 (file)
--- a/server/MetaSnapper.cc
+++ b/server/MetaSnapper.cc
@@ -219,7 +219,7 @@ void
 MetaSnapper::unload()
 {
     delete snapper;
-    snapper = 0;
+    snapper = NULL;
 }
 
 

diff --git a/snapper/Comparison.cc b/snapper/Comparison.cc
index 6224e79c664621fd2e9403e1467a015ecab931e6..f1bdc90b6531b7c87da57ef839b004a920b390e4 100644 (file)
--- a/snapper/Comparison.cc
+++ b/snapper/Comparison.cc
@@ -90,12 +90,22 @@ namespace snapper
 
 
     void
-    Comparison::mount()
+    Comparison::mount() const
     {
        if (!getSnapshot1()->isCurrent())
-           getSnapshot1()->mountFilesystemSnapshot();
+           getSnapshot1()->mountFilesystemSnapshot(false);
        if (!getSnapshot2()->isCurrent())
-           getSnapshot2()->mountFilesystemSnapshot();
+           getSnapshot2()->mountFilesystemSnapshot(false);
+    }
+
+
+    void
+    Comparison::umount() const
+    {
+       if (!getSnapshot1()->isCurrent())
+           getSnapshot1()->umountFilesystemSnapshot(false);
+       if (!getSnapshot2()->isCurrent())
+           getSnapshot2()->umountFilesystemSnapshot(false);
     }
 
 
@@ -104,8 +114,6 @@ namespace snapper
     {
        y2mil("num1:" << getSnapshot1()->getNum() << " num2:" << getSnapshot2()->getNum());
 
-       mount();
-
 #if 1
        cmpdirs_cb_t cb = AppendHelper(&file_paths, files);
 #else
@@ -114,9 +122,15 @@ namespace snapper
        };
 #endif
 
-       SDir dir1 = getSnapshot1()->openSnapshotDir();
-       SDir dir2 = getSnapshot2()->openSnapshotDir();
-       cmpDirs(dir1, dir2, cb);
+       mount();
+
+       {
+           SDir dir1 = getSnapshot1()->openSnapshotDir();
+           SDir dir2 = getSnapshot2()->openSnapshotDir();
+           cmpDirs(dir1, dir2, cb);
+       }
+
+       umount();
 
        files.sort();
 

diff --git a/snapper/Comparison.h b/snapper/Comparison.h
index a32b48c420b0d03883e78bcbd972c30bd0e914ac..7576c8367c04e574168c9b61c97a1060d7adb075 100644 (file)
--- a/snapper/Comparison.h
+++ b/snapper/Comparison.h
@@ -46,7 +46,8 @@ namespace snapper
        Files& getFiles() { return files; }
        const Files& getFiles() const { return files; }
 
-       void mount();
+       void mount() const;
+       void umount() const;
 
        UndoStatistic getUndoStatistic() const;
 

diff --git a/snapper/Snapper.cc b/snapper/Snapper.cc
index 32b47e4bd60636831288dcf44eb7146ec02cc43d..0a009f457e961a6ccb13923fa713cdf88abcbb04 100644 (file)
--- a/snapper/Snapper.cc
+++ b/snapper/Snapper.cc
@@ -85,8 +85,18 @@ namespace snapper
        y2mil("Snapper destructor");
 
        for (Snapshots::iterator it = snapshots.begin(); it != snapshots.end(); ++it)
+       {
            it->flushInfo();
 
+           try
+           {
+               it->handleUmountFilesystemSnapshot();
+           }
+           catch (const UmountSnapshotFailedException& e)
+           {
+           }
+       }
+
        delete filesystem;
        delete config;
     }

diff --git a/snapper/Snapshot.cc b/snapper/Snapshot.cc
index a3a2a271a3f36a52144f4cbadd2e14858261f7ba..072df17bd8677fc0e4218c32cd9857ac2dee25b6 100644 (file)
--- a/snapper/Snapshot.cc
+++ b/snapper/Snapshot.cc
@@ -69,6 +69,19 @@ namespace snapper
     }
 
 
+    Snapshot::Snapshot(const Snapper* snapper, SnapshotType type, unsigned int num, time_t date)
+       : snapper(snapper), type(type), num(num), date(date), uid(0), pre_num(0),
+         info_modified(false), mount_checked(false), mount_user_request(false),
+         mount_use_count(0)
+    {
+    }
+
+
+    Snapshot::~Snapshot()
+    {
+    }
+
+
     // Directory containing the actual content of the snapshot.
     // For btrfs e.g. "/" or "/home" for current and "/.snapshots/1/snapshot"
     // or "/home/.snapshots/1/snapshot" otherwise.
@@ -465,22 +478,56 @@ namespace snapper
 
 
     void
-    Snapshot::mountFilesystemSnapshot() const
+    Snapshot::mountFilesystemSnapshot(bool user_request) const
     {
        if (isCurrent())
            throw IllegalSnapshotException();
 
+       if (!mount_checked)
+       {
+           mount_user_request = snapper->getFilesystem()->isSnapshotMounted(num);
+           mount_checked = true;
+       }
+
+       if (user_request)
+           mount_user_request = true;
+       else
+           mount_use_count++;
+
        snapper->getFilesystem()->mountSnapshot(num);
     }
 
 
     void
-    Snapshot::umountFilesystemSnapshot() const
+    Snapshot::umountFilesystemSnapshot(bool user_request) const
     {
        if (isCurrent())
            throw IllegalSnapshotException();
 
-       snapper->getFilesystem()->umountSnapshot(num);
+       if (!mount_checked)
+       {
+           mount_user_request = snapper->getFilesystem()->isSnapshotMounted(num);
+           mount_checked = true;
+       }
+
+       if (user_request)
+           mount_user_request = false;
+       else
+           mount_use_count--;
+
+       if (user_request && mount_use_count == 0)
+           snapper->getFilesystem()->umountSnapshot(num);
+    }
+
+
+    void
+    Snapshot::handleUmountFilesystemSnapshot() const
+    {
+       if (!mount_checked)
+           return;
+
+       if (!mount_user_request && mount_use_count == 0)
+           snapper->getFilesystem()->umountSnapshot(num);
     }
 
 

diff --git a/snapper/Snapshot.h b/snapper/Snapshot.h
index e6cfdebf16b7376e498ee389e1ca0e0675e1dab0..2770af60d6d2c07a1698c077fa62a0ba71e6aeda 100644 (file)
--- a/snapper/Snapshot.h
+++ b/snapper/Snapshot.h
@@ -84,9 +84,8 @@ namespace snapper
 
        friend class Snapshots;
 
-       Snapshot(const Snapper* snapper, SnapshotType type, unsigned int num, time_t date)
-           : snapper(snapper), type(type), num(num), date(date), uid(0), pre_num(0),
-             info_modified(false) {}
+       Snapshot(const Snapper* snapper, SnapshotType type, unsigned int num, time_t date);
+       ~Snapshot();
 
        SnapshotType getType() const { return type; }
 
@@ -118,8 +117,9 @@ namespace snapper
        SDir openSnapshotDir() const;
 #endif
 
-       void mountFilesystemSnapshot() const;
-       void umountFilesystemSnapshot() const;
+       void mountFilesystemSnapshot(bool user_request) const;
+       void umountFilesystemSnapshot(bool user_request) const;
+       void handleUmountFilesystemSnapshot() const;
 
        friend std::ostream& operator<<(std::ostream& s, const Snapshot& snapshot);
 
@@ -145,6 +145,10 @@ namespace snapper
 
        bool info_modified;
 
+       mutable bool mount_checked;
+       mutable bool mount_user_request;
+       mutable unsigned int mount_use_count;
+
        void writeInfo() const;
 
        void createFilesystemSnapshot() const;