From f6fac118c50633dd368304276c97efa54568331d Mon Sep 17 00:00:00 2001 From: Neal <92747003+onenhansen@users.noreply.github.com> Date: Tue, 28 May 2024 05:34:07 -0600 Subject: [PATCH] F OpenNebula/one#6291: remove disk number and naming limitations (#3061) Signed-off-by: Neal Hansen Co-authored-by: Pavel Czerny --- include/VirtualMachineDisk.h | 4 +-- src/vm/VirtualMachine.cc | 56 +++++++++----------------------- src/vm/VirtualMachineDisk.cc | 63 ++++++++++++++++++++++++++++++------ src/vmm/LibVirtDriverKVM.cc | 10 +++++- 4 files changed, 81 insertions(+), 52 deletions(-) diff --git a/include/VirtualMachineDisk.h b/include/VirtualMachineDisk.h index 1bf4ebb1d67..5b7c7cc3e5e 100644 --- a/include/VirtualMachineDisk.h +++ b/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 disks, VectorAttribute * context, - std::string& error_str); + std::vector disks, VectorAttribute * context, + bool is_q35, std::string& error_str); /** * Release the images in the disk set diff --git a/src/vm/VirtualMachine.cc b/src/vm/VirtualMachine.cc index 01e92cd793f..f34240dfaa4 100644 --- a/src/vm/VirtualMachine.cc +++ b/src/vm/VirtualMachine.cc @@ -3302,53 +3302,20 @@ int VirtualMachine::updateconf(VirtualMachineTemplate* tmpl, string &err, int VirtualMachine::get_disk_images(string& error_str) { - vector adisks; - vector acontext_disks; + vector adisks; + vector 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(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); } /* -------------------------------------------------------------------------- */ diff --git a/src/vm/VirtualMachineDisk.cc b/src/vm/VirtualMachineDisk.cc index 3ca02a63687..e15b180c538 100644 --- a/src/vm/VirtualMachineDisk.cc +++ b/src/vm/VirtualMachineDisk.cc @@ -743,10 +743,22 @@ void VirtualMachineDisks::assign_disk_targets( do { - target = disk_pair.first + static_cast(('a'+ index)); + string temp; + int remainder = index % 26; + int quotient = index / 26; + + while (quotient > 0) { + temp = static_cast('a' + remainder) + temp; + + remainder = (quotient - 1) % 26; + quotient = (quotient - 1) / 26; + } + + temp = static_cast('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 disks, VectorAttribute * vcontext, + vector disks, VectorAttribute * vcontext, bool is_q35, std::string& error_str) { Nebula& nd = Nebula::instance(); ImagePool* ipool = nd.get_ipool(); - vector::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(*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(); diff --git a/src/vmm/LibVirtDriverKVM.cc b/src/vmm/LibVirtDriverKVM.cc index 086e02c2528..256a76c9666 100644 --- a/src/vmm/LibVirtDriverKVM.cc +++ b/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" ) {