You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by syed <gi...@git.apache.org> on 2016/06/30 17:45:48 UTC
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
GitHub user syed opened a pull request:
https://github.com/apache/cloudstack/pull/1600
Support Backup of Snapshots for Managed Storage
This PR adds an ability to Pass a new parameter, locationType,
to the \u201ccreateSnapshot\u201d API command. Depending on the locationType,
we decide where the snapshot should go in case of managed storage.
There are two possible values for the locationType param
1) `Standard`: The standard operation for managed storage is to
keep the snapshot on the device. For non-managed storage, this will
be to upload it to secondary storage. This option will be the
default.
2) `Archive`: Applicable only to managed storage. This will
keep the snapshot on the secondary storage. For non-managed
storage, this will result in an error.
The reason for implementing this feature is to avoid a single
point of failure for primary storage. Right now in case of managed
storage, if the primary storage goes down, there is no easy way
to recover data as all snapshots are also stored on the primary.
This features allows us to mitigate that risk.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/syed/cloudstack snapshot-archive-pr
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1600.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1600
----
commit 252942f29c5c485b7d60b6ae8be33165db1b0cfb
Author: Syed <sy...@gmail.com>
Date: 2016-06-30T17:37:33Z
Support Backup of Snapshots for Managed Storage
This PR adds an ability to Pass a new parameter, locationType,
to the \u201ccreateSnapshot\u201d API command. Depending on the locationType,
we decide where the snapshot should go in case of managed storage.
There are two possible values for the locationType param
1) `Standard`: The standard operation for managed storage is to
keep the snapshot on the device. For non-managed storage, this will
be to upload it to secondary storage. This option will be the
default.
2) `Archive`: Applicable only to managed storage. This will
keep the snapshot on the secondary storage. For non-managed
storage, this will result in an error.
The reason for implementing this feature is to avoid a single
point of failure for primary storage. Right now in case of managed
storage, if the primary storage goes down, there is no easy way
to recover data as all snapshots are also stored on the primary.
This features allows us to mitigate that risk.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r90438253
--- Diff: plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java ---
@@ -538,7 +562,7 @@ public Answer backupSnapshot(final CopyCommand cmd) {
final String[] tmp = result.split("#");
snapshotBackupUuid = tmp[0];
physicalSize = Long.parseLong(tmp[1]);
- finalPath = folder + File.separator + snapshotBackupUuid;
+ finalPath = folder + File.separator + snapshotBackupUuid + ".vhd";
--- End diff --
This change will result in a ".vhd" extension being written to snapshot_store_ref as this is being returned in the CopyCmdAnswer. Although I prefer this, the implication of this change is that there will be inconsistency in snapshot names in a setup upgraded to this version. I am wondering and please confirm that this is not going to cause the UI to not show the older snapshots, only thing will be that the older snapshots' install_path will be without extension. For consistency can be modify the upgrade script to append ".vhd" to all the old Xen based snapshots ?
cc @rhtyd @jburwell
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r90676681
--- Diff: plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java ---
@@ -538,7 +562,7 @@ public Answer backupSnapshot(final CopyCommand cmd) {
final String[] tmp = result.split("#");
snapshotBackupUuid = tmp[0];
physicalSize = Long.parseLong(tmp[1]);
- finalPath = folder + File.separator + snapshotBackupUuid;
+ finalPath = folder + File.separator + snapshotBackupUuid + ".vhd";
--- End diff --
cc @mike-tutkowski
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77259447
--- Diff: core/src/org/apache/cloudstack/storage/to/PrimaryDataStoreTO.java ---
@@ -51,6 +50,7 @@
private final String url;
private Map<String, String> details;
private static final String pathSeparator = "/";
+ private boolean isManaged;
--- End diff --
Made the variable `final`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
My second test run is successful - TestVolumes with resigning:
test_00_check_template_cache (TestVolumes.TestVolumes) ... === TestName: test_00_check_template_cache | Status : SUCCESS ===
ok
Attach a volume to a stopped virtual machine, then start VM ... === TestName: test_01_attach_new_volume_to_stopped_VM | Status : SUCCESS ===
ok
Attach, detach, and attach volume to a running VM ... === TestName: test_02_attach_detach_attach_volume | Status : SUCCESS ===
ok
Attach volume to running VM, then reboot. ... === TestName: test_03_attached_volume_reboot_VM | Status : SUCCESS ===
ok
Detach volume from a running VM, then reboot. ... === TestName: test_04_detach_volume_reboot | Status : SUCCESS ===
ok
Detach volume from a stopped VM, then start. ... === TestName: test_05_detach_vol_stopped_VM_start | Status : SUCCESS ===
ok
Attach a volume to a stopped virtual machine, then start VM ... === TestName: test_06_attach_volume_to_stopped_VM | Status : SUCCESS ===
ok
Destroy and expunge VM with attached volume ... === TestName: test_07_destroy_expunge_VM_with_volume | Status : SUCCESS ===
ok
Delete volume that was attached to a VM and is detached now ... === TestName: test_08_delete_volume_was_attached | Status : SUCCESS ===
ok
Attach a data disk to a VM in one account and attach another data disk to a VM in another account ... === TestName: test_09_attach_volumes_multiple_accounts | Status : SUCCESS ===
ok
Attach more than one disk to a VM ... === TestName: test_10_attach_more_than_one_disk_to_VM | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 11 tests in 2313.359s
OK
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77220737
--- Diff: engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java ---
@@ -328,15 +412,64 @@ private void handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, Templat
copyCommand.setOptions(srcDetails);
copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand);
- }
- catch (CloudRuntimeException | AgentUnavailableException | OperationTimedoutException ex) {
+
+ if (needCache) {
+
+ // If cached storage was needed (in case of object store as secondary
+ // storage), at this point, the data has been copied from the primary
+ // to the NFS cache by the hypervisor. We now invoke another copy
+ // command to copy this data from cache to secondary storage. We
+ // then cleanup the cache
+
+ destOnStore.processEvent(Event.OperationSuccessed, copyCmdAnswer);
+
+ CopyCommand cmd = new CopyCommand(destOnStore.getTO(), destData.getTO(), primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
+ EndPoint ep = selector.select(destOnStore, destData);
+
+ if (ep == null) {
+ errMsg = "No remote endpoint to send command, check if host or ssvm is down?";
+ LOGGER.error(errMsg);
+ copyCmdAnswer = new CopyCmdAnswer(errMsg);
+ } else {
+ copyCmdAnswer = (CopyCmdAnswer) ep.sendMessage(cmd);
+ }
+
+ // clean up snapshot copied to staging
+ performCleanupCacheStorage(destOnStore);
+ }
+
+
+ } catch (CloudRuntimeException | AgentUnavailableException | OperationTimedoutException ex) {
String msg = "Failed to create template from snapshot (Snapshot ID = " + snapshotInfo.getId() + ") : ";
LOGGER.warn(msg, ex);
throw new CloudRuntimeException(msg + ex.getMessage());
- }
- finally {
+
+ } finally {
+
+ // detach and remove access after the volume is created
+ SnapshotObjectTO snapshotObjectTO = (SnapshotObjectTO) snapshotInfo.getTO();
+ DiskTO disk = new DiskTO(snapshotObjectTO, null, snapshotInfo.getPath(), Volume.Type.UNKNOWN);
+ DettachCommand cmd = new DettachCommand(disk, null);
+ StoragePoolVO storagePool = _storagePoolDao.findById(srcDataStore.getId());
+
+ cmd.setManaged(true);
+ cmd.setStorageHost(storagePool.getHostAddress());
+ cmd.setStoragePort(storagePool.getPort());
+ cmd.set_iScsiName(getProperty(snapshotInfo.getId(), DiskTO.IQN));
+
+ try {
+ DettachAnswer dettachAnswer = (DettachAnswer) _agentMgr.send(hostVO.getId(), cmd);
+
+ if (!dettachAnswer.getResult()) {
+ throw new CloudRuntimeException("Error detaching volume:" + dettachAnswer.getDetails());
--- End diff --
I lost track of the catch block below. Why are we throwing an exception to catch a few lines later? It appears to be flow of control using exceptions is a code anti-pattern.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77259343
--- Diff: engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java ---
@@ -380,6 +513,122 @@ private void handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, Templat
}
/**
+ * Creates a volume on the storage from a snapshot that resides on the secondary storage (archived snapshot).
+ * @param snapshotInfo snapshot on secondary
+ * @param volumeInfo volume to be created on the storage
+ * @param callback for async
+ */
+ private void handleCreateVolumeFromSnapshotOnSecondaryStorage(SnapshotInfo snapshotInfo, VolumeInfo volumeInfo, AsyncCompletionCallback<CopyCommandResult> callback) {
+
+ // at this point, the snapshotInfo and volumeInfo should have the same disk offering ID (so either one should be OK to get a DiskOfferingVO instance)
+ DiskOfferingVO diskOffering = _diskOfferingDao.findByIdIncludingRemoved(volumeInfo.getDiskOfferingId());
+ SnapshotVO snapshot = _snapshotDao.findById(snapshotInfo.getId());
+ DataStore destDataStore = volumeInfo.getDataStore();
+
+ // update the volume's hv_ss_reserve (hypervisor snapshot reserve) from a disk offering (used for managed storage)
+ _volumeService.updateHypervisorSnapshotReserveForVolume(diskOffering, volumeInfo.getId(), snapshot.getHypervisorType());
+
+
+ CopyCmdAnswer copyCmdAnswer = null;
+ String errMsg = null;
+
+ HostVO hostVO = null;
+ try {
+
+ //create a volume on the storage
+ AsyncCallFuture<VolumeApiResult> future = _volumeService.createVolumeAsync(volumeInfo, volumeInfo.getDataStore());
+ VolumeApiResult result = future.get();
+
+ if (result.isFailed()) {
+ LOGGER.warn("Failed to create a volume: " + result.getResult());
+ throw new CloudRuntimeException(result.getResult());
+ }
+
+ volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(), volumeInfo.getDataStore());
+
+ volumeInfo.processEvent(Event.MigrationRequested);
+
+ volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(), volumeInfo.getDataStore());
+
+ hostVO = getHost(snapshotInfo.getDataCenterId(), false);
+
+ //copy the volume from secondary via the hypervisor
+ copyCmdAnswer = performCopyOfVdi(volumeInfo, snapshotInfo, hostVO);
+
+ if (copyCmdAnswer == null || !copyCmdAnswer.getResult()) {
+ if (copyCmdAnswer != null && !StringUtils.isEmpty(copyCmdAnswer.getDetails())) {
+ errMsg = copyCmdAnswer.getDetails();
+ }
+ else {
+ errMsg = "Unable to create volume from snapshot";
+ }
+ }
+ }
+ catch (Exception ex) {
+ errMsg = ex.getMessage() != null ? ex.getMessage() : "Copy operation failed in 'StorageSystemDataMotionStrategy.handleCreateVolumeFromSnapshotBothOnStorageSystem'";
+ } finally {
+
+ // detach and remove access after the volume is created
+ DiskTO disk = new DiskTO(volumeInfo.getTO(), volumeInfo.getDeviceId(), volumeInfo.getPath(), volumeInfo.getVolumeType());
+ DettachCommand cmd = new DettachCommand(disk, null);
+ long storagePoolId = volumeInfo.getPoolId();
+ StoragePoolVO storagePool = _storagePoolDao.findById(storagePoolId);
+
+ cmd.setManaged(true);
+ cmd.setStorageHost(storagePool.getHostAddress());
+ cmd.setStoragePort(storagePool.getPort());
+ cmd.set_iScsiName(volumeInfo.get_iScsiName());
+
+ try {
+ DettachAnswer dettachAnswer = (DettachAnswer) _agentMgr.send(hostVO.getId(), cmd);
+
+ if (!dettachAnswer.getResult()) {
+ throw new CloudRuntimeException("Error detaching volume:" + dettachAnswer.getDetails());
+ }
+
+ } catch (Exception e) {
+ LOGGER.warn("Error detaching volume " + volumeInfo.getId() + " Error: " + e.getMessage());
+ }
+
+ _volumeService.revokeAccess(volumeInfo, hostVO, destDataStore);
--- End diff --
moved to a common function
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77080328
--- Diff: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java ---
@@ -92,7 +90,34 @@
@Override
public SnapshotInfo backupSnapshot(SnapshotInfo snapshotInfo) {
- return snapshotInfo;
+
+ if (!snapshotInfo.getLocationType().equals(Snapshot.LocationType.SECONDARY)) {
--- End diff --
Since Enum values are guaranteed to be singletons, the `==` is that same as `equals`. By changing this check to be ``snapshotInfo.getLocationType() != Snapshot.LocationType.SECONDARY``, a potential NPE is avoided and the code is more straightforward.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@syed It actually looks like the rebase lead to the integration tests being put back in their pre-official SolidFire SDK for Python state.
I recommend you just remove all integration tests from this PR as all managed-storage integration tests are now current as of the merge of #1689.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by syed <gi...@git.apache.org>.
Github user syed commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@mike-tutkowski I've fixed the problem where the volume was not being detached after the copy.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by syed <gi...@git.apache.org>.
Github user syed commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@karuturi @jburwell Can we merge this in? Anthing left from my side?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@syed it appears that conflicts were introduced when #1689 was merged. Could you please resolve?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@syed @mike-tutkowski should the tests be re-executed to make sure everything is super green?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1600
Packaging result: \u2716centos6 \u2716centos7 \u2716debian repo: http://packages.shapeblue.com/cloudstack/pr/1600
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1600
Packaging result: \u2714centos6 \u2714centos7 \u2714debian repo: http://packages.shapeblue.com/cloudstack/pr/1600
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@blueorangutan package
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77218636
--- Diff: engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java ---
@@ -380,6 +513,122 @@ private void handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, Templat
}
/**
+ * Creates a volume on the storage from a snapshot that resides on the secondary storage (archived snapshot).
+ * @param snapshotInfo snapshot on secondary
+ * @param volumeInfo volume to be created on the storage
+ * @param callback for async
+ */
+ private void handleCreateVolumeFromSnapshotOnSecondaryStorage(SnapshotInfo snapshotInfo, VolumeInfo volumeInfo, AsyncCompletionCallback<CopyCommandResult> callback) {
+
+ // at this point, the snapshotInfo and volumeInfo should have the same disk offering ID (so either one should be OK to get a DiskOfferingVO instance)
+ DiskOfferingVO diskOffering = _diskOfferingDao.findByIdIncludingRemoved(volumeInfo.getDiskOfferingId());
+ SnapshotVO snapshot = _snapshotDao.findById(snapshotInfo.getId());
+ DataStore destDataStore = volumeInfo.getDataStore();
+
+ // update the volume's hv_ss_reserve (hypervisor snapshot reserve) from a disk offering (used for managed storage)
+ _volumeService.updateHypervisorSnapshotReserveForVolume(diskOffering, volumeInfo.getId(), snapshot.getHypervisorType());
+
+
+ CopyCmdAnswer copyCmdAnswer = null;
+ String errMsg = null;
+
+ HostVO hostVO = null;
+ try {
+
+ //create a volume on the storage
+ AsyncCallFuture<VolumeApiResult> future = _volumeService.createVolumeAsync(volumeInfo, volumeInfo.getDataStore());
+ VolumeApiResult result = future.get();
--- End diff --
@syed it is not safe to assume that current/future underlying storage driver will properly implement timeouts. The consequences of a buggy implementation are to starve the Management Server of resources causing it to become unresponsive or crash. Please bound this call in a timeout -- ideally specified as a global setting.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@jburwell a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
My fourth test run is successful - TestSnapshots.py:
test_01_create_volume_snapshot_using_sf_snapshot (TestSnapshots.TestSnapshots) ... === TestName: test_01_create_volume_snapshot_using_sf_snapshot | Status : SUCCESS ===
ok
test_02_create_volume_snapshot_using_sf_volume (TestSnapshots.TestSnapshots) ... === TestName: test_02_create_volume_snapshot_using_sf_volume | Status : SUCCESS ===
ok
test_03_create_volume_snapshot_using_sf_volume_and_sf_snapshot (TestSnapshots.TestSnapshots) ... === TestName: test_03_create_volume_snapshot_using_sf_volume_and_sf_snapshot | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 3 tests in 7114.793s
OK
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77074073
--- Diff: api/test/org/apache/cloudstack/api/command/test/CreateSnapshotCmdTest.java ---
@@ -0,0 +1,130 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.test;
+
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.VolumeApiService;
+import com.cloud.user.Account;
+import com.cloud.user.AccountService;
+import junit.framework.TestCase;
+import org.apache.cloudstack.api.ResponseGenerator;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.user.snapshot.CreateSnapshotCmd;
+import org.apache.cloudstack.api.response.SnapshotResponse;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.mockito.Mockito;
+
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyBoolean;
+import static org.mockito.Matchers.anyLong;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Matchers.isNull;
+
+public class CreateSnapshotCmdTest extends TestCase {
+
+ private CreateSnapshotCmd createSnapshotCmd;
+ private ResponseGenerator responseGenerator;
+
+ @Rule
+ public ExpectedException expectedException = ExpectedException.none();
+
+ @Override
+ @Before
+ public void setUp() {
+
+ createSnapshotCmd = new CreateSnapshotCmd() {
+
+ @Override
+ public String getCommandName() {
+ return "createsnapshotresponse";
+ }
+
+ @Override
+ public Long getVolumeId(){
+ return 1L;
+ }
+
+ @Override
+ public long getEntityOwnerId(){
+ return 1L;
+ }
+ };
+
+ }
+
+ @Test
+ public void testCreateSuccess() {
+
+ AccountService accountService = Mockito.mock(AccountService.class);
+ Account account = Mockito.mock(Account.class);
+ Mockito.when(accountService.getAccount(anyLong())).thenReturn(account);
+
+ VolumeApiService volumeApiService = Mockito.mock(VolumeApiService.class);
+ Snapshot snapshot = Mockito.mock(Snapshot.class);
+ try {
+
+ Mockito.when(volumeApiService.takeSnapshot(anyLong(), anyLong(), anyLong(),
+ any(Account.class), anyBoolean(), isNull(Snapshot.LocationType.class))).thenReturn(snapshot);
+
+ } catch (Exception e) {
+ Assert.fail("Received exception when success expected " + e.getMessage());
+ }
+
+ responseGenerator = Mockito.mock(ResponseGenerator.class);
+ SnapshotResponse snapshotResponse = Mockito.mock(SnapshotResponse.class);
+ Mockito.when(responseGenerator.createSnapshotResponse(snapshot)).thenReturn(snapshotResponse);
+ Mockito.doNothing().when(snapshotResponse).setAccountName(anyString());
+
+ createSnapshotCmd._accountService = accountService;
+ createSnapshotCmd._responseGenerator = responseGenerator;
+ createSnapshotCmd._volumeService = volumeApiService;
+
+ createSnapshotCmd.execute();
+ }
+
+ @Test
+ @Ignore
--- End diff --
Why are we adding a new unit test that is being ignored?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
GitHub user syed reopened a pull request:
https://github.com/apache/cloudstack/pull/1600
Support Backup of Snapshots for Managed Storage
This PR adds an ability to Pass a new parameter, locationType,
to the \u201ccreateSnapshot\u201d API command. Depending on the locationType,
we decide where the snapshot should go in case of managed storage.
There are two possible values for the locationType param
1) `Standard`: The standard operation for managed storage is to
keep the snapshot on the device. For non-managed storage, this will
be to upload it to secondary storage. This option will be the
default.
2) `Archive`: Applicable only to managed storage. This will
keep the snapshot on the secondary storage. For non-managed
storage, this will result in an error.
The reason for implementing this feature is to avoid a single
point of failure for primary storage. Right now in case of managed
storage, if the primary storage goes down, there is no easy way
to recover data as all snapshots are also stored on the primary.
This features allows us to mitigate that risk.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/syed/cloudstack snapshot-archive-pr
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1600.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1600
----
commit 252942f29c5c485b7d60b6ae8be33165db1b0cfb
Author: Syed <sy...@gmail.com>
Date: 2016-06-30T17:37:33Z
Support Backup of Snapshots for Managed Storage
This PR adds an ability to Pass a new parameter, locationType,
to the \u201ccreateSnapshot\u201d API command. Depending on the locationType,
we decide where the snapshot should go in case of managed storage.
There are two possible values for the locationType param
1) `Standard`: The standard operation for managed storage is to
keep the snapshot on the device. For non-managed storage, this will
be to upload it to secondary storage. This option will be the
default.
2) `Archive`: Applicable only to managed storage. This will
keep the snapshot on the secondary storage. For non-managed
storage, this will result in an error.
The reason for implementing this feature is to avoid a single
point of failure for primary storage. Right now in case of managed
storage, if the primary storage goes down, there is no easy way
to recover data as all snapshots are also stored on the primary.
This features allows us to mitigate that risk.
commit 374944a3e2670f78ebdcdffe065a64ee89e3ac96
Author: Syed <sy...@gmail.com>
Date: 2016-07-05T17:21:56Z
Detach from hypervisor after copy to secondary storage
commit 3d30645baca863b31f2e10e68def606de25666b1
Author: Mike Tutkowski <mi...@solidfire.com>
Date: 2016-07-13T19:35:43Z
A couple Marvin changes and new/updated Marvin tests
commit 50d0b17ae86476a3ef6b17fd3f8e2a6e311ceff5
Author: Syed Mushtaq Ahmed <sy...@gmail.com>
Date: 2016-07-13T23:53:00Z
Merge pull request #5 from mike-tutkowski/snapshot-archive-pr
A couple Marvin changes and new/updated Marvin tests
commit a96519970d09c22be5470422c6f208f6812f7c90
Author: Mike Tutkowski <mi...@solidfire.com>
Date: 2016-07-14T16:35:13Z
Changing locationType to PRIMARY and SECONDARY
commit 6baa19635e074923e797d748c0bb6b7efdf24a4f
Author: Syed <sy...@gmail.com>
Date: 2016-09-01T21:38:46Z
Fix review comments
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
Github user syed closed the pull request at:
https://github.com/apache/cloudstack/pull/1600
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77080602
--- Diff: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java ---
@@ -212,20 +251,47 @@ public SnapshotInfo takeSnapshot(SnapshotInfo snapshotInfo) {
performSnapshotAndCopyOnHostSide(volumeInfo, snapshotInfo);
}
- markAsBackedUp((SnapshotObject)result.getSnashot());
+ snapshotOnPrimary = result.getSnapshot();
+ backedUpSnapshot = backupSnapshot(snapshotOnPrimary);
+
+ updateLocationTypeInDb(backedUpSnapshot);
+
}
finally {
if (result != null && result.isSuccess()) {
volumeInfo.stateTransit(Volume.Event.OperationSucceeded);
- }
- else {
+
+ Preconditions.checkState(snapshotOnPrimary != null);
--- End diff --
Should an exception be thrown in this ``finally`` block or just skip processing?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
The code LGTM.
In addition, my automated regression tests have all come back successful (documented above).
I plan to write new automated tests for this PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
In manual testing, I've noticed that the SR that contains the VDI that we copy from primary to secondary storage is not removed from XenServer.
I see that the SolidFire volume that contains the SR is no longer in a volume access group (good) and it is deleted (good), but the SR is still attached to by XenServer.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@mike-tutkowski yes, those failing tests are known to cause intermittent failures. LGTM on tests. If all the reported issues have been fixed, this can be merged. /cc @jburwell
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77259257
--- Diff: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java ---
@@ -212,20 +251,47 @@ public SnapshotInfo takeSnapshot(SnapshotInfo snapshotInfo) {
performSnapshotAndCopyOnHostSide(volumeInfo, snapshotInfo);
}
- markAsBackedUp((SnapshotObject)result.getSnashot());
+ snapshotOnPrimary = result.getSnapshot();
+ backedUpSnapshot = backupSnapshot(snapshotOnPrimary);
+
+ updateLocationTypeInDb(backedUpSnapshot);
+
}
finally {
if (result != null && result.isSuccess()) {
volumeInfo.stateTransit(Volume.Event.OperationSucceeded);
- }
- else {
+
+ Preconditions.checkState(snapshotOnPrimary != null);
--- End diff --
Removed the precondition, added an if check which better handles the situation
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
My fifth and final test run is successful - TestAddRemoveHosts.py:
test_add_remove_host_with_solidfire_plugin_1 (TestAddRemoveHosts.TestAddRemoveHosts) ... === TestName: test_add_remove_host_with_solidfire_plugin_1 | Status : SUCCESS ===
ok
test_add_remove_host_with_solidfire_plugin_2 (TestAddRemoveHosts.TestAddRemoveHosts) ... === TestName: test_add_remove_host_with_solidfire_plugin_2 | Status : SUCCESS ===
ok
test_add_remove_host_with_solidfire_plugin_3 (TestAddRemoveHosts.TestAddRemoveHosts) ... === TestName: test_add_remove_host_with_solidfire_plugin_3 | Status : SUCCESS ===
ok
test_add_remove_host_with_solidfire_plugin_4 (TestAddRemoveHosts.TestAddRemoveHosts) ... === TestName: test_add_remove_host_with_solidfire_plugin_4 | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 4 tests in 3665.055s
OK
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@mike-tutkowski sounds good. I am putting together a test results email which should be out shortly. It outlines the issues we have found thus far. As such, I expect the testing period for the releases to be extended while we address those issues and then rebase and retest a number of open PRs.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@syed Just an FYI that this branch had two conflicts with master (after recent changes to master). I went ahead and fixed the conflicts and re-pushed the commit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@blueorangutan package
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77079861
--- Diff: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java ---
@@ -92,7 +90,34 @@
@Override
public SnapshotInfo backupSnapshot(SnapshotInfo snapshotInfo) {
- return snapshotInfo;
+
--- End diff --
Please add a ``Preconditions.checkArgument(snapshotInfo != null, "<explaination of the precondition>")`` to protect against potential NPEs.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@syed @karuturi @jburwell Just wondering how we're doing on this one. Is there anything else that Syed needs to do here? Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@rhtyd I don't think either of those two errors are related to this PR (probably environment issues). I think we are good to go here. What do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77077921
--- Diff: engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java ---
@@ -380,6 +513,122 @@ private void handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, Templat
}
/**
+ * Creates a volume on the storage from a snapshot that resides on the secondary storage (archived snapshot).
+ * @param snapshotInfo snapshot on secondary
+ * @param volumeInfo volume to be created on the storage
+ * @param callback for async
+ */
+ private void handleCreateVolumeFromSnapshotOnSecondaryStorage(SnapshotInfo snapshotInfo, VolumeInfo volumeInfo, AsyncCompletionCallback<CopyCommandResult> callback) {
+
+ // at this point, the snapshotInfo and volumeInfo should have the same disk offering ID (so either one should be OK to get a DiskOfferingVO instance)
+ DiskOfferingVO diskOffering = _diskOfferingDao.findByIdIncludingRemoved(volumeInfo.getDiskOfferingId());
+ SnapshotVO snapshot = _snapshotDao.findById(snapshotInfo.getId());
+ DataStore destDataStore = volumeInfo.getDataStore();
+
+ // update the volume's hv_ss_reserve (hypervisor snapshot reserve) from a disk offering (used for managed storage)
+ _volumeService.updateHypervisorSnapshotReserveForVolume(diskOffering, volumeInfo.getId(), snapshot.getHypervisorType());
+
+
+ CopyCmdAnswer copyCmdAnswer = null;
+ String errMsg = null;
+
+ HostVO hostVO = null;
+ try {
+
+ //create a volume on the storage
+ AsyncCallFuture<VolumeApiResult> future = _volumeService.createVolumeAsync(volumeInfo, volumeInfo.getDataStore());
+ VolumeApiResult result = future.get();
+
+ if (result.isFailed()) {
+ LOGGER.warn("Failed to create a volume: " + result.getResult());
--- End diff --
Why is this message being logged to ``WARN`` and not ``ERROR``?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
Can you return the new locationtype parameter in the listSnapshots API call?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@jburwell Yes, the managed-storage tests should be re-executed. I can do this over the weekend as my CloudStack systems in the lab here are being fully utilized for the time being with another project I'm working on at the moment. I should be able to provide the results by Monday.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@syed could you please squash your commits? Once that is done, the following items will be need to be completed:
* [ ] Re-execute functional test since the code has changed since they were last run (@mike-tutowski)
* [ ] Execute regression tests on KVM, VMware, and Xen with unmanaged storage (@jburwell using blueorangutan)
Are there any specific component tests that should be run in addition to the smoke tests?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@mike-tutkowski @syed needs to resolve the conflicts before we can proceed with package and functional testing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77209873
--- Diff: engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java ---
@@ -328,15 +412,64 @@ private void handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, Templat
copyCommand.setOptions(srcDetails);
copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand);
- }
- catch (CloudRuntimeException | AgentUnavailableException | OperationTimedoutException ex) {
+
+ if (needCache) {
+
+ // If cached storage was needed (in case of object store as secondary
+ // storage), at this point, the data has been copied from the primary
+ // to the NFS cache by the hypervisor. We now invoke another copy
+ // command to copy this data from cache to secondary storage. We
+ // then cleanup the cache
+
+ destOnStore.processEvent(Event.OperationSuccessed, copyCmdAnswer);
+
+ CopyCommand cmd = new CopyCommand(destOnStore.getTO(), destData.getTO(), primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
+ EndPoint ep = selector.select(destOnStore, destData);
+
+ if (ep == null) {
+ errMsg = "No remote endpoint to send command, check if host or ssvm is down?";
+ LOGGER.error(errMsg);
+ copyCmdAnswer = new CopyCmdAnswer(errMsg);
+ } else {
+ copyCmdAnswer = (CopyCmdAnswer) ep.sendMessage(cmd);
+ }
+
+ // clean up snapshot copied to staging
+ performCleanupCacheStorage(destOnStore);
+ }
+
+
+ } catch (CloudRuntimeException | AgentUnavailableException | OperationTimedoutException ex) {
String msg = "Failed to create template from snapshot (Snapshot ID = " + snapshotInfo.getId() + ") : ";
LOGGER.warn(msg, ex);
throw new CloudRuntimeException(msg + ex.getMessage());
- }
- finally {
+
+ } finally {
+
+ // detach and remove access after the volume is created
+ SnapshotObjectTO snapshotObjectTO = (SnapshotObjectTO) snapshotInfo.getTO();
+ DiskTO disk = new DiskTO(snapshotObjectTO, null, snapshotInfo.getPath(), Volume.Type.UNKNOWN);
+ DettachCommand cmd = new DettachCommand(disk, null);
+ StoragePoolVO storagePool = _storagePoolDao.findById(srcDataStore.getId());
+
+ cmd.setManaged(true);
+ cmd.setStorageHost(storagePool.getHostAddress());
+ cmd.setStoragePort(storagePool.getPort());
+ cmd.set_iScsiName(getProperty(snapshotInfo.getId(), DiskTO.IQN));
+
+ try {
+ DettachAnswer dettachAnswer = (DettachAnswer) _agentMgr.send(hostVO.getId(), cmd);
+
+ if (!dettachAnswer.getResult()) {
+ throw new CloudRuntimeException("Error detaching volume:" + dettachAnswer.getDetails());
--- End diff --
There is a `catch` statement which catches this excepiton. The `revokeAccess()` call will always happen.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77073521
--- Diff: api/src/org/apache/cloudstack/api/response/SnapshotResponse.java ---
@@ -82,6 +82,10 @@
@Param(description = "valid types are hourly, daily, weekly, monthy, template, and none.")
private String intervalType;
+ @SerializedName(ApiConstants.LOCATION_TYPE)
+ @Param(description = "valid location types are primary and archive.")
+ private String locationType;
--- End diff --
+1 @karuturi -- types should be symmetric between request and response.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
My first test run is successful - TestVolumes.py without resigning:
test_00_check_template_cache (TestVolumes.TestVolumes) ... === TestName: test_00_check_template_cache | Status : SUCCESS ===
ok
Attach a volume to a stopped virtual machine, then start VM ... === TestName: test_01_attach_new_volume_to_stopped_VM | Status : SUCCESS ===
ok
Attach, detach, and attach volume to a running VM ... === TestName: test_02_attach_detach_attach_volume | Status : SUCCESS ===
ok
Attach volume to running VM, then reboot. ... === TestName: test_03_attached_volume_reboot_VM | Status : SUCCESS ===
ok
Detach volume from a running VM, then reboot. ... === TestName: test_04_detach_volume_reboot | Status : SUCCESS ===
ok
Detach volume from a stopped VM, then start. ... === TestName: test_05_detach_vol_stopped_VM_start | Status : SUCCESS ===
ok
Attach a volume to a stopped virtual machine, then start VM ... === TestName: test_06_attach_volume_to_stopped_VM | Status : SUCCESS ===
ok
Destroy and expunge VM with attached volume ... === TestName: test_07_destroy_expunge_VM_with_volume | Status : SUCCESS ===
ok
Delete volume that was attached to a VM and is detached now ... === TestName: test_08_delete_volume_was_attached | Status : SUCCESS ===
ok
Attach a data disk to a VM in one account and attach another data disk to a VM in another account ... === TestName: test_09_attach_volumes_multiple_accounts | Status : SUCCESS ===
ok
Attach more than one disk to a VM ... === TestName: test_10_attach_more_than_one_disk_to_VM | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 11 tests in 2000.082s
OK
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1600
Packaging result: \u2714centos6 \u2714centos7 \u2714debian. JID-34
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by syed <gi...@git.apache.org>.
Github user syed commented on the issue:
https://github.com/apache/cloudstack/pull/1600
Good catch @mike-tutkowski. I will fix it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@mike-tutkowski we are holding merges until we get all smoke tests passing on #1692 (and the subsequent PRs for the 4.9 and master branches). When those fixes are merged, we have you rebase, run the full smoke test suite, and and merge to master. This PR is the list to make 4.10.0.0.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r90676133
--- Diff: plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java ---
@@ -538,7 +562,7 @@ public Answer backupSnapshot(final CopyCommand cmd) {
final String[] tmp = result.split("#");
snapshotBackupUuid = tmp[0];
physicalSize = Long.parseLong(tmp[1]);
- finalPath = folder + File.separator + snapshotBackupUuid;
+ finalPath = folder + File.separator + snapshotBackupUuid + ".vhd";
--- End diff --
@abhinandanprateek @jburwell @rhtyd So I've looked at why I added that change. This was required for object store as secondary storage to work.
Let's say you are creating a template from a snapshot in a standard scenario where you have NFS as secondary storage. Now the request for this will go to the SSVM. Without the extension, SSVM will try to guess the extension based on the hypervisor and invoke a different function for XenServer which is not ideal but works see: `NfsSecondaryStorageResource.java:copySnapshotToTemplateFromNfsToNfsXenserver` However, if you want to backup to an object storage, it doesn't have this logic. It basically takes the install path as the source of truth see: `NfsSecondaryStorageResource.java:copyFromNfsToImage` Same thing when we create a volume from snapshot but this time the request goes to the hypervisor and it will add the `.vhd` extension if it doesn't find one see `XenServer625StorageProcessor:createVolumeFromSnapshot` where it invokes the `getSnapshotUuid` to extract the UUID from the installpath.
So I've basically fixed the case where we use Object storage and want to backup the snapshot. Now, in the normal case where we have NFS as secondary storage, are the snapshot operations failing? My impression was that they were working correctly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@mike-tutkowski I will kick off regression tests now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@syed I want to hold this PR until we have a chance to review #1642. On the surface, there appears to be some overlap, and I like to understand it before proceeding.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@jburwell a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1600
Packaging result: \u2714centos6 \u2714centos7 \u2714debian. JID-121
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77079527
--- Diff: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java ---
@@ -92,7 +90,34 @@
@Override
public SnapshotInfo backupSnapshot(SnapshotInfo snapshotInfo) {
- return snapshotInfo;
+
+ if (!snapshotInfo.getLocationType().equals(Snapshot.LocationType.SECONDARY)) {
+
+ markAsBackedUp((SnapshotObject)snapshotInfo);
+ return snapshotInfo;
+ }
+
+ // At this point the snapshot is either taken as a native
+ // snapshot on the storage or exists as a volume on the storage (clone).
+ // If archive flag is passed, we will copy this snapshot to secondary
+ // storage and delete it from the primary storage.
+
+ Preconditions.checkState(Snapshot.LocationType.SECONDARY.equals(snapshotInfo.getLocationType()));
+
+ HostVO host = getHost(snapshotInfo.getVolumeId());
+ boolean canStorageSystemCreateVolumeFromSnapshot = canStorageSystemCreateVolumeFromSnapshot(snapshotInfo.getBaseVolume().getPoolId());
+ boolean computeClusterSupportsResign = clusterDao.getSupportsResigning(host.getClusterId());
+
+ if (!canStorageSystemCreateVolumeFromSnapshot || !computeClusterSupportsResign) {
+ String mesg = "Cannot archive snapshot: Either canStorageSystemCreateVolumeFromSnapshot or " +
+ "computeClusterSupportsResign were false. ";
+
+ s_logger.warn(mesg);
--- End diff --
Why is the message being logged to ``WARN`` and not ``ERROR``?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77077963
--- Diff: engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java ---
@@ -380,6 +513,122 @@ private void handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, Templat
}
/**
+ * Creates a volume on the storage from a snapshot that resides on the secondary storage (archived snapshot).
+ * @param snapshotInfo snapshot on secondary
+ * @param volumeInfo volume to be created on the storage
+ * @param callback for async
+ */
+ private void handleCreateVolumeFromSnapshotOnSecondaryStorage(SnapshotInfo snapshotInfo, VolumeInfo volumeInfo, AsyncCompletionCallback<CopyCommandResult> callback) {
+
+ // at this point, the snapshotInfo and volumeInfo should have the same disk offering ID (so either one should be OK to get a DiskOfferingVO instance)
+ DiskOfferingVO diskOffering = _diskOfferingDao.findByIdIncludingRemoved(volumeInfo.getDiskOfferingId());
+ SnapshotVO snapshot = _snapshotDao.findById(snapshotInfo.getId());
+ DataStore destDataStore = volumeInfo.getDataStore();
+
+ // update the volume's hv_ss_reserve (hypervisor snapshot reserve) from a disk offering (used for managed storage)
+ _volumeService.updateHypervisorSnapshotReserveForVolume(diskOffering, volumeInfo.getId(), snapshot.getHypervisorType());
+
+
+ CopyCmdAnswer copyCmdAnswer = null;
+ String errMsg = null;
+
+ HostVO hostVO = null;
+ try {
+
+ //create a volume on the storage
+ AsyncCallFuture<VolumeApiResult> future = _volumeService.createVolumeAsync(volumeInfo, volumeInfo.getDataStore());
+ VolumeApiResult result = future.get();
--- End diff --
Should this call be bounded by a timeout?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
My mistake. I forgot that I added that a while ago. :)
On Jul 4, 2016, at 12:40 PM, Syed Mushtaq Ahmed <no...@github.com>> wrote:
@mike-tutkowski<https://github.com/mike-tutkowski> , the list snapshot already returns the loacationType. I think you've already added that.
-
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<https://github.com/apache/cloudstack/pull/1600#issuecomment-230340112>, or mute the thread<https://github.com/notifications/unsubscribe/AC4SHwCngBVFvcsRcZLcRdKHyNBdMY2xks5qSVORgaJpZM4JCaMg>.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77259462
--- Diff: api/test/org/apache/cloudstack/api/command/test/CreateSnapshotCmdTest.java ---
@@ -0,0 +1,130 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.test;
+
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.VolumeApiService;
+import com.cloud.user.Account;
+import com.cloud.user.AccountService;
+import junit.framework.TestCase;
+import org.apache.cloudstack.api.ResponseGenerator;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.user.snapshot.CreateSnapshotCmd;
+import org.apache.cloudstack.api.response.SnapshotResponse;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.mockito.Mockito;
+
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyBoolean;
+import static org.mockito.Matchers.anyLong;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Matchers.isNull;
+
+public class CreateSnapshotCmdTest extends TestCase {
+
+ private CreateSnapshotCmd createSnapshotCmd;
+ private ResponseGenerator responseGenerator;
+
+ @Rule
+ public ExpectedException expectedException = ExpectedException.none();
+
+ @Override
+ @Before
+ public void setUp() {
+
+ createSnapshotCmd = new CreateSnapshotCmd() {
+
+ @Override
+ public String getCommandName() {
+ return "createsnapshotresponse";
+ }
+
+ @Override
+ public Long getVolumeId(){
+ return 1L;
+ }
+
+ @Override
+ public long getEntityOwnerId(){
+ return 1L;
+ }
+ };
+
+ }
+
+ @Test
+ public void testCreateSuccess() {
+
+ AccountService accountService = Mockito.mock(AccountService.class);
+ Account account = Mockito.mock(Account.class);
+ Mockito.when(accountService.getAccount(anyLong())).thenReturn(account);
+
+ VolumeApiService volumeApiService = Mockito.mock(VolumeApiService.class);
+ Snapshot snapshot = Mockito.mock(Snapshot.class);
+ try {
+
+ Mockito.when(volumeApiService.takeSnapshot(anyLong(), anyLong(), anyLong(),
+ any(Account.class), anyBoolean(), isNull(Snapshot.LocationType.class))).thenReturn(snapshot);
+
+ } catch (Exception e) {
+ Assert.fail("Received exception when success expected " + e.getMessage());
+ }
+
+ responseGenerator = Mockito.mock(ResponseGenerator.class);
+ SnapshotResponse snapshotResponse = Mockito.mock(SnapshotResponse.class);
+ Mockito.when(responseGenerator.createSnapshotResponse(snapshot)).thenReturn(snapshotResponse);
+ Mockito.doNothing().when(snapshotResponse).setAccountName(anyString());
+
+ createSnapshotCmd._accountService = accountService;
+ createSnapshotCmd._responseGenerator = responseGenerator;
+ createSnapshotCmd._volumeService = volumeApiService;
+
+ createSnapshotCmd.execute();
--- End diff --
Added assertion
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by syed <gi...@git.apache.org>.
Github user syed commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@mike-tutkowski @jburwell I've re-opened the PR to trigger jenkins again.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by syed <gi...@git.apache.org>.
Github user syed commented on the issue:
https://github.com/apache/cloudstack/pull/1600
Thanks @karuturi for your comments, I will get to them as soon as I have some free cycles. Right now this is only supported for XenServer as this requires the backend to be able to do native snapshots. If there is need, we can add VMWare/KVM support as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the issue:
https://github.com/apache/cloudstack/pull/1600
Looks like all the issues are resolved and the tests are good. I am merging this now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77080424
--- Diff: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java ---
@@ -92,7 +90,34 @@
@Override
public SnapshotInfo backupSnapshot(SnapshotInfo snapshotInfo) {
- return snapshotInfo;
+
+ if (!snapshotInfo.getLocationType().equals(Snapshot.LocationType.SECONDARY)) {
+
+ markAsBackedUp((SnapshotObject)snapshotInfo);
+ return snapshotInfo;
+ }
+
+ // At this point the snapshot is either taken as a native
+ // snapshot on the storage or exists as a volume on the storage (clone).
+ // If archive flag is passed, we will copy this snapshot to secondary
+ // storage and delete it from the primary storage.
+
+ Preconditions.checkState(Snapshot.LocationType.SECONDARY.equals(snapshotInfo.getLocationType()));
--- End diff --
Why is the check necessary given the check on Line 94?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77259356
--- Diff: engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java ---
@@ -380,6 +513,122 @@ private void handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, Templat
}
/**
+ * Creates a volume on the storage from a snapshot that resides on the secondary storage (archived snapshot).
+ * @param snapshotInfo snapshot on secondary
+ * @param volumeInfo volume to be created on the storage
+ * @param callback for async
+ */
+ private void handleCreateVolumeFromSnapshotOnSecondaryStorage(SnapshotInfo snapshotInfo, VolumeInfo volumeInfo, AsyncCompletionCallback<CopyCommandResult> callback) {
+
+ // at this point, the snapshotInfo and volumeInfo should have the same disk offering ID (so either one should be OK to get a DiskOfferingVO instance)
+ DiskOfferingVO diskOffering = _diskOfferingDao.findByIdIncludingRemoved(volumeInfo.getDiskOfferingId());
+ SnapshotVO snapshot = _snapshotDao.findById(snapshotInfo.getId());
+ DataStore destDataStore = volumeInfo.getDataStore();
+
+ // update the volume's hv_ss_reserve (hypervisor snapshot reserve) from a disk offering (used for managed storage)
+ _volumeService.updateHypervisorSnapshotReserveForVolume(diskOffering, volumeInfo.getId(), snapshot.getHypervisorType());
+
+
+ CopyCmdAnswer copyCmdAnswer = null;
+ String errMsg = null;
+
+ HostVO hostVO = null;
+ try {
+
+ //create a volume on the storage
+ AsyncCallFuture<VolumeApiResult> future = _volumeService.createVolumeAsync(volumeInfo, volumeInfo.getDataStore());
+ VolumeApiResult result = future.get();
--- End diff --
Added timout
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77074337
--- Diff: core/src/org/apache/cloudstack/storage/to/PrimaryDataStoreTO.java ---
@@ -51,6 +50,7 @@
private final String url;
private Map<String, String> details;
private static final String pathSeparator = "/";
+ private boolean isManaged;
--- End diff --
Since no mutator is exposed and the value gets set in the constructor, considering making it ``final``.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77259176
--- Diff: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java ---
@@ -92,7 +90,34 @@
@Override
public SnapshotInfo backupSnapshot(SnapshotInfo snapshotInfo) {
- return snapshotInfo;
+
+ if (!snapshotInfo.getLocationType().equals(Snapshot.LocationType.SECONDARY)) {
--- End diff --
Done
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by syed <gi...@git.apache.org>.
Github user syed commented on the issue:
https://github.com/apache/cloudstack/pull/1600
Sure @mike-tutkowski
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77254138
--- Diff: engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java ---
@@ -380,6 +513,122 @@ private void handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, Templat
}
/**
+ * Creates a volume on the storage from a snapshot that resides on the secondary storage (archived snapshot).
+ * @param snapshotInfo snapshot on secondary
+ * @param volumeInfo volume to be created on the storage
+ * @param callback for async
+ */
+ private void handleCreateVolumeFromSnapshotOnSecondaryStorage(SnapshotInfo snapshotInfo, VolumeInfo volumeInfo, AsyncCompletionCallback<CopyCommandResult> callback) {
+
+ // at this point, the snapshotInfo and volumeInfo should have the same disk offering ID (so either one should be OK to get a DiskOfferingVO instance)
+ DiskOfferingVO diskOffering = _diskOfferingDao.findByIdIncludingRemoved(volumeInfo.getDiskOfferingId());
+ SnapshotVO snapshot = _snapshotDao.findById(snapshotInfo.getId());
+ DataStore destDataStore = volumeInfo.getDataStore();
+
+ // update the volume's hv_ss_reserve (hypervisor snapshot reserve) from a disk offering (used for managed storage)
+ _volumeService.updateHypervisorSnapshotReserveForVolume(diskOffering, volumeInfo.getId(), snapshot.getHypervisorType());
+
+
+ CopyCmdAnswer copyCmdAnswer = null;
+ String errMsg = null;
+
+ HostVO hostVO = null;
+ try {
+
+ //create a volume on the storage
+ AsyncCallFuture<VolumeApiResult> future = _volumeService.createVolumeAsync(volumeInfo, volumeInfo.getDataStore());
+ VolumeApiResult result = future.get();
+
+ if (result.isFailed()) {
+ LOGGER.warn("Failed to create a volume: " + result.getResult());
+ throw new CloudRuntimeException(result.getResult());
+ }
+
+ volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(), volumeInfo.getDataStore());
+
+ volumeInfo.processEvent(Event.MigrationRequested);
+
+ volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(), volumeInfo.getDataStore());
+
+ hostVO = getHost(snapshotInfo.getDataCenterId(), false);
+
+ //copy the volume from secondary via the hypervisor
+ copyCmdAnswer = performCopyOfVdi(volumeInfo, snapshotInfo, hostVO);
+
+ if (copyCmdAnswer == null || !copyCmdAnswer.getResult()) {
+ if (copyCmdAnswer != null && !StringUtils.isEmpty(copyCmdAnswer.getDetails())) {
+ errMsg = copyCmdAnswer.getDetails();
+ }
+ else {
+ errMsg = "Unable to create volume from snapshot";
+ }
+ }
+ }
+ catch (Exception ex) {
+ errMsg = ex.getMessage() != null ? ex.getMessage() : "Copy operation failed in 'StorageSystemDataMotionStrategy.handleCreateVolumeFromSnapshotBothOnStorageSystem'";
+ } finally {
+
+ // detach and remove access after the volume is created
+ DiskTO disk = new DiskTO(volumeInfo.getTO(), volumeInfo.getDeviceId(), volumeInfo.getPath(), volumeInfo.getVolumeType());
+ DettachCommand cmd = new DettachCommand(disk, null);
+ long storagePoolId = volumeInfo.getPoolId();
+ StoragePoolVO storagePool = _storagePoolDao.findById(storagePoolId);
+
+ cmd.setManaged(true);
+ cmd.setStorageHost(storagePool.getHostAddress());
+ cmd.setStoragePort(storagePool.getPort());
+ cmd.set_iScsiName(volumeInfo.get_iScsiName());
+
+ try {
+ DettachAnswer dettachAnswer = (DettachAnswer) _agentMgr.send(hostVO.getId(), cmd);
+
+ if (!dettachAnswer.getResult()) {
+ throw new CloudRuntimeException("Error detaching volume:" + dettachAnswer.getDetails());
+ }
+
+ } catch (Exception e) {
+ LOGGER.warn("Error detaching volume " + volumeInfo.getId() + " Error: " + e.getMessage());
+ }
+
+ _volumeService.revokeAccess(volumeInfo, hostVO, destDataStore);
+ }
+
+ CopyCommandResult result = new CopyCommandResult(null, copyCmdAnswer);
+
+ result.setResult(errMsg);
+
+ callback.complete(result);
+
+ }
+
+ private void performCleanupCacheStorage(DataObject destOnStore) {
+ destOnStore.processEvent(Event.DestroyRequested);
+
+ DeleteCommand deleteCommand = new DeleteCommand(destOnStore.getTO());
+ EndPoint ep = selector.select(destOnStore);
+ try {
+ if (ep == null) {
+ LOGGER.warn("Unable to cleanup staging NFS because no endpoint was found " +
+ "Object: " + destOnStore.getType() + " ID: " + destOnStore.getId());
+
+ destOnStore.processEvent(Event.OperationFailed);
+ } else {
+ Answer deleteAnswer = ep.sendMessage(deleteCommand);
+ if (deleteAnswer != null && deleteAnswer.getResult()) {
+ LOGGER.warn("Unable to cleanup staging NFS " + deleteAnswer.getDetails() +
+ "Object: " + destOnStore.getType() + " ID: " + destOnStore.getId());
+ destOnStore.processEvent(Event.OperationFailed);
+ }
+ }
+
+ destOnStore.processEvent(Event.OperationSuccessed);
+ } catch (Exception e) {
--- End diff --
We don't want to error out when we fail to clean up cache storage. I have added a statemachine transition using the event `OperationFailed` in the catch block
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r70425148
--- Diff: api/src/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotCmd.java ---
@@ -195,24 +196,37 @@ public void create() throws ResourceAllocationException {
@Override
public void execute() {
- s_logger.info("VOLSS: createSnapshotCmd starts:" + System.currentTimeMillis());
- CallContext.current().setEventDetails("Volume Id: " + getVolumeUuid());
Snapshot snapshot;
try {
snapshot =
- _volumeService.takeSnapshot(getVolumeId(), getPolicyId(), getEntityId(), _accountService.getAccount(getEntityOwnerId()), getQuiescevm());
+ _volumeService.takeSnapshot(getVolumeId(), getPolicyId(), getEntityId(), _accountService.getAccount(getEntityOwnerId()), getQuiescevm(), getLocationType());
+
if (snapshot != null) {
SnapshotResponse response = _responseGenerator.createSnapshotResponse(snapshot);
response.setResponseName(getCommandName());
setResponseObject(response);
} else {
- throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to create snapshot due to an internal error creating snapshot for volume " + volumeId);
+ throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to create snapshot due to an internal error creating snapshot for volume " + getVolumeId());
}
} catch (Exception e) {
- throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to create snapshot due to an internal error creating snapshot for volume " + volumeId);
+ throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to create snapshot due to an internal error creating snapshot for volume " + getVolumeId());
}
}
+ private Snapshot.LocationType getLocationType() {
+ if (Snapshot.LocationType.values() == null || Snapshot.LocationType.values().length == 0) {
+ return null;
+ }
+
+ Snapshot.LocationType locationTypeToReturn = Snapshot.LocationType.values()[0];
+
+ if (locationType != null && locationType >= 1 && locationType <= Snapshot.LocationType.values().length) {
+ locationTypeToReturn = Snapshot.LocationType.values()[locationType-1];
--- End diff --
I would suggest keeping the enum values all upper case, make locationType string in cmd and use LocationType.valueOf(cmd.locationType.toUpperCase())
https://books.google.co.in/books?id=ka2VUBqHiWkC&lpg=PA158&dq=Never%20derive%20a%20value%20associated%20with%20an%20enum%20from%20its%20ordinal&pg=PA158#v=onepage&q&f=false
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@syed @jburwell I should be able to run the tests toward the end of the week or during the weekend. At the moment, my resources in the lab are devoted to other CloudStack dev activities.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77210539
--- Diff: engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java ---
@@ -380,6 +513,122 @@ private void handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, Templat
}
/**
+ * Creates a volume on the storage from a snapshot that resides on the secondary storage (archived snapshot).
+ * @param snapshotInfo snapshot on secondary
+ * @param volumeInfo volume to be created on the storage
+ * @param callback for async
+ */
+ private void handleCreateVolumeFromSnapshotOnSecondaryStorage(SnapshotInfo snapshotInfo, VolumeInfo volumeInfo, AsyncCompletionCallback<CopyCommandResult> callback) {
+
+ // at this point, the snapshotInfo and volumeInfo should have the same disk offering ID (so either one should be OK to get a DiskOfferingVO instance)
+ DiskOfferingVO diskOffering = _diskOfferingDao.findByIdIncludingRemoved(volumeInfo.getDiskOfferingId());
+ SnapshotVO snapshot = _snapshotDao.findById(snapshotInfo.getId());
+ DataStore destDataStore = volumeInfo.getDataStore();
+
+ // update the volume's hv_ss_reserve (hypervisor snapshot reserve) from a disk offering (used for managed storage)
+ _volumeService.updateHypervisorSnapshotReserveForVolume(diskOffering, volumeInfo.getId(), snapshot.getHypervisorType());
+
+
+ CopyCmdAnswer copyCmdAnswer = null;
+ String errMsg = null;
+
+ HostVO hostVO = null;
+ try {
+
+ //create a volume on the storage
+ AsyncCallFuture<VolumeApiResult> future = _volumeService.createVolumeAsync(volumeInfo, volumeInfo.getDataStore());
+ VolumeApiResult result = future.get();
--- End diff --
The storage drivers have a timeout logic in them already.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77073945
--- Diff: api/test/org/apache/cloudstack/api/command/test/CreateSnapshotCmdTest.java ---
@@ -0,0 +1,130 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.test;
+
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.VolumeApiService;
+import com.cloud.user.Account;
+import com.cloud.user.AccountService;
+import junit.framework.TestCase;
+import org.apache.cloudstack.api.ResponseGenerator;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.user.snapshot.CreateSnapshotCmd;
+import org.apache.cloudstack.api.response.SnapshotResponse;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.mockito.Mockito;
+
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyBoolean;
+import static org.mockito.Matchers.anyLong;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Matchers.isNull;
+
+public class CreateSnapshotCmdTest extends TestCase {
+
+ private CreateSnapshotCmd createSnapshotCmd;
+ private ResponseGenerator responseGenerator;
+
+ @Rule
+ public ExpectedException expectedException = ExpectedException.none();
+
+ @Override
+ @Before
+ public void setUp() {
+
+ createSnapshotCmd = new CreateSnapshotCmd() {
+
+ @Override
+ public String getCommandName() {
+ return "createsnapshotresponse";
+ }
+
+ @Override
+ public Long getVolumeId(){
+ return 1L;
+ }
+
+ @Override
+ public long getEntityOwnerId(){
+ return 1L;
+ }
+ };
+
+ }
+
+ @Test
+ public void testCreateSuccess() {
+
+ AccountService accountService = Mockito.mock(AccountService.class);
+ Account account = Mockito.mock(Account.class);
+ Mockito.when(accountService.getAccount(anyLong())).thenReturn(account);
+
+ VolumeApiService volumeApiService = Mockito.mock(VolumeApiService.class);
+ Snapshot snapshot = Mockito.mock(Snapshot.class);
+ try {
+
+ Mockito.when(volumeApiService.takeSnapshot(anyLong(), anyLong(), anyLong(),
+ any(Account.class), anyBoolean(), isNull(Snapshot.LocationType.class))).thenReturn(snapshot);
+
+ } catch (Exception e) {
+ Assert.fail("Received exception when success expected " + e.getMessage());
+ }
+
+ responseGenerator = Mockito.mock(ResponseGenerator.class);
+ SnapshotResponse snapshotResponse = Mockito.mock(SnapshotResponse.class);
+ Mockito.when(responseGenerator.createSnapshotResponse(snapshot)).thenReturn(snapshotResponse);
+ Mockito.doNothing().when(snapshotResponse).setAccountName(anyString());
+
+ createSnapshotCmd._accountService = accountService;
+ createSnapshotCmd._responseGenerator = responseGenerator;
+ createSnapshotCmd._volumeService = volumeApiService;
+
+ createSnapshotCmd.execute();
--- End diff --
Why aren't there any assertions in this test method? Seems like it should at least validate the response returned by ``createSnapshotCmd.execute()``.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@karuturi Anything else @syed needs to do for this one? Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the issue:
https://github.com/apache/cloudstack/pull/1600
BTw, does this work only with xenserver? I dont see any vmware related changes
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
GitHub user syed reopened a pull request:
https://github.com/apache/cloudstack/pull/1600
Support Backup of Snapshots for Managed Storage
This PR adds an ability to Pass a new parameter, locationType,
to the \u201ccreateSnapshot\u201d API command. Depending on the locationType,
we decide where the snapshot should go in case of managed storage.
There are two possible values for the locationType param
1) `Primary`: The standard operation for managed storage is to
keep the snapshot on the device (primary). For non-managed storage, this will
give an error as this option is only supported for managed storage
2) `Secondary`: Applicable only to managed storage. This will
keep the snapshot on the secondary storage. For non-managed
storage, this will result in an error.
The reason for implementing this feature is to avoid a single
point of failure for primary storage. Right now in case of managed
storage, if the primary storage goes down, there is no easy way
to recover data as all snapshots are also stored on the primary.
This features allows us to mitigate that risk.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/syed/cloudstack snapshot-archive-pr
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1600.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1600
----
commit bfad68efa35b94f67470fd8b549aca0cc7ff7d30
Author: Syed <sy...@gmail.com>
Date: 2016-06-30T17:37:33Z
Support Backup of Snapshots for Managed Storage
This PR adds an ability to Pass a new parameter, locationType,
to the \u201ccreateSnapshot\u201d API command. Depending on the locationType,
we decide where the snapshot should go in case of managed storage.
There are two possible values for the locationType param
1) `Standard`: The standard operation for managed storage is to
keep the snapshot on the device. For non-managed storage, this will
be to upload it to secondary storage. This option will be the
default.
2) `Archive`: Applicable only to managed storage. This will
keep the snapshot on the secondary storage. For non-managed
storage, this will result in an error.
The reason for implementing this feature is to avoid a single
point of failure for primary storage. Right now in case of managed
storage, if the primary storage goes down, there is no easy way
to recover data as all snapshots are also stored on the primary.
This features allows us to mitigate that risk.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77075736
--- Diff: engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java ---
@@ -328,15 +412,64 @@ private void handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, Templat
copyCommand.setOptions(srcDetails);
copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand);
- }
- catch (CloudRuntimeException | AgentUnavailableException | OperationTimedoutException ex) {
+
+ if (needCache) {
+
+ // If cached storage was needed (in case of object store as secondary
+ // storage), at this point, the data has been copied from the primary
+ // to the NFS cache by the hypervisor. We now invoke another copy
+ // command to copy this data from cache to secondary storage. We
+ // then cleanup the cache
+
+ destOnStore.processEvent(Event.OperationSuccessed, copyCmdAnswer);
+
+ CopyCommand cmd = new CopyCommand(destOnStore.getTO(), destData.getTO(), primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
+ EndPoint ep = selector.select(destOnStore, destData);
+
+ if (ep == null) {
+ errMsg = "No remote endpoint to send command, check if host or ssvm is down?";
+ LOGGER.error(errMsg);
+ copyCmdAnswer = new CopyCmdAnswer(errMsg);
+ } else {
+ copyCmdAnswer = (CopyCmdAnswer) ep.sendMessage(cmd);
+ }
+
+ // clean up snapshot copied to staging
+ performCleanupCacheStorage(destOnStore);
+ }
+
+
+ } catch (CloudRuntimeException | AgentUnavailableException | OperationTimedoutException ex) {
String msg = "Failed to create template from snapshot (Snapshot ID = " + snapshotInfo.getId() + ") : ";
LOGGER.warn(msg, ex);
throw new CloudRuntimeException(msg + ex.getMessage());
- }
- finally {
+
+ } finally {
+
+ // detach and remove access after the volume is created
+ SnapshotObjectTO snapshotObjectTO = (SnapshotObjectTO) snapshotInfo.getTO();
+ DiskTO disk = new DiskTO(snapshotObjectTO, null, snapshotInfo.getPath(), Volume.Type.UNKNOWN);
+ DettachCommand cmd = new DettachCommand(disk, null);
+ StoragePoolVO storagePool = _storagePoolDao.findById(srcDataStore.getId());
+
+ cmd.setManaged(true);
+ cmd.setStorageHost(storagePool.getHostAddress());
+ cmd.setStoragePort(storagePool.getPort());
+ cmd.set_iScsiName(getProperty(snapshotInfo.getId(), DiskTO.IQN));
+
+ try {
+ DettachAnswer dettachAnswer = (DettachAnswer) _agentMgr.send(hostVO.getId(), cmd);
+
+ if (!dettachAnswer.getResult()) {
+ throw new CloudRuntimeException("Error detaching volume:" + dettachAnswer.getDetails());
--- End diff --
Throwing an exception here will end processing of this ``finally`` block, and skipping the ``_volumeService.revokeAccess`` call at the end of the method. What would be the impact of skipping that call in the event an exception is thrown here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
My third test run is successful - TestVMSnapshots.py:
test_01_take_VM_snapshot (TestVMSnapshots.TestVMSnapshots) ... === TestName: test_01_take_VM_snapshot | Status : SUCCESS ===
ok
test_02_take_VM_snapshot_with_data_disk (TestVMSnapshots.TestVMSnapshots) ... === TestName: test_02_take_VM_snapshot_with_data_disk | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 2 tests in 765.252s
OK
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
Sounds good, @syed Since what you added is the same as what's currently in master, it looks like your list of changed files for this PR is down to 35 (only one of which is a Python file, base, py). That looks good.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
Github user syed closed the pull request at:
https://github.com/apache/cloudstack/pull/1600
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77259286
--- Diff: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java ---
@@ -92,7 +90,34 @@
@Override
public SnapshotInfo backupSnapshot(SnapshotInfo snapshotInfo) {
- return snapshotInfo;
+
--- End diff --
Added precondition
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77079000
--- Diff: engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java ---
@@ -380,6 +513,122 @@ private void handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, Templat
}
/**
+ * Creates a volume on the storage from a snapshot that resides on the secondary storage (archived snapshot).
+ * @param snapshotInfo snapshot on secondary
+ * @param volumeInfo volume to be created on the storage
+ * @param callback for async
+ */
+ private void handleCreateVolumeFromSnapshotOnSecondaryStorage(SnapshotInfo snapshotInfo, VolumeInfo volumeInfo, AsyncCompletionCallback<CopyCommandResult> callback) {
+
+ // at this point, the snapshotInfo and volumeInfo should have the same disk offering ID (so either one should be OK to get a DiskOfferingVO instance)
+ DiskOfferingVO diskOffering = _diskOfferingDao.findByIdIncludingRemoved(volumeInfo.getDiskOfferingId());
+ SnapshotVO snapshot = _snapshotDao.findById(snapshotInfo.getId());
+ DataStore destDataStore = volumeInfo.getDataStore();
+
+ // update the volume's hv_ss_reserve (hypervisor snapshot reserve) from a disk offering (used for managed storage)
+ _volumeService.updateHypervisorSnapshotReserveForVolume(diskOffering, volumeInfo.getId(), snapshot.getHypervisorType());
+
+
+ CopyCmdAnswer copyCmdAnswer = null;
+ String errMsg = null;
+
+ HostVO hostVO = null;
+ try {
+
+ //create a volume on the storage
+ AsyncCallFuture<VolumeApiResult> future = _volumeService.createVolumeAsync(volumeInfo, volumeInfo.getDataStore());
+ VolumeApiResult result = future.get();
+
+ if (result.isFailed()) {
+ LOGGER.warn("Failed to create a volume: " + result.getResult());
+ throw new CloudRuntimeException(result.getResult());
+ }
+
+ volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(), volumeInfo.getDataStore());
+
+ volumeInfo.processEvent(Event.MigrationRequested);
+
+ volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(), volumeInfo.getDataStore());
+
+ hostVO = getHost(snapshotInfo.getDataCenterId(), false);
+
+ //copy the volume from secondary via the hypervisor
+ copyCmdAnswer = performCopyOfVdi(volumeInfo, snapshotInfo, hostVO);
+
+ if (copyCmdAnswer == null || !copyCmdAnswer.getResult()) {
+ if (copyCmdAnswer != null && !StringUtils.isEmpty(copyCmdAnswer.getDetails())) {
+ errMsg = copyCmdAnswer.getDetails();
+ }
+ else {
+ errMsg = "Unable to create volume from snapshot";
+ }
+ }
+ }
+ catch (Exception ex) {
+ errMsg = ex.getMessage() != null ? ex.getMessage() : "Copy operation failed in 'StorageSystemDataMotionStrategy.handleCreateVolumeFromSnapshotBothOnStorageSystem'";
+ } finally {
+
+ // detach and remove access after the volume is created
+ DiskTO disk = new DiskTO(volumeInfo.getTO(), volumeInfo.getDeviceId(), volumeInfo.getPath(), volumeInfo.getVolumeType());
+ DettachCommand cmd = new DettachCommand(disk, null);
+ long storagePoolId = volumeInfo.getPoolId();
+ StoragePoolVO storagePool = _storagePoolDao.findById(storagePoolId);
+
+ cmd.setManaged(true);
+ cmd.setStorageHost(storagePool.getHostAddress());
+ cmd.setStoragePort(storagePool.getPort());
+ cmd.set_iScsiName(volumeInfo.get_iScsiName());
+
+ try {
+ DettachAnswer dettachAnswer = (DettachAnswer) _agentMgr.send(hostVO.getId(), cmd);
+
+ if (!dettachAnswer.getResult()) {
+ throw new CloudRuntimeException("Error detaching volume:" + dettachAnswer.getDetails());
+ }
+
+ } catch (Exception e) {
+ LOGGER.warn("Error detaching volume " + volumeInfo.getId() + " Error: " + e.getMessage());
+ }
+
+ _volumeService.revokeAccess(volumeInfo, hostVO, destDataStore);
+ }
+
+ CopyCommandResult result = new CopyCommandResult(null, copyCmdAnswer);
+
+ result.setResult(errMsg);
+
+ callback.complete(result);
+
+ }
+
+ private void performCleanupCacheStorage(DataObject destOnStore) {
+ destOnStore.processEvent(Event.DestroyRequested);
+
+ DeleteCommand deleteCommand = new DeleteCommand(destOnStore.getTO());
+ EndPoint ep = selector.select(destOnStore);
+ try {
+ if (ep == null) {
+ LOGGER.warn("Unable to cleanup staging NFS because no endpoint was found " +
+ "Object: " + destOnStore.getType() + " ID: " + destOnStore.getId());
+
+ destOnStore.processEvent(Event.OperationFailed);
+ } else {
+ Answer deleteAnswer = ep.sendMessage(deleteCommand);
+ if (deleteAnswer != null && deleteAnswer.getResult()) {
+ LOGGER.warn("Unable to cleanup staging NFS " + deleteAnswer.getDetails() +
+ "Object: " + destOnStore.getType() + " ID: " + destOnStore.getId());
+ destOnStore.processEvent(Event.OperationFailed);
+ }
+ }
+
+ destOnStore.processEvent(Event.OperationSuccessed);
+ } catch (Exception e) {
--- End diff --
Why are all checked and unchecked exceptions being caught?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the issue:
https://github.com/apache/cloudstack/pull/1600
`locationType` Standard and Archive refer to either secondary or primary based on the storage type and is confusing(atleast for me). Can we make this simple and use the locationType as primary or secondary?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@blueorangutan test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by syed <gi...@git.apache.org>.
Github user syed commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@mike-tutkowski , the list snapshot already returns the `loacationType`. I think you've already added that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@blueorangutan package
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1600
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by syed <gi...@git.apache.org>.
Github user syed commented on the issue:
https://github.com/apache/cloudstack/pull/1600
This was done in two parts:
The work to add the API parameter was done by @mike-tutkowski and the work to enable this on the backend was done by me. The rebase destroyed history but I would like to correctly credit the contributors :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77259407
--- Diff: engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java ---
@@ -328,15 +412,64 @@ private void handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, Templat
copyCommand.setOptions(srcDetails);
copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand);
- }
- catch (CloudRuntimeException | AgentUnavailableException | OperationTimedoutException ex) {
+
+ if (needCache) {
+
+ // If cached storage was needed (in case of object store as secondary
+ // storage), at this point, the data has been copied from the primary
+ // to the NFS cache by the hypervisor. We now invoke another copy
+ // command to copy this data from cache to secondary storage. We
+ // then cleanup the cache
+
+ destOnStore.processEvent(Event.OperationSuccessed, copyCmdAnswer);
+
+ CopyCommand cmd = new CopyCommand(destOnStore.getTO(), destData.getTO(), primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
+ EndPoint ep = selector.select(destOnStore, destData);
+
+ if (ep == null) {
+ errMsg = "No remote endpoint to send command, check if host or ssvm is down?";
+ LOGGER.error(errMsg);
+ copyCmdAnswer = new CopyCmdAnswer(errMsg);
+ } else {
+ copyCmdAnswer = (CopyCmdAnswer) ep.sendMessage(cmd);
+ }
+
+ // clean up snapshot copied to staging
+ performCleanupCacheStorage(destOnStore);
+ }
+
+
+ } catch (CloudRuntimeException | AgentUnavailableException | OperationTimedoutException ex) {
String msg = "Failed to create template from snapshot (Snapshot ID = " + snapshotInfo.getId() + ") : ";
LOGGER.warn(msg, ex);
throw new CloudRuntimeException(msg + ex.getMessage());
- }
- finally {
+
+ } finally {
+
+ // detach and remove access after the volume is created
+ SnapshotObjectTO snapshotObjectTO = (SnapshotObjectTO) snapshotInfo.getTO();
+ DiskTO disk = new DiskTO(snapshotObjectTO, null, snapshotInfo.getPath(), Volume.Type.UNKNOWN);
+ DettachCommand cmd = new DettachCommand(disk, null);
+ StoragePoolVO storagePool = _storagePoolDao.findById(srcDataStore.getId());
+
+ cmd.setManaged(true);
+ cmd.setStorageHost(storagePool.getHostAddress());
+ cmd.setStoragePort(storagePool.getPort());
+ cmd.set_iScsiName(getProperty(snapshotInfo.getId(), DiskTO.IQN));
+
+ try {
+ DettachAnswer dettachAnswer = (DettachAnswer) _agentMgr.send(hostVO.getId(), cmd);
+
+ if (!dettachAnswer.getResult()) {
+ throw new CloudRuntimeException("Error detaching volume:" + dettachAnswer.getDetails());
--- End diff --
Logging, instead of throwing exception
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
I have run all of the relevant managed-storage tests and all passed (details below).
I did encounter one issue:
When the fourth test of TestSnapshots ran, it failed because I did not have the base.py file from this PR in my Marvin install. I went ahead and rebuilt Marvin and re-installed it.
The same test then encountered another issue:
As it turns out, the new (optional) parameter, locationtype, was still annotated as a CommandType.SHORT (the original design), even though the variable it annotates was changed to be a String during the PR review. I went ahead and made changes to CreateSnapshot.java (now the variable is annotated as a CommandType.STRING) and to the Marvin test, TestSnapshot.py.
The only code that would have noticed this parameter issue is this one test (none of the other tests in our system pass in this new (optional) parameter).
Here are the details of the test results:
TestSnapshots:
Note the error on the fourth test (and then that it passes after the noted code changes):
test_01_create_volume_snapshot_using_sf_snapshot (TestSnapshots.TestSnapshots) ... === TestName: test_01_create_volume_snapshot_using_sf_snapshot | Status : SUCCESS ===
ok
test_02_create_volume_snapshot_using_sf_volume (TestSnapshots.TestSnapshots) ... === TestName: test_02_create_volume_snapshot_using_sf_volume | Status : SUCCESS ===
ok
test_03_create_volume_snapshot_using_sf_volume_and_sf_snapshot (TestSnapshots.TestSnapshots) ... === TestName: test_03_create_volume_snapshot_using_sf_volume_and_sf_snapshot | Status : SUCCESS ===
ok
test_04_create_volume_snapshot_using_sf_snapshot_and_archiving (TestSnapshots.TestSnapshots) ... === TestName: test_04_create_volume_snapshot_using_sf_snapshot_and_archiving | Status : EXCEPTION ===
ERROR
After the noted code changes:
test_04_create_volume_snapshot_using_sf_snapshot_and_archiving (TestSnapshots.TestSnapshots) ... === TestName: test_04_create_volume_snapshot_using_sf_snapshot_and_archiving | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 1 test in 3107.229s
OK
TestAddRemoveHosts:
test_add_remove_host_with_solidfire_plugin_1 (TestAddRemoveHosts.TestAddRemoveHosts) ... === TestName: test_add_remove_host_with_solidfire_plugin_1 | Status : SUCCESS ===
ok
test_add_remove_host_with_solidfire_plugin_2 (TestAddRemoveHosts.TestAddRemoveHosts) ... === TestName: test_add_remove_host_with_solidfire_plugin_2 | Status : SUCCESS ===
ok
test_add_remove_host_with_solidfire_plugin_3 (TestAddRemoveHosts.TestAddRemoveHosts) ... === TestName: test_add_remove_host_with_solidfire_plugin_3 | Status : SUCCESS ===
ok
test_add_remove_host_with_solidfire_plugin_4 (TestAddRemoveHosts.TestAddRemoveHosts) ... === TestName: test_add_remove_host_with_solidfire_plugin_4 | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 4 tests in 4357.199s
OK
TestVMSnapshots:
test_01_take_VM_snapshot (TestVMSnapshots.TestVMSnapshots) ... === TestName: test_01_take_VM_snapshot | Status : SUCCESS ===
ok
test_02_take_VM_snapshot_with_data_disk (TestVMSnapshots.TestVMSnapshots) ... === TestName: test_02_take_VM_snapshot_with_data_disk | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 2 tests in 750.531s
OK
TestVolumes (no XenServer SR resigning):
test_00_check_template_cache (TestVolumes.TestVolumes) ... === TestName: test_00_check_template_cache | Status : SUCCESS ===
ok
Attach a volume to a stopped virtual machine, then start VM ... === TestName: test_01_attach_new_volume_to_stopped_VM | Status : SUCCESS ===
ok
Attach, detach, and attach volume to a running VM ... === TestName: test_02_attach_detach_attach_volume | Status : SUCCESS ===
ok
Attach volume to running VM, then reboot. ... === TestName: test_03_attached_volume_reboot_VM | Status : SUCCESS ===
ok
Detach volume from a running VM, then reboot. ... === TestName: test_04_detach_volume_reboot | Status : SUCCESS ===
ok
Detach volume from a stopped VM, then start. ... === TestName: test_05_detach_vol_stopped_VM_start | Status : SUCCESS ===
ok
Attach a volume to a stopped virtual machine, then start VM ... === TestName: test_06_attach_volume_to_stopped_VM | Status : SUCCESS ===
ok
Destroy and expunge VM with attached volume ... === TestName: test_07_destroy_expunge_VM_with_volume | Status : SUCCESS ===
ok
Delete volume that was attached to a VM and is detached now ... === TestName: test_08_delete_volume_was_attached | Status : SUCCESS ===
ok
Attach a data disk to a VM in one account and attach another data disk to a VM in another account ... === TestName: test_09_attach_volumes_multiple_accounts | Status : SUCCESS ===
ok
Attach more than one disk to a VM ... === TestName: test_10_attach_more_than_one_disk_to_VM | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 11 tests in 1955.862s
OK
TestVolumes (XenServer SR resigning):
test_00_check_template_cache (TestVolumes.TestVolumes) ... === TestName: test_00_check_template_cache | Status : SUCCESS ===
ok
Attach a volume to a stopped virtual machine, then start VM ... === TestName: test_01_attach_new_volume_to_stopped_VM | Status : SUCCESS ===
ok
Attach, detach, and attach volume to a running VM ... === TestName: test_02_attach_detach_attach_volume | Status : SUCCESS ===
ok
Attach volume to running VM, then reboot. ... === TestName: test_03_attached_volume_reboot_VM | Status : SUCCESS ===
ok
Detach volume from a running VM, then reboot. ... === TestName: test_04_detach_volume_reboot | Status : SUCCESS ===
ok
Detach volume from a stopped VM, then start. ... === TestName: test_05_detach_vol_stopped_VM_start | Status : SUCCESS ===
ok
Attach a volume to a stopped virtual machine, then start VM ... === TestName: test_06_attach_volume_to_stopped_VM | Status : SUCCESS ===
ok
Destroy and expunge VM with attached volume ... === TestName: test_07_destroy_expunge_VM_with_volume | Status : SUCCESS ===
ok
Delete volume that was attached to a VM and is detached now ... === TestName: test_08_delete_volume_was_attached | Status : SUCCESS ===
ok
Attach a data disk to a VM in one account and attach another data disk to a VM in another account ... === TestName: test_09_attach_volumes_multiple_accounts | Status : SUCCESS ===
ok
Attach more than one disk to a VM ... === TestName: test_10_attach_more_than_one_disk_to_VM | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 11 tests in 2362.066s
OK
TestVMMigrationWithStorage:
test_01_storage_migrate_root_and_data_disks (TestVMMigrationWithStorage.TestVMMigrationWithStorage) ... === TestName: test_01_storage_migrate_root_and_data_disks | Status : SUCCESS ===
ok
test_02_storage_migrate_root_and_data_disks (TestVMMigrationWithStorage.TestVMMigrationWithStorage) ... === TestName: test_02_storage_migrate_root_and_data_disks | Status : SUCCESS ===
ok
test_03_storage_migrate_root_and_data_disks_fail (TestVMMigrationWithStorage.TestVMMigrationWithStorage) ... === TestName: test_03_storage_migrate_root_and_data_disks_fail | Status : SUCCESS ===
ok
test_04_storage_migrate_root_disk_fails (TestVMMigrationWithStorage.TestVMMigrationWithStorage) ... === TestName: test_04_storage_migrate_root_disk_fails | Status : SUCCESS ===
ok
test_05_storage_migrate_data_disk_fails (TestVMMigrationWithStorage.TestVMMigrationWithStorage) ... === TestName: test_05_storage_migrate_data_disk_fails | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 5 tests in 3589.890s
OK
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by syed <gi...@git.apache.org>.
Github user syed commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@mike-tutkowski @jburwell looks like we are green now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by syed <gi...@git.apache.org>.
Github user syed commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@mike-tutkowski I think the `Destroyed` state should suffice.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by syed <gi...@git.apache.org>.
Github user syed commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@jburwell Rebased with master and resolved conflicts
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
Re: [GitHub] cloudstack issue #1600: Support Backup of Snapshots for
Managed Storage
Posted by "Tutkowski, Mike" <Mi...@netapp.com>.
Well...it does say it's in the Destroyed state.
I thought we removed these kinds of rows from this table, but it looks like we don't.
That being the case, I think we can ignore what I asked for here.
________________________________________
From: mike-tutkowski <gi...@git.apache.org>
Sent: Monday, July 11, 2016 1:39 PM
To: dev@cloudstack.apache.org
Subject: [GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
A comment:
I have observed that the SR that was previously not being detached from XenServer is now being detached with your newer version of the code.
One thing I noticed in the DB:
In the snapshot_store_ref table, we still have a row for the snapshot that indicates there is a version of the snapshot on primary storage. We also - correctly - have a row indicating that there is a version of the snapshot on secondary storage.
I think we should remove the row from the snapshot_store_ref table that links the snapshot to a version on primary storage.
________________________________
From: Tutkowski, Mike
Sent: Monday, July 4, 2016 1:24 PM
To: apache/cloudstack
Cc: apache/cloudstack; Mention
Subject: Re: [apache/cloudstack] Support Backup of Snapshots for Managed Storage (#1600)
My mistake. I forgot that I added that a while ago. :)
On Jul 4, 2016, at 12:40 PM, Syed Mushtaq Ahmed <no...@github.com>> wrote:
@mike-tutkowski<https://github.com/mike-tutkowski> , the list snapshot already returns the loacationType. I think you've already added that.
-
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<https://github.com/apache/cloudstack/pull/1600#issuecomment-230340112>, or mute the thread<https://github.com/notifications/unsubscribe/AC4SHwCngBVFvcsRcZLcRdKHyNBdMY2xks5qSVORgaJpZM4JCaMg>.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
A comment:
I have observed that the SR that was previously not being detached from XenServer is now being detached with your newer version of the code.
One thing I noticed in the DB:
In the snapshot_store_ref table, we still have a row for the snapshot that indicates there is a version of the snapshot on primary storage. We also - correctly - have a row indicating that there is a version of the snapshot on secondary storage.
I think we should remove the row from the snapshot_store_ref table that links the snapshot to a version on primary storage.
________________________________
From: Tutkowski, Mike
Sent: Monday, July 4, 2016 1:24 PM
To: apache/cloudstack
Cc: apache/cloudstack; Mention
Subject: Re: [apache/cloudstack] Support Backup of Snapshots for Managed Storage (#1600)
My mistake. I forgot that I added that a while ago. :)
On Jul 4, 2016, at 12:40 PM, Syed Mushtaq Ahmed <no...@github.com>> wrote:
@mike-tutkowski<https://github.com/mike-tutkowski> , the list snapshot already returns the loacationType. I think you've already added that.
-
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<https://github.com/apache/cloudstack/pull/1600#issuecomment-230340112>, or mute the thread<https://github.com/notifications/unsubscribe/AC4SHwCngBVFvcsRcZLcRdKHyNBdMY2xks5qSVORgaJpZM4JCaMg>.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by syed <gi...@git.apache.org>.
Github user syed commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@jburwell I've rebased against the master. The merge should be good now. I've addressed your comments as well. @mike-tutkowski has ran his tests on this functionality.
https://github.com/apache/cloudstack/pull/1600#issuecomment-229981990
https://github.com/apache/cloudstack/pull/1600#issuecomment-229965967
What would be the next steps here
cc: @karuturi
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
Just following up on this one. Is there any pending work item that remains or are we able to merge? Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77259373
--- Diff: engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java ---
@@ -380,6 +513,122 @@ private void handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, Templat
}
/**
+ * Creates a volume on the storage from a snapshot that resides on the secondary storage (archived snapshot).
+ * @param snapshotInfo snapshot on secondary
+ * @param volumeInfo volume to be created on the storage
+ * @param callback for async
+ */
+ private void handleCreateVolumeFromSnapshotOnSecondaryStorage(SnapshotInfo snapshotInfo, VolumeInfo volumeInfo, AsyncCompletionCallback<CopyCommandResult> callback) {
+
+ // at this point, the snapshotInfo and volumeInfo should have the same disk offering ID (so either one should be OK to get a DiskOfferingVO instance)
+ DiskOfferingVO diskOffering = _diskOfferingDao.findByIdIncludingRemoved(volumeInfo.getDiskOfferingId());
+ SnapshotVO snapshot = _snapshotDao.findById(snapshotInfo.getId());
+ DataStore destDataStore = volumeInfo.getDataStore();
+
+ // update the volume's hv_ss_reserve (hypervisor snapshot reserve) from a disk offering (used for managed storage)
+ _volumeService.updateHypervisorSnapshotReserveForVolume(diskOffering, volumeInfo.getId(), snapshot.getHypervisorType());
+
+
+ CopyCmdAnswer copyCmdAnswer = null;
+ String errMsg = null;
+
+ HostVO hostVO = null;
+ try {
+
+ //create a volume on the storage
+ AsyncCallFuture<VolumeApiResult> future = _volumeService.createVolumeAsync(volumeInfo, volumeInfo.getDataStore());
+ VolumeApiResult result = future.get();
+
+ if (result.isFailed()) {
+ LOGGER.warn("Failed to create a volume: " + result.getResult());
--- End diff --
changed to ERROR
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77259303
--- Diff: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java ---
@@ -92,7 +90,34 @@
@Override
public SnapshotInfo backupSnapshot(SnapshotInfo snapshotInfo) {
- return snapshotInfo;
+
+ if (!snapshotInfo.getLocationType().equals(Snapshot.LocationType.SECONDARY)) {
+
+ markAsBackedUp((SnapshotObject)snapshotInfo);
+ return snapshotInfo;
+ }
+
+ // At this point the snapshot is either taken as a native
+ // snapshot on the storage or exists as a volume on the storage (clone).
+ // If archive flag is passed, we will copy this snapshot to secondary
+ // storage and delete it from the primary storage.
+
+ Preconditions.checkState(Snapshot.LocationType.SECONDARY.equals(snapshotInfo.getLocationType()));
+
+ HostVO host = getHost(snapshotInfo.getVolumeId());
+ boolean canStorageSystemCreateVolumeFromSnapshot = canStorageSystemCreateVolumeFromSnapshot(snapshotInfo.getBaseVolume().getPoolId());
+ boolean computeClusterSupportsResign = clusterDao.getSupportsResigning(host.getClusterId());
+
+ if (!canStorageSystemCreateVolumeFromSnapshot || !computeClusterSupportsResign) {
+ String mesg = "Cannot archive snapshot: Either canStorageSystemCreateVolumeFromSnapshot or " +
+ "computeClusterSupportsResign were false. ";
+
+ s_logger.warn(mesg);
--- End diff --
Changed to ERROR
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@syed You can actually just remove the integration tests from this PR as they were added (along with updates to use the new SolidFire SDK for Python) in #1689.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@jburwell @syed There is no real overlap. Syed and I are working on items from a shared to-do list.
The PR here is something he and I built so that volume snapshots can be taken using a SAN (like SolidFire) and then moved to secondary storage later (the benefits are outlined in his description section above).
The other PR is related to running system VMs using managed storage. At a high level, that means having a virtual disk of a system VM on its own backend volume (for QoS reasons).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by syed <gi...@git.apache.org>.
Github user syed commented on the issue:
https://github.com/apache/cloudstack/pull/1600
Thanks @mike-tutkowski
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@syed Thanks! I had a similar issue with #1642 and fixed it the same way.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1600
<b>Trillian test result (tid-239)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 27277 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr1600-t239-kvm-centos7.zip
Test completed. 46 look ok, 2 have error(s)
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 374.76 | test_vpc_redundant.py
ContextSuite context=TestVpcRemoteAccessVpn>:setup | `Error` | 1847.33 | test_vpc_vpn.py
test_01_vpc_site2site_vpn | Success | 160.64 | test_vpc_vpn.py
test_01_redundant_vpc_site2site_vpn | Success | 255.88 | test_vpc_vpn.py
test_02_VPC_default_routes | Success | 276.09 | test_vpc_router_nics.py
test_01_VPC_nics_after_destroy | Success | 563.89 | test_vpc_router_nics.py
test_05_rvpc_multi_tiers | Success | 531.69 | test_vpc_redundant.py
test_04_rvpc_network_garbage_collector_nics | Success | 1442.35 | test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | Success | 565.29 | test_vpc_redundant.py
test_02_redundant_VPC_default_routes | Success | 748.56 | test_vpc_redundant.py
test_09_delete_detached_volume | Success | 15.48 | test_volumes.py
test_08_resize_volume | Success | 15.39 | test_volumes.py
test_07_resize_fail | Success | 20.49 | test_volumes.py
test_06_download_detached_volume | Success | 15.32 | test_volumes.py
test_05_detach_volume | Success | 100.26 | test_volumes.py
test_04_delete_attached_volume | Success | 10.19 | test_volumes.py
test_03_download_attached_volume | Success | 15.29 | test_volumes.py
test_02_attach_volume | Success | 73.77 | test_volumes.py
test_01_create_volume | Success | 712.02 | test_volumes.py
test_deploy_vm_multiple | Success | 304.13 | test_vm_life_cycle.py
test_deploy_vm | Success | 0.03 | test_vm_life_cycle.py
test_advZoneVirtualRouter | Success | 0.02 | test_vm_life_cycle.py
test_10_attachAndDetach_iso | Success | 26.66 | test_vm_life_cycle.py
test_09_expunge_vm | Success | 125.21 | test_vm_life_cycle.py
test_08_migrate_vm | Success | 40.94 | test_vm_life_cycle.py
test_07_restore_vm | Success | 0.12 | test_vm_life_cycle.py
test_06_destroy_vm | Success | 126.03 | test_vm_life_cycle.py
test_03_reboot_vm | Success | 126.41 | test_vm_life_cycle.py
test_02_start_vm | Success | 10.19 | test_vm_life_cycle.py
test_01_stop_vm | Success | 40.34 | test_vm_life_cycle.py
test_CreateTemplateWithDuplicateName | Success | 120.96 | test_templates.py
test_08_list_system_templates | Success | 0.03 | test_templates.py
test_07_list_public_templates | Success | 0.04 | test_templates.py
test_05_template_permissions | Success | 0.07 | test_templates.py
test_04_extract_template | Success | 5.37 | test_templates.py
test_03_delete_template | Success | 5.14 | test_templates.py
test_02_edit_template | Success | 90.19 | test_templates.py
test_01_create_template | Success | 40.54 | test_templates.py
test_10_destroy_cpvm | Success | 161.68 | test_ssvm.py
test_09_destroy_ssvm | Success | 168.68 | test_ssvm.py
test_08_reboot_cpvm | Success | 131.59 | test_ssvm.py
test_07_reboot_ssvm | Success | 133.66 | test_ssvm.py
test_06_stop_cpvm | Success | 132.17 | test_ssvm.py
test_05_stop_ssvm | Success | 133.88 | test_ssvm.py
test_04_cpvm_internals | Success | 1.21 | test_ssvm.py
test_03_ssvm_internals | Success | 3.39 | test_ssvm.py
test_02_list_cpvm_vm | Success | 0.13 | test_ssvm.py
test_01_list_sec_storage_vm | Success | 0.13 | test_ssvm.py
test_01_snapshot_root_disk | Success | 11.21 | test_snapshots.py
test_04_change_offering_small | Success | 240.79 | test_service_offerings.py
test_03_delete_service_offering | Success | 0.04 | test_service_offerings.py
test_02_edit_service_offering | Success | 0.06 | test_service_offerings.py
test_01_create_service_offering | Success | 0.11 | test_service_offerings.py
test_02_sys_template_ready | Success | 0.13 | test_secondary_storage.py
test_01_sys_vm_start | Success | 0.18 | test_secondary_storage.py
test_09_reboot_router | Success | 45.40 | test_routers.py
test_08_start_router | Success | 30.28 | test_routers.py
test_07_stop_router | Success | 10.17 | test_routers.py
test_06_router_advanced | Success | 0.06 | test_routers.py
test_05_router_basic | Success | 0.04 | test_routers.py
test_04_restart_network_wo_cleanup | Success | 5.72 | test_routers.py
test_03_restart_network_cleanup | Success | 60.52 | test_routers.py
test_02_router_internal_adv | Success | 1.06 | test_routers.py
test_01_router_internal_basic | Success | 0.59 | test_routers.py
test_router_dns_guestipquery | Success | 76.95 | test_router_dns.py
test_router_dns_externalipquery | Success | 0.10 | test_router_dns.py
test_router_dhcphosts | Success | 279.58 | test_router_dhcphosts.py
test_01_updatevolumedetail | Success | 0.08 | test_resource_detail.py
test_01_reset_vm_on_reboot | Success | 135.98 | test_reset_vm_on_reboot.py
test_createRegion | Success | 0.04 | test_regions.py
test_create_pvlan_network | Success | 5.21 | test_pvlan.py
test_dedicatePublicIpRange | Success | 0.46 | test_public_ip_range.py
test_04_rvpc_privategw_static_routes | Success | 603.69 | test_privategw_acl.py
test_03_vpc_privategw_restart_vpc_cleanup | Success | 546.98 | test_privategw_acl.py
test_02_vpc_privategw_static_routes | Success | 371.44 | test_privategw_acl.py
test_01_vpc_privategw_acl | Success | 98.22 | test_privategw_acl.py
test_01_primary_storage_nfs | Success | 36.11 | test_primary_storage.py
test_createPortablePublicIPRange | Success | 15.44 | test_portable_publicip.py
test_createPortablePublicIPAcquire | Success | 15.44 | test_portable_publicip.py
test_isolate_network_password_server | Success | 89.34 | test_password_server.py
test_UpdateStorageOverProvisioningFactor | Success | 0.14 | test_over_provisioning.py
test_oobm_zchange_password | Success | 20.52 | test_outofbandmanagement.py
test_oobm_multiple_mgmt_server_ownership | Success | 16.52 | test_outofbandmanagement.py
test_oobm_issue_power_status | Success | 10.51 | test_outofbandmanagement.py
test_oobm_issue_power_soft | Success | 15.52 | test_outofbandmanagement.py
test_oobm_issue_power_reset | Success | 15.52 | test_outofbandmanagement.py
test_oobm_issue_power_on | Success | 15.52 | test_outofbandmanagement.py
test_oobm_issue_power_off | Success | 15.52 | test_outofbandmanagement.py
test_oobm_issue_power_cycle | Success | 15.52 | test_outofbandmanagement.py
test_oobm_enabledisable_across_clusterzones | Success | 52.65 | test_outofbandmanagement.py
test_oobm_enable_feature_valid | Success | 5.18 | test_outofbandmanagement.py
test_oobm_enable_feature_invalid | Success | 0.12 | test_outofbandmanagement.py
test_oobm_disable_feature_valid | Success | 0.20 | test_outofbandmanagement.py
test_oobm_disable_feature_invalid | Success | 0.14 | test_outofbandmanagement.py
test_oobm_configure_invalid_driver | Success | 0.10 | test_outofbandmanagement.py
test_oobm_configure_default_driver | Success | 0.10 | test_outofbandmanagement.py
test_oobm_background_powerstate_sync | Success | 29.87 | test_outofbandmanagement.py
test_extendPhysicalNetworkVlan | Success | 15.31 | test_non_contigiousvlan.py
test_01_nic | Success | 636.91 | test_nic.py
test_releaseIP | Success | 248.78 | test_network.py
test_reboot_router | Success | 434.73 | test_network.py
test_public_ip_user_account | Success | 10.26 | test_network.py
test_public_ip_admin_account | Success | 40.29 | test_network.py
test_network_rules_acquired_public_ip_3_Load_Balancer_Rule | Success | 66.85 | test_network.py
test_network_rules_acquired_public_ip_2_nat_rule | Success | 61.76 | test_network.py
test_network_rules_acquired_public_ip_1_static_nat_rule | Success | 124.20 | test_network.py
test_delete_account | Success | 318.98 | test_network.py
test_02_port_fwd_on_non_src_nat | Success | 55.71 | test_network.py
test_01_port_fwd_on_src_nat | Success | 111.92 | test_network.py
test_nic_secondaryip_add_remove | Success | 233.58 | test_multipleips_per_nic.py
login_test_saml_user | Success | 24.70 | test_login.py
test_assign_and_removal_lb | Success | 133.47 | test_loadbalance.py
test_02_create_lb_rule_non_nat | Success | 187.32 | test_loadbalance.py
test_01_create_lb_rule_src_nat | Success | 218.53 | test_loadbalance.py
test_03_list_snapshots | Success | 0.08 | test_list_ids_parameter.py
test_02_list_templates | Success | 0.04 | test_list_ids_parameter.py
test_01_list_volumes | Success | 0.03 | test_list_ids_parameter.py
test_07_list_default_iso | Success | 0.06 | test_iso.py
test_05_iso_permissions | Success | 0.07 | test_iso.py
test_04_extract_Iso | Success | 5.18 | test_iso.py
test_03_delete_iso | Success | 95.23 | test_iso.py
test_02_edit_iso | Success | 0.06 | test_iso.py
test_01_create_iso | Success | 21.93 | test_iso.py
test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | Success | 253.13 | test_internal_lb.py
test_03_vpc_internallb_haproxy_stats_on_all_interfaces | Success | 198.46 | test_internal_lb.py
test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | Success | 515.34 | test_internal_lb.py
test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | Success | 442.13 | test_internal_lb.py
test_dedicateGuestVlanRange | Success | 10.31 | test_guest_vlan_range.py
test_UpdateConfigParamWithScope | Success | 0.15 | test_global_settings.py
test_rolepermission_lifecycle_update | Success | 7.36 | test_dynamicroles.py
test_rolepermission_lifecycle_list | Success | 6.91 | test_dynamicroles.py
test_rolepermission_lifecycle_delete | Success | 6.78 | test_dynamicroles.py
test_rolepermission_lifecycle_create | Success | 6.80 | test_dynamicroles.py
test_rolepermission_lifecycle_concurrent_updates | Success | 6.93 | test_dynamicroles.py
test_role_lifecycle_update_role_inuse | Success | 6.78 | test_dynamicroles.py
test_role_lifecycle_update | Success | 12.00 | test_dynamicroles.py
test_role_lifecycle_list | Success | 6.78 | test_dynamicroles.py
test_role_lifecycle_delete | Success | 11.84 | test_dynamicroles.py
test_role_lifecycle_create | Success | 6.79 | test_dynamicroles.py
test_role_inuse_deletion | Success | 6.77 | test_dynamicroles.py
test_role_account_acls_multiple_mgmt_servers | Success | 9.08 | test_dynamicroles.py
test_role_account_acls | Success | 9.16 | test_dynamicroles.py
test_default_role_deletion | Success | 7.05 | test_dynamicroles.py
test_04_create_fat_type_disk_offering | Success | 0.09 | test_disk_offerings.py
test_03_delete_disk_offering | Success | 0.04 | test_disk_offerings.py
test_02_edit_disk_offering | Success | 0.06 | test_disk_offerings.py
test_02_create_sparse_type_disk_offering | Success | 0.07 | test_disk_offerings.py
test_01_create_disk_offering | Success | 0.11 | test_disk_offerings.py
test_deployvm_userdispersing | Success | 20.60 | test_deploy_vms_with_varied_deploymentplanners.py
test_deployvm_userconcentrated | Success | 55.82 | test_deploy_vms_with_varied_deploymentplanners.py
test_deployvm_firstfit | Success | 121.06 | test_deploy_vms_with_varied_deploymentplanners.py
test_deployvm_userdata_post | Success | 10.68 | test_deploy_vm_with_userdata.py
test_deployvm_userdata | Success | 61.07 | test_deploy_vm_with_userdata.py
test_02_deploy_vm_root_resize | Success | 7.02 | test_deploy_vm_root_resize.py
test_01_deploy_vm_root_resize | Success | 6.91 | test_deploy_vm_root_resize.py
test_00_deploy_vm_root_resize | Success | 224.03 | test_deploy_vm_root_resize.py
test_deploy_vm_from_iso | Success | 243.70 | test_deploy_vm_iso.py
test_DeployVmAntiAffinityGroup | Success | 76.06 | test_affinity_groups.py
test_03_delete_vm_snapshots | Skipped | 0.00 | test_vm_snapshots.py
test_02_revert_vm_snapshots | Skipped | 0.00 | test_vm_snapshots.py
test_01_test_vm_volume_snapshot | Skipped | 0.00 | test_vm_snapshots.py
test_01_create_vm_snapshots | Skipped | 0.00 | test_vm_snapshots.py
test_06_copy_template | Skipped | 0.00 | test_templates.py
test_static_role_account_acls | Skipped | 0.02 | test_staticroles.py
test_11_ss_nfs_version_on_ssvm | Skipped | 0.02 | test_ssvm.py
test_01_scale_vm | Skipped | 0.00 | test_scale_vm.py
test_01_primary_storage_iscsi | Skipped | 0.04 | test_primary_storage.py
test_06_copy_iso | Skipped | 0.00 | test_iso.py
test_deploy_vgpu_enabled_vm | Skipped | 0.04 | test_deploy_vgpu_enabled_vm.py
test_3d_gpu_support | Skipped | 0.04 | test_deploy_vgpu_enabled_vm.py
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by syed <gi...@git.apache.org>.
Github user syed commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@jburwell Squashed the commits. You can run the smoke tests on this. This change is mostly on the managed storage side so running any volume related tests would be helpful apart from the tests @mike-tutkowski has added.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by syed <gi...@git.apache.org>.
Github user syed commented on the issue:
https://github.com/apache/cloudstack/pull/1600
A bit of background on how this feature works. When locationType is specified as `Archve`, a snaphsot is taken at the storage, but the copy to seconday storage is done via the hypervisor as it is able to correctly attach the LUN and extract the underlying virtual disk. This was done as an alternative to taking the snapshot on the hypervisor and upload it to secondary storage directly becuse, we've seen cases where hypervisor snapshots cause a significant delay in read/write performance and snapshot on storage device allows us to take snaspots without sacrificing performance.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@syed #1692 is testing now. It addresses ping test related failures in the smoke test suite. I am hoping the tests pass, and we can merge it first thing tomorrow. Once it has, we can rebase this branch and kick of the smoke test run.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77259202
--- Diff: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java ---
@@ -92,7 +90,34 @@
@Override
public SnapshotInfo backupSnapshot(SnapshotInfo snapshotInfo) {
- return snapshotInfo;
+
+ if (!snapshotInfo.getLocationType().equals(Snapshot.LocationType.SECONDARY)) {
+
+ markAsBackedUp((SnapshotObject)snapshotInfo);
+ return snapshotInfo;
+ }
+
+ // At this point the snapshot is either taken as a native
+ // snapshot on the storage or exists as a volume on the storage (clone).
+ // If archive flag is passed, we will copy this snapshot to secondary
+ // storage and delete it from the primary storage.
+
+ Preconditions.checkState(Snapshot.LocationType.SECONDARY.equals(snapshotInfo.getLocationType()));
--- End diff --
Removed the check
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
It looks like we're having an issue with Travis?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r77078902
--- Diff: engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java ---
@@ -380,6 +513,122 @@ private void handleCreateTemplateFromSnapshot(SnapshotInfo snapshotInfo, Templat
}
/**
+ * Creates a volume on the storage from a snapshot that resides on the secondary storage (archived snapshot).
+ * @param snapshotInfo snapshot on secondary
+ * @param volumeInfo volume to be created on the storage
+ * @param callback for async
+ */
+ private void handleCreateVolumeFromSnapshotOnSecondaryStorage(SnapshotInfo snapshotInfo, VolumeInfo volumeInfo, AsyncCompletionCallback<CopyCommandResult> callback) {
+
+ // at this point, the snapshotInfo and volumeInfo should have the same disk offering ID (so either one should be OK to get a DiskOfferingVO instance)
+ DiskOfferingVO diskOffering = _diskOfferingDao.findByIdIncludingRemoved(volumeInfo.getDiskOfferingId());
+ SnapshotVO snapshot = _snapshotDao.findById(snapshotInfo.getId());
+ DataStore destDataStore = volumeInfo.getDataStore();
+
+ // update the volume's hv_ss_reserve (hypervisor snapshot reserve) from a disk offering (used for managed storage)
+ _volumeService.updateHypervisorSnapshotReserveForVolume(diskOffering, volumeInfo.getId(), snapshot.getHypervisorType());
+
+
+ CopyCmdAnswer copyCmdAnswer = null;
+ String errMsg = null;
+
+ HostVO hostVO = null;
+ try {
+
+ //create a volume on the storage
+ AsyncCallFuture<VolumeApiResult> future = _volumeService.createVolumeAsync(volumeInfo, volumeInfo.getDataStore());
+ VolumeApiResult result = future.get();
+
+ if (result.isFailed()) {
+ LOGGER.warn("Failed to create a volume: " + result.getResult());
+ throw new CloudRuntimeException(result.getResult());
+ }
+
+ volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(), volumeInfo.getDataStore());
+
+ volumeInfo.processEvent(Event.MigrationRequested);
+
+ volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(), volumeInfo.getDataStore());
+
+ hostVO = getHost(snapshotInfo.getDataCenterId(), false);
+
+ //copy the volume from secondary via the hypervisor
+ copyCmdAnswer = performCopyOfVdi(volumeInfo, snapshotInfo, hostVO);
+
+ if (copyCmdAnswer == null || !copyCmdAnswer.getResult()) {
+ if (copyCmdAnswer != null && !StringUtils.isEmpty(copyCmdAnswer.getDetails())) {
+ errMsg = copyCmdAnswer.getDetails();
+ }
+ else {
+ errMsg = "Unable to create volume from snapshot";
+ }
+ }
+ }
+ catch (Exception ex) {
+ errMsg = ex.getMessage() != null ? ex.getMessage() : "Copy operation failed in 'StorageSystemDataMotionStrategy.handleCreateVolumeFromSnapshotBothOnStorageSystem'";
+ } finally {
+
+ // detach and remove access after the volume is created
+ DiskTO disk = new DiskTO(volumeInfo.getTO(), volumeInfo.getDeviceId(), volumeInfo.getPath(), volumeInfo.getVolumeType());
+ DettachCommand cmd = new DettachCommand(disk, null);
+ long storagePoolId = volumeInfo.getPoolId();
+ StoragePoolVO storagePool = _storagePoolDao.findById(storagePoolId);
+
+ cmd.setManaged(true);
+ cmd.setStorageHost(storagePool.getHostAddress());
+ cmd.setStoragePort(storagePool.getPort());
+ cmd.set_iScsiName(volumeInfo.get_iScsiName());
+
+ try {
+ DettachAnswer dettachAnswer = (DettachAnswer) _agentMgr.send(hostVO.getId(), cmd);
+
+ if (!dettachAnswer.getResult()) {
+ throw new CloudRuntimeException("Error detaching volume:" + dettachAnswer.getDetails());
+ }
+
+ } catch (Exception e) {
+ LOGGER.warn("Error detaching volume " + volumeInfo.getId() + " Error: " + e.getMessage());
+ }
+
+ _volumeService.revokeAccess(volumeInfo, hostVO, destDataStore);
--- End diff --
Lines 571-593 appear to duplicate Line 463-475 in ``StorageSystemDataMotionStrategy``. Please consider consolidation of the commonalities.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r70563575
--- Diff: api/src/org/apache/cloudstack/api/response/SnapshotResponse.java ---
@@ -82,6 +82,10 @@
@Param(description = "valid types are hourly, daily, weekly, monthy, template, and none.")
private String intervalType;
+ @SerializedName(ApiConstants.LOCATION_TYPE)
+ @Param(description = "valid location types are primary and archive.")
+ private String locationType;
--- End diff --
LocationType is a String in response. It makes sense for it be String in request as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:
https://github.com/apache/cloudstack/pull/1600
Thanks for the clarification, @jburwell. I had the same question for #1642, but your answer here works for both PRs.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by syed <gi...@git.apache.org>.
Github user syed commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@mike-tutkowski I've added your updated tests and rebased.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1600: Support Backup of Snapshots for Managed Storage
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1600
@blueorangutan test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r91006590
--- Diff: plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java ---
@@ -538,7 +562,7 @@ public Answer backupSnapshot(final CopyCommand cmd) {
final String[] tmp = result.split("#");
snapshotBackupUuid = tmp[0];
physicalSize = Long.parseLong(tmp[1]);
- finalPath = folder + File.separator + snapshotBackupUuid;
+ finalPath = folder + File.separator + snapshotBackupUuid + ".vhd";
--- End diff --
@syed This is the way it should have been, but now that in a upgraded setup there will be mixed names with ".vhd" and without, do you think this will not cause issues ? If it does we need to come up with a plan to deal with them like a upgrade step ? I think the cleanup threads may not be able to handle the old naming convention for one. @rhtyd
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1600: Support Backup of Snapshots for Managed Stora...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1600#discussion_r90595496
--- Diff: plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java ---
@@ -538,7 +562,7 @@ public Answer backupSnapshot(final CopyCommand cmd) {
final String[] tmp = result.split("#");
snapshotBackupUuid = tmp[0];
physicalSize = Long.parseLong(tmp[1]);
- finalPath = folder + File.separator + snapshotBackupUuid;
+ finalPath = folder + File.separator + snapshotBackupUuid + ".vhd";
--- End diff --
While I would want to add the extension, but I think we should revert this specific change, as otherwise it would add upgrade complexities. Given the short time for next RCs, such a change can be properly addressed in future with tooling around upgrading/renaming the files and paths.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---