CLOUDSTACK-2568: ACS41 regression in storage subsystem (seen with local storage and 2 or more hosts)
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. Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/dce42581 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/dce42581 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/dce42581 Branch: refs/heads/rbd-snap-clone Commit: dce42581710ce3613f4bf765d713fab9552747ca Parents: 0a44369 Author: Prachi Damle <pra...@cloud.com> Authored: Tue May 21 16:05:07 2013 -0700 Committer: Prachi Damle <pra...@cloud.com> Committed: Tue May 21 16:06:29 2013 -0700 ---------------------------------------------------------------------- .../cloud/entity/api/db/VolumeReservationVO.java | 22 ++------------- .../api/db/dao/VolumeReservationDaoImpl.java | 2 +- .../com/cloud/utils/db/GenericSearchBuilder.java | 18 +++++++----- 3 files changed, 15 insertions(+), 27 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/dce42581/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/db/VolumeReservationVO.java ---------------------------------------------------------------------- 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 index f064623..e858f74 100644 --- 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,10 +45,6 @@ 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. @@ -64,7 +52,7 @@ public class VolumeReservationVO implements InternalIdentity{ 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 class VolumeReservationVO implements InternalIdentity{ return vmId; } - public Long geVmReservationId() { + public long getVmReservationId() { return vmReservationId; } @@ -93,8 +81,4 @@ public class VolumeReservationVO implements InternalIdentity{ } - public Map<String,String> getVolumeReservation(){ - return volumeReservationMap; - } - } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/dce42581/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/db/dao/VolumeReservationDaoImpl.java ---------------------------------------------------------------------- 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 index 26bc65f..4f3761b 100644 --- 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 class VolumeReservationDaoImpl extends GenericDaoBase<VolumeReservationVO VmIdSearch.done(); VmReservationIdSearch = createSearchBuilder(); - VmReservationIdSearch.and("vmReservationId", VmReservationIdSearch.entity().geVmReservationId(), SearchCriteria.Op.EQ); + VmReservationIdSearch.and("vmReservationId", VmReservationIdSearch.entity().getVmReservationId(), SearchCriteria.Op.EQ); VmReservationIdSearch.done(); } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/dce42581/utils/src/com/cloud/utils/db/GenericSearchBuilder.java ---------------------------------------------------------------------- diff --git a/utils/src/com/cloud/utils/db/GenericSearchBuilder.java b/utils/src/com/cloud/utils/db/GenericSearchBuilder.java index 8bbe07c..337d444 100755 --- 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.HashMap; 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 class GenericSearchBuilder<T, K> implements MethodInterceptor { 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?"; } }