F #727: Improve shareable disks (#970)

* Shareable only on supported hypervisors
* Fix disk-attach
* Restrict shareable disks to SHARED datastores
* On image clone: copy SHAREABLE depends on DEFAULT_IMAGE_PERSISTENT flag

include/VirtualMachine.h

@@ -1008,12 +1008,7 @@ public:
      *    @param error_str Returns the error reason, if any
      *    @return 0 on success
      */
-    int automatic_requirements(std::set<int>& cluster_ids, std::string& error_str)
-    {
-        std::set<int> datastore_ids;
-
-        return automatic_requirements(cluster_ids, datastore_ids, error_str);
-    }
+    int automatic_requirements(std::set<int>& cluster_ids, std::string& error_str);
 
     /**
      *  Resize the VM capacity
@@ -1674,6 +1669,12 @@ public:
      */
     int check_tm_mad_disks(const std::string& tm_mad, std::string& error);
 
+    /**
+     *  Check if VM has shareable disks and vmm_mad supports them
+     *  @param vmm_mad is the tm_mad for system datastore chosen
+     */
+    int check_shareable_disks(const std::string& vmm_mad, std::string& error);
+
 private:
 
     // -------------------------------------------------------------------------
@@ -2094,17 +2095,6 @@ private:
      */
     int get_vmgroup(std::string& error);
 
-    /**
-     * Adds automatic placement requirements: Datastore and Cluster
-     *    @param cluster_ids set of viable clusters for this VM
-     *    @param ds_ids set of viable datastores for this VM
-     *    @param error_str Returns the error reason, if any
-     *    @return 0 on success
-     */
-    int automatic_requirements(std::set<int>& cluster_ids,
-                               std::set<int>& ds_ids,
-                               std::string& error_str);
-
     // ------------------------------------------------------------------------
     // Public cloud templates related functions
     // ------------------------------------------------------------------------

include/VirtualMachineManagerDriver.h

@@ -107,6 +107,14 @@ public:
         return live_resize;
     }
 
+    /**
+     *  @return true if shareable disks are supported
+     */
+    bool support_shareable() const
+    {
+        return support_shareable_;
+    }
+
 protected:
     /**
      *  Gets a configuration attr from driver configuration file (single
@@ -276,6 +284,11 @@ private:
     */
     bool live_resize;
 
+    /**
+    * Set to true if hypervisor supports shareable disks
+    */
+    bool support_shareable_;
+
     /**
      *  Sends a deploy request to the MAD: "DEPLOY ID XML_DRV_MSG"
      *    @param oid the virtual machine id.

share/etc/oned.conf

@@ -448,6 +448,7 @@ VM_MAD = [
     TYPE           = "kvm",
     KEEP_SNAPSHOTS = "yes",
     LIVE_RESIZE    = "yes",
+    SUPPORT_SHAREABLE    = "yes",
     IMPORTED_VMS_ACTIONS = "terminate, terminate-hard, hold, release, suspend,
         resume, delete, reboot, reboot-hard, resched, unresched, disk-attach,
         disk-detach, nic-attach, nic-detach, snapshot-create, snapshot-delete"
@@ -463,6 +464,7 @@ VM_MAD = [
     TYPE           = "qemu",
     KEEP_SNAPSHOTS = "yes",
     LIVE_RESIZE    = "yes",
+    SUPPORT_SHAREABLE    = "yes",
     IMPORTED_VMS_ACTIONS = "terminate, terminate-hard, hold, release, suspend,
         resume, delete, reboot, reboot-hard, resched, unresched, disk-attach,
         disk-detach, nic-attach, nic-detach, snapshot-create, snapshot-delete"

src/image/Image.cc

@@ -1088,6 +1088,14 @@ bool Image::test_set_persistent(Template * image_template, int uid, int gid,
 
     image_template->replace("PERSISTENT", persistent);
 
+    string persistent_type;
+    if (!persistent &&
+        image_template->get("PERSISTENT_TYPE", persistent_type) &&
+        one_util::toupper(persistent_type) == "SHAREABLE" )
+    {
+        image_template->erase("PERSISTENT_TYPE");
+    }
+
     return persistent;
 }
 

src/mad/sh/scripts_common.sh

@@ -799,6 +799,7 @@ function get_source_xml {
 # * DRIVER
 # * TYPE
 # * READONLY
+# * SHAREABLE
 # * CACHE
 # * DISCARD
 # * IMG_SRC
@@ -865,6 +866,7 @@ function get_disk_information {
                         $DISK_XPATH/DRIVER \
                         $DISK_XPATH/TYPE \
                         $DISK_XPATH/READONLY \
+                        $DISK_XPATH/SHAREABLE \
                         $DISK_XPATH/CACHE \
                         $DISK_XPATH/DISCARD \
                         $DISK_XPATH/SOURCE \
@@ -910,6 +912,7 @@ function get_disk_information {
     DRIVER="${XPATH_ELEMENTS[j++]:-$DEFAULT_TYPE}"
     TYPE="${XPATH_ELEMENTS[j++]}"
     READONLY="${XPATH_ELEMENTS[j++]}"
+    SHAREABLE="${XPATH_ELEMENTS[j++]}"
     CACHE="${XPATH_ELEMENTS[j++]}"
     DISCARD="${XPATH_ELEMENTS[j++]}"
     IMG_SRC="${XPATH_ELEMENTS[j++]}"
@@ -953,6 +956,7 @@ function get_disk_information {
 
     TYPE=$(echo "$TYPE"|tr A-Z a-z)
     READONLY=$(echo "$READONLY"|tr A-Z a-z)
+    SHAREABLE=$(echo "$SHAREABLE"|tr A-Z a-z)
 
     NAME="$SOURCE"
 

src/rm/RequestManagerVirtualMachine.cc

@@ -998,6 +998,12 @@ void VirtualMachineDeploy::request_execute(xmlrpc_c::paramList const& paramList,
         return;
     }
 
+    if ( vm->check_shareable_disks(vmm_mad, att.resp_msg) != 0)
+    {
+        failure_response(ACTION, att);
+        return;
+    }
+
     static_cast<VirtualMachinePool *>(pool)->update(vm.get());
 
     // ------------------------------------------------------------------------

src/vm/VirtualMachine.cc

@@ -755,7 +755,6 @@ int VirtualMachine::insert(SqlDB * db, string& error_str)
     long int memory;
     float fvalue;
     set<int> cluster_ids;
-    set<int> datastore_ids;
     vector<Template *> quotas;
     ostringstream oss;
 
@@ -1030,7 +1029,7 @@ int VirtualMachine::insert(SqlDB * db, string& error_str)
         goto error_requirements;
     }
 
-    rc = automatic_requirements(cluster_ids, datastore_ids, error_str);
+    rc = automatic_requirements(cluster_ids, error_str);
 
     if ( rc != 0 )
     {
@@ -1498,12 +1497,13 @@ error_common:
  * @return 0 on success
  */
 static int get_datastore_requirements(Template *tmpl, set<int>& ds_ids,
-        string& error_str)
+        bool& shareable, string& error_str)
 {
     ostringstream oss;
 
     vector<VectorAttribute*> vatts;
     set<int> csystem_ds;
+    shareable = false;
 
     DatastorePool * ds_pool = Nebula::instance().get_dspool();
 
@@ -1514,6 +1514,10 @@ static int get_datastore_requirements(Template *tmpl, set<int>& ds_ids,
 
     for (int i=0; i<num_vatts; i++, csystem_ds.clear())
     {
+        bool is_shareable;
+        vatts[i]->vector_value("SHAREABLE", is_shareable);
+        shareable = shareable || is_shareable;
+
         int val;
 
         if (vatts[i]->vector_value("DATASTORE_ID", val) != 0)
@@ -1521,7 +1525,7 @@ static int get_datastore_requirements(Template *tmpl, set<int>& ds_ids,
             continue;
         }
 
-        if (auto ds = ds_pool->get(val))
+        if (auto ds = ds_pool->get_ro(val))
         {
             ds->get_compatible_system_ds(csystem_ds);
 
@@ -1552,11 +1556,13 @@ error_disk:
 /* ------------------------------------------------------------------------ */
 
 int VirtualMachine::automatic_requirements(set<int>& cluster_ids,
-        set<int>& datastore_ids, string& error_str)
+                                           string& error_str)
 {
     string tm_mad_system;
     ostringstream   oss;
     set<string>     clouds;
+    bool            shareable = false;
+    set<int>        datastore_ids;
 
     obj_template->erase("AUTOMATIC_REQUIREMENTS");
     obj_template->erase("AUTOMATIC_DS_REQUIREMENTS");
@@ -1569,7 +1575,8 @@ int VirtualMachine::automatic_requirements(set<int>& cluster_ids,
         return -1;
     }
 
-    rc = get_datastore_requirements(obj_template.get(), datastore_ids, error_str);
+    rc = get_datastore_requirements(obj_template.get(), datastore_ids,
+                                    shareable, error_str);
 
     if (rc == -1)
     {
@@ -1669,7 +1676,20 @@ int VirtualMachine::automatic_requirements(set<int>& cluster_ids,
         {
             oss << " & (TM_MAD = \"" << one_util::trim(tm_mad_system) << "\")";
         }
+    }
 
+    if (shareable)
+    {
+        if (!oss.str().empty())
+        {
+            oss << " & ";
+        }
+
+        oss << "(SHARED = \"YES\")";
+    }
+
+    if (!oss.str().empty())
+    {
         obj_template->add("AUTOMATIC_DS_REQUIREMENTS", oss.str());
     }
 
@@ -3563,6 +3583,43 @@ int VirtualMachine::check_tm_mad_disks(const string& tm_mad, string& error)
 /* ------------------------------------------------------------------------ */
 /* ------------------------------------------------------------------------ */
 
+int VirtualMachine::check_shareable_disks(const string& vmm_mad, string& error)
+{
+    VirtualMachineManager * vmm = Nebula::instance().get_vmm();
+    const VirtualMachineManagerDriver * vmmd = vmm->get(vmm_mad);
+
+    if ( vmmd == nullptr )
+    {
+        error = "Cannot find vmm driver: " + vmm_mad;
+
+        return -1;
+    }
+
+    if ( vmmd->support_shareable() )
+    {
+        return 0;
+    }
+
+    for (const auto disk : disks)
+    {
+        bool shareable;
+        disk->vector_value("SHAREABLE", shareable);
+
+        if (shareable)
+        {
+            error = "VM has shareable disk, but vmm driver: '"
+                + vmm_mad + "' doesn't support shareable disks";
+
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+/* ------------------------------------------------------------------------ */
+/* ------------------------------------------------------------------------ */
+
 void VirtualMachine::encrypt()
 {
     std::string one_key;

src/vmm/VirtualMachineManagerDriver.cc

@@ -43,7 +43,8 @@ VirtualMachineManagerDriver::VirtualMachineManagerDriver(
         keep_snapshots(false),
         ds_live_migration(false),
         cold_nic_attach(false),
-        live_resize(false)
+        live_resize(false),
+        support_shareable_(false)
 {
     char *          error_msg = nullptr;
     const char *    cfile;
@@ -114,6 +115,8 @@ VirtualMachineManagerDriver::VirtualMachineManagerDriver(
 
     driver_conf.get("LIVE_RESIZE", live_resize);
 
+    driver_conf.get("SUPPORT_SHAREABLE", support_shareable_);
+
     // -------------------------------------------------------------------------
     // Parse IMPORTED_VMS_ACTIONS string and init the action set
     // -------------------------------------------------------------------------

src/vmm_mad/remotes/kvm/attach_disk

@@ -99,6 +99,7 @@ XML+="</source>"
 XML+="<target dev='$(xml_esc "${TARGET}")'/>"
 [ -n "${ORDER}" ] && XML+=" <boot order='$(xml_esc "${ORDER}")'/>"
 [ "${READONLY}" = 'yes' ] && XML+="<readonly/>"
+[ "${SHAREABLE}" = 'yes' ] && XML+="<shareable/>"
 [ -n "${LUKS}" ] && XML+="${LUKS}"
 
 if [ -n "${TOTAL_BYTES_SEC}${READ_BYTES_SEC}${WRITE_BYTES_SEC}" ] || \