Details
-
Bug
-
Status: Resolved
-
Critical
-
Resolution: Fixed
-
4.10.0.0, 4.9.2.0, 4.5.2.2, 4.9.3.0
-
Security Level: Public (Anyone can view this level - this is the default.)
-
None
-
Production environment running ACS 4.5.2.2, 4.9.2.0, 4.9.3.0
XenServer 6.5
Primary Storage NFS and ISCSI
Fresh install running 4.5.2.2, 4.9.2.0, 4.9.3.0 and 4.10
Description
If we have two or more snapshots of a ROOT disk and try to create a template from differencial snapshots, CloudStack not merging the vhd files on secondary storage.
Steps to reproduce
1. Create virtual machine - OK
2. Create a manual snapshot from ROOT volume - OK
The vhd of first snapshot (snapshot-base) is stored on secondary storage correctly
[root@acs01 6]# ls -l /exports/secondary/snapshots/2/6/011a7650-a13f-4332-8c72-2428a002affc.vhd
rw-rr- 1 root root 1767191040 Nov 1 12:12 /exports/secondary/snapshots/2/6/011a7650-a13f-4332-8c72-2428a002affc.vhd
[root@acs01 6]# md5sum /exports/secondary/snapshots/2/6/011a7650-a13f-4332-8c72-2428a002affc.vhd
240e5e424552d44b5d297895b064beee /exports/secondary/snapshots/2/6/011a7650-a13f-4332-8c72-2428a002affc.vhd
3. Create a template from snapshot-base - OK
[root@acs01 6]# ls -l /exports/secondary/template/tmpl/2/206/be3311d9-8e59-4240-968d-73d7a6eecfdd.vhd
rw-rr- 1 root root 1767191040 Nov 1 12:17 /exports/secondary/template/tmpl/2/206/be3311d9-8e59-4240-968d-73d7a6eecfdd.vhd
[root@acs01 6]# md5sum /exports/secondary/template/tmpl/2/206/be3311d9-8e59-4240-968d-73d7a6eecfdd.vhd
240e5e424552d44b5d297895b064beee /exports/secondary/template/tmpl/2/206/be3311d9-8e59-4240-968d-73d7a6eecfdd.vhd
This template from snapshot-base boot normally
4. Create new file (1GB) on ROOT volume
5. Create new snapshot (snapshot-diff) from ROOT volume - OK
[root@acs01 6]# ls -l /exports/secondary/snapshots/2/6/
total 2840056
rw-rr- 1 root root 1767191040 Nov 1 12:12 011a7650-a13f-4332-8c72-2428a002affc.vhd <- snapshot-base
rw-rr- 1 root root 1141019136 Nov 1 12:21 1503adaa-4f6e-4f9c-8d80-68663baa86b8.vhd <- snapshot-diff
6. Create a template from second snapshot - NOK
[root@acs01 6]# ls -lh /exports/secondary/template/tmpl/2/207/4c5c51a1-fcb4-4cc8-8f97-1664a63af262.vhd
rw-rr- 1 root root 1141019136 Nov 1 12:30 /exports/secondary/template/tmpl/2/207/4c5c51a1-fcb4-4cc8-8f97-1664a63af262.vhd
CloudStack not merging snapshot-base and snapshot-diff. Template from second snapshot is corrupted because is based just on differential vhd.
Volume from snapshot work very well
1. Create a volume from snapshot-diff - OK
CloudStack merge two snapshots in one file
[root@acs01 primary]# ls -l /exports/primary/70f1db23-cc59-4ec7-b8d4-ed036228593b.vhd
rw-rr- 1 root root 2843030016 Nov 1 12:34 /exports/primary/70f1db23-cc59-4ec7-b8d4-ed036228593b.vhd
2. Download the new volume to check vhd on secondary storage
[root@acs01 9]# ls -l /exports/secondary/volumes/2/9/ec6e0f65-bb71-4c91-bac3-287cde2106b7.vhd
rw-rr- 1 root root 2843030016 Nov 1 12:35 /exports/secondary/volumes/2/9/ec6e0f65-bb71-4c91-bac3-287cde2106b7.vhd
We test on 4.5.2.2, 4.9.2.0, 4.9.3.0 and 4.10. XenServer 6.5 with SP1 and XenServer 6.5 with all patches.
A workaround is create a volume (data-disk) from latest snapshot, create new snapshot from data-disk and convert to template.
Attachments
Issue Links
- links to
Activity
Obrigado Rafael
Feliz ano novo pra ti
Abraço
–
Marcelo Lima
Mobile: +55 51 99630 4055 | Phone: +55 51 3465 6916 | Fax: +55 51 3465 6916
Rua Brasil 1100/24, Canoas, RS - Brazil - CEP 92310-150
mlima@lotic.com.br | www.lotic.com.br | Twitter:@LoticArq
rafaelweingartner opened a new pull request #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402
@mike-tutkowski found a problem in #2315 when converting a snapshot to a template from a storage pool that has been deleted. Following his feedback in #2298 I am creating this PR to address the issue.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
rafaelweingartner commented on issue #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402#issuecomment-356916761
@DaanHoogland, @rhtyd, and @mike-tutkowski can you review these changes?
I believe that the problem described by Mike in #2298 can be considered a blocker, and this PR is fixing that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
rhtyd commented on issue #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402#issuecomment-356918259
@rafaelweingartner do you mean it's blocker for 4.11 or for #2298 , we're already past freeze now and we should not be accepting any feature PRs now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
rafaelweingartner commented on issue #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402#issuecomment-356919058
Hmm, that is a good question.
This affects only XenServers and the case that is broken is the following.
> If you have a snapshot from a storage pool that has been deleted, you will not be able to create a volume or template from it.
It feels like a critical problem, but I am not sure now if it is a blocker or not.
Do we have a place where the concept of blocker is described?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
rhtyd commented on issue #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402#issuecomment-356922041
@rafaelweingartner I think you've answered yourself. Wrt a release, I would consider blocker to be something that would literally stop some part or the whole of CloudStack to be working (installation, upgrade failures included). We can always fix rest of the bugs as part of a minor (for example the next, 4.11.1.0) release.
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
blueorangutan commented on issue #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402#issuecomment-356922073
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
rafaelweingartner commented on issue #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402#issuecomment-356922299
Ah ok. Thanks for the feedback.
I will leave the PR open, so we can work on it after 4.11
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
blueorangutan commented on issue #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402#issuecomment-356928436
Packaging result: ✖centos6 ✔centos7 ✖debian. JID-1644
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
mike-tutkowski commented on issue #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402#issuecomment-357002394
This should definitely at least go into a maintenance release for 4.11.
I can test this out soon.
Thanks, @rafaelweingartner!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
rafaelweingartner commented on issue #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402#issuecomment-357004830
@mike-tutkowski Welcome!
I should thank you for the help
Travis is failing, but I have not yet figured out why. It looks like a timeout thing.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
mike-tutkowski commented on issue #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402#issuecomment-357005067
@rafaelweingartner I have the same problem with Travis on my PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
rhtyd commented on issue #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402#issuecomment-357519652
@rafaelweingartner @mike-tutkowski /cc @DaanHoogland and others - can you comment if this should be considered for 4.11.0.0, is this blocker/critical from your perspective if we merge/accept #2298 ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
rhtyd commented on issue #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402#issuecomment-357626214
Okay @mike-tutkowski @rafaelweingartner @DaanHoogland I've moved this to 4.11.1 milestone.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
DaanHoogland commented on issue #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402#issuecomment-360055719
@rafaelweingartner no extra PR please, we will merge forward 4.11 after merge of any fixes
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
rafaelweingartner commented on issue #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402#issuecomment-366796206
reviewers (@DaanHoogland, @mike-tutkowski, @rhtyd ) what is your positions here? It seems that the tests passed and we have a consensus. If you are ok with this one, I would like to proceed and merge this to 4.11 and then forward to 4.12 (master branch)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
mike-tutkowski commented on issue #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402#issuecomment-366797995
Yes, per my prior comments, the code LGTM and the two tests that previously failed now pass.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
rafaelweingartner commented on issue #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402#issuecomment-366798899
@mike-tutkowski thanks!
I am getting used to look directly to the "reviweres" section on Github and checking the approved ones. I did not seen any for this PR, that is why I asked again.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
mike-tutkowski commented on issue #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402#issuecomment-366800222
Sounds good - we should all get used to marking that field.
On Feb 19, 2018, at 12:46 PM, Rafael Weingärtner <notifications@github.com<notifications@github.com>> wrote:
@mike-tutkowski<https://github.com/mike-tutkowski> thanks!
I am getting used to look directly to the "reviweres" section on Github and checking the approved ones. I did not seen any for this PR, that is why I asked again.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<https://github.com/apache/cloudstack/pull/2402#issuecomment-366798899>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AC4SHxDTguzYeKeTROs1yhRE0tmZFpSMks5tWd2pgaJpZM4RasNN>.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
rafaelweingartner commented on issue #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402#issuecomment-366800251
Since everything is ok, I will merge this one.
Let's see if I still know how to do it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
rafaelweingartner closed pull request #2402: CLOUDSTACK-10128 Simplify the search for host to execute command
URL: https://github.com/apache/cloudstack/pull/2402
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/engine/schema/src/com/cloud/host/dao/HostDao.java b/engine/schema/src/com/cloud/host/dao/HostDao.java
index 9e967e34efd..71c9cd23867 100644
— a/engine/schema/src/com/cloud/host/dao/HostDao.java
+++ b/engine/schema/src/com/cloud/host/dao/HostDao.java
@@ -19,13 +19,12 @@
import java.util.Date;
import java.util.List;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
-
import com.cloud.host.Host;
import com.cloud.host.Host.Type;
import com.cloud.host.HostVO;
import com.cloud.host.Status;
import com.cloud.hypervisor.Hypervisor;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
import com.cloud.info.RunningHostCountInfo;
import com.cloud.resource.ResourceState;
import com.cloud.utils.db.GenericDao;
@@ -99,14 +98,13 @@
HostVO findByIp(String ip);
/**
- * This method will look for a host that is of the same hypervisor and same zone as the storage pool where the volume of the Snapshot is stored.
+ * This method will look for a host that is of the same hypervisor and zone as indicated in its parameters.
- <ul>
- * <li>(this is applicable only for XenServer) If the storage pool is managed, then we will look for a host that has the property 'supportsResign' in cloud.cluster_details
- <li>We give priority to 'Enabled' hosts, but if no 'Enabled' hosts are found, we use 'Disabled' hosts
- <li>If no host is found, we throw a runtime exception
- </ul>
* - Side note: this method is currently only used in XenServerGuru; therefore, it was designed to meet XenServer deployment scenarios requirements.
*/
- HostVO findHostToOperateOnSnapshotBasedOnStoragePool(StoragePoolVO storagePoolVO);
+ HostVO findHostInZoneToExecuteCommand(long zoneId, HypervisorType hypervisorType);
}
diff --git a/engine/schema/src/com/cloud/host/dao/HostDaoImpl.java b/engine/schema/src/com/cloud/host/dao/HostDaoImpl.java
index c1403d08609..e48bfd58a6a 100644-
- a/engine/schema/src/com/cloud/host/dao/HostDaoImpl.java
+++ b/engine/schema/src/com/cloud/host/dao/HostDaoImpl.java
@@ -24,14 +24,13 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.TimeZone;
- a/engine/schema/src/com/cloud/host/dao/HostDaoImpl.java
-
import javax.annotation.PostConstruct;
import javax.inject.Inject;
import javax.persistence.TableGenerator;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
-import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
@@ -50,6 +49,7 @@
import com.cloud.host.Status;
import com.cloud.host.Status.Event;
import com.cloud.hypervisor.Hypervisor;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
import com.cloud.info.RunningHostCountInfo;
import com.cloud.org.Grouping;
import com.cloud.org.Managed;
@@ -1168,25 +1168,23 @@ public HostVO findByIp(final String ipAddress)
- String sqlFindHostConnectedToStoragePoolToExecuteCommand = "select h.id from storage_pool pool "
- + " join cluster c on pool.cluster_id = c.id "
- + " %s "
- + " join host h on h.data_center_id = c.data_center_id and h.hypervisor_type = c.hypervisor_type"
- + " where pool.id = ? and h.status = 'Up' and h.type = 'Routing' and resource_state = '%s' "
- + " ORDER by rand() limit 1 ";
+ String sqlFindHostInZoneToExecuteCommand = "Select id from host "
+ + " where type = 'Routing' and hypervisor_type = ? and data_center_id = ? and status = 'Up' "
+ + " and resource_state = '%s' "
+ + " ORDER by rand() limit 1";
@Override
- public HostVO findHostToOperateOnSnapshotBasedOnStoragePool(StoragePoolVO storagePoolVO) {
+ public HostVO findHostInZoneToExecuteCommand(long zoneId, HypervisorType hypervisorType) {
try (TransactionLegacy tx = TransactionLegacy.currentTxn()) { - String sql = createSqlFindHostConnectedToStoragePoolToExecuteCommand(storagePoolVO, false);
- ResultSet rs = executeSqlGetResultSetForMethodFindHostToOperateBasedOnStoragePool(storagePoolVO, tx, sql);
+ String sql = createSqlFindHostToExecuteCommand(false);
+ ResultSet rs = executeSqlGetResultsetForMethodFindHostInZoneToExecuteCommand(hypervisorType, zoneId, tx, sql);
if (rs.next()) { return findById(rs.getLong("id")); } - sql = createSqlFindHostConnectedToStoragePoolToExecuteCommand(storagePoolVO, true);
- rs = executeSqlGetResultSetForMethodFindHostToOperateBasedOnStoragePool(storagePoolVO, tx, sql);
+ sql = createSqlFindHostToExecuteCommand(true);
+ rs = executeSqlGetResultsetForMethodFindHostInZoneToExecuteCommand(hypervisorType, zoneId, tx, sql);
if (!rs.next()) { - throw new CloudRuntimeException(String.format("Could not find a host connected to the storage pool [storagepool=%d]. ", storagePoolVO.getId())); + throw new CloudRuntimeException(String.format("Could not find a host in zone [zoneId=%d] to operate on. ", zoneId)); }return findById(rs.getLong("id"));
{ @@ -1194,21 +1192,18 @@ public HostVO findHostToOperateOnSnapshotBasedOnStoragePool(StoragePoolVO storag }
} catch (SQLException e)}
- private ResultSet executeSqlGetResultSetForMethodFindHostToOperateBasedOnStoragePool(StoragePoolVO storagePoolVO, TransactionLegacy tx, String sql) throws SQLException {
+ private ResultSet executeSqlGetResultsetForMethodFindHostInZoneToExecuteCommand(HypervisorType hypervisorType, long zoneId, TransactionLegacy tx, String sql) throws SQLException { PreparedStatement pstmt = tx.prepareAutoCloseStatement(sql); - pstmt.setLong(1, storagePoolVO.getId()); + pstmt.setString(1, Objects.toString(hypervisorType)); + pstmt.setLong(2, zoneId); return pstmt.executeQuery(); }
- private String createSqlFindHostConnectedToStoragePoolToExecuteCommand(StoragePoolVO storagePoolVO, boolean useDisabledHosts) {
+ private String createSqlFindHostToExecuteCommand(boolean useDisabledHosts) {
String hostResourceStatus = "Enabled";
if (useDisabledHosts) { hostResourceStatus = "Disabled"; } - String joinForManagedStorage = StringUtils.EMPTY;
- if (storagePoolVO.isManaged()) { - joinForManagedStorage = " join cluster_details cd on cd.cluster_id = c.id and cd.name = 'supportsResign' and cd.value = 'true' "; - }
- return String.format(sqlFindHostConnectedToStoragePoolToExecuteCommand, joinForManagedStorage, hostResourceStatus);
+ return String.format(sqlFindHostInZoneToExecuteCommand, hostResourceStatus);
}
}
diff --git a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java
index e665906c289..2398e91c90c 100644-
- a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java
+++ b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java
@@ -123,11 +123,4 @@
List<StoragePoolVO> listLocalStoragePoolByPath(long datacenterId, String path);
- a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java
-
void deletePoolTags(long poolId);
-
- /**
- * Looks for a storage pool where the original volume of the snapshot was taken.
- * Even if the volume has already been deleted, we will return the last storage pool where it was stored.
- */
- StoragePoolVO findStoragePoolForSnapshot(long snapshotId);
-
}
diff --git a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java
index 6fd4808f7d4..3cd59055774 100644-
- a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java
+++ b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java
@@ -554,25 +554,4 @@ public long countPoolsByStatus(StoragePoolStatus... statuses) {
public void deletePoolTags(long poolId) { _tagsDao.deleteTags(poolId); }-
- a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java
-
- private String sqlIsSnapshotStoragePoolManaged = "select pool.id from snapshots s "
- + " join volumes v on v.id = s.volume_id "
- + " join storage_pool pool on pool.id = v.pool_id "
- + " where s.id = ?";
- - @Override
- public StoragePoolVO findStoragePoolForSnapshot(long snapshotId) {
- try (TransactionLegacy tx = TransactionLegacy.currentTxn();
- PreparedStatement pstmt = tx.prepareAutoCloseStatement(sqlIsSnapshotStoragePoolManaged) {
- pstmt.setLong(1, snapshotId);
- ResultSet rs = pstmt.executeQuery();
- if (!rs.next()) { - throw new CloudRuntimeException(String.format("Could not find a storage pool for snapshot [snapshotId=%d]. ", snapshotId)); - }
- long storagePoolId = rs.getLong("id");
- return findById(storagePoolId);
- } catch (SQLException e) { - throw new CloudRuntimeException(e); - }
- }
}
diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/XenServerGuru.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/XenServerGuru.java
index 7d5279cd7cc..2ef942cd53a 100644-
- a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/XenServerGuru.java
+++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/XenServerGuru.java
@@ -202,9 +202,9 @@ public boolean trackVmHostChange() { logger.debug("We are returning the default host to execute commands because the source and destination objects are not snapshot and template respectively."); return defaultHostToExecuteCommands; }
- a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/XenServerGuru.java
-
- long snapshotId = srcData.getId();
- StoragePoolVO storagePoolVO = storagePoolDao.findStoragePoolForSnapshot(snapshotId);
- HostVO hostCandidateToExecutedCommand = hostDao.findHostToOperateOnSnapshotBasedOnStoragePool(storagePoolVO);
+ HostVO defaultHostToExecuteCommand = hostDao.findById(hostId);
+
+ HostVO hostCandidateToExecutedCommand = hostDao.findHostInZoneToExecuteCommand(defaultHostToExecuteCommand.getDataCenterId(), srcData.getHypervisorType());
hostDao.loadDetails(hostCandidateToExecutedCommand);
String hypervisorVersion = hostCandidateToExecutedCommand.getHypervisorVersion();
if (StringUtils.isBlank(hypervisorVersion)) {
@@ -219,9 +219,7 @@ public boolean trackVmHostChange() {
String snapshotHotFixVersion = hostCandidateToExecutedCommand.getDetail(XenserverConfigs.XS620HotFix);
boolean isXenServer620 = StringUtils.equals(hypervisorVersion, "6.2.0");
if (isXenServer620 && !StringUtils.equalsIgnoreCase(XenserverConfigs.XSHotFix62ESP1004, snapshotHotFixVersion)) { - logger.debug(String.format( - "We are returning the default host to execute commands because the hypervisor version is not 6.2.0 with hotfix ESP1004 [hypervisorVersion=%s, hotfixVersion=%s]", - hypervisorVersion, snapshotHotFixVersion)); + logger.debug(String.format("We are returning the default host to execute commands because the hypervisor version is not 6.2.0 with hotfix ESP1004 [hypervisorVersion=%s, hotfixVersion=%s]", hypervisorVersion, snapshotHotFixVersion)); return defaultHostToExecuteCommands; }logger.debug(String.format("We are changing the hostId to executed command from %d to %d.", hostId, hostCandidateToExecutedCommand.getId()));
diff --git a/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/XenServerGuruTest.java b/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/XenServerGuruTest.java
index d67a4f18f0b..a9d01436479 100644-
- a/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/XenServerGuruTest.java
+++ b/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/XenServerGuruTest.java
@@ -21,7 +21,6 @@
import org.apache.cloudstack.storage.command.CopyCommand;
import org.apache.cloudstack.storage.command.StorageSubSystemCommand;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.commons.lang.StringUtils;
import org.junit.Assert;
import org.junit.Before;
@@ -54,8 +53,6 @@
@Mock
private PrimaryDataStoreDao storagePoolDao;
- a/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/XenServerGuruTest.java
-
- private Long defaultHostId = 1l;
-
@Mock
private CopyCommand copyCommandMock;
@@ -65,23 +62,31 @@
@Mock
private DataTO destinationDataMock;
+ private Long defaultHostId = 1l;
+
@Mock
- private HostVO hostMock;
+ private HostVO defaultHost;
@Mock
- private StoragePoolVO storagePoolMock;
+ private HostVO changedHost;
private Long changedHostId = 12l;
- private long snapshotId = 5l;
+ private long zoneId = 100l;
@Before
public void beforeTest()
@Test
@@ -162,8 +167,8 @@ public void getCommandHostDelegationTestCommandIsCopyCommandAndSourceDataHypervi
configureSourceAndDestinationDataMockDataStoreAsNfsToType();
configureSourceHypervisorAsXenServerAndSourceTypeAsSnapshotAndDestinationTypeAsTemplate();
- Mockito.when(hostMock.getHypervisorVersion()).thenReturn(StringUtils.EMPTY);
- Mockito.when(hostDaoMock.findHostToOperateOnSnapshotBasedOnStoragePool(storagePoolMock)).thenReturn(hostMock);
+ Mockito.when(changedHost.getHypervisorVersion()).thenReturn(StringUtils.EMPTY);
+ Mockito.when(hostDaoMock.findHostInZoneToExecuteCommand(zoneId, HypervisorType.XenServer)).thenReturn(changedHost);
Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, copyCommandMock);
@@ -181,8 +186,8 @@ public void getCommandHostDelegationTestCommandIsCopyCommandAndSourceDataHypervi
configureSourceAndDestinationDataMockDataStoreAsNfsToType();
configureSourceHypervisorAsXenServerAndSourceTypeAsSnapshotAndDestinationTypeAsTemplate();
- Mockito.when(hostMock.getHypervisorVersion()).thenReturn("6.1.0");
- Mockito.when(hostDaoMock.findHostToOperateOnSnapshotBasedOnStoragePool(storagePoolMock)).thenReturn(hostMock);
+ Mockito.when(changedHost.getHypervisorVersion()).thenReturn("6.1.0");
+ Mockito.when(hostDaoMock.findHostInZoneToExecuteCommand(zoneId, HypervisorType.XenServer)).thenReturn(changedHost);
Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, copyCommandMock);
@@ -194,8 +199,8 @@ public void getCommandHostDelegationTestCommandIsCopyCommandAndSourceDataHypervi
configureSourceAndDestinationDataMockDataStoreAsNfsToType();
configureSourceHypervisorAsXenServerAndSourceTypeAsSnapshotAndDestinationTypeAsTemplate();
- Mockito.when(hostMock.getHypervisorVersion()).thenReturn("6.2.0");
- Mockito.when(hostDaoMock.findHostToOperateOnSnapshotBasedOnStoragePool(storagePoolMock)).thenReturn(hostMock);
+ Mockito.when(changedHost.getHypervisorVersion()).thenReturn("6.2.0");
+ Mockito.when(hostDaoMock.findHostInZoneToExecuteCommand(zoneId, HypervisorType.XenServer)).thenReturn(changedHost);
Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, copyCommandMock);
@@ -207,10 +212,10 @@ public void getCommandHostDelegationTestCommandIsCopyCommandAndSourceDataHypervi
configureSourceAndDestinationDataMockDataStoreAsNfsToType();
configureSourceHypervisorAsXenServerAndSourceTypeAsSnapshotAndDestinationTypeAsTemplate();
- Mockito.when(hostMock.getHypervisorVersion()).thenReturn("6.2.0");
- Mockito.when(hostMock.getDetail(XenserverConfigs.XS620HotFix)).thenReturn(XenserverConfigs.XSHotFix62ESP1004);
+ Mockito.when(changedHost.getHypervisorVersion()).thenReturn("6.2.0");
+ Mockito.when(changedHost.getDetail(XenserverConfigs.XS620HotFix)).thenReturn(XenserverConfigs.XSHotFix62ESP1004);
- Mockito.when(hostDaoMock.findHostToOperateOnSnapshotBasedOnStoragePool(storagePoolMock)).thenReturn(hostMock);
+ Mockito.when(hostDaoMock.findHostInZoneToExecuteCommand(zoneId, HypervisorType.XenServer)).thenReturn(changedHost);
Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, copyCommandMock);
@@ -223,9 +228,9 @@ public void getCommandHostDelegationTestCommandIsCopyCommandAndSourceDataHypervi
configureSourceAndDestinationDataMockDataStoreAsNfsToType();
configureSourceHypervisorAsXenServerAndSourceTypeAsSnapshotAndDestinationTypeAsTemplate();
- Mockito.when(hostMock.getHypervisorVersion()).thenReturn("6.5.0");
+ Mockito.when(changedHost.getHypervisorVersion()).thenReturn("6.5.0");
- Mockito.when(hostDaoMock.findHostToOperateOnSnapshotBasedOnStoragePool(storagePoolMock)).thenReturn(hostMock);
+ Mockito.when(hostDaoMock.findHostInZoneToExecuteCommand(zoneId, HypervisorType.XenServer)).thenReturn(changedHost);
Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, copyCommandMock);
diff --git a/requirements.txt b/requirements.txt
index 4485f5d73f9..5f2551e9f4f 100644
— a/requirements.txt
+++ b/requirements.txt
@@ -21,4 +21,5 @@ cloudmonkey
- Marvin dependencies are installed via its bundle
- Install the SolidFire SDK for Python
-solidfire-sdk-python
\ No newline at end of file
+solidfire-sdk-python
+
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Commit 1d0f2128f62ff32b9cff27f6de1ed711563b656b in cloudstack's branch refs/heads/master from rafaelweingartner
[ https://gitbox.apache.org/repos/asf?p=cloudstack.git;h=1d0f212 ]
CLOUDSTACK-9025CLOUDSTACK-10128solve problems of templates created based on snapshots (#2315)The first PR(#1176) intended to solve #
CLOUDSTACK-9025was only tackling the problem for CloudStack deployments that use single hypervisor types (restricted to XenServer). Additionally, the lack of information regarding that solution (poor documentation, test cases and description in PRs and Jira ticket) led the code to be removed in #1124 after a long discussion and analysis in #1056. That piece of code seemed logicless (and it was!). It would receive a hostId and then change that hostId for other hostId of the zone without doing any check; it was not even checking the hypervisor and storage in which the host was plugged into.The problem reported in #
CLOUDSTACK-9025is caused by partial snapshots that are taken in XenServer. This means, we do not take a complete snapshot, but a partial one that contains only the modified data. This requires rebuilding the VHD hierarchy when creating a template out of the snapshot. The point is that the first hostId received is not a hostId, but a system VM ID(SSVM). That is why the code in #1176 fixed the problem for some deployment scenarios, but would cause problems for scenarios where we have multiple hypervisors in the same zone. We need to execute the creation of the VHD that represents the template in the hypervisor, so the VHD chain can be built using the parent links.This commit changes the method com.cloud.hypervisor.XenServerGuru.getCommandHostDelegation(long, Command). From now on we replace the hostId that is intended to execute the “copy command” that will create the VHD of the template according to some conditions that were already in place. The idea is that starting with XenServer 6.2.0 hotFix ESP1004 we need to execute the command in the hypervisor host and not from the SSVM. Moreover, the method was improved making it readable and understandable; it was also created test cases assuring that from XenServer 6.2.0 hotFix ESP1004 and upward versions we change the hostId that will be used to execute the “copy command”.
Furthermore, we are not selecting a random host from a zone anymore. A new method was introduced in the HostDao called “findHostConnectedToSnapshotStoragePoolToExecuteCommand”, using this method we look for a host that is in the cluster that is using the storage pool where the volume from which the Snaphost is taken of. By doing this, we guarantee that the host that is connected to the primary storage where all of the snapshots parent VHDs are stored is used to create the template.
Consider using Disabled hosts when no Enabled hosts are found
This also closes #2317