remove getDomainId and write the workaround code

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

api/src/main/java/com/cloud/offering/ServiceOffering.java

@@ -104,8 +104,6 @@ public interface ServiceOffering extends DiskOffering, InfrastructureEntity, Int
     @Override
     boolean isUseLocalStorage();
 
-    Long getDomainId();
-
     /**
      * @return tag that should be present on the host needed, optional parameter
      */

api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java

@@ -16,7 +16,6 @@
 // under the License.
 package org.apache.cloudstack.api.command.admin.offering;
 
-import java.util.ArrayList;
 import java.util.List;
 
 import org.apache.cloudstack.api.APICommand;
@@ -62,24 +61,17 @@ public class CreateDiskOfferingCmd extends BaseCmd {
     private Boolean customized;
 
     @Parameter(name = ApiConstants.DOMAIN_ID,
-               type = CommandType.UUID,
-               entityType = DomainResponse.class,
-               description = "the ID of the containing domain, null for public offerings")
-    private Long domainId;
-
-    @Parameter(name = ApiConstants.DOMAIN_ID_LIST,
             type = CommandType.LIST,
             collectionType = CommandType.UUID,
             entityType = DomainResponse.class,
-            description = "the ID of the domains offering is associated with, null for all domain offerings",
+            description = "the ID of the containing domain(s), null for public offerings")
-            since = "4.13")
     private List<Long> domainIds;
 
-    @Parameter(name = ApiConstants.ZONE_ID_LIST,
+    @Parameter(name = ApiConstants.ZONE_ID,
             type = CommandType.LIST,
             collectionType = CommandType.UUID,
             entityType = ZoneResponse.class,
-            description = "the ID of the zones offering is associated with, null for all zone offerings",
+            description = "the ID of the containing zone(s), null for public offerings",
             since = "4.13")
     private List<Long> zoneIds;
 
@@ -186,12 +178,6 @@ public class CreateDiskOfferingCmd extends BaseCmd {
     }
 
     public List<Long> getDomainIds() {
-        if (domainIds == null) {
-            domainIds = new ArrayList<>();
-        }
-        if (domainId != null) {
-            domainIds.add(domainId);
-        }
         return domainIds;
     }
 

api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java

@@ -18,6 +18,7 @@ package org.apache.cloudstack.api.command.admin.offering;
 
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 import org.apache.cloudstack.api.APICommand;
@@ -83,10 +84,11 @@ public class CreateServiceOfferingCmd extends BaseCmd {
     private String tags;
 
     @Parameter(name = ApiConstants.DOMAIN_ID,
-               type = CommandType.UUID,
+            type = CommandType.LIST,
-               entityType = DomainResponse.class,
+            collectionType = CommandType.UUID,
-               description = "the ID of the containing domain, null for public offerings")
+            entityType = DomainResponse.class,
-    private Long domainId;
+            description = "the ID of the containing domain(s), null for public offerings")
+    private List<Long> domainIds;
 
     @Parameter(name = ApiConstants.HOST_TAGS, type = CommandType.STRING, description = "the host tag for this service offering.")
     private String hostTag;
@@ -249,8 +251,8 @@ public class CreateServiceOfferingCmd extends BaseCmd {
         return tags;
     }
 
-    public Long getDomainId() {
+    public List<Long> getDomainId() {
-        return domainId;
+        return domainIds;
     }
 
     public String getHostTag() {

engine/schema/src/main/java/com/cloud/domain/dao/DomainDao.java

@@ -27,7 +27,7 @@ public interface DomainDao extends GenericDao<DomainVO, Long> {
 
     public DomainVO findDomainByPath(String domainPath);
 
-    public boolean isChildDomain(Long parentId, Long childId);
+    boolean isChildDomain(Long parentId, Long childId);
 
     DomainVO findImmediateChildForParent(Long parentId);
 

engine/schema/src/main/java/com/cloud/service/ServiceOfferingVO.java

@@ -106,7 +106,7 @@ public class ServiceOfferingVO extends DiskOfferingVO implements ServiceOffering
     }
 
     public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer speed, Integer rateMbps, Integer multicastRateMbps, boolean offerHA, boolean limitCpuUse,
-            boolean volatileVm, String displayText, ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse, VirtualMachine.Type vmType, Long domainId) {
+                             boolean volatileVm, String displayText, ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse, VirtualMachine.Type vmType) {
         super(name, displayText, provisioningType, false, tags, recreatable, useLocalStorage, systemUse, true);
         this.cpu = cpu;
         this.ramSize = ramSize;
@@ -120,8 +120,8 @@ public class ServiceOfferingVO extends DiskOfferingVO implements ServiceOffering
     }
 
     public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer speed, Integer rateMbps, Integer multicastRateMbps, boolean offerHA,
-            boolean limitResourceUse, boolean volatileVm, String displayText, ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse,
+                             boolean limitResourceUse, boolean volatileVm, String displayText, ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse,
-            VirtualMachine.Type vmType, Long domainId, String hostTag) {
+                             VirtualMachine.Type vmType, String hostTag) {
         this(name,
             cpu,
             ramSize,
@@ -137,14 +137,14 @@ public class ServiceOfferingVO extends DiskOfferingVO implements ServiceOffering
             recreatable,
             tags,
             systemUse,
-            vmType,
+            vmType
-            domainId);
+        );
         this.hostTag = hostTag;
     }
 
     public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer speed, Integer rateMbps, Integer multicastRateMbps, boolean offerHA,
-            boolean limitResourceUse, boolean volatileVm, String displayText, ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse,
+                             boolean limitResourceUse, boolean volatileVm, String displayText, ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse,
-            VirtualMachine.Type vmType, Long domainId, String hostTag, String deploymentPlanner) {
+                             VirtualMachine.Type vmType, String hostTag, String deploymentPlanner) {
         this(name,
             cpu,
             ramSize,
@@ -161,8 +161,7 @@ public class ServiceOfferingVO extends DiskOfferingVO implements ServiceOffering
             tags,
             systemUse,
             vmType,
-            domainId,
+                hostTag);
-            hostTag);
         this.deploymentPlanner = deploymentPlanner;
     }
 
@@ -192,12 +191,6 @@ public class ServiceOfferingVO extends DiskOfferingVO implements ServiceOffering
         vmType = offering.getSystemVmType();
     }
 
-    @Override
-    public Long getDomainId() {
-        // TODO: get rid of me
-        return null;
-    }
-
     @Override
     public boolean isOfferHA() {
         return offerHA;

engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDao.java

@@ -30,7 +30,15 @@ public interface ResourceDetailsDao<R extends ResourceDetail> extends GenericDao
      * @param name
      * @return
      */
-    public R findDetail(long resourceId, String name);
+    R findDetail(long resourceId, String name);
+
+    /**
+     * Find details by resourceId and key
+     * @param resourceId
+     * @param key
+     * @return
+     */
+    List<R> findDetails(long resourceId, String key);
 
     /**
      * Find details by key,value pair
@@ -39,7 +47,7 @@ public interface ResourceDetailsDao<R extends ResourceDetail> extends GenericDao
      * @param display
      * @return
      */
-    public List<R> findDetails(String key, String value, Boolean display);
+    List<R> findDetails(String key, String value, Boolean display);
 
     /**
      * Removes all details for the resource specified
@@ -47,12 +55,21 @@ public interface ResourceDetailsDao<R extends ResourceDetail> extends GenericDao
      */
     public void removeDetails(long resourceId);
 
+
     /**
      * Removes detail having resourceId and key specified (unique combination)
      * @param resourceId
      * @param key
      */
-    public void removeDetail(long resourceId, String key);
+    void removeDetail(long resourceId, String key);
+
+    /**
+     * Removes detail having resourceId and key, value specified (unique combination)
+     * @param resourceId
+     * @param key
+     * @param value
+     */
+    void removeDetail(long resourceId, String key, String value);
 
     /**
      * Lists all details for the resourceId

engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDaoBase.java

@@ -50,6 +50,13 @@ public abstract class ResourceDetailsDaoBase<R extends ResourceDetail> extends G
         return findOneBy(sc);
     }
 
+    public List<R> findDetails(long resourceId, String key) {
+        SearchCriteria<R> sc = AllFieldsSearch.create();
+        sc.setParameters("resourceId", resourceId);
+        sc.setParameters("name", key);
+        return listBy(sc);
+    }
+
     public List<R> findDetails(String name, String value, Boolean display) {
         SearchCriteria<R> sc = AllFieldsSearch.create();
 
@@ -107,6 +114,16 @@ public abstract class ResourceDetailsDaoBase<R extends ResourceDetail> extends G
         remove(sc);
     }
 
+    public void removeDetail(long resourceId, String key, String value) {
+        if (key != null) {
+            SearchCriteria<R> sc = AllFieldsSearch.create();
+            sc.setParameters("resourceId", resourceId);
+            sc.setParameters("name", key);
+            sc.setParameters("value", value);
+            remove(sc);
+        }
+    }
+
     public void removeDetail(long resourceId, String key) {
         if (key != null) {
             SearchCriteria<R> sc = AllFieldsSearch.create();

engine/schema/src/main/resources/META-INF/db/schema-41200to41300.sql

@@ -26,7 +26,7 @@ ALTER TABLE `cloud`.`disk_offering` DROP COLUMN `domain_id`;
 -- Disk offering with multi-domains and multi-zones
 DROP VIEW IF EXISTS `cloud`.`disk_offering_view`;
 CREATE VIEW `cloud`.`disk_offering_view` AS
-    select
+    SELECT
         `disk_offering`.`id` AS `id`,
         `disk_offering`.`uuid` AS `uuid`,
         `disk_offering`.`name` AS `name`,
@@ -60,25 +60,27 @@ CREATE VIEW `cloud`.`disk_offering_view` AS
         `disk_offering`.`type` AS `type`,
         `disk_offering`.`display_offering` AS `display_offering`,
         `disk_offering`.`state`  AS `state`,
-        GROUP_CONCAT(domain.id) AS domain_id,
+        GROUP_CONCAT(DISTINCT(domain.id)) AS domain_id,
-        GROUP_CONCAT(domain.uuid) AS domain_uuid,
+        GROUP_CONCAT(DISTINCT(domain.uuid)) AS domain_uuid,
-        GROUP_CONCAT(domain.name) AS domain_name,
+        GROUP_CONCAT(DISTINCT(domain.name)) AS domain_name,
-        GROUP_CONCAT(domain.path) AS domain_path,
+        GROUP_CONCAT(DISTINCT(domain.path)) AS domain_path,
-        GROUP_CONCAT(zone.id) AS zone_id,
+        GROUP_CONCAT(DISTINCT(zone.id)) AS zone_id,
-        GROUP_CONCAT(zone.uuid) AS zone_uuid,
+        GROUP_CONCAT(DISTINCT(zone.uuid)) AS zone_uuid,
-        GROUP_CONCAT(zone.name) AS zone_name
+        GROUP_CONCAT(DISTINCT(zone.name)) AS zone_name
-    from
+    FROM
         `cloud`.`disk_offering`
-            left join
+            LEFT JOIN
         `cloud`.`disk_offering_details` AS `domain_details` ON `domain_details`.`offering_id` = `disk_offering`.`id` AND `domain_details`.`name`='domainid'
-            left join
+            LEFT JOIN
-        `cloud`.`domain` AS `domain` ON `domain`.`id` = `domain_details`.`value`
+        `cloud`.`domain` AS `domain` ON FIND_IN_SET(`domain`.`id`, `domain_details`.`value`)
-            left join
+            LEFT JOIN
         `cloud`.`disk_offering_details` AS `zone_details` ON `zone_details`.`offering_id` = `disk_offering`.`id` AND `zone_details`.`name`='zoneid'
-            left join
+            LEFT JOIN
-        `cloud`.`data_center` AS `zone` ON `zone`.`id` = `zone_details`.`value`
+        `cloud`.`data_center` AS `zone` ON FIND_IN_SET(`zone`.`id`, `zone_details`.`value`)
-    where
+    WHERE
-        disk_offering.state='ACTIVE' GROUP BY id;
+        `disk_offering`.`state`='ACTIVE'
+    GROUP BY
+        `disk_offering`.`id`;
 
 -- Service offering with multi-domains and multi-zones
 DROP VIEW IF EXISTS `cloud`.`service_offering_view`;

framework/db/src/main/java/com/cloud/utils/db/SearchBase.java

@@ -28,15 +28,15 @@ import java.util.Map;
 import javax.persistence.Column;
 import javax.persistence.Transient;
 
-import net.sf.cglib.proxy.Factory;
-import net.sf.cglib.proxy.MethodInterceptor;
-import net.sf.cglib.proxy.MethodProxy;
-
 import com.cloud.utils.db.SearchCriteria.Func;
 import com.cloud.utils.db.SearchCriteria.Op;
 import com.cloud.utils.db.SearchCriteria.SelectType;
 import com.cloud.utils.exception.CloudRuntimeException;
 
+import net.sf.cglib.proxy.Factory;
+import net.sf.cglib.proxy.MethodInterceptor;
+import net.sf.cglib.proxy.MethodProxy;
+
 /**
  * SearchBase contains the methods that are used to build up search
  * queries.  While this class is public it's not really meant for public
@@ -410,6 +410,10 @@ public abstract class SearchBase<J extends SearchBase<?, T, K>, T, K> {
                 return;
             }
 
+            if (op == Op.FIND_IN_SET) {
+                sql.append(" FIND_IN_SET(?, ");
+            }
+
             sql.append(attr.table).append(".").append(attr.columnName).append(op.toString());
             if (op == Op.IN && params.length == 1) {
                 sql.delete(sql.length() - op.toString().length(), sql.length());

framework/db/src/main/java/com/cloud/utils/db/SearchCriteria.java

@@ -36,7 +36,7 @@ public class SearchCriteria<K> {
                 " NOT BETWEEN ? AND ? ",
                 2), IN(" IN () ", -1), NOTIN(" NOT IN () ", -1), LIKE(" LIKE ? ", 1), NLIKE(" NOT LIKE ? ", 1), NIN(" NOT IN () ", -1), NULL(" IS NULL ", 0), NNULL(
                 " IS NOT NULL ",
-                0), SC(" () ", 1), TEXT("  () ", 1), RP("", 0), AND(" AND ", 0), OR(" OR ", 0), NOT(" NOT ", 0);
+                0), SC(" () ", 1), TEXT("  () ", 1), RP("", 0), AND(" AND ", 0), OR(" OR ", 0), NOT(" NOT ", 0), FIND_IN_SET(" ) ", 1);
 
         private final String op;
         int params;
@@ -57,7 +57,7 @@ public class SearchCriteria<K> {
     }
 
     public enum Func {
-        NATIVE("@", 1), MAX("MAX(@)", 1), MIN("MIN(@)", 1), FIRST("FIRST(@)", 1), LAST("LAST(@)", 1), SUM("SUM(@)", 1), COUNT("COUNT(@)", 1), DISTINCT("DISTINCT(@)", 1), FIND_IN_SET("FIND_IN_SET(@, @)", 2);
+        NATIVE("@", 1), MAX("MAX(@)", 1), MIN("MIN(@)", 1), FIRST("FIRST(@)", 1), LAST("LAST(@)", 1), SUM("SUM(@)", 1), COUNT("COUNT(@)", 1), DISTINCT("DISTINCT(@)", 1);
 
         private String func;
         private int count;

framework/quota/src/main/java/org/apache/cloudstack/quota/vo/ServiceOfferingVO.java

@@ -284,10 +284,4 @@ public class ServiceOfferingVO extends DiskOfferingVO implements ServiceOffering
     public void setDynamicFlag(boolean isdynamic) {
         isDynamic = isdynamic;
     }
-
-    @Override
-    public Long getDomainId() {
-        // TODO: get rid of me
-        return null;
-    }
 }

server/src/main/java/com/cloud/acl/DomainChecker.java

@@ -16,6 +16,7 @@
 // under the License.
 package com.cloud.acl;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
@@ -41,6 +42,8 @@ import com.cloud.offering.DiskOffering;
 import com.cloud.offering.ServiceOffering;
 import com.cloud.projects.ProjectManager;
 import com.cloud.projects.dao.ProjectAccountDao;
+import com.cloud.service.ServiceOfferingDetailsVO;
+import com.cloud.service.dao.ServiceOfferingDetailsDao;
 import com.cloud.storage.LaunchPermissionVO;
 import com.cloud.storage.dao.LaunchPermissionDao;
 import com.cloud.template.VirtualMachineTemplate;
@@ -72,6 +75,8 @@ public class DomainChecker extends AdapterBase implements SecurityChecker {
     AccountService _accountService;
     @Inject
     DiskOfferingDetailsDao diskOfferingDetailsDao;
+    @Inject
+    ServiceOfferingDetailsDao serviceOfferingDetailsDao;
 
     protected DomainChecker() {
         super();
@@ -222,7 +227,11 @@ public class DomainChecker extends AdapterBase implements SecurityChecker {
 
     @Override
     public boolean checkAccess(Account account, ServiceOffering so) throws PermissionDeniedException {
-        if (account == null || so.getDomainId() == null) {//public offering
+        final List<Long> soDomainIds = new ArrayList<>();
+        for (final ServiceOfferingDetailsVO detail: serviceOfferingDetailsDao.findDetails(so.getId(), ApiConstants.DOMAIN_ID)) {
+            soDomainIds.add(Long.valueOf(detail.getValue()));
+        }
+        if (account == null || soDomainIds.isEmpty()) { //public offering
             return true;
         } else {
             //admin has all permissions
@@ -235,13 +244,13 @@ public class DomainChecker extends AdapterBase implements SecurityChecker {
                     || account.getType() == Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN
                     || _accountService.isDomainAdmin(account.getId())
                     || account.getType() == Account.ACCOUNT_TYPE_PROJECT) {
-                if (account.getDomainId() == so.getDomainId()) {
+                if (soDomainIds.contains(account.getDomainId())) {
                     return true; //service offering and account at exact node
                 } else {
                     Domain domainRecord = _domainDao.findById(account.getDomainId());
                     if (domainRecord != null) {
                         while (true) {
-                            if (domainRecord.getId() == so.getDomainId()) {
+                            if (soDomainIds.contains(domainRecord.getId())) {
                                 //found as a child
                                 return true;
                             }

server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java

@@ -50,12 +50,13 @@ public class DiskOfferingJoinDaoImpl extends GenericDaoBase<DiskOfferingJoinVO,
 
     @Override
     public List<DiskOfferingJoinVO> findByDomainId(long domainId) {
-        SearchBuilder<DiskOfferingJoinVO> DiskOfferingsByDomainIdSearch = createSearchBuilder();
+        SearchBuilder<DiskOfferingJoinVO> sb = createSearchBuilder();
-        DiskOfferingsByDomainIdSearch.and("domainId", DiskOfferingsByDomainIdSearch.entity().getDomainId(), SearchCriteria.Op.NNULL);
+        sb.and("domainId", sb.entity().getDomainId(), SearchCriteria.Op.FIND_IN_SET);
-        DiskOfferingsByDomainIdSearch.select("domainId", SearchCriteria.Func.FIND_IN_SET, DiskOfferingsByDomainIdSearch.entity().getDomainId(), String.valueOf(domainId));
+        sb.done();
-        DiskOfferingsByDomainIdSearch.done();
 
-        return listBy(DiskOfferingsByDomainIdSearch.create());
+        SearchCriteria<DiskOfferingJoinVO> sc = sb.create();
+        sc.setParameters("domainId", String.valueOf(domainId));
+        return listBy(sc);
     }
 
     @Override

server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java

@@ -47,11 +47,13 @@ public class ServiceOfferingJoinDaoImpl extends GenericDaoBase<ServiceOfferingJo
 
     @Override
     public List<ServiceOfferingJoinVO> findByDomainId(long domainId) {
-        SearchBuilder<ServiceOfferingJoinVO> ServiceOfferingsByDomainIdSearch = createSearchBuilder();
+        SearchBuilder<ServiceOfferingJoinVO> sb = createSearchBuilder();
-        ServiceOfferingsByDomainIdSearch.select("domainId", SearchCriteria.Func.FIND_IN_SET, ServiceOfferingsByDomainIdSearch.entity().getDomainId(), String.valueOf(domainId));
+        sb.and("domainId", sb.entity().getDomainId(), SearchCriteria.Op.FIND_IN_SET);
-        ServiceOfferingsByDomainIdSearch.done();
+        sb.done();
 
-        return listBy(ServiceOfferingsByDomainIdSearch.create());
+        SearchCriteria<ServiceOfferingJoinVO> sc = sb.create();
+        sc.setParameters("domainId", String.valueOf(domainId));
+        return listBy(sc);
     }
 
     @Override

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

@@ -2289,8 +2289,12 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
         }
 
         // check if valid domain
-        if (cmd.getDomainId() != null && _domainDao.findById(cmd.getDomainId()) == null) {
+        if (cmd.getDomainId() != null && !cmd.getDomainId().isEmpty()) {
-            throw new InvalidParameterValueException("Please specify a valid domain id");
+            for (final Long domainId: cmd.getDomainId()) {
+                if (_domainDao.findById(domainId) == null) {
+                    throw new InvalidParameterValueException("Please specify a valid domain id");
+                }
+            }
         }
 
         final Boolean offerHA = cmd.isOfferHa();
@@ -2378,7 +2382,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
 
     protected ServiceOfferingVO createServiceOffering(final long userId, final boolean isSystem, final VirtualMachine.Type vmType,
             final String name, final Integer cpu, final Integer ramSize, final Integer speed, final String displayText, final String provisioningType, final boolean localStorageRequired,
-            final boolean offerHA, final boolean limitResourceUse, final boolean volatileVm,  String tags, final Long domainId, final String hostTag,
+            final boolean offerHA, final boolean limitResourceUse, final boolean volatileVm,  String tags, final List<Long> domainIds, final String hostTag,
             final Integer networkRate, final String deploymentPlanner, final Map<String, String> details, final Boolean isCustomizedIops, Long minIops, Long maxIops,
             Long bytesReadRate, Long bytesReadRateMax, Long bytesReadRateMaxLength,
             Long bytesWriteRate, Long bytesWriteRateMax, Long bytesWriteRateMaxLength,
@@ -2393,14 +2397,16 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
         }
         final Account account = _accountDao.findById(user.getAccountId());
         if (account.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) {
-            if (domainId == null) {
+            if (domainIds == null || domainIds.isEmpty()) {
                 throw new InvalidParameterValueException("Unable to create public service offering by id " + userId + " because it is domain-admin");
             }
             if (tags != null || hostTag != null) {
                 throw new InvalidParameterValueException("Unable to create service offering with storage tags or host tags by id " + userId + " because it is domain-admin");
             }
-            if (! _domainDao.isChildDomain(account.getDomainId(), domainId)) {
+            for (Long domainId: domainIds) {
-                throw new InvalidParameterValueException("Unable to create service offering by another domain admin with id " + userId);
+                if (! _domainDao.isChildDomain(account.getDomainId(), domainId)) {
+                    throw new InvalidParameterValueException("Unable to create service offering by another domain admin with id " + userId);
+                }
             }
         } else if (account.getType() != Account.ACCOUNT_TYPE_ADMIN) {
             throw new InvalidParameterValueException("Unable to create service offering by id " + userId + " because it is not root-admin or domain-admin");
@@ -2412,7 +2418,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
 
         ServiceOfferingVO offering = new ServiceOfferingVO(name, cpu, ramSize, speed, networkRate, null, offerHA,
                 limitResourceUse, volatileVm, displayText, typedProvisioningType, localStorageRequired, false, tags, isSystem, vmType,
-                domainId, hostTag, deploymentPlanner);
+                hostTag, deploymentPlanner);
 
         if (Boolean.TRUE.equals(isCustomizedIops) || isCustomizedIops == null) {
                 minIops = null;
@@ -2484,7 +2490,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
 
         offering.setHypervisorSnapshotReserve(hypervisorSnapshotReserve);
 
-        List<ServiceOfferingDetailsVO> detailsVO = null;
+        List<ServiceOfferingDetailsVO> detailsVO = new ArrayList<ServiceOfferingDetailsVO>();
         if (details != null) {
             // To have correct input, either both gpu card name and VGPU type should be passed or nothing should be passed.
             // Use XOR condition to verify that.
@@ -2493,7 +2499,6 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
             if ((entry1 || entry2) && !(entry1 && entry2)) {
                 throw new InvalidParameterValueException("Please specify the pciDevice and vgpuType correctly.");
             }
-            detailsVO = new ArrayList<ServiceOfferingDetailsVO>();
             for (final Entry<String, String> detailEntry : details.entrySet()) {
                 if (detailEntry.getKey().equals(GPU.Keys.pciDevice.toString())) {
                     if (detailEntry.getValue() == null) {
@@ -2510,9 +2515,14 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
         }
 
         if ((offering = _serviceOfferingDao.persist(offering)) != null) {
-            if (detailsVO != null && !detailsVO.isEmpty()) {
+            if (domainIds != null && !domainIds.isEmpty()) {
-                for (int index = 0; index < detailsVO.size(); index++) {
+                for (Long domainId: domainIds) {
-                    detailsVO.get(index).setResourceId(offering.getId());
+                    detailsVO.add(new ServiceOfferingDetailsVO(offering.getId(), ApiConstants.DOMAIN_ID, String.valueOf(domainId), true));
+                }
+            }
+            if (!detailsVO.isEmpty()) {
+                for (ServiceOfferingDetailsVO detail: detailsVO) {
+                    detail.setResourceId(offering.getId());
                 }
                 _serviceOfferingDetailsDao.saveDetails(detailsVO);
             }
@@ -2549,11 +2559,15 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
         }
         final Account account = _accountDao.findById(user.getAccountId());
         if (account.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) {
-            if (offeringHandle.getDomainId() == null) {
+            final List<ServiceOfferingDetailsVO> details = _serviceOfferingDetailsDao.findDetails(offeringHandle.getId(), ApiConstants.DOMAIN_ID);
+            if (details.isEmpty()) {
                 throw new InvalidParameterValueException("Unable to update public service offering by id " + userId + " because it is domain-admin");
             }
-            if (! _domainDao.isChildDomain(account.getDomainId(), offeringHandle.getDomainId() )) {
+            for (final ServiceOfferingDetailsVO detail : details) {
-                throw new InvalidParameterValueException("Unable to update service offering by another domain admin with id " + userId);
+                final Long domainId = Long.valueOf(detail.getValue(), 0);
+                if (!_domainDao.isChildDomain(account.getDomainId(), domainId)) {
+                    throw new InvalidParameterValueException("Unable to update service offering by another domain admin with id " + userId);
+                }
             }
         } else if (account.getType() != Account.ACCOUNT_TYPE_ADMIN) {
             throw new InvalidParameterValueException("Unable to update service offering by id " + userId + " because it is not root-admin or domain-admin");
@@ -2997,11 +3011,15 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
         }
         final Account account = _accountDao.findById(user.getAccountId());
         if (account.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) {
-            if (offering.getDomainId() == null) {
+            final List<ServiceOfferingDetailsVO> details = _serviceOfferingDetailsDao.findDetails(offering.getId(), ApiConstants.DOMAIN_ID);
+            if (details.isEmpty()) {
                 throw new InvalidParameterValueException("Unable to delete public service offering by id " + userId + " because it is domain-admin");
             }
-            if (! _domainDao.isChildDomain(account.getDomainId(), offering.getDomainId() )) {
+            for (final ServiceOfferingDetailsVO detail : details) {
-                throw new InvalidParameterValueException("Unable to delete service offering by another domain admin with id " + userId);
+                final Long domainId = Long.valueOf(detail.getValue(), 0);
+                if (!_domainDao.isChildDomain(account.getDomainId(), domainId)) {
+                    throw new InvalidParameterValueException("Unable to delete service offering by another domain admin with id " + userId);
+                }
             }
         } else if (account.getType() != Account.ACCOUNT_TYPE_ADMIN) {
             throw new InvalidParameterValueException("Unable to delete service offering by id " + userId + " because it is not root-admin or domain-admin");

server/src/main/java/com/cloud/user/DomainManagerImpl.java

@@ -63,7 +63,9 @@ import com.cloud.network.dao.NetworkDomainDao;
 import com.cloud.projects.ProjectManager;
 import com.cloud.projects.ProjectVO;
 import com.cloud.projects.dao.ProjectDao;
+import com.cloud.service.dao.ServiceOfferingDao;
 import com.cloud.service.dao.ServiceOfferingDetailsDao;
+import com.cloud.storage.dao.DiskOfferingDao;
 import com.cloud.user.dao.AccountDao;
 import com.cloud.utils.Pair;
 import com.cloud.utils.component.ManagerBase;
@@ -97,10 +99,14 @@ public class DomainManagerImpl extends ManagerBase implements DomainManager, Dom
     @Inject
     private DiskOfferingJoinDao diskOfferingJoinDao;
     @Inject
+    private DiskOfferingDao diskOfferingDao;
+    @Inject
     private DiskOfferingDetailsDao diskOfferingDetailsDao;
     @Inject
     private ServiceOfferingJoinDao serviceOfferingJoinDao;
     @Inject
+    private ServiceOfferingDao serviceOfferingDao;
+    @Inject
     private ServiceOfferingDetailsDao serviceOfferingDetailsDao;
     @Inject
     private ProjectDao _projectDao;
@@ -446,22 +452,23 @@ public class DomainManagerImpl extends ManagerBase implements DomainManager, Dom
             return;
         }
 
+        String domainIdString = String.valueOf(domainId);
         List<Long> diskOfferingsDetailsToRemove = new ArrayList<>();
         List<Long> serviceOfferingsDetailsToRemove = new ArrayList<>();
 
         // delete the service and disk offerings associated with this domain
         List<DiskOfferingJoinVO> diskOfferingsForThisDomain = diskOfferingJoinDao.findByDomainId(domainId);
         for (DiskOfferingJoinVO diskOffering : diskOfferingsForThisDomain) {
-            if (String.valueOf(domainId).equals(diskOffering.getDomainId())) {
+            if (domainIdString.equals(diskOffering.getDomainId())) {
-                diskOfferingJoinDao.remove(diskOffering.getId());
+                diskOfferingDao.remove(diskOffering.getId());
             } else {
                 diskOfferingsDetailsToRemove.add(diskOffering.getId());
             }
         }
         List<ServiceOfferingJoinVO> serviceOfferingsForThisDomain = serviceOfferingJoinDao.findByDomainId(domainId);
         for (ServiceOfferingJoinVO serviceOffering : serviceOfferingsForThisDomain) {
-            if (String.valueOf(domainId).equals(serviceOffering.getDomainId())) {
+            if (domainIdString.equals(serviceOffering.getDomainId())) {
-                serviceOfferingJoinDao.remove(serviceOffering.getId());
+                serviceOfferingDao.remove(serviceOffering.getId());
             } else {
                 serviceOfferingsDetailsToRemove.add(serviceOffering.getId());
             }
@@ -469,10 +476,10 @@ public class DomainManagerImpl extends ManagerBase implements DomainManager, Dom
 
         // Remove domain IDs for offerings which may be multi-domain
         for (final Long diskOfferingId : diskOfferingsDetailsToRemove) {
-            diskOfferingDetailsDao.removeDetail(diskOfferingId, ApiConstants.DOMAIN_ID);
+            diskOfferingDetailsDao.removeDetail(diskOfferingId, ApiConstants.DOMAIN_ID, domainIdString);
         }
         for (final Long serviceOfferingId : serviceOfferingsDetailsToRemove) {
-            serviceOfferingDetailsDao.removeDetail(serviceOfferingId, ApiConstants.DOMAIN_ID);
+            serviceOfferingDetailsDao.removeDetail(serviceOfferingId, ApiConstants.DOMAIN_ID, domainIdString);
         }
     }
 

server/src/test/java/com/cloud/vm/DeploymentPlanningManagerImplTest.java

@@ -143,7 +143,6 @@ public class DeploymentPlanningManagerImplTest {
     @Mock
     Host host;
 
-    private static long domainId = 5L;
     private static long dataCenterId = 1L;
     private static long hostId = 1l;
 
@@ -186,8 +185,8 @@ public class DeploymentPlanningManagerImplTest {
     public void dataCenterAvoidTest() throws InsufficientServerCapacityException, AffinityConflictException {
         ServiceOfferingVO svcOffering =
             new ServiceOfferingVO("testOffering", 1, 512, 500, 1, 1, false, false, false, "test dpm",
-                    ProvisioningType.THIN, false, false, null, false, VirtualMachine.Type.User, domainId,
+                    ProvisioningType.THIN, false, false, null, false, VirtualMachine.Type.User,
-                null, "FirstFitPlanner");
+                    null, "FirstFitPlanner");
         Mockito.when(vmProfile.getServiceOffering()).thenReturn(svcOffering);
 
         DataCenterDeployment plan = new DataCenterDeployment(dataCenterId);
@@ -201,8 +200,8 @@ public class DeploymentPlanningManagerImplTest {
     public void plannerCannotHandleTest() throws InsufficientServerCapacityException, AffinityConflictException {
         ServiceOfferingVO svcOffering =
             new ServiceOfferingVO("testOffering", 1, 512, 500, 1, 1, false, false, false, "test dpm",
-                    ProvisioningType.THIN, false, false, null, false, VirtualMachine.Type.User, domainId,
+                    ProvisioningType.THIN, false, false, null, false, VirtualMachine.Type.User,
-                null, "UserDispersingPlanner");
+                    null, "UserDispersingPlanner");
         Mockito.when(vmProfile.getServiceOffering()).thenReturn(svcOffering);
 
         DataCenterDeployment plan = new DataCenterDeployment(dataCenterId);
@@ -217,8 +216,8 @@ public class DeploymentPlanningManagerImplTest {
     public void emptyClusterListTest() throws InsufficientServerCapacityException, AffinityConflictException {
         ServiceOfferingVO svcOffering =
             new ServiceOfferingVO("testOffering", 1, 512, 500, 1, 1, false, false, false, "test dpm",
-                ProvisioningType.THIN, false, false, null, false, VirtualMachine.Type.User, domainId,
+                ProvisioningType.THIN, false, false, null, false, VirtualMachine.Type.User,
-                null, "FirstFitPlanner");
+                    null, "FirstFitPlanner");
         Mockito.when(vmProfile.getServiceOffering()).thenReturn(svcOffering);
 
         DataCenterDeployment plan = new DataCenterDeployment(dataCenterId);