F #6291: remove disk number and naming limitations (#3061)

Signed-off-by: Neal Hansen <[email protected]>
Co-authored-by: Pavel Czerny <[email protected]>

include/VirtualMachineDisk.h

@@ -544,8 +544,8 @@ class VirtualMachineDisks : public VirtualMachineAttributeSet
      *  @return 0 if success
      */
     int get_images(int vm_id, int uid, const std::string& tm_mad_sys,
-            std::vector<Attribute *> disks, VectorAttribute * context,
-            std::string& error_str);
+            std::vector<VectorAttribute *> disks, VectorAttribute * context,
+            bool is_q35, std::string& error_str);
 
     /**
      *  Release the images in the disk set

src/vm/VirtualMachine.cc

@@ -3302,53 +3302,20 @@ int VirtualMachine::updateconf(VirtualMachineTemplate* tmpl, string &err,
 
 int VirtualMachine::get_disk_images(string& error_str)
 {
-    vector<Attribute *> adisks;
-    vector<Attribute *> acontext_disks;
+    vector<VectorAttribute *> adisks;
+    vector<VectorAttribute *> acontext_disks;
 
     int num_context = user_obj_template->remove("CONTEXT", acontext_disks);
-    int num_disks   = user_obj_template->remove("DISK", adisks);
+    user_obj_template->remove("DISK", adisks);
 
-    for (auto it = acontext_disks.begin(); it != acontext_disks.end(); )
-    {
-        if ( (*it)->type() != Attribute::VECTOR )
-        {
-            delete *it;
-            num_context--;
-            it = acontext_disks.erase(it);
-        }
-        else
-        {
-            obj_template->set(*it);
-            ++it;
-        }
-    }
-
-    for (auto it = adisks.begin(); it != adisks.end(); )
-    {
-        if ( (*it)->type() != Attribute::VECTOR )
-        {
-            delete *it;
-            num_disks--;
-            it = adisks.erase(it);
-        }
-        else
-        {
-            obj_template->set(*it);
-            ++it;
-        }
-    }
-
-    if ( num_disks > 20 )
-    {
-        error_str = "Exceeded the maximum number of disks (20)";
-        return -1;
-    }
+    obj_template->set(acontext_disks);
+    obj_template->set(adisks);
 
     VectorAttribute * context = 0;
 
     if ( num_context > 0 )
     {
-        context = static_cast<VectorAttribute * >(acontext_disks[0]);
+        context = acontext_disks[0];
     }
 
     // -------------------------------------------------------------------------
@@ -3363,7 +3330,16 @@ int VirtualMachine::get_disk_images(string& error_str)
         obj_template->add("TM_MAD_SYSTEM", tm_mad_sys);
     }
 
-    return disks.get_images(oid, uid, tm_mad_sys, adisks, context, error_str);
+    VectorAttribute* os = obj_template->get("OS");
+    bool is_q35         = false;
+
+    if (os)
+    {
+        string machine = os->vector_value("MACHINE");
+        is_q35         = machine.find("q35") != std::string::npos;
+    }
+
+    return disks.get_images(oid, uid, tm_mad_sys, adisks, context, is_q35, error_str);
 }
 
 /* -------------------------------------------------------------------------- */

src/vm/VirtualMachineDisk.cc

@@ -743,10 +743,22 @@ void VirtualMachineDisks::assign_disk_targets(
 
         do
         {
-            target = disk_pair.first + static_cast<char>(('a'+ index));
+            string temp;
+            int remainder = index % 26;
+            int quotient  = index / 26;
+
+            while (quotient > 0) {
+                temp = static_cast<char>('a' + remainder) + temp;
+
+                remainder = (quotient - 1) % 26;
+                quotient  = (quotient - 1) / 26;
+            }
+
+            temp   = static_cast<char>('a' + remainder) + temp;
+            target = disk_pair.first + temp;
+
             index++;
-        }
-        while ( used_targets.count(target) > 0 && index < 26 );
+        } while (used_targets.count(target) > 0);
 
         disk_pair.second->replace("TARGET", target);
 
@@ -760,15 +772,13 @@ void VirtualMachineDisks::assign_disk_targets(
 /* -------------------------------------------------------------------------- */
 
 int VirtualMachineDisks::get_images(int vm_id, int uid, const std::string& tsys,
-        vector<Attribute *> disks, VectorAttribute * vcontext,
+        vector<VectorAttribute *> disks, VectorAttribute * vcontext, bool is_q35,
         std::string& error_str)
 {
     Nebula&    nd    = Nebula::instance();
     ImagePool* ipool = nd.get_ipool();
 
-    vector<Attribute*>::iterator it;
-
-    int         disk_id, image_id;
+    int         disk_id = 0, image_id, hd_disks = 0, sd_disks = 0;
     std::string dev_prefix, target;
 
     Image::ImageType image_type;
@@ -782,10 +792,10 @@ int VirtualMachineDisks::get_images(int vm_id, int uid, const std::string& tsys,
 
     std::ostringstream oss;
 
-    for(it = disks.begin(), disk_id = 0; it != disks.end(); ++it, ++disk_id)
+    for(auto it = disks.begin(); it != disks.end(); ++it, ++disk_id)
     {
         Snapshots*       snapshots;
-        VectorAttribute* vdisk = static_cast<VectorAttribute * >(*it);
+        VectorAttribute* vdisk = *it;
 
         // ---------------------------------------------------------------------
         // Initialize DISK attribute information and acquire associated IMAGE
@@ -867,8 +877,33 @@ int VirtualMachineDisks::get_images(int vm_id, int uid, const std::string& tsys,
                     break;
             }
         }
+
+        if ( dev_prefix == "sd" )
+        {
+            sd_disks++;
+        }
+
+        if ( dev_prefix == "hd" )
+        {
+            hd_disks++;
+        }
     }
 
+    // -------------------------------------------------------------------------
+    // Check for too many disks
+    // -------------------------------------------------------------------------
+
+    if ( hd_disks > 4 && !is_q35 )
+    {
+        goto error_too_many_hd_disks;
+    }
+
+    if ( sd_disks > 256 )
+    {
+        goto error_too_many_sd_disks;
+    }
+
+
     // -------------------------------------------------------------------------
     // Targets for OS Disks
     // -------------------------------------------------------------------------
@@ -919,6 +954,16 @@ int VirtualMachineDisks::get_images(int vm_id, int uid, const std::string& tsys,
 
     return 0;
 
+error_too_many_hd_disks:
+    oss << "Non-q35 VM " << vm_id << " has more than 4 hd disks";
+    error_str = oss.str();
+    goto error_common;
+
+error_too_many_sd_disks:
+    oss << "VM " << vm_id << " has more than 256 disks";
+    error_str = oss.str();
+    goto error_common;
+
 error_duplicated_target:
     oss << "Two disks have defined the same target " << target;
     error_str = oss.str();

src/vmm/LibVirtDriverKVM.cc

@@ -1579,7 +1579,15 @@ int LibVirtDriver::deployment_description_kvm(
         //   - target is based on dev target to have a predictable order
         if ( target[0] == 's' && target[1] == 'd' )
         {
-            int target_number = target[2] - 'a';
+            string suffix        = target.substr(2);
+            int    target_number = 0;
+
+            for (char ch : suffix)
+            {
+                target_number = target_number * 26 + (ch - 'a' + 1);
+            }
+
+            target_number--;
 
             if ( disk_bus == "sata" )
             {