From 78186c3b0201ecf55779b3f4bb6a3105fec1288d Mon Sep 17 00:00:00 2001 From: Prachi Damle Date: Wed, 22 May 2013 20:14:32 +0100 Subject: [PATCH] CLOUDSTACK-2568: ACS41 regression in storage subsystem (seen with local storage and 2 or more hosts) Patch for 4.1, 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 = "") and indicate which column this method refers to. This will cause the GenericSearchBuilder to identify the field correctly. - Also, let planner search for pools instead of selecting the one reserved - because there is no way currently to pass multiple pool information to the planner and this may cause issues when a VM has multiple disks. Signed-off-by: Chip Childers --- .../cloud/entity/api/VMEntityManagerImpl.java | 2 +- .../entity/api/db/VolumeReservationVO.java | 35 +++++-------------- .../api/db/dao/VolumeReservationDaoImpl.java | 2 +- .../src/com/cloud/deploy/FirstFitPlanner.java | 2 +- .../cloud/utils/db/GenericSearchBuilder.java | 18 ++++++---- 5 files changed, 23 insertions(+), 36 deletions(-) diff --git a/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java b/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java index 6bfe953c33c..3a87997e177 100755 --- a/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java +++ b/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java @@ -207,7 +207,7 @@ public class VMEntityManagerImpl implements VMEntityManager { } DataCenterDeployment plan = new DataCenterDeployment(vm.getDataCenterId(), vmReservation.getPodId(), vmReservation.getClusterId(), - vmReservation.getHostId(), poolId , null); + vmReservation.getHostId(), null , null); VMInstanceVO vmDeployed = _itMgr.start(vm, params, _userDao.findById(new Long(caller)), _accountDao.findById(vm.getAccountId()), plan); 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 cbad9aba568..52eeff5eed6 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,21 +34,17 @@ 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; - + @Column(name="volume_id") private long volumeId; @Column(name="pool_id") private long poolId; - - // VolumeId -> poolId - @Transient - Map 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,14 +52,14 @@ 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; this.vmReservationId = vmReservationId; } - - + + public long getId() { return id; } @@ -80,10 +68,10 @@ public class VolumeReservationVO implements InternalIdentity{ return vmId; } - public Long geVmReservationId() { + public long getVmReservationId() { return vmReservationId; } - + public long getVolumeId() { return volumeId; } @@ -92,9 +80,4 @@ public class VolumeReservationVO implements InternalIdentity{ return poolId; } - - public Map 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 index 4b1b1e667d6..1779cde1f45 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 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 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?"; } }