CLOUDSTACK-2568: ACS41 regression in storage subsystem (seen with local storage and 2 or more hosts)

Prachi Damle · Prachi Damle · commit dce42581710c · 2013-05-21T16:06:29.000-07:00
Changes: - In VolumeReservationVO, the getter method of a column had a typo, causing us to create a wrong searchbuilder. It was searching over the 'id' column instead of 'vm_reservation_id' causing - This bug was causing the vm deployment to choose a wrong pool during deployment since the search was choosing incorrectly - This bug in the GenericSearchBuilder is also fixed - if the getter method does not use the standard 'get' or 'is' prefix, one should annotate that method using @column(name = "<column_name>") and indicate which column this method refers to. This will cause the GenericSearchBuilder to identify the field correctly.
diff --git a/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/db/VolumeReservationVO.java b/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/db/VolumeReservationVO.java
@@ -16,22 +16,14 @@
 // under the License.
 package org.apache.cloudstack.engine.cloud.entity.api.db;
 
-import java.util.Date;
-import java.util.Map;
-
 import javax.persistence.Column;
 import javax.persistence.Entity;
 import javax.persistence.GeneratedValue;
 import javax.persistence.GenerationType;
 import javax.persistence.Id;
 import javax.persistence.Table;
-import javax.persistence.Transient;
-
-import org.apache.cloudstack.api.Identity;
 import org.apache.cloudstack.api.InternalIdentity;
 
-import com.cloud.utils.db.GenericDao;
-
 @Entity
 @Table(name = "volume_reservation")
 public class VolumeReservationVO implements InternalIdentity{
@@ -42,7 +34,7 @@ public class VolumeReservationVO implements InternalIdentity{
     private long id;
 
     @Column(name = "vm_reservation_id")
-    private Long vmReservationId;
+    private long vmReservationId;
 
     @Column(name = "vm_id")
     private long vmId;
@@ -53,18 +45,14 @@ public class VolumeReservationVO implements InternalIdentity{
     @Column(name="pool_id")
     private long poolId;
 
-    // VolumeId -> poolId
-    @Transient
-    Map<String, String> volumeReservationMap;
-
     /**
      * There should never be a public constructor for this class. Since it's
      * only here to define the table for the DAO class.
      */
     protected VolumeReservationVO() {
     }
 
-    public VolumeReservationVO(long vmId, long volumeId, long poolId, Long vmReservationId) {
+    public VolumeReservationVO(long vmId, long volumeId, long poolId, long vmReservationId) {
         this.vmId = vmId;
         this.volumeId = volumeId;
         this.poolId = poolId;
@@ -80,7 +68,7 @@ public long getVmId() {
         return vmId;
     }
 
-    public Long geVmReservationId() {
+    public long getVmReservationId() {
         return vmReservationId;
     }
 
@@ -93,8 +81,4 @@ public Long getPoolId() {
     }
 
 
-    public Map<String,String> getVolumeReservation(){
-        return volumeReservationMap;
-    }
-
 }
diff --git a/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/db/dao/VolumeReservationDaoImpl.java b/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/db/dao/VolumeReservationDaoImpl.java
@@ -49,7 +49,7 @@ public void init() {
         VmIdSearch.done();
 
         VmReservationIdSearch = createSearchBuilder();
-        VmReservationIdSearch.and("vmReservationId", VmReservationIdSearch.entity().geVmReservationId(), SearchCriteria.Op.EQ);
+        VmReservationIdSearch.and("vmReservationId", VmReservationIdSearch.entity().getVmReservationId(), SearchCriteria.Op.EQ);
         VmReservationIdSearch.done();
     }
 
diff --git a/utils/src/com/cloud/utils/db/GenericSearchBuilder.java b/utils/src/com/cloud/utils/db/GenericSearchBuilder.java
@@ -23,6 +23,7 @@
 import java.util.List;
 import java.util.Map;
 
+import javax.persistence.Column;
 import javax.persistence.Transient;
 
 import net.sf.cglib.proxy.Factory;
@@ -163,13 +164,16 @@ public Object intercept(Object object, Method method, Object[] args, MethodProxy
 				set(fieldName);
 				return null;
 			} else {
-				name = name.toLowerCase();
-				for (String fieldName : _attrs.keySet()) {
-					if (name.endsWith(fieldName.toLowerCase())) {
-						set(fieldName);
-						return null;
-					}
-				}
+			    Column ann = method.getAnnotation(Column.class);
+			    if (ann != null) {
+			        String colName = ann.name();
+			        for (Map.Entry<String, Attribute> attr : _attrs.entrySet()) {
+			            if (colName.equals(attr.getValue().columnName)) {
+			                set(attr.getKey());
+			                return null;
+			            }
+			        }
+			    }
 				assert false : "Perhaps you need to make the method start with get or is?";
 			}
 		}

Original file line number	Diff line number	Diff line change
`@@ -49,7 +49,7 @@ public void init() {`
`49`	`49`	`VmIdSearch.done();`
`50`	`50`
`51`	`51`	`VmReservationIdSearch = createSearchBuilder();`
`52`		`- VmReservationIdSearch.and("vmReservationId", VmReservationIdSearch.entity().geVmReservationId(), SearchCriteria.Op.EQ);`
	`52`	`+ VmReservationIdSearch.and("vmReservationId", VmReservationIdSearch.entity().getVmReservationId(), SearchCriteria.Op.EQ);`
`53`	`53`	`VmReservationIdSearch.done();`
`54`	`54`	`}`
`55`	`55`