]> git.ipfire.org Git - thirdparty/snapper.git/commitdiff
- implement all-time unique snapshot numbers 1106/head
authorArvin Schnell <aschnell@suse.de>
Tue, 10 Mar 2026 12:00:44 +0000 (13:00 +0100)
committerArvin Schnell <aschnell@suse.de>
Tue, 10 Mar 2026 13:36:52 +0000 (14:36 +0100)
doc/unique-numbers.txt [new file with mode: 0644]
package/snapper.changes
snapper/Snapshot.cc

diff --git a/doc/unique-numbers.txt b/doc/unique-numbers.txt
new file mode 100644 (file)
index 0000000..0cd12ea
--- /dev/null
@@ -0,0 +1,10 @@
+With snapper 0.13.1 the snapshot numbers should be all-time unique in
+that even after deleting a snapshot the number is not reused for new
+snapshots.
+
+This is basically done by 1. keeping the directory if the snapshot
+with the highest number is removed and 2. also looking at empty
+directories when finding the number for the next snapshot.
+
+As a consequence there can temporarily be empty directories in the
+infos directories.
index af0782dba60853ffbf869e29439511b3202db001..23cb67ea54f60ee335b4fe3e4450067af06fa848 100644 (file)
@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Tue Mar 10 10:55:15 CET 2026 - aschnell@suse.com
+
+- implement all-time unique snapshot numbers
+  (gh#openSUSE/snapper#131)
+
 -------------------------------------------------------------------
 Wed Dec 17 12:57:19 UTC 2025 - Cheng-Ling Lai <jamesljlster@gmail.com>
 
index c71b4f0a224af78813cebf70e524ec422efb3b01..727777f3aac5206c398df7cfc6544ca2c5778906 100644 (file)
@@ -1,6 +1,6 @@
 /*
  * Copyright (c) [2011-2015] Novell, Inc.
- * Copyright (c) [2016-2025] SUSE LLC
+ * Copyright (c) [2016-2026] SUSE LLC
  *
  * All Rights Reserved.
  *
@@ -236,25 +236,36 @@ namespace snapper
     Snapshots::~Snapshots() = default;
 
 
+    static bool
+    number_entries(unsigned char type, const char* name)
+    {
+       // Snapshot '0' is internal and not saved on disks. So simply ignore directories
+       // starting with '0'.
+
+       static const regex rx_num("[1-9][0-9]*", regex::extended);
+
+       return regex_match(name, rx_num);
+    }
+
+
     void
     Snapshots::read()
     {
-       static const regex rx("[0-9]+", regex::extended);
-
        SDir infos_dir = snapper->openInfosDir();
 
-       vector<string> infos = infos_dir.entries();
-       for (vector<string>::const_iterator it1 = infos.begin(); it1 != infos.end(); ++it1)
+       for (const string& info : infos_dir.entries(number_entries))
        {
-           if (!regex_match(*it1, rx))
-               continue;
-
            try
            {
-               SDir info_dir(infos_dir, *it1);
+               SDir info_dir(infos_dir, info);
                int fd = info_dir.open("info.xml", O_NOFOLLOW | O_CLOEXEC);
                if (fd < 0)
-                   SN_THROW(IOErrorException("open info.xml failed"));
+               {
+                   // Since we want all-time unique snapshots number we might have empty
+                   // directories. So do not raise an exception here.
+
+                   continue;
+               }
 
                XmlFile file(fd, "");
 
@@ -265,30 +276,30 @@ namespace snapper
                SnapshotType type;
                if (!getChildValue(node, "type", tmp) || !toValue(tmp, type, true))
                {
-                   y2err("type missing or invalid. not adding snapshot " << *it1);
+                   y2err("type missing or invalid. not adding snapshot " << info);
                    continue;
                }
 
                unsigned int num;
                if (!getChildValue(node, "num", num) || num == 0)
                {
-                   y2err("num missing or invalid. not adding snapshot " << *it1);
+                   y2err("num missing or invalid. not adding snapshot " << info);
                    continue;
                }
 
                time_t date;
                if (!getChildValue(node, "date", tmp) || (date = scan_datetime(tmp, true)) == (time_t)(-1))
                {
-                   y2err("date missing or invalid. not adding snapshot " << *it1);
+                   y2err("date missing or invalid. not adding snapshot " << info);
                    continue;
                }
 
                Snapshot snapshot(snapper, type, num, date);
 
-               *it1 >> num;
+               info >> num;
                if (num != snapshot.num)
                {
-                   y2err("num mismatch. not adding snapshot " << *it1);
+                   y2err("num mismatch. not adding snapshot " << info);
                    continue;
                }
 
@@ -314,7 +325,7 @@ namespace snapper
 
                if (!filesystem->checkSnapshot(snapshot.num))
                {
-                   y2err("snapshot check failed. not adding snapshot " << *it1);
+                   y2err("snapshot check failed. not adding snapshot " << info);
                    continue;
                }
 
@@ -326,7 +337,7 @@ namespace snapper
            {
                SN_CAUGHT(e);
 
-               y2err("loading " << *it1 << " failed");
+               y2err("loading " << info << " failed");
            }
        }
 
@@ -487,10 +498,26 @@ namespace snapper
     unsigned int
     Snapshots::nextNumber() const
     {
-       unsigned int num = entries.empty() ? 0 : entries.rbegin()->num;
+       // We want all-time unique snapshot numbers.
 
        SDir infos_dir = snapper->openInfosDir();
 
+       // Numbers of directories (and files) found in infos-dir.
+
+       vector<unsigned int> nums;
+       for (const string& tmp : infos_dir.entries(number_entries))
+           nums.push_back(stoi(tmp));
+
+       // Set num to next available free number. All entries should also be included in
+       // nums but better be save.
+
+       unsigned int num = entries.empty() ? 0 : entries.rbegin()->num;
+       for (unsigned int tmp : nums)
+           num = std::max(num, tmp);
+
+       // Now try to create the next directory. EEXIST should not happen since we just
+       // looked at all directories but better be save.
+
        while (true)
        {
            ++num;
@@ -510,6 +537,15 @@ namespace snapper
 
        infos_dir.chmod(decString(num), 0755, 0);
 
+       // Now try to remove directories not belonging to any snapshot. The new num is not
+       // included in nums.
+
+       for (unsigned int tmp : nums)
+       {
+           if (find(tmp) == end())
+               infos_dir.rmdir(decString(tmp));
+       }
+
        return num;
     }
 
@@ -837,9 +873,16 @@ namespace snapper
        if (info_dir.unlink("info.xml") < 0)
            y2err("unlink 'info.xml' failed errno: " << errno << " (" << stringerror(errno) << ")");
 
-       SDir infos_dir = snapper->openInfosDir();
-       if (infos_dir.rmdir(decString(snapshot->getNum())) < 0)
-           y2err("rmdir '" << snapshot->getNum() << "' failed errno: " << errno << " (" << stringerror(errno) << ")");
+       // We want all-time unique snapshot numbers. So keep directory of snapshot with
+       // highest number.
+
+       if (snapshot->num != entries.rbegin()->num)
+       {
+           SDir infos_dir = snapper->openInfosDir();
+           if (infos_dir.rmdir(decString(snapshot->getNum())) < 0)
+               y2err("rmdir '" << snapshot->getNum() << "' failed errno: " << errno << " (" <<
+                     stringerror(errno) << ")");
+       }
 
        Plugins::delete_snapshot(Plugins::Stage::POST_ACTION, snapper->subvolumeDir(), snapper->getFilesystem(),
                                 *snapshot, report);