From f7d9d69206e7ca168549bea00aea1dfb8c7bb903 Mon Sep 17 00:00:00 2001 From: "Ruben S. Montero" Date: Tue, 11 Nov 2014 18:52:06 +0100 Subject: [PATCH] feature #3175: Do not duplicate SG information --- include/VirtualMachine.h | 32 +++++----- src/dm/DispatchManagerActions.cc | 5 ++ src/vm/VirtualMachine.cc | 101 ++++++++++++++++--------------- 3 files changed, 74 insertions(+), 64 deletions(-) diff --git a/include/VirtualMachine.h b/include/VirtualMachine.h index 5386027026..d7ac634df3 100644 --- a/include/VirtualMachine.h +++ b/include/VirtualMachine.h @@ -1090,6 +1090,12 @@ public: */ static long long get_volatile_disk_size(Template * tmpl); + /** + * Returns a set of the security group IDs in use in this VM + * @param sgs a set of security group IDs + */ + void get_security_groups(set& sgs) const; + // ------------------------------------------------------------------------ // Context related functions // ------------------------------------------------------------------------ @@ -1280,6 +1286,7 @@ public: * Setups the new NIC attribute to be attached to the VM. * * @param vm_id Id of the VM where this nic will be attached + * @param vm_sgs the securty group ids already present in the VM * @param new_nic New NIC vector attribute, obtained from get_attach_nic_info * @param rules Security Group rules will be added at the end of this * vector. If not used, the VectorAttributes must be freed by the calling @@ -1291,6 +1298,7 @@ public: */ static int set_up_attach_nic( int vm_id, + set& vm_sgs, VectorAttribute * new_nic, vector &rules, int max_nic_id, @@ -1681,34 +1689,30 @@ private: * Acquires the security groups of this NIC * * @param vm_id Virtual Machine oid - * @param nic NIC to get the security groups from + * @param sgs security group ID set * @param rules Security Group rules will be added at the end of this vector - * @param error_str Returns the error reason, if any - * @return 0 on success, -1 otherwise */ - static int get_security_groups( - int vm_id, - VectorAttribute const * nic, - vector &rules, - string &error_str); + static void get_security_group_rules(int vm_id, set& sgs, + vector &rules); /** * Releases the security groups of this NIC * * @param vm_id Virtual Machine oid * @param nic NIC to release the security groups - * @param error_str Returns the error reason, if any * @return 0 on success, -1 otherwise */ - static int release_security_groups( - int vm_id, VectorAttribute const * nic, string &error_str); + static void release_security_groups(int vm_id, VectorAttribute const * nic); /** * Returns a set of the security group IDs of this NIC * @param nic NIC to get the security groups from - * @return a set of security group IDs + * @param sgs a set of security group IDs */ - static set nic_security_groups(VectorAttribute const * nic); + static void get_security_groups(VectorAttribute const * nic, set& sgs) + { + one_util::split_unique(nic->vector_value("SECURITY_GROUPS"), ',', sgs); + } /** * Get all disk images for this Virtual Machine @@ -1717,8 +1721,6 @@ private: */ int get_disk_images(string &error_str); - - protected: //************************************************************************** diff --git a/src/dm/DispatchManagerActions.cc b/src/dm/DispatchManagerActions.cc index d98f2252c7..30b17dbff5 100644 --- a/src/dm/DispatchManagerActions.cc +++ b/src/dm/DispatchManagerActions.cc @@ -1359,6 +1359,8 @@ int DispatchManager::attach_nic( int oid; int rc; + set vm_sgs; + VectorAttribute * nic; vector sg_rules; @@ -1400,6 +1402,8 @@ int DispatchManager::attach_nic( return -1; } + vm->get_security_groups(vm_sgs); + vm->set_state(VirtualMachine::HOTPLUG_NIC); vm->set_resched(false); @@ -1412,6 +1416,7 @@ int DispatchManager::attach_nic( vm->unlock(); rc = VirtualMachine::set_up_attach_nic(oid, + vm_sgs, nic, sg_rules, max_nic_id, diff --git a/src/vm/VirtualMachine.cc b/src/vm/VirtualMachine.cc index eaac2300d0..8275dae336 100644 --- a/src/vm/VirtualMachine.cc +++ b/src/vm/VirtualMachine.cc @@ -2246,6 +2246,7 @@ VectorAttribute * VirtualMachine::get_attach_nic_info( int VirtualMachine::set_up_attach_nic( int vm_id, + set& vm_sgs, VectorAttribute * new_nic, vector &rules, int max_nic_id, @@ -2255,9 +2256,7 @@ int VirtualMachine::set_up_attach_nic( Nebula& nd = Nebula::instance(); VirtualNetworkPool* vnpool = nd.get_vnpool(); - // ------------------------------------------------------------------------- - // Acquire the new network lease - // ------------------------------------------------------------------------- + set nic_sgs; int rc = vnpool->nic_attribute(new_nic, max_nic_id+1, uid, vm_id, error_str); @@ -2266,9 +2265,16 @@ int VirtualMachine::set_up_attach_nic( return -1; } - rc = get_security_groups(vm_id, new_nic, rules, error_str); + get_security_groups(new_nic, nic_sgs); - return rc; + for (set::iterator it = vm_sgs.begin(); it != vm_sgs.end(); it++) + { + nic_sgs.erase(*it); + } + + get_security_group_rules(vm_id, nic_sgs, rules); + + return 0; } /* -------------------------------------------------------------------------- */ @@ -2626,6 +2632,8 @@ int VirtualMachine::get_network_leases(string& estr) vector sg_rules; vector::iterator it; + set vm_sgs; + num_nics = user_obj_template->remove("NIC",nics); for (vector::iterator it=nics.begin(); it != nics.end(); it++) @@ -2650,15 +2658,12 @@ int VirtualMachine::get_network_leases(string& estr) { return -1; } - - rc = get_security_groups(this->get_oid(), nic, sg_rules, estr); - - if (rc == -1) - { - return -1; - } } + get_security_groups(vm_sgs); + + get_security_group_rules(get_oid(), vm_sgs, sg_rules); + for(it = sg_rules.begin(); it != sg_rules.end(); it++ ) { obj_template->set(*it); @@ -2735,7 +2740,7 @@ int VirtualMachine::release_network_leases(VectorAttribute const * nic, int vmid return -1; } - release_security_groups(vmid, nic, error_msg); + release_security_groups(vmid, nic); if (nic->vector_value("NETWORK_ID", vnid) != 0) { @@ -2775,26 +2780,28 @@ int VirtualMachine::release_network_leases(VectorAttribute const * nic, int vmid /* -------------------------------------------------------------------------- */ /* -------------------------------------------------------------------------- */ -set VirtualMachine::nic_security_groups(VectorAttribute const * nic) +void VirtualMachine::get_security_groups(set& sgs) const { - set result; - one_util::split_unique(nic->vector_value("SECURITY_GROUPS"), ',', result); + string vnid; + string ip; + int num_nics; + vector nics; - return result; + num_nics = get_template_attribute("NIC", nics); + + for(int i=0; i(nics[i]),sgs); + } } - /* -------------------------------------------------------------------------- */ /* -------------------------------------------------------------------------- */ -int VirtualMachine::get_security_groups( - int vm_id, - VectorAttribute const * nic, - vector &rules, - string &error_str) +void VirtualMachine::get_security_group_rules(int id, set& secgroups, + vector &rules) { - set::const_iterator sg_it; - set secgroups = nic_security_groups(nic); + set::iterator sg_it; vector::iterator rule_it; @@ -2816,7 +2823,7 @@ int VirtualMachine::get_security_groups( continue; } - sgroup->add_vm(vm_id); + sgroup->add_vm(id); sgroup_pool->update(sgroup); @@ -2826,10 +2833,12 @@ int VirtualMachine::get_security_groups( for (rule_it = sgroup_rules.begin(); rule_it != sgroup_rules.end(); rule_it++) { - VectorAttribute* rule = *rule_it; - - if ( rule->vector_value("NETWORK_ID", vnet_id) != -1 ) + if ( (*rule_it)->vector_value("NETWORK_ID", vnet_id) != -1 ) { + vector vnet_rules; + + VectorAttribute * rule = *rule_it; + vnet = vnet_pool->get(vnet_id, true); if (vnet == 0) @@ -2837,7 +2846,6 @@ int VirtualMachine::get_security_groups( continue; } - vector vnet_rules; vnet->process_security_rule(rule, vnet_rules); delete rule; @@ -2852,23 +2860,21 @@ int VirtualMachine::get_security_groups( } } } - - // TODO: error handling - return 0; } /* -------------------------------------------------------------------------- */ /* -------------------------------------------------------------------------- */ -int VirtualMachine::release_security_groups( - int vm_id, VectorAttribute const * nic, string &error_str) +void VirtualMachine::release_security_groups(int id, VectorAttribute const * nic) { - set::const_iterator it; - set secgroups = nic_security_groups(nic); + set::iterator it; + set secgroups; SecurityGroup* sgroup; SecurityGroupPool* sgroup_pool = Nebula::instance().get_secgrouppool(); + get_security_groups(nic, secgroups); + for (it = secgroups.begin(); it != secgroups.end(); it++) { sgroup = sgroup_pool->get(*it, true); @@ -2878,15 +2884,12 @@ int VirtualMachine::release_security_groups( continue; } - sgroup->del_vm(vm_id); + sgroup->del_vm(id); sgroup_pool->update(sgroup); sgroup->unlock(); } - - // TODO: error handling - return 0; } /* -------------------------------------------------------------------------- */ @@ -3382,15 +3385,15 @@ void VirtualMachine::set_auth_request(int uid, int num; vector vectors; VectorAttribute * vector; - set::iterator it; Nebula& nd = Nebula::instance(); - ImagePool * ipool = nd.get_ipool(); - VirtualNetworkPool * vnpool = nd.get_vnpool(); - SecurityGroupPool * sgpool = nd.get_secgrouppool(); + ImagePool * ipool = nd.get_ipool(); + VirtualNetworkPool * vnpool = nd.get_vnpool(); + SecurityGroupPool * sgpool = nd.get_secgrouppool(); - SecurityGroup * sgroup; + set sgroups; + SecurityGroup * sgroup; num = tmpl->get("DISK",vectors); @@ -3411,7 +3414,7 @@ void VirtualMachine::set_auth_request(int uid, num = tmpl->get("NIC",vectors); - for(int i=0; i(vectors[i]); @@ -3422,9 +3425,9 @@ void VirtualMachine::set_auth_request(int uid, vnpool->authorize_nic(vector,uid,&ar); - set sgroups = nic_security_groups(vector); + get_security_groups(vector, sgroups); - for(it = sgroups.begin(); it != sgroups.end(); it++) + for(set::iterator it = sgroups.begin(); it != sgroups.end(); it++) { sgroup = sgpool->get(*it, true);