bug 9873: always add default security group to the SG list when deploy vm in 1) Basic zone 2) Advance zone using SG enabled network

status 9873: resolved fixed

Following fixes were made as a part of the checkin:

* When deploy user vm and SG doesn't exist in the DB, create it automatically.
* SecurityGroup enabled use vm start: if map to default group is not present in security_group_vm_map table, create one.
* Added "name" (securityGroupName) parameter back to deleteSecurityGroup/authorizeSecurityGroupIngress/deployVm. Mutually exclusive with security group id parameter.

Conflicts:

	api/src/com/cloud/api/commands/AuthorizeSecurityGroupIngressCmd.java
	api/src/com/cloud/api/commands/DeleteSecurityGroupCmd.java
	api/src/com/cloud/api/commands/DeployVMCmd.java
	server/src/com/cloud/api/ApiDBUtils.java
	server/src/com/cloud/vm/UserVmManagerImpl.java

api/src/com/cloud/api/ApiConstants.java

@@ -140,6 +140,7 @@ public class ApiConstants {
     public static final String SCOPE = "scope";
     public static final String SECRET_KEY = "secretkey";
     public static final String SECURITY_GROUP_IDS = "securitygroupids";
+    public static final String SECURITY_GROUP_NAMES = "securitygroupnames";
     public static final String SECURITY_GROUP_NAME = "securitygroupname";
     public static final String SECURITY_GROUP_ID = "securitygroupid";
     public static final String SECURITY_GROUP_EANBLED = "securitygroupenabled";

api/src/com/cloud/api/ResponseGenerator.java

@@ -204,5 +204,7 @@ public interface ResponseGenerator {
 	UserResponse createUserResponse(User user);
 
 	AccountResponse createUserAccountResponse(UserAccount user);
+	
+	Long getSecurityGroupId(String groupName, long accountId);
 
 }

api/src/com/cloud/api/commands/AuthorizeSecurityGroupIngressCmd.java

@@ -36,9 +36,10 @@ import com.cloud.api.response.IngressRuleResponse;
 import com.cloud.api.response.SecurityGroupResponse;
 import com.cloud.async.AsyncJob;
 import com.cloud.event.EventTypes;
+import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.network.security.IngressRule;
-import com.cloud.network.security.SecurityGroup;
 import com.cloud.user.Account;
+import com.cloud.user.UserContext;
 import com.cloud.utils.StringUtils;
 
 @Implementation(responseObject = IngressRuleResponse.class, description = "Authorizes a particular ingress rule for this security group")
@@ -67,20 +68,33 @@ public class AuthorizeSecurityGroupIngressCmd extends BaseAsyncCmd {
     @Parameter(name = ApiConstants.ICMP_CODE, type = CommandType.INTEGER, description = "error code for this icmp message")
     private Integer icmpCode;
 
-    @Parameter(name = ApiConstants.SECURITY_GROUP_ID, type = CommandType.LONG, required = true, description = "The ID of the security group")
+    @Parameter(name=ApiConstants.CIDR_LIST, type=CommandType.LIST, collectionType=CommandType.STRING, description="the cidr list associated")
-    private Long securityGroupId;
+    private List cidrList;
-
-    @Parameter(name = ApiConstants.CIDR_LIST, type = CommandType.LIST, collectionType = CommandType.STRING, description = "the cidr list associated")
-    private List<String> cidrList;
 
     @Parameter(name = ApiConstants.USER_SECURITY_GROUP_LIST, type = CommandType.MAP, description = "user to security group mapping")
     private Map userSecurityGroupList;
+    
+    @Parameter(name=ApiConstants.DOMAIN_ID, type=CommandType.LONG, description="an optional domainId for the security group. If the account parameter is used, domainId must also be used.")
+    private Long domainId;
+    
+    @Parameter(name=ApiConstants.ACCOUNT, type=CommandType.STRING, description="an optional account for the virtual machine. Must be used with domainId.")
+    private String accountName;
+    
+    @Parameter(name=ApiConstants.SECURITY_GROUP_ID, type=CommandType.LONG, description="The ID of the security group. Mutually exclusive with securityGroupName parameter")
+    private Long securityGroupId;
+    
+    @Parameter(name=ApiConstants.SECURITY_GROUP_NAME, type=CommandType.STRING, description="The name of the security group. Mutually exclusive with securityGroupName parameter")
+    private String securityGroupName;
 
-    // ///////////////////////////////////////////////////
+    /////////////////////////////////////////////////////
-    // ///////////////// Accessors ///////////////////////
+    /////////////////// Accessors ///////////////////////
-    // ///////////////////////////////////////////////////
+    /////////////////////////////////////////////////////
 
-    public List<String> getCidrList() {
+    public String getAccountName() {
+        return accountName;
+    }
+
+    public List getCidrList() {
         return cidrList;
     }
 
@@ -97,6 +111,17 @@ public class AuthorizeSecurityGroupIngressCmd extends BaseAsyncCmd {
     }
 
     public Long getSecurityGroupId() {
+        if (securityGroupId != null && securityGroupName != null) {
+            throw new InvalidParameterValueException("securityGroupId and securityGroupName parameters are mutually exclusive");
+        }
+        
+        if (securityGroupName != null) {
+            securityGroupId = _responseGenerator.getSecurityGroupId(securityGroupName, getEntityOwnerId());
+            if (securityGroupId == null) {
+                throw new InvalidParameterValueException("Unable to find security group " + securityGroupName + " for account id=" + getEntityOwnerId());
+            }
+        }
+        
         return securityGroupId;
     }
 
@@ -130,12 +155,19 @@ public class AuthorizeSecurityGroupIngressCmd extends BaseAsyncCmd {
 
     @Override
     public long getEntityOwnerId() {
-        SecurityGroup group = _entityMgr.findById(SecurityGroup.class, getSecurityGroupId());
+        Account account = UserContext.current().getCaller();
-        if (group != null) {
+        if ((account == null) || isAdmin(account.getType())) {
-            return group.getAccountId();
+            if ((domainId != null) && (accountName != null)) {
+                Account userAccount = _responseGenerator.findAccountByNameDomain(accountName, domainId);
+                if (userAccount != null) {
+                    return userAccount.getId();
+                } else {
+                    throw new InvalidParameterValueException("Unable to find account by name " + accountName + " in domain " + domainId);
+                }
+            }
         }
-
+        
-        return Account.ACCOUNT_ID_SYSTEM; // no account info given, parent this command to SYSTEM so ERROR events are tracked
+        return account.getId();
     }
 
     @Override

api/src/com/cloud/api/commands/CreateSecurityGroupCmd.java

@@ -41,13 +41,13 @@ public class CreateSecurityGroupCmd extends BaseCmd {
 
     @Parameter(name=ApiConstants.ACCOUNT, type=CommandType.STRING, description="an optional account for the security group. Must be used with domainId.")
     private String accountName;
+    
+    @Parameter(name=ApiConstants.DOMAIN_ID, type=CommandType.LONG, description="an optional domainId for the security group. If the account parameter is used, domainId must also be used.")
+    private Long domainId;
 
     @Parameter(name=ApiConstants.DESCRIPTION, type=CommandType.STRING, description="the description of the security group")
     private String description;
 
-    @Parameter(name=ApiConstants.DOMAIN_ID, type=CommandType.LONG, description="an optional domainId for the security group. If the account parameter is used, domainId must also be used.")
-    private Long domainId;
-
     @Parameter(name=ApiConstants.NAME, type=CommandType.STRING, required=true, description="name of the security group")
     private String securityGroupName;
 

api/src/com/cloud/api/commands/DeleteSecurityGroupCmd.java

@@ -1,87 +1,108 @@
-/**
+package com.cloud.api.commands;
- *  Copyright (C) 2010 Cloud.com, Inc.  All rights reserved.
+
- * 
+import org.apache.log4j.Logger;
- * This software is licensed under the GNU General Public License v3 or later.
+
- * 
+import com.cloud.api.ApiConstants;
- * It is free software: you can redistribute it and/or modify
+import com.cloud.api.BaseCmd;
- * it under the terms of the GNU General Public License as published by
+import com.cloud.api.Implementation;
- * the Free Software Foundation, either version 3 of the License, or any later version.
+import com.cloud.api.Parameter;
- * This program is distributed in the hope that it will be useful,
+import com.cloud.api.ServerApiException;
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
+import com.cloud.api.response.SuccessResponse;
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+import com.cloud.exception.InvalidParameterValueException;
- * GNU General Public License for more details.
+import com.cloud.exception.ResourceInUseException;
- * 
+import com.cloud.user.Account;
- * You should have received a copy of the GNU General Public License
+import com.cloud.user.UserContext;
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
- * 
+@Implementation(description="Deletes security group", responseObject=SuccessResponse.class)
- */
+public class DeleteSecurityGroupCmd extends BaseCmd {
-
+    public static final Logger s_logger = Logger.getLogger(DeleteSecurityGroupCmd.class.getName());
-package com.cloud.api.commands;
+    private static final String s_name = "deletesecuritygroupresponse";
-
+
-import org.apache.log4j.Logger;
+    /////////////////////////////////////////////////////
-
+    //////////////// API parameters /////////////////////
-import com.cloud.api.ApiConstants;
+    /////////////////////////////////////////////////////
-import com.cloud.api.BaseCmd;
+
-import com.cloud.api.Implementation;
+    @Parameter(name=ApiConstants.ACCOUNT, type=CommandType.STRING, description="the account of the security group. Must be specified with domain ID")
-import com.cloud.api.Parameter;
+    private String accountName;
-import com.cloud.api.ServerApiException;
+
-import com.cloud.api.response.SuccessResponse;
+    @Parameter(name=ApiConstants.DOMAIN_ID, type=CommandType.LONG, description="the domain ID of account owning the security group")
-import com.cloud.exception.ResourceInUseException;
+    private Long domainId;
-import com.cloud.network.security.SecurityGroup;
+
-import com.cloud.user.Account;
+    @Parameter(name=ApiConstants.ID, type=CommandType.LONG, description="The ID of the security group. Mutually exclusive with name parameter")
-
+    private Long id;
-@Implementation(description = "Deletes security group", responseObject = SuccessResponse.class)
+    
-public class DeleteSecurityGroupCmd extends BaseCmd {
+    @Parameter(name=ApiConstants.NAME, type=CommandType.STRING, description="The ID of the security group. Mutually exclusive with id parameter")
-    public static final Logger s_logger = Logger.getLogger(DeleteSecurityGroupCmd.class.getName());
+    private String name;
-    private static final String s_name = "deletesecuritygroupresponse";
+
-
+
-    // ///////////////////////////////////////////////////
+    /////////////////////////////////////////////////////
-    // ////////////// API parameters /////////////////////
+    /////////////////// Accessors ///////////////////////
-    // ///////////////////////////////////////////////////
+    /////////////////////////////////////////////////////
-
+
-    @Parameter(name = ApiConstants.ID, type = CommandType.LONG, required = true, description = "The ID of the security group")
+    public String getAccountName() {
-    private Long id;
+        return accountName;
-
+    }
-    // ///////////////////////////////////////////////////
+
-    // ///////////////// Accessors ///////////////////////
+    public Long getDomainId() {
-    // ///////////////////////////////////////////////////
+        return domainId;
-
+    }
-    public Long getId() {
+
-        return id;
+    public Long getId() {
-    }
+        if (id != null && name != null) {
-
+            throw new InvalidParameterValueException("name and id parameters are mutually exclusive");
-    // ///////////////////////////////////////////////////
+        }
-    // ///////////// API Implementation///////////////////
+        
-    // ///////////////////////////////////////////////////
+        if (name != null) {
-
+            id = _responseGenerator.getSecurityGroupId(name, getEntityOwnerId());
-    @Override
+            if (id == null) {
-    public String getCommandName() {
+                throw new InvalidParameterValueException("Unable to find security group by name " + name + " for the account id=" + getEntityOwnerId());
-        return s_name;
+            }
-    }
+        }
-
+        
-    @Override
+        return id;
-    public long getEntityOwnerId() {
+    }
-        SecurityGroup group = _entityMgr.findById(SecurityGroup.class, getId());
+
-        if (group != null) {
+
-            return group.getAccountId();
+
-        }
+    /////////////////////////////////////////////////////
-
+    /////////////// API Implementation///////////////////
-        return Account.ACCOUNT_ID_SYSTEM; // no account info given, parent this command to SYSTEM so ERROR events are tracked
+    /////////////////////////////////////////////////////
-    }
+
-
+    @Override
-    @Override
+    public String getCommandName() {
-    public void execute() {
+        return s_name;
-        try {
+    }
-            boolean result = _securityGroupService.deleteSecurityGroup(this);
+    
-            if (result) {
+    @Override
-                SuccessResponse response = new SuccessResponse(getCommandName());
+    public long getEntityOwnerId() {
-                this.setResponseObject(response);
+        Account account = UserContext.current().getCaller();
-            } else {
+        if ((account == null) || isAdmin(account.getType())) {
-                throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to delete security group");
+            if ((domainId != null) && (accountName != null)) {
-            }
+                Account userAccount = _responseGenerator.findAccountByNameDomain(accountName, domainId);
-        } catch (ResourceInUseException ex) {
+                if (userAccount != null) {
-            s_logger.warn("Exception: ", ex);
+                    return userAccount.getId();
-            throw new ServerApiException(BaseCmd.RESOURCE_IN_USE_ERROR, ex.getMessage());
+                } else {
-        }
+                    throw new InvalidParameterValueException("Unable to find account by name " + accountName + " in domain " + domainId);
-    }
+                }
-}
+            }
+        }
+        
+        return account.getId();
+    }
+	
+    @Override
+    public void execute(){
+        try{
+            boolean result = _securityGroupService.deleteSecurityGroup(this);
+            if (result) {
+                SuccessResponse response = new SuccessResponse(getCommandName());
+                this.setResponseObject(response);
+            } else {
+                throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to delete security group");
+            }
+        } catch (ResourceInUseException ex) {
+            s_logger.warn("Exception: ", ex);
+            throw new ServerApiException(BaseCmd.RESOURCE_IN_USE_ERROR, ex.getMessage());
+        }
+    }
+}

api/src/com/cloud/api/commands/DeployVMCmd.java

@@ -18,6 +18,7 @@
 
 package com.cloud.api.commands;
 
+import java.util.ArrayList;
 import java.util.List;
 
 import org.apache.log4j.Logger;
@@ -81,9 +82,6 @@ public class DeployVMCmd extends BaseAsyncCreateCmd {
     @Parameter(name=ApiConstants.NETWORK_IDS, type=CommandType.LIST, collectionType=CommandType.LONG, description="list of network ids used by virtual machine")
     private List<Long> networkIds;
 
-    @Parameter(name=ApiConstants.SECURITY_GROUP_IDS, type=CommandType.LIST, collectionType=CommandType.LONG, description="comma separated list of security groups id that going to be applied to the virtual machine. Should be passed only when vm is created from a zone with Basic Network support")
-    private List<Long> securityGroupIdList;
-
     //DataDisk information
     @Parameter(name=ApiConstants.DISK_OFFERING_ID, type=CommandType.LONG, description="the ID of the disk offering for the virtual machine. If the template is of ISO format, the diskOfferingId is for the root disk volume. Otherwise this parameter is used to indicate the offering for the data disk volume. If the templateId parameter passed is from a Template object, the diskOfferingId refers to a DATA Disk Volume created. If the templateId parameter passed is from an ISO object, the diskOfferingId refers to a ROOT Disk Volume created.")
     private Long diskOfferingId;
@@ -105,6 +103,12 @@ public class DeployVMCmd extends BaseAsyncCreateCmd {
 
     @Parameter(name=ApiConstants.HOST_ID, type=CommandType.LONG, description="destination Host ID to deploy the VM to - parameter available for root admin only")
     private Long hostId;
+    
+    @Parameter(name=ApiConstants.SECURITY_GROUP_IDS, type=CommandType.LIST, collectionType=CommandType.LONG, description="comma separated list of security groups id that going to be applied to the virtual machine. Should be passed only when vm is created from a zone with Basic Network support. Mutually exclusive with securitygroupnames parameter")
+    private List<Long> securityGroupIdList;
+    
+    @Parameter(name=ApiConstants.SECURITY_GROUP_NAMES, type=CommandType.LIST, collectionType=CommandType.STRING, description="comma separated list of security groups names that going to be applied to the virtual machine. Should be passed only when vm is created from a zone with Basic Network support. Mutually exclusive with securitygroupids parameter")
+    private List<String> securityGroupNameList;
 
     /////////////////////////////////////////////////////
     /////////////////// Accessors ///////////////////////
@@ -141,7 +145,24 @@ public class DeployVMCmd extends BaseAsyncCreateCmd {
     }
 
     public List<Long> getSecurityGroupIdList() {
-        return securityGroupIdList;
+        if (securityGroupIdList != null && securityGroupIdList != null) {
+            throw new InvalidParameterValueException("securitygroupids parameter is mutually exclusive with securitygroupnames parameter");
+        }
+        
+       //transform group names to ids here
+       if (securityGroupNameList != null) {
+            securityGroupIdList = new ArrayList<Long>();
+            for (String groupName : securityGroupNameList) {
+                Long groupId = _responseGenerator.getSecurityGroupId(groupName, getEntityOwnerId());
+                if (groupId == null) {
+                    throw new InvalidParameterValueException("Unable to find group by name " + groupName + " for account " + getEntityOwnerId());
+                } else {
+                    securityGroupIdList.add(groupId);
+                }
+            }    
+        }
+       
+       return securityGroupIdList;
     }
 
     public Long getServiceOfferingId() {
@@ -201,15 +222,13 @@ public class DeployVMCmd extends BaseAsyncCreateCmd {
                 Account userAccount = _responseGenerator.findAccountByNameDomain(accountName, domainId);
                 if (userAccount != null) {
                     return userAccount.getId();
+                } else {
+                    throw new InvalidParameterValueException("Unable to find account by name " + getAccountName() + " in domain " + getDomainId());
                 }
             }
         }
-
+        
-        if (account != null) {
+        return account.getId();
-            return account.getId();
-        }
-
-        return Account.ACCOUNT_ID_SYSTEM; // no account info given, parent this command to SYSTEM so ERROR events are tracked
     }
 
     @Override

scripts/vm/hypervisor/xenserver/vmops

@@ -654,7 +654,7 @@ def default_network_rules(session, args):
         return 'false'
     
     for v in vifs:
-        default_ebtables_rules(vm_chain, v, vm_ip, vm_mac)
+        default_ebtables_rules(vmchain, v, vm_ip, vm_mac)
     
     if write_rule_log_for_vm(vm_name, vm_id, vm_ip, domid, '_initial_', '-1') == False:
         util.SMlog("Failed to log default network rules, ignoring")

server/src/com/cloud/api/ApiDBUtils.java

@@ -581,6 +581,10 @@ public class ApiDBUtils {
         float cpuOverprovisioningFactor = NumbersUtil.parseFloat(opFactor, 1);
         return cpuOverprovisioningFactor;
     }
+    
+    public static SecurityGroup getSecurityGroup(String groupName, long ownerId) {
+        return _securityGroupMgr.getSecurityGroup(groupName, ownerId);
+    }
 
     public static ConsoleProxyVO findConsoleProxy(long id) {
         return _consoleProxyDao.findById(id);

server/src/com/cloud/api/ApiResponseHelper.java

@@ -2485,4 +2485,14 @@ public class ApiResponseHelper implements ResponseGenerator {
         response.setObjectName("network");
         return response;
     }
+    
+    @Override
+    public Long getSecurityGroupId (String groupName, long accountId) {
+        SecurityGroup sg = ApiDBUtils.getSecurityGroup(groupName, accountId);
+        if (sg == null) {
+            return null;
+        } else {
+            return sg.getId();
+        }
+    }
 }

server/src/com/cloud/network/NetworkManagerImpl.java

@@ -1446,7 +1446,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag
 
     @Override
     public void expungeNics(VirtualMachineProfile<? extends VMInstanceVO> vm) {
-        List<NicVO> nics = _nicDao.listIncludingRemovedBy(vm.getId());
+        List<NicVO> nics = _nicDao.listByVmIdIncludingRemoved(vm.getId());
         for (NicVO nic : nics) {
             _nicDao.expunge(nic.getId());
         }

server/src/com/cloud/network/security/SecurityGroupManager.java

@@ -34,7 +34,7 @@ public interface SecurityGroupManager {
 	
 	public SecurityGroupVO createSecurityGroup(String name, String description, Long domainId, Long accountId, String accountName);
 	
-	public SecurityGroupVO createDefaultSecurityGroup( Long accountId);
+	public SecurityGroupVO createDefaultSecurityGroup(Long accountId);
 	
 	public boolean addInstanceToGroups(Long userVmId, List<Long> groups);
 
@@ -47,4 +47,10 @@ public interface SecurityGroupManager {
     public List<SecurityGroupVO> getSecurityGroupsForVm(long vmId);
 
     public boolean isVmSecurityGroupEnabled(Long vmId);
+    
+    SecurityGroup getDefaultSecurityGroup(long accountId);
+    
+    SecurityGroup getSecurityGroup(String name, long accountId);
+    
+    boolean isVmMappedToDefaultSecurityGroup(long vmId);
 }

server/src/com/cloud/network/security/SecurityGroupManagerImpl.java

@@ -1168,4 +1168,30 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG
         }
         return false;
     }
+    
+    @Override
+    public SecurityGroupVO getDefaultSecurityGroup(long accountId) {
+        return _securityGroupDao.findByAccountAndName(accountId, DEFAULT_GROUP_NAME);
+    }
+    
+    @Override
+    public SecurityGroup getSecurityGroup(String name, long accountId) {
+        return _securityGroupDao.findByAccountAndName(accountId, name);
+    }
+    
+    @Override
+    public boolean isVmMappedToDefaultSecurityGroup(long vmId) {
+        UserVmVO vm = _userVmMgr.getVirtualMachine(vmId);
+        SecurityGroup defaultGroup = getDefaultSecurityGroup(vm.getAccountId());
+        if (defaultGroup == null) {
+            s_logger.warn("Unable to find default security group for account id=" + vm.getAccountId());
+            return false;
+        }
+        SecurityGroupVMMapVO map = _securityGroupVMMapDao.findByVmIdGroupId(vmId, defaultGroup.getId());
+        if (map == null) {
+            return false;
+        } else {
+            return true;
+        }
+    }
 }

server/src/com/cloud/vm/UserVmManagerImpl.java

@@ -126,6 +126,7 @@ import com.cloud.network.dao.NetworkDao;
 import com.cloud.network.lb.LoadBalancingRulesManager;
 import com.cloud.network.router.VirtualNetworkApplianceManager;
 import com.cloud.network.rules.RulesManager;
+import com.cloud.network.security.SecurityGroup;
 import com.cloud.network.security.SecurityGroupManager;
 import com.cloud.network.vpn.PasswordResetElement;
 import com.cloud.offering.NetworkOffering;
@@ -209,6 +210,7 @@ import com.cloud.vm.dao.UserVmDetailsDao;
 @Local(value = { UserVmManager.class, UserVmService.class })
 public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager {
     private static final Logger s_logger = Logger.getLogger(UserVmManagerImpl.class);
+
     private static final int ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_COOPERATION = 3; // 3 seconds
 
     @Inject
@@ -292,7 +294,7 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
     @Inject
     protected VMTemplateHostDao _vmTemplateHostDao;
     @Inject
-    protected SecurityGroupManager _networkGroupMgr;
+    protected SecurityGroupManager _securityGroupMgr;
     @Inject
     protected ServiceOfferingDao _serviceOfferingDao;
     @Inject
@@ -1216,11 +1218,10 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
 
     private boolean cleanupVmResources(long vmId) {
         boolean success = true;
-
+        //Remove vm from security groups
-        // Remove vm from security groups
+        _securityGroupMgr.removeInstanceFromGroups(vmId);
-        _networkGroupMgr.removeInstanceFromGroups(vmId);
+        
-
+        //Remove vm from instance group
-        // Remove vm from instance group
         removeInstanceFromInstanceGroup(vmId);
 
         // cleanup port forwarding rules
@@ -2010,9 +2011,37 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
         } else {
             networkList.add(_networkDao.findById(defaultNetwork.getId()));
         }
-
+        
-        return createVirtualMachine(zone, serviceOffering, template, hostName, displayName, owner, diskOfferingId, diskSize, networkList, securityGroupIdList, group, userData, sshKeyPair, hypervisor,
+        if (securityGroupIdList == null) {
-                caller);
+            securityGroupIdList = new ArrayList<Long>();
+        }
+        
+        SecurityGroup defaultGroup = _securityGroupMgr.getDefaultSecurityGroup(owner.getId());
+        if (defaultGroup != null) {
+          //check if security group id list already contains Default security group, and if not - add it
+            boolean defaultGroupPresent = false;
+            for (Long securityGroupId : securityGroupIdList) {
+                if (securityGroupId.longValue() == defaultGroup.getId()) {
+                    defaultGroupPresent = true;
+                    break;
+                }
+            }
+            
+            if (!defaultGroupPresent) {
+                securityGroupIdList.add(defaultGroup.getId());
+            }
+          
+        } else {
+            //create default security group for the account
+            if (s_logger.isDebugEnabled()) {
+                s_logger.debug("Couldn't find default security group for the account " + owner + " so creating a new one");
+            }
+            defaultGroup = _securityGroupMgr.createSecurityGroup(SecurityGroupManager.DEFAULT_GROUP_NAME, SecurityGroupManager.DEFAULT_GROUP_DESCRIPTION, owner.getDomainId(), owner.getId(), owner.getAccountName());
+            securityGroupIdList.add(defaultGroup.getId());
+        }
+        
+        return createVirtualMachine(zone, serviceOffering, template, hostName, displayName, owner, diskOfferingId, 
+                                    diskSize, networkList, securityGroupIdList, group, userData, sshKeyPair, hypervisor, caller);
     }
 
     @Override
@@ -2023,8 +2052,9 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
 
         Account caller = UserContext.current().getCaller();
         List<NetworkVO> networkList = new ArrayList<NetworkVO>();
-
+        boolean isSecurityGroupEnabledNetworkUsed = false;
-        // Verify that caller can perform actions in behalf of vm owner
+        
+        //Verify that caller can perform actions in behalf of vm owner
         _accountMgr.checkAccess(caller, owner);
 
         // If no network is specified, find system security group enabled network
@@ -2053,7 +2083,8 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
             }
 
             networkList.add(network);
-
+            isSecurityGroupEnabledNetworkUsed = true;
+            
         } else {
             // Verify that all the networks are Direct/Guest/AccountSpecific; can't create combination of SG enabled network and
             // regular networks
@@ -2084,9 +2115,40 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
                 networkList.add(network);
             }
         }
-
+        
-        return createVirtualMachine(zone, serviceOffering, template, hostName, displayName, owner, diskOfferingId, diskSize, networkList, securityGroupIdList, group, userData, sshKeyPair, hypervisor,
+        // if network is security group enabled, and default security group is not present in the list of groups specified, add it automatically
-                caller);
+        if (isSecurityGroupEnabledNetworkUsed) {
+            if (securityGroupIdList == null) {
+                securityGroupIdList = new ArrayList<Long>();
+            }
+            
+            SecurityGroup defaultGroup = _securityGroupMgr.getDefaultSecurityGroup(owner.getId());
+            if (defaultGroup != null) {
+              //check if security group id list already contains Default security group, and if not - add it
+                boolean defaultGroupPresent = false;
+                for (Long securityGroupId : securityGroupIdList) {
+                    if (securityGroupId.longValue() == defaultGroup.getId()) {
+                        defaultGroupPresent = true;
+                        break;
+                    }
+                }
+                
+                if (!defaultGroupPresent) {
+                    securityGroupIdList.add(defaultGroup.getId());
+                }
+              
+            } else {
+                //create default security group for the account
+                if (s_logger.isDebugEnabled()) {
+                    s_logger.debug("Couldn't find default security group for the account " + owner + " so creating a new one");
+                }
+                defaultGroup = _securityGroupMgr.createSecurityGroup(SecurityGroupManager.DEFAULT_GROUP_NAME, SecurityGroupManager.DEFAULT_GROUP_DESCRIPTION, owner.getDomainId(), owner.getId(), owner.getAccountName());
+                securityGroupIdList.add(defaultGroup.getId());
+            }
+        }
+        
+        return createVirtualMachine(zone, serviceOffering, template, hostName, displayName, owner, diskOfferingId, 
+                diskSize, networkList, securityGroupIdList, group, userData, sshKeyPair, hypervisor, caller);
     }
 
     @Override
@@ -2403,8 +2465,9 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
             throw new CloudRuntimeException("Unable to assign Vm to the group " + group);
         }
 
-        _networkGroupMgr.addInstanceToGroups(vm.getId(), securityGroupIdList);
+        
-
+        _securityGroupMgr.addInstanceToGroups(vm.getId(), securityGroupIdList);
+        
         return vm;
     }
 
@@ -2720,6 +2783,22 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
 
         userId = accountAndUserValidation(vmId, account, userId, vm);
         UserVO user = _userDao.findById(userId);
+        
+        //check if vm is security group enabled
+        if (_securityGroupMgr.isVmSecurityGroupEnabled(vmId) && !_securityGroupMgr.isVmMappedToDefaultSecurityGroup(vmId)) {
+            //if vm is not mapped to security group, create a mapping
+            if (s_logger.isDebugEnabled()) {
+                s_logger.debug("Vm " + vm + " is security group enabled, but not mapped to default security group; creating the mapping automatically");
+            }
+            
+            SecurityGroup defaultSecurityGroup = _securityGroupMgr.getDefaultSecurityGroup(vm.getAccountId());
+            if (defaultSecurityGroup != null) {
+                List<Long> groupList = new ArrayList<Long>();
+                groupList.add(defaultSecurityGroup.getId());
+                _securityGroupMgr.addInstanceToGroups(vmId, groupList);
+            }
+        }
+  
         return _itMgr.start(vm, null, user, account);
     }
 

server/src/com/cloud/vm/dao/NicDao.java

@@ -30,10 +30,8 @@ import com.cloud.vm.VirtualMachine;
 public interface NicDao extends GenericDao<NicVO, Long> {
     List<NicVO> listByVmId(long instanceId);
     
-    List<NicVO> listByVmIdIncludingRemoved(long instanceId);
-    
     List<String> listIpAddressInNetwork(long networkConfigId);
-    List<NicVO> listIncludingRemovedBy(long instanceId);
+    List<NicVO> listByVmIdIncludingRemoved(long instanceId);
     
     List<NicVO> listByNetworkId(long networkId);
     

server/src/com/cloud/vm/dao/NicDaoImpl.java

@@ -77,13 +77,6 @@ public class NicDaoImpl extends GenericDaoBase<NicVO, Long> implements NicDao {
     }
     
     
-    @Override
-    public List<NicVO> listIncludingRemovedBy(long instanceId) {
-        SearchCriteria<NicVO> sc = AllFieldsSearch.create();
-        sc.setParameters("instance", instanceId);
-        return listIncludingRemovedBy(sc);
-    }
-    
     @Override
     public List<String> listIpAddressInNetwork(long networkId) {
         SearchCriteria<String> sc = IpSearch.create();