- move lvcreate/lvremove cmds in cache code so that ops are atomic and cache retains...

diff --git a/configure.ac b/configure.ac
index ec6feab4b2ad1ce9dd5f207b9f20771b9f617ca8..9cee3029c3ba8a12ff9303e94187a7426ee25f18 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -37,6 +37,7 @@ AC_PATH_PROG([LVREMOVEBIN], [lvremove], [/sbin/lvremove])
 AC_PATH_PROG([LVSBIN], [lvs], [/sbin/lvs])
 AC_PATH_PROG([LVCHANGEBIN], [lvchange], [/sbin/lvchange])
 AC_PATH_PROG([LVMBIN], [lvm], [/sbin/lvm])
+AC_PATH_PROG([LVRENAMEBIN], [lvrename], [/sbin/lvrename])
 
 AC_DEFINE_UNQUOTED([CHSNAPBIN], ["$CHSNAPBIN"], [Path of chsnap program.])
 AC_DEFINE_UNQUOTED([CPBIN], ["$CPBIN"], [Path of cp program.])
@@ -49,6 +50,7 @@ AC_DEFINE_UNQUOTED([LVREMOVEBIN], ["$LVREMOVEBIN"], [Path of lvremove program.])
 AC_DEFINE_UNQUOTED([LVSBIN], ["$LVSBIN"], [Path of lvs program.])
 AC_DEFINE_UNQUOTED([LVCHANGEBIN], ["$LVCHANGEBIN"], [Path of lvchange program.])
 AC_DEFINE_UNQUOTED([LVMBIN], ["$LVMBIN"], [Path of lvm program.])
+AC_DEFINE_UNQUOTED([LVRENAMEBIN], ["$LVRENAMEBIN"], [Path of lvrename program.])
 
 dnl Automake 1.11 enables silent compilation
 dnl Disable it by "configure --disable-silent-rules" or "make V=1"

diff --git a/snapper/Lvm.cc b/snapper/Lvm.cc
index d285ad6fe404b47feaa449a5742a8e9917dc948f..7ba15f37242b160d618f56fb54c252a128000687 100644 (file)
--- a/snapper/Lvm.cc
+++ b/snapper/Lvm.cc
@@ -188,11 +188,6 @@ namespace snapper
     void
     Lvm::createSnapshot(unsigned int num) const
     {
-       SystemCmd cmd(LVCREATEBIN " --permission r --snapshot --name " +
-                     quote(snapshotLvName(num)) + " " + quote(vg_name + "/" + lv_name));
-       if (cmd.retcode() != 0)
-           throw CreateSnapshotFailedException();
-
        SDir info_dir = openInfoDir(num);
        int r1 = info_dir.mkdir("snapshot", 0755);
        if (r1 != 0 && errno != EEXIST)
@@ -203,12 +198,12 @@ namespace snapper
 
        try
        {
-           cache->add(vg_name, snapshotLvName(num));
+           cache->create_snapshot(vg_name, lv_name, snapshotLvName(num));
        }
        catch (const LvmCacheException& e)
        {
-           y2war("lvm cache failed");
            y2deb(cache);
+           throw CreateSnapshotFailedException();
        }
     }
 
@@ -216,18 +211,14 @@ namespace snapper
     void
     Lvm::deleteSnapshot(unsigned int num) const
     {
-       SystemCmd cmd(LVREMOVEBIN " --force " + quote(vg_name + "/" + snapshotLvName(num)));
-       if (cmd.retcode() != 0)
-           throw DeleteSnapshotFailedException();
-
        try
        {
-           cache->remove(vg_name, snapshotLvName(num));
+           cache->delete_snapshot(vg_name, snapshotLvName(num));
        }
        catch (const LvmCacheException& e)
        {
-           y2war("lvm cache failed");
            y2deb(cache);
+           throw DeleteSnapshotFailedException();
        }
 
        SDir info_dir = openInfoDir(num);

diff --git a/snapper/LvmCache.cc b/snapper/LvmCache.cc
index 2f21bdc2533e4b3f88dbbc2481388228478c680d..4ed87b5ceee97d440b8447ea3308130497ed00cc 100644 (file)
--- a/snapper/LvmCache.cc
+++ b/snapper/LvmCache.cc
@@ -94,16 +94,20 @@ namespace snapper
 
        if (!attrs.active)
        {
-           boost::upgrade_to_unique_lock<boost::upgrade_mutex> unique_lock(upg_lock);
-
-           SystemCmd cmd(LVCHANGEBIN + caps->get_ignoreactivationskip() + " -ay " + quote(vg->get_vg_name() + "/" + lv_name));
-           if (cmd.retcode() != 0)
            {
-               y2err("Couldn't activate snapshot " << vg->get_vg_name() << "/" << lv_name);
-               throw LvmActivationException();
+               boost::upgrade_to_unique_lock<boost::upgrade_mutex> unique_lock(upg_lock);
+
+               SystemCmd cmd(LVCHANGEBIN + caps->get_ignoreactivationskip() + " -ay " + quote(vg->get_vg_name() + "/" + lv_name));
+               if (cmd.retcode() != 0)
+               {
+                   y2err("lvm cache: " << vg->get_vg_name() << "/" << lv_name << " activation failed!");
+                   throw LvmCacheException();
+               }
+
+               attrs.active = true;
            }
 
-           attrs.active = true;
+           y2deb("lvm cache: " << vg->get_vg_name() << "/" << lv_name << " activated");
        }
     }
 
@@ -125,16 +129,20 @@ namespace snapper
 
        if (attrs.active)
        {
-           boost::upgrade_to_unique_lock<boost::upgrade_mutex> unique_lock(upg_lock);
-
-           SystemCmd cmd(LVCHANGEBIN " -an " + quote(vg->get_vg_name() + "/" + lv_name));
-           if (cmd.retcode() != 0)
            {
-               y2err("Couldn't activate snapshot " << vg->get_vg_name() << "/" << lv_name);
-               throw LvmDeactivatationException();
+               boost::upgrade_to_unique_lock<boost::upgrade_mutex> unique_lock(upg_lock);
+
+               SystemCmd cmd(LVCHANGEBIN " -an " + quote(vg->get_vg_name() + "/" + lv_name));
+               if (cmd.retcode() != 0)
+               {
+                   y2err("lvm cache: " << vg->get_vg_name() << "/" << lv_name << " deactivation failed!");
+                   throw LvmCacheException();
+               }
+
+               attrs.active = false;
            }
 
-           attrs.active = false;
+           y2deb("lvm cache: " << vg->get_vg_name() << "/" << lv_name << " deactivated");
        }
     }
 
@@ -148,7 +156,7 @@ namespace snapper
 
        if (cmd.retcode() != 0 || cmd.numLines() < 1)
        {
-           y2err("lvm cache failed to get info about " << vg->get_vg_name() << "/" << lv_name);
+           y2err("lvm cache: failed to get info about " << vg->get_vg_name() << "/" << lv_name);
            throw LvmCacheException();
        }
 
@@ -216,7 +224,7 @@ namespace snapper
        iterator it = lv_info_map.find(lv_name);
        if (it == lv_info_map.end())
        {
-           y2err(vg_name << "/" << lv_name << " is not in cache!");
+           y2err("lvm cache: " << vg_name << "/" << lv_name << " is not in cache!");
            throw LvmCacheException();
        }
 
@@ -232,7 +240,7 @@ namespace snapper
        iterator it = lv_info_map.find(lv_name);
        if (it == lv_info_map.end())
        {
-           y2err(vg_name << "/" << lv_name << " is not in cache!");
+           y2err("lvm cache: " << vg_name << "/" << lv_name << " is not in cache!");
            throw LvmCacheException();
        }
 
@@ -272,12 +280,25 @@ namespace snapper
 
 
     void
-    VolumeGroup::add(const string& lv_name)
+    VolumeGroup::create_snapshot(const string& lv_origin_name, const string& lv_snapshot_name)
     {
-       boost::unique_lock<boost::upgrade_mutex> lock(vg_mutex);
+       boost::upgrade_lock<boost::upgrade_mutex> upg_lock(vg_mutex);
+
+       if (lv_info_map.find(lv_snapshot_name) != lv_info_map.end())
+       {
+           y2err("lvm cache: " << vg_name << "/" << lv_snapshot_name << " already in cache!");
+           throw LvmCacheException();
+       }
 
-       if (!lv_info_map.insert(make_pair(lv_name, new LogicalVolume(this, lv_name))).second)
+       boost::upgrade_to_unique_lock<boost::upgrade_mutex> unique_lock(upg_lock);
+
+       SystemCmd cmd(LVCREATEBIN " --permission r --snapshot --name " +
+                     quote(lv_snapshot_name) + " " + quote(vg_name + "/" + lv_origin_name));
+
+       if (cmd.retcode() != 0)
            throw LvmCacheException();
+
+       lv_info_map.insert(make_pair(lv_snapshot_name, new LogicalVolume(this, lv_snapshot_name)));
     }
 
 
@@ -300,7 +321,7 @@ namespace snapper
            SystemCmd cmd(LVSBIN " --noheadings -o lv_attr,segtype,pool_lv " + quote(vg_name + "/" + lv_name));
            if (cmd.retcode() != 0 || cmd.numLines() < 1)
            {
-               y2err("lvm cache failed to get info about " << vg_name << "/" << lv_name);
+               y2err("lvm cache: failed to get info about " << vg_name << "/" << lv_name);
                throw LvmCacheException();
            }
 
@@ -319,13 +340,23 @@ namespace snapper
 
 
     void
-    VolumeGroup::remove(const string& lv_name)
+    VolumeGroup::remove_lv(const string& lv_name)
     {
-       boost::unique_lock<boost::upgrade_mutex> unique_lock(vg_mutex);
+       boost::upgrade_lock<boost::upgrade_mutex> upg_lock(vg_mutex);
 
        const_iterator cit = lv_info_map.find(lv_name);
 
        if (cit == lv_info_map.end())
+       {
+           y2err("lvm cache: " << vg_name << "/" << lv_name << " is not in cache!");
+           throw LvmCacheException();
+       }
+
+       // wait for all invidual lv cache operations under shared vg lock to finish
+       boost::upgrade_to_unique_lock<boost::upgrade_mutex> unique_lock(upg_lock);
+
+       SystemCmd cmd(LVREMOVEBIN " --force " + quote(vg_name + "/" + lv_name));
+       if (cmd.retcode() != 0)
            throw LvmCacheException();
 
        delete cit->second;
@@ -341,10 +372,26 @@ namespace snapper
        const_iterator cit = lv_info_map.find(old_name);
 
        if (cit == lv_info_map.end() || lv_info_map.find(new_name) != lv_info_map.end())
+       {
+           y2err("lvm cache: " << vg_name << "/" << old_name <<
+                 " is missing or " << vg_name << "/" << new_name <<
+                 " already in cache!");
            throw LvmCacheException();
+       }
 
+       // wait for all invidual lv cache operations under shared vg lock to finish
        boost::upgrade_to_unique_lock<boost::upgrade_mutex> unique_lock(upg_lock);
 
+       SystemCmd cmd(LVRENAMEBIN " " + quote(vg_name) + " " + quote(old_name) +
+                     " " + quote(new_name));
+
+       if (cmd.retcode() != 0)
+       {
+           y2err("lvm cache: " << vg_name <<  "/" << old_name << " -> " <<
+                 vg_name << "/" << new_name << " rename command failed!");
+           throw LvmCacheException();
+       }
+
        lv_info_map.insert(make_pair(new_name, new LogicalVolume(this, new_name, cit->second->attrs)));
 
        delete cit->second;
@@ -385,7 +432,7 @@ namespace snapper
 
        if (cit == vgroups.end())
        {
-           y2err("VG " << vg_name << " is not in cache!");
+           y2err("lvm cache: VG " << vg_name << " is not in cache!");
            throw LvmCacheException();
        }
 
@@ -400,7 +447,7 @@ namespace snapper
 
        if (cit == vgroups.end())
        {
-           y2err("VG " << vg_name << " is not in cache!");
+           y2err("lvm cache: VG " << vg_name << " is not in cache!");
            throw LvmCacheException();
        }
 
@@ -453,7 +500,7 @@ namespace snapper
 
 
     void
-    LvmCache::add(const string& vg_name, const string& lv_name) const
+    LvmCache::create_snapshot(const string& vg_name, const string& lv_origin_name, const string& lv_snapshot_name)
     {
        const_iterator cit = vgroups.find(vg_name);
        if (cit == vgroups.end())
@@ -462,17 +509,9 @@ namespace snapper
            throw LvmCacheException();
        }
 
-       try
-       {
-           cit->second->add(lv_name);
-       }
-       catch(const LvmCacheException& e)
-       {
-           y2err(vg_name << "/" << lv_name << " already in cache!");
-           throw;
-       }
+       cit->second->create_snapshot(lv_origin_name, lv_snapshot_name);
 
-       y2deb("lvm cache: added new lv: " << lv_name << " in vg: " << vg_name);
+       y2deb("lvm cache: created new snapshot: " << lv_snapshot_name << " in vg: " << vg_name);
     }
 
 
@@ -482,7 +521,7 @@ namespace snapper
        SystemCmd cmd(LVSBIN " --noheadings -o lv_name,lv_attr,segtype,pool_lv " + quote(vg_name));
        if (cmd.retcode() != 0)
        {
-           y2err("lvm cache failed to get info about VG: " << vg_name);
+           y2err("lvm cache: failed to get info about VG " << vg_name);
            throw LvmCacheException();
        }
 
@@ -507,14 +546,19 @@ namespace snapper
 
 
     void
-    LvmCache::remove(const string& vg_name, const string& lv_name) const
+    LvmCache::delete_snapshot(const string& vg_name, const string& lv_name) const
     {
        const_iterator cit = vgroups.find(vg_name);
 
-       if (cit != vgroups.end())
-           cit->second->remove(lv_name);
+       if (cit == vgroups.end())
+       {
+           y2err("lvm cache: VG " << vg_name << " not in cache!");
+           throw LvmCacheException();
+       }
+
+       cit->second->remove_lv(lv_name);
 
-       y2deb("lvm cache: removed lv " << lv_name << " from vg " << vg_name);
+       y2deb("lvm cache: removed " << vg_name << "/" << lv_name);
     }
 
 
@@ -525,21 +569,13 @@ namespace snapper
 
        if (cit == vgroups.end())
        {
-           y2err("VG " << vg_name << " is not in cache!");
+           y2err("lvm cache: VG " << vg_name << " is not in cache!");
            throw LvmCacheException();
        }
 
-       try
-       {
-           cit->second->rename(old_name, new_name);
-       }
-       catch (const LvmCacheException& e)
-       {
-           y2err("lvm cache failed to rename " << vg_name << "/" << old_name << " ->  " << vg_name << "/" << new_name);
-           throw;
-       }
+       cit->second->rename(old_name, new_name);
 
-       y2deb("lvm cache: in vg " << vg_name << " " << old_name << " was renamed to " << new_name);
+       y2deb("lvm cache: " << vg_name << "/" << old_name << " renamed to " << vg_name << "/" << new_name);
     }
 
 

diff --git a/snapper/LvmCache.h b/snapper/LvmCache.h
index 65dafd0684839dd0808efe06989d5f800f9d8648..2ee992090c483fdb30b33300e94b4ecb282e816d 100644 (file)
--- a/snapper/LvmCache.h
+++ b/snapper/LvmCache.h
@@ -119,10 +119,10 @@ namespace snapper
 
        bool read_only(const string& lv_name) const; // shared lock
 
-       void add(const string& lv_name); // excl lock
+       void create_snapshot(const string& lv_origin_name, const string& lv_snapshot_name); // upg lock -> excl
        void add_or_update(const string& lv_name); // upg lock -> excl
 
-       void remove(const string& lv_name); // excl lock
+       void remove_lv(const string& lv_name); // upg lock -> excl
        void rename(const string& old_name, const string& new_name); // upg lock -> excl
 
        friend std::ostream& operator<<(std::ostream& out, const VolumeGroup* vg);
@@ -156,13 +156,13 @@ namespace snapper
        bool contains_thin(const string& vg_name, const string& lv_name) const;
        bool read_only(const string& vg_name, const string& lv_name) const;
 
-       // add snapshot owned by snapper
-       void add(const string& vg_name, const string& lv_name) const;
+       // create snapper owned snapshot
+       void create_snapshot(const string& vg_name, const string&lv_origin_name, const string& lv_snapshot_name);
        // used to actualise info about origin volume
        void add_or_update(const string& vg_name, const string& lv_name);
 
        // remove snapshot owned by snapper
-       void remove(const string& vg_name, const string& lv_name) const;
+       void delete_snapshot(const string& vg_name, const string& lv_name) const;
 
        // rename snapshots (used during import)
        void rename(const string& vg_name, const string& old_name, const string& new_name) const;