B #2052: Add mixed mode for ALLOW_ORPHANS to accomodate Ceph snapshots

dependencies
Co-authored-by: Christian González <cgonzalez@opennebula.systems>

include/Image.h

@@ -549,7 +549,7 @@ class Image : public PoolObjectSQL
 
     void revert_snapshot(int snap_id)
     {
-        snapshots.active_snapshot(snap_id);
+        snapshots.active_snapshot(snap_id, true);
     };
 
     void set_target_snapshot(int snap_id)

include/Snapshots.h

@@ -45,7 +45,60 @@ class VectorAttribute;
 class Snapshots
 {
 public:
-    Snapshots(int _disk_id, bool orphans);
+
+    /**
+     *  ALLOW_ORPHANS: Define how child snapshots are handled.
+     *    - ALLOW: Children can be orphan (no parent snapshot)
+     *      |- snap_1
+     *      |- snap_2
+     *      |- snap_3
+     *
+     *    - DENY: New snapshots are set active and child of the previous one
+     *      |- snap_1
+     *         |- snap_2
+     *            |- snap_3
+     *
+     *    - MIXED: Snapshots are children of last snapshot reverted to
+     *      |- snap_1 (<--- revert)
+     *         |- snap_2
+     *         |- snap_3
+     */
+    enum AllowOrphansMode
+    {
+        ALLOW = 0,
+        DENY  = 1,
+        MIXED = 2
+    };
+
+    static string allow_orphans_mode_to_str(AllowOrphansMode aom)
+    {
+        switch (aom)
+        {
+            case ALLOW: return "YES";
+            case DENY:  return "NO";
+            case MIXED: return "MIXED";
+        }
+
+        return "NO";
+    };
+
+    static AllowOrphansMode str_to_allow_orphans_mode(const string& aom)
+    {
+        if (aom == "YES")
+        {
+            return ALLOW;
+        }
+        else if (aom == "MIXED")
+        {
+            return MIXED;
+        }
+        else
+        {
+            return DENY;
+        }
+    };
+
+    Snapshots(int _disk_id, AllowOrphansMode orphans);
 
     Snapshots(const Snapshots& s);
 
@@ -98,8 +151,12 @@ class Snapshots
     /**
      *  Set the given snapshot as active. Updates the values of the current
      *  snapshot
+     *
+     *    @param id id of the snapshot
+     *    @param revert true if the cause of changing the active snapshot
+     *                  is because a revert
      */
-    int active_snapshot(int id);
+    int active_snapshot(int id, bool revert);
 
     /**
      * Rename the given snapshot by the given name
@@ -224,6 +281,19 @@ class Snapshots
      */
     void init();
 
+    /**
+     *  Updates children list for the current base snapshot in the tree
+     *    @param snapshot new child to be added
+     */
+    void add_child_mixed(VectorAttribute *snapshot);
+
+    /**
+     *  Updates children list of the active snapshot
+     *    @param snapshot new child to be added
+     *    @return -1 in case of error (current active does not exist)
+     */
+    int add_child_deny(VectorAttribute *snapshot);
+
     /**
      *  Text representation of the snapshot pool. To be stored as part of the
      *  VM or Image Templates
@@ -248,12 +318,17 @@ class Snapshots
     /**
      * Allow to remove parent snapshots and active one
      */
-    bool orphans;
+    AllowOrphansMode orphans;
 
     /**
      * Snapshot pointer map
      */
     map<int, VectorAttribute *> snapshot_pool;
+
+    /**
+     * Current snaphsot base for mixed mode
+     */
+    int current_base;
 };
 
 #endif /*SNAPSHOTS_H_*/

include/VirtualMachine.h

@@ -1491,11 +1491,13 @@ class VirtualMachine : public PoolObjectSQL
      *    @param disk_id of the disk
      *    @param snap_id of the snapshot
      *    @param error if any
+     *    @param revert true if the cause of changing the active snapshot
+     *                  is because a revert
      *    @return -1 if error
      */
-    int revert_disk_snapshot(int disk_id, int snap_id)
+    int revert_disk_snapshot(int disk_id, int snap_id, bool revert)
     {
-        return disks.revert_snapshot(disk_id, snap_id);
+        return disks.revert_snapshot(disk_id, snap_id, revert);
     }
 
     /**

include/VirtualMachineDisk.h

@@ -22,6 +22,7 @@
 
 #include "VirtualMachineAttribute.h"
 #include "Snapshots.h"
+#include "NebulaUtil.h"
 
 class AuthRequest;
 
@@ -54,16 +55,16 @@ class VirtualMachineDisk : public VirtualMachineAttribute
         return is_flag("PERSISTENT");
     }
 
-    bool allow_orphans() const
+    Snapshots::AllowOrphansMode allow_orphans() const
     {
-        bool orphans;
+        string orphans;
 
         if (vector_value("ALLOW_ORPHANS", orphans) == -1)
         {
-            orphans = false;
+            orphans = Snapshots::DENY;
         }
 
-        return orphans;
+        return Snapshots::str_to_allow_orphans_mode(one_util::toupper(orphans));
     }
 
     void set_attach()
@@ -273,9 +274,11 @@ class VirtualMachineDisk : public VirtualMachineAttribute
     /**
      *  Sets the snap_id as active, the VM will boot from it next time
      *    @param snap_id of the snapshot
+     *    @param revert true if the cause of changing the active snapshot
+     *                  is because a revert
      *    @return -1 if error
      */
-    int revert_snapshot(int snap_id);
+    int revert_snapshot(int snap_id, bool revert);
 
     /**
      *  Deletes the snap_id from the list
@@ -730,9 +733,11 @@ class VirtualMachineDisks : public VirtualMachineAttributeSet
      *  Sets the snap_id as active, the VM will boot from it next time
      *    @param disk_id of the disk
      *    @param snap_id of the snapshot
+     *    @param revert true if the cause of changing the active snapshot
+     *                  is because a revert
      *    @return -1 if error
      */
-    int revert_snapshot(int disk_id, int snap_id);
+    int revert_snapshot(int disk_id, int snap_id, bool revert);
 
     /**
      *  Deletes the snap_id from the list

share/etc/oned.conf

@@ -1239,7 +1239,20 @@ INHERIT_VNET_ATTR       = "VCENTER_INSTANCE_ID"
 #            among the different hosts or not. Valid values: "yes" or "no"
 #   ds_migrate : The driver allows migrations across datastores. Valid values:
 #               "yes" or "no". Note: THIS ONLY APPLIES TO SYSTEM DS.
-#   allow_orphans: Snapshots can live without parents
+#   allow_orphans: Snapshots can live without parents. Suported values:
+#        YES: Children can be orphan (no parent snapshot)
+#         |- snap_1
+#         |- snap_2
+#         |- snap_3
+#        NO: New snapshots are set active and child of the previous one
+#         |- snap_1
+#            |- snap_2
+#               |- snap_3
+#        MIXED: Snapshots are children of last snapshot reverted to
+#         |- snap_1 (<--- revert)
+#            |- snap_3
+#            |- snap_4
+#         |- snap_2
 #*******************************************************************************
 
 TM_MAD_CONF = [
@@ -1274,7 +1287,7 @@ TM_MAD_CONF = [
 
 TM_MAD_CONF = [
     NAME = "ceph", LN_TARGET = "NONE", CLONE_TARGET = "SELF", SHARED = "YES",
-    DS_MIGRATE = "NO", DRIVER = "raw", ALLOW_ORPHANS="yes",
+    DS_MIGRATE = "NO", DRIVER = "raw", ALLOW_ORPHANS="mixed",
     TM_MAD_SYSTEM = "ssh", LN_TARGET_SSH = "SYSTEM", CLONE_TARGET_SSH = "SYSTEM",
     DISK_TYPE_SSH = "FILE", TM_MAD_SYSTEM = "shared", LN_TARGET_SHARED = "NONE",
     CLONE_TARGET_SHARED = "SELF", DISK_TYPE_SHARED = "RBD"

src/datastore/Datastore.cc

@@ -326,6 +326,8 @@ int Datastore::set_tm_mad(string &tm_mad, string &error_str)
     ostringstream oss;
     std::stringstream ss;
 
+    string orph;
+
     if ( Nebula::instance().get_tm_conf_attribute(tm_mad, vatt) != 0 )
     {
         goto error_conf;
@@ -443,11 +445,9 @@ int Datastore::set_tm_mad(string &tm_mad, string &error_str)
         remove_template_attribute("SHARED");
     }
 
-    bool orph;
-
     if ( vatt->vector_value("ALLOW_ORPHANS", orph) == -1 )
     {
-        orph = false;
+        orph = "NO";
     }
 
     replace_template_attribute("ALLOW_ORPHANS", orph);

src/image/Image.cc

@@ -60,7 +60,7 @@ Image::Image(int             _uid,
         vm_collection("VMS"),
         img_clone_collection("CLONES"),
         app_clone_collection("APP_CLONES"),
-        snapshots(-1, false),
+        snapshots(-1, Snapshots::DENY),
         target_snapshot(-1)
     {
         if (_image_template != 0)

src/image/ImageManagerActions.cc

@@ -1064,7 +1064,7 @@ void ImageManager::set_image_snapshots(int iid, const Snapshots& s)
 
 void ImageManager::clear_image_snapshots(int iid)
 {
-    Snapshots _snaps(-1, false);
+    Snapshots _snaps(-1, Snapshots::DENY);
 
     set_image_snapshots(iid, _snaps);
 }

src/lcm/LifeCycleStates.cc

@@ -1893,7 +1893,7 @@ void LifeCycleManager::disk_snapshot_success(int vid)
     bool img_owner, vm_owner;
 
     const VirtualMachineDisk * disk;
-    Snapshots           snaps(-1, false);
+    Snapshots           snaps(-1, Snapshots::DENY);
     const Snapshots*    tmp_snaps;
     string              error_str;
 
@@ -1924,10 +1924,14 @@ void LifeCycleManager::disk_snapshot_success(int vid)
             vm->set_state(VirtualMachine::RUNNING);
         case VirtualMachine::DISK_SNAPSHOT_POWEROFF:
         case VirtualMachine::DISK_SNAPSHOT_SUSPENDED:
+            vm->log("LCM", Log::INFO, "VM disk snapshot operation completed.");
+            vm->revert_disk_snapshot(disk_id, snap_id, false);
+            break;
+
         case VirtualMachine::DISK_SNAPSHOT_REVERT_POWEROFF:
         case VirtualMachine::DISK_SNAPSHOT_REVERT_SUSPENDED:
             vm->log("LCM", Log::INFO, "VM disk snapshot operation completed.");
-            vm->revert_disk_snapshot(disk_id, snap_id);
+            vm->revert_disk_snapshot(disk_id, snap_id, true);
             break;
 
         case VirtualMachine::DISK_SNAPSHOT_DELETE:
@@ -2036,7 +2040,7 @@ void LifeCycleManager::disk_snapshot_failure(int vid)
     Template *vm_quotas = 0;
 
     const VirtualMachineDisk* disk;
-    Snapshots           snaps(-1, false);
+    Snapshots           snaps(-1, Snapshots::DENY);
     const Snapshots*    tmp_snaps;
     string              error_str;
 

src/nebula/NebulaTemplate.cc

@@ -157,8 +157,8 @@ void OpenNebulaTemplate::set_multiple_conf_default()
     set_conf_ds("ssh",            "",                     "NO");
     set_conf_ds("vmfs",           "BRIDGE_LIST",          "NO");
     set_conf_ds("vcenter",
-		"VCENTER_INSTANCE_ID, VCENTER_DS_REF, VCENTER_DC_REF, VCENTER_HOST, VCENTER_USER, VCENTER_PASSWORD",
-		"NO");
+        "VCENTER_INSTANCE_ID, VCENTER_DS_REF, VCENTER_DC_REF, VCENTER_HOST, VCENTER_USER, VCENTER_PASSWORD",
+        "NO");
     set_conf_ds("ceph",
                 "DISK_TYPE,BRIDGE_LIST,CEPH_HOST,CEPH_USER,CEPH_SECRET",
                 "NO");