You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/07/13 02:11:34 UTC
[GitHub] [cloudstack] skattoju4 opened a new pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
skattoju4 opened a new pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875
### Description
This PR is a fix for the data volume import and export functionality when using swift as secondary storage.
Background:
Secondary storage is used for templates, snapshots and iso's. A data disk is a volume and is meant to reside on primary storage. However, when importing a volume it is first uploaded to secondary storage and then copied to over to primary storage when attached to a VM. Similarly when exporting a volume, it is copied to secondary storage before it is made available to the user for download. When using swift as a secondary storage, an intermediate staging nfs store is needed when copying volumes between primary and secondary storage. This staging nfs store is leveraged for the volume import and export functionality since volumes need not have a footprint on secondary storage.
Currently this functionality does not work:
When importing a volume the following error is observed:
`org.apache.cloudstack.storage.to.VolumeObjectTO cannot be cast to org.apache.cloudstack.storage.to.TemplateObjectTO
`
Further investigation reveals that there is no provision in the code to handle import of volumes. Currently only template import is supported.
When exporting a volume a URL to the secondary storage is generated but it clicking it does not initiate a download of the exported volume. Further investigation revealed that the url points to a broken symlink which points to an invalid path on the SSVM
Use Cases
1. User should be able to import a volume from a URL when using swift as secondary storage and XenServer as the hypervisor.
2. User should be able to export a volume to a secondary storage VM URL when using swift as secondary storage and XenServer as the hypervisor.
<!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
<!--- ********************************************************************************* -->
<!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
<!--- PLEASE PUT AN 'X' in only **ONE** box -->
<!--- ********************************************************************************* -->
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
This addresses https://github.com/apache/cloudstack/issues/4874
#### Feature/Enhancement Scale
- [ ] Major
- [*] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [*] Minor
- [ ] Trivial
### How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
1. Create data volume (non-managed storage)
2. Attach it to a VM.
3. Create a file system and a test file on the new data volume.
4. Export volume.
5. Upload exported volume to a bucket and generate a URL.
6. Import volume back into CloudStack from URL.
7. Attach imported disk to a VM.
8. Verify previously created test file.
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] skattoju4 commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
skattoju4 commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r620554844
##########
File path: server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
##########
@@ -1596,6 +1596,19 @@ private Volume orchestrateAttachVolumeToVM(Long vmId, Long volumeId, Long device
StoragePoolVO destPrimaryStorage = null;
if (exstingVolumeOfVm != null && !exstingVolumeOfVm.getState().equals(Volume.State.Allocated)) {
destPrimaryStorage = _storagePoolDao.findById(exstingVolumeOfVm.getPoolId());
+ if(destPrimaryStorage.getPodId() == null) {
+ destPrimaryStorage.setPodId(vm.getPodIdToDeployIn());
+
+ }
+ if(destPrimaryStorage.getClusterId() == null) {
+ HostVO hostVO = null;
+ if (vm.getHostId() != null) {
+ hostVO = _hostDao.findById(vm.getHostId());
+ } else {
+ hostVO = _hostDao.findById(vm.getLastHostId());
+ }
+ destPrimaryStorage.setClusterId(hostVO.getClusterId());
Review comment:
This was added because we were encountering a null pointer exception in some scenarios I don't recall exactly. I added the bit to get the last host id of the vm as attaching a disk to a stopped vm would fail. Have not encountered any side effects with regards to zone wide storage so far..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-895049183
Ping @skattoju4 cc @syed @swill - can you check and fix the build failure? Is this PR ready for reviewing/merging?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-867307043
<b>Trillian test result (tid-1071)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35011 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4875-t1071-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_usage.py
Smoke tests completed. 87 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_volume_usage | `Error` | 127.92 | test_usage.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r601771583
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -185,6 +108,86 @@
import io.netty.handler.codec.http.HttpResponseEncoder;
import io.netty.handler.logging.LogLevel;
import io.netty.handler.logging.LoggingHandler;
+import org.apache.cloudstack.framework.security.keystore.KeystoreManager;
+import org.apache.cloudstack.storage.NfsMountManagerImpl.PathParser;
+import org.apache.cloudstack.storage.command.CopyCmdAnswer;
+import org.apache.cloudstack.storage.command.CopyCommand;
+import org.apache.cloudstack.storage.command.DeleteCommand;
+import org.apache.cloudstack.storage.command.DownloadCommand;
+import org.apache.cloudstack.storage.command.DownloadProgressCommand;
+import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
+import org.apache.cloudstack.storage.command.UploadStatusAnswer;
+import org.apache.cloudstack.storage.command.UploadStatusAnswer.UploadStatus;
+import org.apache.cloudstack.storage.command.UploadStatusCommand;
+import org.apache.cloudstack.storage.configdrive.ConfigDrive;
+import org.apache.cloudstack.storage.configdrive.ConfigDriveBuilder;
+import org.apache.cloudstack.storage.template.DownloadManager;
+import org.apache.cloudstack.storage.template.DownloadManagerImpl;
+import org.apache.cloudstack.storage.template.UploadEntity;
+import org.apache.cloudstack.storage.template.UploadManager;
+import org.apache.cloudstack.storage.template.UploadManagerImpl;
+import org.apache.cloudstack.storage.to.SnapshotObjectTO;
+import org.apache.cloudstack.storage.to.TemplateObjectTO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
+import org.apache.cloudstack.utils.security.DigestHelper;
+import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.FilenameUtils;
+import org.apache.commons.lang.StringUtils;
+import org.apache.http.HttpEntity;
+import org.apache.http.HttpResponse;
+import org.apache.http.NameValuePair;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.utils.URLEncodedUtils;
+import org.apache.http.conn.ssl.NoopHostnameVerifier;
+import org.apache.http.conn.ssl.TrustStrategy;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClients;
+import org.apache.http.ssl.SSLContextBuilder;
+import org.apache.log4j.Logger;
+import org.joda.time.DateTime;
+import org.joda.time.format.ISODateTimeFormat;
+
+import javax.naming.ConfigurationException;
+import javax.net.ssl.SSLContext;
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.FileReader;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStreamWriter;
+import java.io.UnsupportedEncodingException;
+import java.net.InetAddress;
+import java.net.URI;
+import java.net.UnknownHostException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.KeyManagementException;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+
+import static com.cloud.network.NetworkModel.METATDATA_DIR;
+import static com.cloud.network.NetworkModel.PASSWORD_DIR;
+import static com.cloud.network.NetworkModel.PASSWORD_FILE;
+import static com.cloud.network.NetworkModel.PUBLIC_KEYS_FILE;
+import static com.cloud.network.NetworkModel.USERDATA_DIR;
+import static com.cloud.network.NetworkModel.USERDATA_FILE;
+import static com.cloud.utils.StringUtils.join;
+import static com.cloud.utils.storage.S3.S3Utils.putFile;
+import static java.lang.String.format;
+import static java.util.Arrays.asList;
+import static org.apache.commons.lang.StringUtils.substringAfterLast;
Review comment:
Question:
Why reorder the imports instead of only add what you need?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] skattoju4 commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
skattoju4 commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r605324912
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -1137,11 +1167,11 @@ protected File downloadFromUrlToNfs(String url, NfsTO nfs, String path, String n
try (FileOutputStream outputStream = new FileOutputStream(destFile);) {
entity.writeTo(outputStream);
} catch (IOException e) {
- s_logger.debug("downloadFromUrlToNfs:Exception:" + e.getMessage(), e);
+ s_logger.error("downloadFromUrlToNfs:Exception:" + e.getMessage(), e);
}
return new File(destFile.getAbsolutePath());
} catch (IOException e) {
- s_logger.debug("Faild to get url:" + url + ", due to " + e.toString());
+ s_logger.error("Faild to get url:" + url + ", due to " + e.toString());
Review comment:
thanks will do it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] skattoju4 commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
skattoju4 commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r620535834
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -772,7 +780,12 @@ protected Answer copyFromSwiftToNfs(CopyCommand cmd, DataTO srcData, SwiftTO swi
}
}
- File destFile = SwiftUtil.getObject(swiftTO, downloadDirectory, srcData.getPath());
+ String filePath = downloadPath + File.separator + destData.getName();
Review comment:
Hey Syed, thanks for the review. We do call getPath() on line 766 so the downloadPath will contain the path to folder depending on whether its a volume, snapshot or template. Here i'm skipping the copy from swift if the file is already there since we don't upload volumes to swift just to the staging nfs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] skattoju4 commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
skattoju4 commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r605324600
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -1137,11 +1167,11 @@ protected File downloadFromUrlToNfs(String url, NfsTO nfs, String path, String n
try (FileOutputStream outputStream = new FileOutputStream(destFile);) {
entity.writeTo(outputStream);
} catch (IOException e) {
- s_logger.debug("downloadFromUrlToNfs:Exception:" + e.getMessage(), e);
+ s_logger.error("downloadFromUrlToNfs:Exception:" + e.getMessage(), e);
Review comment:
thanks will do it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r601768716
##########
File path: plugins/storage/image/swift/src/main/java/org/apache/cloudstack/storage/datastore/driver/SwiftImageStoreDriverImpl.java
##########
@@ -100,12 +102,28 @@ public String createEntityExtractUrl(DataStore store, String installPath, ImageF
@Override
public void createAsync(DataStore dataStore, DataObject data, AsyncCompletionCallback<CreateCmdResult> callback) {
- Long maxTemplateSizeInBytes = getMaxTemplateSizeInBytes();
- VirtualMachineTemplate tmpl = _templateDao.findById(data.getId());
+
+ DownloadCommand downloadCommand = null;
+ if (data.getType() == DataObjectType.TEMPLATE) {
+ Long maxTemplateSizeInBytes = getMaxTemplateSizeInBytes();
+ downloadCommand = new DownloadCommand((TemplateObjectTO) (data.getTO()), maxTemplateSizeInBytes);
+ }else if (data.getType() == DataObjectType.VOLUME){
+ Long maxDownloadSizeInBytes = getMaxVolumeSizeInBytes();
+ VolumeInfo volumeInfo = (VolumeInfo) data;
+ RegisterVolumePayload payload = (RegisterVolumePayload) volumeInfo.getpayload();
+ ImageFormat format = ImageFormat.valueOf(payload.getFormat());
+ downloadCommand = new DownloadCommand((VolumeObjectTO) (data.getTO()), maxDownloadSizeInBytes, payload.getChecksum(), payload.getUrl(), format);
+ }
+
+ if (downloadCommand == null){
+ String errMsg = "Unable to build download command, DataObject is of neither VOLUME or TEMPLATE type";
Review comment:
Suggestion:
Improve logging by adding DataObject data in the message.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] skattoju4 edited a comment on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
skattoju4 edited a comment on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-824403305
@syed @khos2ow could you take a look at this if you have some time ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] skattoju4 commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
skattoju4 commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-824403305
@khos2ow could you take a look at this ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] skattoju4 commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
skattoju4 commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r620537589
##########
File path: server/src/main/java/com/cloud/storage/download/DownloadMonitorImpl.java
##########
@@ -218,7 +221,8 @@ public void downloadVolumeToStorage(DataObject volume, AsyncCompletionCallback<D
Long maxVolumeSizeInBytes = getMaxVolumeSizeInBytes();
start();
DownloadCommand dcmd = new DownloadCommand((VolumeObjectTO)(volume.getTO()), maxVolumeSizeInBytes, checkSum, url, format);
- dcmd.setProxy(getHttpProxy());
+ DataStore cacheStore = cacheManager.getCacheStorage(store.getScope());
+ dcmd.setCacheStore(cacheStore.getTO());dcmd.setProxy(getHttpProxy());
Review comment:
fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-867014265
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] skattoju4 commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
skattoju4 commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r620535834
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -772,7 +780,12 @@ protected Answer copyFromSwiftToNfs(CopyCommand cmd, DataTO srcData, SwiftTO swi
}
}
- File destFile = SwiftUtil.getObject(swiftTO, downloadDirectory, srcData.getPath());
+ String filePath = downloadPath + File.separator + destData.getName();
Review comment:
Hey Syed, thanks for the review. We do call getPath() on line 766 so the downloadPath will contain the path to folder depending on whether its a volume, snapshot or template. Here i'm skipping the copy from swift if the file is already there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-894600349
@nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-866953851
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 339
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-889639428
Let's try again
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-866899868
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r601775952
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -1180,13 +1210,13 @@ protected Answer registerTemplateOnSwift(DownloadCommand cmd) {
try (FileInputStream fs = new FileInputStream(file)) {
md5sum = DigestUtils.md5Hex(fs);
} catch (IOException e) {
- s_logger.debug("Failed to get md5sum: " + file.getAbsoluteFile());
+ s_logger.error("Failed to get md5sum: " + file.getAbsoluteFile());
Review comment:
Suggestion:
Pass the exception as parameter to `s_logger.error`:
```java
s_logger.error("Failed to get md5sum: " + file.getAbsoluteFile(), e);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r601773887
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -1137,11 +1167,11 @@ protected File downloadFromUrlToNfs(String url, NfsTO nfs, String path, String n
try (FileOutputStream outputStream = new FileOutputStream(destFile);) {
entity.writeTo(outputStream);
} catch (IOException e) {
- s_logger.debug("downloadFromUrlToNfs:Exception:" + e.getMessage(), e);
+ s_logger.error("downloadFromUrlToNfs:Exception:" + e.getMessage(), e);
Review comment:
Suggestion:
We could improve logging here by adding some data, like:
```java
s_logger.error(String.format("Unable to write downloaded data from [%s] to [%s] due to [%s].", url, destFile, e.getMessage()), e);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-923692312
Ping @skattoju4
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r704048909
##########
File path: plugins/storage/image/swift/src/main/java/org/apache/cloudstack/storage/datastore/driver/SwiftImageStoreDriverImpl.java
##########
@@ -100,12 +102,28 @@ public String createEntityExtractUrl(DataStore store, String installPath, ImageF
@Override
public void createAsync(DataStore dataStore, DataObject data, AsyncCompletionCallback<CreateCmdResult> callback) {
- Long maxTemplateSizeInBytes = getMaxTemplateSizeInBytes();
- VirtualMachineTemplate tmpl = _templateDao.findById(data.getId());
+
+ DownloadCommand downloadCommand = null;
+ if (data.getType() == DataObjectType.TEMPLATE) {
+ Long maxTemplateSizeInBytes = getMaxTemplateSizeInBytes();
+ downloadCommand = new DownloadCommand((TemplateObjectTO) (data.getTO()), maxTemplateSizeInBytes);
+ }else if (data.getType() == DataObjectType.VOLUME){
+ Long maxDownloadSizeInBytes = getMaxVolumeSizeInBytes();
+ VolumeInfo volumeInfo = (VolumeInfo) data;
+ RegisterVolumePayload payload = (RegisterVolumePayload) volumeInfo.getpayload();
+ ImageFormat format = ImageFormat.valueOf(payload.getFormat());
+ downloadCommand = new DownloadCommand((VolumeObjectTO) (data.getTO()), maxDownloadSizeInBytes, payload.getChecksum(), payload.getUrl(), format);
+ }
+
+ if (downloadCommand == null){
Review comment:
```suggestion
if (downloadCommand == null) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r601774227
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -1137,11 +1167,11 @@ protected File downloadFromUrlToNfs(String url, NfsTO nfs, String path, String n
try (FileOutputStream outputStream = new FileOutputStream(destFile);) {
entity.writeTo(outputStream);
} catch (IOException e) {
- s_logger.debug("downloadFromUrlToNfs:Exception:" + e.getMessage(), e);
+ s_logger.error("downloadFromUrlToNfs:Exception:" + e.getMessage(), e);
}
return new File(destFile.getAbsolutePath());
} catch (IOException e) {
- s_logger.debug("Faild to get url:" + url + ", due to " + e.toString());
+ s_logger.error("Faild to get url:" + url + ", due to " + e.toString());
Review comment:
Typo:
`Faild` -> `Failed`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-863659529
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] skattoju4 commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
skattoju4 commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-837474777
@rhtyd it should be good to go. A CloudStack setup that uses swift as secondary storage is needed to test. This was tested manually with the following high level steps:
1. Create data volume (only non-managed storage is supported for now)
2. Attach it to a VM.
3. Create a file system and a test file on the new data volume.
4. Export volume. (tests volume export feature)
5. Upload exported volume to a bucket and generate a URL.
6. Import volume back into CloudStack from URL. (tests volume import feature)
7. Attach imported disk to a VM.
8. Verify previously created test file.
Its been in production for some time with no side effects so far @pdion891 can attest.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez closed pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
nvazquez closed pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] syed commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
syed commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r619394901
##########
File path: server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
##########
@@ -1596,6 +1596,19 @@ private Volume orchestrateAttachVolumeToVM(Long vmId, Long volumeId, Long device
StoragePoolVO destPrimaryStorage = null;
if (exstingVolumeOfVm != null && !exstingVolumeOfVm.getState().equals(Volume.State.Allocated)) {
destPrimaryStorage = _storagePoolDao.findById(exstingVolumeOfVm.getPoolId());
+ if(destPrimaryStorage.getPodId() == null) {
+ destPrimaryStorage.setPodId(vm.getPodIdToDeployIn());
+
+ }
+ if(destPrimaryStorage.getClusterId() == null) {
+ HostVO hostVO = null;
+ if (vm.getHostId() != null) {
+ hostVO = _hostDao.findById(vm.getHostId());
+ } else {
+ hostVO = _hostDao.findById(vm.getLastHostId());
+ }
+ destPrimaryStorage.setClusterId(hostVO.getClusterId());
Review comment:
Do we want to set the clusterID for zonewide storage too?
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -772,7 +780,12 @@ protected Answer copyFromSwiftToNfs(CopyCommand cmd, DataTO srcData, SwiftTO swi
}
}
- File destFile = SwiftUtil.getObject(swiftTO, downloadDirectory, srcData.getPath());
+ String filePath = downloadPath + File.separator + destData.getName();
Review comment:
Are you not using the `getPath` to find the path of the object anymore? does `downloadPath` consider the difference between snapshot and template and now volume?
##########
File path: core/src/main/java/com/cloud/storage/template/SwiftVolumeDownloader.java
##########
@@ -0,0 +1,397 @@
+//
Review comment:
Where would this run? On the mgmt server or on the SSVM?
##########
File path: server/src/main/java/com/cloud/storage/download/DownloadMonitorImpl.java
##########
@@ -218,7 +221,8 @@ public void downloadVolumeToStorage(DataObject volume, AsyncCompletionCallback<D
Long maxVolumeSizeInBytes = getMaxVolumeSizeInBytes();
start();
DownloadCommand dcmd = new DownloadCommand((VolumeObjectTO)(volume.getTO()), maxVolumeSizeInBytes, checkSum, url, format);
- dcmd.setProxy(getHttpProxy());
+ DataStore cacheStore = cacheManager.getCacheStorage(store.getScope());
+ dcmd.setCacheStore(cacheStore.getTO());dcmd.setProxy(getHttpProxy());
Review comment:
fix formatting here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r704050498
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java
##########
@@ -304,8 +317,27 @@ public void setDownloadStatus(String jobId, Status status) {
td.setStatus(Status.POST_DOWNLOAD_FINISHED);
td.setDownloadError("Install completed successfully at " + new SimpleDateFormat().format(new Date()));
}
- }
- else {
+ } else if (td instanceof SwiftVolumeDownloader) {
+ dj.setCheckSum(((SwiftVolumeDownloader) td).getMd5sum());
+ if ("vhd".equals(((SwiftVolumeDownloader) td).getFileExtension()) ||
Review comment:
```suggestion
if ("vhd".equalsIgnoreCase(((SwiftVolumeDownloader) td).getFileExtension()))
```
Ignore if it is strictly all lower/upper case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] skattoju4 commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
skattoju4 commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r620555054
##########
File path: core/src/main/java/com/cloud/storage/template/SwiftVolumeDownloader.java
##########
@@ -0,0 +1,397 @@
+//
Review comment:
This would run on the SSVM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-837030480
@skattoju4 is this ready, how do we test it or can you provide test results?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-889649524
Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian. SL-JID 705
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-866900403
@nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] skattoju4 commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
skattoju4 commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r605323100
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -185,6 +108,86 @@
import io.netty.handler.codec.http.HttpResponseEncoder;
import io.netty.handler.logging.LogLevel;
import io.netty.handler.logging.LoggingHandler;
+import org.apache.cloudstack.framework.security.keystore.KeystoreManager;
+import org.apache.cloudstack.storage.NfsMountManagerImpl.PathParser;
+import org.apache.cloudstack.storage.command.CopyCmdAnswer;
+import org.apache.cloudstack.storage.command.CopyCommand;
+import org.apache.cloudstack.storage.command.DeleteCommand;
+import org.apache.cloudstack.storage.command.DownloadCommand;
+import org.apache.cloudstack.storage.command.DownloadProgressCommand;
+import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
+import org.apache.cloudstack.storage.command.UploadStatusAnswer;
+import org.apache.cloudstack.storage.command.UploadStatusAnswer.UploadStatus;
+import org.apache.cloudstack.storage.command.UploadStatusCommand;
+import org.apache.cloudstack.storage.configdrive.ConfigDrive;
+import org.apache.cloudstack.storage.configdrive.ConfigDriveBuilder;
+import org.apache.cloudstack.storage.template.DownloadManager;
+import org.apache.cloudstack.storage.template.DownloadManagerImpl;
+import org.apache.cloudstack.storage.template.UploadEntity;
+import org.apache.cloudstack.storage.template.UploadManager;
+import org.apache.cloudstack.storage.template.UploadManagerImpl;
+import org.apache.cloudstack.storage.to.SnapshotObjectTO;
+import org.apache.cloudstack.storage.to.TemplateObjectTO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
+import org.apache.cloudstack.utils.security.DigestHelper;
+import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.FilenameUtils;
+import org.apache.commons.lang.StringUtils;
+import org.apache.http.HttpEntity;
+import org.apache.http.HttpResponse;
+import org.apache.http.NameValuePair;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.utils.URLEncodedUtils;
+import org.apache.http.conn.ssl.NoopHostnameVerifier;
+import org.apache.http.conn.ssl.TrustStrategy;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClients;
+import org.apache.http.ssl.SSLContextBuilder;
+import org.apache.log4j.Logger;
+import org.joda.time.DateTime;
+import org.joda.time.format.ISODateTimeFormat;
+
+import javax.naming.ConfigurationException;
+import javax.net.ssl.SSLContext;
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.FileReader;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStreamWriter;
+import java.io.UnsupportedEncodingException;
+import java.net.InetAddress;
+import java.net.URI;
+import java.net.UnknownHostException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.KeyManagementException;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+
+import static com.cloud.network.NetworkModel.METATDATA_DIR;
+import static com.cloud.network.NetworkModel.PASSWORD_DIR;
+import static com.cloud.network.NetworkModel.PASSWORD_FILE;
+import static com.cloud.network.NetworkModel.PUBLIC_KEYS_FILE;
+import static com.cloud.network.NetworkModel.USERDATA_DIR;
+import static com.cloud.network.NetworkModel.USERDATA_FILE;
+import static com.cloud.utils.StringUtils.join;
+import static com.cloud.utils.storage.S3.S3Utils.putFile;
+import static java.lang.String.format;
+import static java.util.Arrays.asList;
+import static org.apache.commons.lang.StringUtils.substringAfterLast;
Review comment:
The ide does it according to style guidelines, but i agree its results in a bunch of extraneous changes :/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r704048728
##########
File path: plugins/storage/image/swift/src/main/java/org/apache/cloudstack/storage/datastore/driver/SwiftImageStoreDriverImpl.java
##########
@@ -100,12 +102,28 @@ public String createEntityExtractUrl(DataStore store, String installPath, ImageF
@Override
public void createAsync(DataStore dataStore, DataObject data, AsyncCompletionCallback<CreateCmdResult> callback) {
- Long maxTemplateSizeInBytes = getMaxTemplateSizeInBytes();
- VirtualMachineTemplate tmpl = _templateDao.findById(data.getId());
+
+ DownloadCommand downloadCommand = null;
+ if (data.getType() == DataObjectType.TEMPLATE) {
+ Long maxTemplateSizeInBytes = getMaxTemplateSizeInBytes();
+ downloadCommand = new DownloadCommand((TemplateObjectTO) (data.getTO()), maxTemplateSizeInBytes);
+ }else if (data.getType() == DataObjectType.VOLUME){
Review comment:
```suggestion
} else if (data.getType() == DataObjectType.VOLUME) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] skattoju4 commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
skattoju4 commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-837475076
The travis ci failures are intermittent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] skattoju4 commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
skattoju4 commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r605323186
##########
File path: plugins/storage/image/swift/src/main/java/org/apache/cloudstack/storage/datastore/driver/SwiftImageStoreDriverImpl.java
##########
@@ -100,12 +102,28 @@ public String createEntityExtractUrl(DataStore store, String installPath, ImageF
@Override
public void createAsync(DataStore dataStore, DataObject data, AsyncCompletionCallback<CreateCmdResult> callback) {
- Long maxTemplateSizeInBytes = getMaxTemplateSizeInBytes();
- VirtualMachineTemplate tmpl = _templateDao.findById(data.getId());
+
+ DownloadCommand downloadCommand = null;
+ if (data.getType() == DataObjectType.TEMPLATE) {
+ Long maxTemplateSizeInBytes = getMaxTemplateSizeInBytes();
+ downloadCommand = new DownloadCommand((TemplateObjectTO) (data.getTO()), maxTemplateSizeInBytes);
+ }else if (data.getType() == DataObjectType.VOLUME){
+ Long maxDownloadSizeInBytes = getMaxVolumeSizeInBytes();
+ VolumeInfo volumeInfo = (VolumeInfo) data;
+ RegisterVolumePayload payload = (RegisterVolumePayload) volumeInfo.getpayload();
+ ImageFormat format = ImageFormat.valueOf(payload.getFormat());
+ downloadCommand = new DownloadCommand((VolumeObjectTO) (data.getTO()), maxDownloadSizeInBytes, payload.getChecksum(), payload.getUrl(), format);
+ }
+
+ if (downloadCommand == null){
+ String errMsg = "Unable to build download command, DataObject is of neither VOLUME or TEMPLATE type";
Review comment:
thanks will do it.
##########
File path: core/src/main/java/com/cloud/storage/template/SwiftVolumeDownloader.java
##########
@@ -0,0 +1,396 @@
+//
+// 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 com.cloud.storage.template;
+
+import com.cloud.agent.api.to.SwiftTO;
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.storage.command.DownloadCommand;
+import org.apache.cloudstack.storage.command.DownloadCommand.ResourceType;
+import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.http.Header;
+import org.apache.http.HttpEntityEnclosingRequest;
+import org.apache.http.HttpRequest;
+import org.apache.http.HttpResponse;
+import org.apache.http.client.HttpRequestRetryHandler;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.protocol.HttpClientContext;
+import org.apache.http.conn.ConnectTimeoutException;
+import org.apache.http.conn.ssl.NoopHostnameVerifier;
+import org.apache.http.conn.ssl.TrustStrategy;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClients;
+import org.apache.http.protocol.HttpContext;
+import org.apache.http.ssl.SSLContextBuilder;
+import org.apache.log4j.Logger;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLException;
+import java.io.BufferedInputStream;
+import java.io.BufferedOutputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InterruptedIOException;
+import java.net.UnknownHostException;
+import java.security.KeyManagementException;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.util.Date;
+
+/**
+ * Download a volume file using HTTP(S)
+ *
+ * This class, once instantiated, has the purpose to download a Volume to staging nfs or cache when using as Swift Image Store.
+ *
+ * Execution of the instance is started when runInContext() is called.
+ */
+public class SwiftVolumeDownloader extends ManagedContextRunnable implements TemplateDownloader {
+ private static final Logger LOGGER = Logger.getLogger(SwiftVolumeDownloader.class.getName());
+ private static final int DOWNLOAD_BUFFER_SIZE_BYTES = 1024* 1024;
+
+ private final String downloadUrl;
+ private final String fileName;
+ private final String fileExtension;
+ private final long volumeId;
+ private final CloseableHttpClient httpClient;
+ private final HttpGet httpGet;
+ private final DownloadCompleteCallback downloadCompleteCallback;
+ private final SwiftTO swiftTO;
+ private String errorString = "";
+ private Status status = Status.NOT_STARTED;
+ private final ResourceType resourceType = ResourceType.VOLUME;
+ private long remoteSize;
+ private String md5sum;
+ private long downloadTime;
+ private long totalBytes;
+ private final long maxVolumeSizeInBytes;
+ private final String installPathPrefix;
+ private final String installPath;
+ private File volumeFile;
+ private boolean resume = false;
+
+ public SwiftVolumeDownloader(DownloadCommand cmd, DownloadCompleteCallback downloadCompleteCallback, long maxVolumeSizeInBytes, String installPathPrefix) {
+ this.downloadUrl = cmd.getUrl();
+ this.swiftTO = (SwiftTO) cmd.getDataStore();
+ this.maxVolumeSizeInBytes = maxVolumeSizeInBytes;
+ this.httpClient = initializeHttpClient();
+ this.downloadCompleteCallback = downloadCompleteCallback;
+ this.fileName = cmd.getName();
+ this.fileExtension = cmd.getFormat().getFileExtension();
+ this.volumeId = cmd.getId();
+ this.installPathPrefix = installPathPrefix;
+ this.installPath = cmd.getInstallPath();
+ this.httpGet = new HttpGet(downloadUrl);
+ }
+
+ private CloseableHttpClient initializeHttpClient(){
+
+ CloseableHttpClient client = null;
+ try {
+ //trust all certs
+ SSLContext sslContext = new SSLContextBuilder()
+ .loadTrustMaterial(null, (TrustStrategy) (chain, authType) -> true)
+ .build();
+ client = HttpClients.custom().setSSLContext(sslContext)
+ .setSSLHostnameVerifier(new NoopHostnameVerifier())
+ .setRetryHandler(buildRetryHandler(5))
+ .build();
+ } catch (NoSuchAlgorithmException e) {
+ e.printStackTrace();
+ } catch (KeyManagementException e) {
+ e.printStackTrace();
+ } catch (KeyStoreException e) {
+ e.printStackTrace();
+ }
+
+ return client;
+ }
+
+ private HttpRequestRetryHandler buildRetryHandler(int retryCount){
+
+ HttpRequestRetryHandler customRetryHandler = new HttpRequestRetryHandler() {
+ @Override
+ public boolean retryRequest(
+ IOException exception,
+ int executionCount,
+ HttpContext context) {
+ if (executionCount >= retryCount) {
+ // Do not retry if over max retry count
+ return false;
+ }
+ if (exception instanceof InterruptedIOException) {
+ // Timeout
+ return false;
+ }
+ if (exception instanceof UnknownHostException) {
+ // Unknown host
+ return false;
+ }
+ if (exception instanceof ConnectTimeoutException) {
+ // Connection refused
+ return false;
+ }
+ if (exception instanceof SSLException) {
+ // SSL handshake exception
+ return false;
+ }
+ HttpClientContext clientContext = HttpClientContext.adapt(context);
+ HttpRequest request = clientContext.getRequest();
+ boolean idempotent = !(request instanceof HttpEntityEnclosingRequest);
+ if (idempotent) {
+ // Retry if the request is considered idempotent
+ return true;
+ }
+ return false;
+ }
+
+ };
+ return customRetryHandler;
+ }
+
+ @Override
+ public long download(boolean resume, DownloadCompleteCallback callback) {
+ if (!status.equals(Status.NOT_STARTED)) {
+ // Only start downloading if we haven't started yet.
+ LOGGER.info("Volume download is already started, not starting again. Volume: " + downloadUrl);
+ return 0;
+ }
+
+ HttpResponse response = null;
+ try {
+ response = httpClient.execute(httpGet);
+ } catch (IOException e) {
+ e.printStackTrace();
+ errorString = "Exception while executing HttpMethod " + httpGet.getMethod() + " on URL " + downloadUrl + " "
+ + response.getStatusLine().getStatusCode() + " " + response.getStatusLine().getReasonPhrase();
+ LOGGER.error(errorString);
+ status = Status.UNRECOVERABLE_ERROR;
+ return 0;
+ }
+
+ // Headers
+ long contentLength = response.getEntity().getContentLength();
+ Header contentType = response.getEntity().getContentType();
+
+ // Check the contentLengthHeader and transferEncodingHeader.
+ if (contentLength <= 0) {
+ errorString = "The Content Length of " + downloadUrl + " is <= 0 and content Type is "+contentType.toString();
+ LOGGER.error(errorString);
+ status = Status.UNRECOVERABLE_ERROR;
+ return 0;
+ } else {
+ // The ContentLengthHeader is supplied, parse it's value.
+ remoteSize = contentLength;
+ }
+
+ if (remoteSize > maxVolumeSizeInBytes) {
+ errorString = "Remote size is too large for volume " + downloadUrl + " remote size is " + remoteSize + " max allowed is " + maxVolumeSizeInBytes;
+ LOGGER.error(errorString);
+ status = Status.UNRECOVERABLE_ERROR;
+ return 0;
+ }
+
+ InputStream inputStream;
+ try {
+ inputStream = new BufferedInputStream(response.getEntity().getContent());
+ } catch (IOException e) {
+ errorString = "Exception occurred while opening InputStream for volume from " + downloadUrl;
+ LOGGER.error(errorString);
+ status = Status.UNRECOVERABLE_ERROR;
+ return 0;
+ }
+
+ String filePath = installPathPrefix + File.separator + installPath;
+ File directory = new File(filePath);
+ File srcFile = new File(filePath + File.separator + fileName);
+ try {
+ if (!directory.exists()) {
+ LOGGER.info("Creating directories "+filePath);
+ directory.mkdirs();
+ }
+ if (!srcFile.createNewFile()) {
+ LOGGER.info("Reusing existing file " + srcFile.getPath());
+ }
+ } catch (IOException e) {
+ errorString = "Exception occurred while creating temp file " + srcFile.getPath();
+ LOGGER.error(errorString);
+ status = Status.UNRECOVERABLE_ERROR;
+ return 0;
+ }
+
+ LOGGER.info("Starting download from " + downloadUrl + " to staging with size " + remoteSize + " bytes to "+filePath);
+ final Date downloadStart = new Date();
+
+ try (FileOutputStream fileOutputStream = new FileOutputStream(srcFile);) {
+ BufferedOutputStream outputStream = new BufferedOutputStream(fileOutputStream,DOWNLOAD_BUFFER_SIZE_BYTES);
+ byte[] data = new byte[DOWNLOAD_BUFFER_SIZE_BYTES];
+ int bufferLength = 0;
+ while((bufferLength = inputStream.read(data,0,DOWNLOAD_BUFFER_SIZE_BYTES)) >= 0){
+ totalBytes += bufferLength;
+ outputStream.write(data,0,bufferLength);
+ status = Status.IN_PROGRESS;
+ LOGGER.trace("Download in progress: " + getDownloadPercent() + "%");
+ if(totalBytes >= remoteSize){
+ volumeFile = srcFile;
+ status = Status.DOWNLOAD_FINISHED;
+ }
+ }
+ outputStream.close();
+ inputStream.close();
+ } catch (IOException e) {
+ LOGGER.error("Exception when downloading from url to staging nfs:" + e.getMessage(), e);
Review comment:
thanks will do it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-867014987
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-889639711
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-889957017
Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian. SL-JID 713
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-914909253
Ping @skattoju4
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-918862818
Ping @skattoju4 can you fix conflicts and address comments?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r601767135
##########
File path: core/src/main/java/com/cloud/storage/template/SwiftVolumeDownloader.java
##########
@@ -0,0 +1,396 @@
+//
+// 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 com.cloud.storage.template;
+
+import com.cloud.agent.api.to.SwiftTO;
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.storage.command.DownloadCommand;
+import org.apache.cloudstack.storage.command.DownloadCommand.ResourceType;
+import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.http.Header;
+import org.apache.http.HttpEntityEnclosingRequest;
+import org.apache.http.HttpRequest;
+import org.apache.http.HttpResponse;
+import org.apache.http.client.HttpRequestRetryHandler;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.protocol.HttpClientContext;
+import org.apache.http.conn.ConnectTimeoutException;
+import org.apache.http.conn.ssl.NoopHostnameVerifier;
+import org.apache.http.conn.ssl.TrustStrategy;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClients;
+import org.apache.http.protocol.HttpContext;
+import org.apache.http.ssl.SSLContextBuilder;
+import org.apache.log4j.Logger;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLException;
+import java.io.BufferedInputStream;
+import java.io.BufferedOutputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InterruptedIOException;
+import java.net.UnknownHostException;
+import java.security.KeyManagementException;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.util.Date;
+
+/**
+ * Download a volume file using HTTP(S)
+ *
+ * This class, once instantiated, has the purpose to download a Volume to staging nfs or cache when using as Swift Image Store.
+ *
+ * Execution of the instance is started when runInContext() is called.
+ */
+public class SwiftVolumeDownloader extends ManagedContextRunnable implements TemplateDownloader {
+ private static final Logger LOGGER = Logger.getLogger(SwiftVolumeDownloader.class.getName());
+ private static final int DOWNLOAD_BUFFER_SIZE_BYTES = 1024* 1024;
+
+ private final String downloadUrl;
+ private final String fileName;
+ private final String fileExtension;
+ private final long volumeId;
+ private final CloseableHttpClient httpClient;
+ private final HttpGet httpGet;
+ private final DownloadCompleteCallback downloadCompleteCallback;
+ private final SwiftTO swiftTO;
+ private String errorString = "";
+ private Status status = Status.NOT_STARTED;
+ private final ResourceType resourceType = ResourceType.VOLUME;
+ private long remoteSize;
+ private String md5sum;
+ private long downloadTime;
+ private long totalBytes;
+ private final long maxVolumeSizeInBytes;
+ private final String installPathPrefix;
+ private final String installPath;
+ private File volumeFile;
+ private boolean resume = false;
+
+ public SwiftVolumeDownloader(DownloadCommand cmd, DownloadCompleteCallback downloadCompleteCallback, long maxVolumeSizeInBytes, String installPathPrefix) {
+ this.downloadUrl = cmd.getUrl();
+ this.swiftTO = (SwiftTO) cmd.getDataStore();
+ this.maxVolumeSizeInBytes = maxVolumeSizeInBytes;
+ this.httpClient = initializeHttpClient();
+ this.downloadCompleteCallback = downloadCompleteCallback;
+ this.fileName = cmd.getName();
+ this.fileExtension = cmd.getFormat().getFileExtension();
+ this.volumeId = cmd.getId();
+ this.installPathPrefix = installPathPrefix;
+ this.installPath = cmd.getInstallPath();
+ this.httpGet = new HttpGet(downloadUrl);
+ }
+
+ private CloseableHttpClient initializeHttpClient(){
+
+ CloseableHttpClient client = null;
+ try {
+ //trust all certs
+ SSLContext sslContext = new SSLContextBuilder()
+ .loadTrustMaterial(null, (TrustStrategy) (chain, authType) -> true)
+ .build();
+ client = HttpClients.custom().setSSLContext(sslContext)
+ .setSSLHostnameVerifier(new NoopHostnameVerifier())
+ .setRetryHandler(buildRetryHandler(5))
+ .build();
+ } catch (NoSuchAlgorithmException e) {
+ e.printStackTrace();
+ } catch (KeyManagementException e) {
+ e.printStackTrace();
+ } catch (KeyStoreException e) {
+ e.printStackTrace();
+ }
+
+ return client;
+ }
+
+ private HttpRequestRetryHandler buildRetryHandler(int retryCount){
+
+ HttpRequestRetryHandler customRetryHandler = new HttpRequestRetryHandler() {
+ @Override
+ public boolean retryRequest(
+ IOException exception,
+ int executionCount,
+ HttpContext context) {
+ if (executionCount >= retryCount) {
+ // Do not retry if over max retry count
+ return false;
+ }
+ if (exception instanceof InterruptedIOException) {
+ // Timeout
+ return false;
+ }
+ if (exception instanceof UnknownHostException) {
+ // Unknown host
+ return false;
+ }
+ if (exception instanceof ConnectTimeoutException) {
+ // Connection refused
+ return false;
+ }
+ if (exception instanceof SSLException) {
+ // SSL handshake exception
+ return false;
+ }
+ HttpClientContext clientContext = HttpClientContext.adapt(context);
+ HttpRequest request = clientContext.getRequest();
+ boolean idempotent = !(request instanceof HttpEntityEnclosingRequest);
+ if (idempotent) {
+ // Retry if the request is considered idempotent
+ return true;
+ }
+ return false;
+ }
+
+ };
+ return customRetryHandler;
+ }
+
+ @Override
+ public long download(boolean resume, DownloadCompleteCallback callback) {
+ if (!status.equals(Status.NOT_STARTED)) {
+ // Only start downloading if we haven't started yet.
+ LOGGER.info("Volume download is already started, not starting again. Volume: " + downloadUrl);
+ return 0;
+ }
+
+ HttpResponse response = null;
+ try {
+ response = httpClient.execute(httpGet);
+ } catch (IOException e) {
+ e.printStackTrace();
+ errorString = "Exception while executing HttpMethod " + httpGet.getMethod() + " on URL " + downloadUrl + " "
+ + response.getStatusLine().getStatusCode() + " " + response.getStatusLine().getReasonPhrase();
+ LOGGER.error(errorString);
+ status = Status.UNRECOVERABLE_ERROR;
+ return 0;
+ }
+
+ // Headers
+ long contentLength = response.getEntity().getContentLength();
+ Header contentType = response.getEntity().getContentType();
+
+ // Check the contentLengthHeader and transferEncodingHeader.
+ if (contentLength <= 0) {
+ errorString = "The Content Length of " + downloadUrl + " is <= 0 and content Type is "+contentType.toString();
+ LOGGER.error(errorString);
+ status = Status.UNRECOVERABLE_ERROR;
+ return 0;
+ } else {
+ // The ContentLengthHeader is supplied, parse it's value.
+ remoteSize = contentLength;
+ }
+
+ if (remoteSize > maxVolumeSizeInBytes) {
+ errorString = "Remote size is too large for volume " + downloadUrl + " remote size is " + remoteSize + " max allowed is " + maxVolumeSizeInBytes;
+ LOGGER.error(errorString);
+ status = Status.UNRECOVERABLE_ERROR;
+ return 0;
+ }
+
+ InputStream inputStream;
+ try {
+ inputStream = new BufferedInputStream(response.getEntity().getContent());
+ } catch (IOException e) {
+ errorString = "Exception occurred while opening InputStream for volume from " + downloadUrl;
+ LOGGER.error(errorString);
+ status = Status.UNRECOVERABLE_ERROR;
+ return 0;
+ }
+
+ String filePath = installPathPrefix + File.separator + installPath;
+ File directory = new File(filePath);
+ File srcFile = new File(filePath + File.separator + fileName);
+ try {
+ if (!directory.exists()) {
+ LOGGER.info("Creating directories "+filePath);
+ directory.mkdirs();
+ }
+ if (!srcFile.createNewFile()) {
+ LOGGER.info("Reusing existing file " + srcFile.getPath());
+ }
+ } catch (IOException e) {
+ errorString = "Exception occurred while creating temp file " + srcFile.getPath();
+ LOGGER.error(errorString);
+ status = Status.UNRECOVERABLE_ERROR;
+ return 0;
+ }
+
+ LOGGER.info("Starting download from " + downloadUrl + " to staging with size " + remoteSize + " bytes to "+filePath);
+ final Date downloadStart = new Date();
+
+ try (FileOutputStream fileOutputStream = new FileOutputStream(srcFile);) {
+ BufferedOutputStream outputStream = new BufferedOutputStream(fileOutputStream,DOWNLOAD_BUFFER_SIZE_BYTES);
+ byte[] data = new byte[DOWNLOAD_BUFFER_SIZE_BYTES];
+ int bufferLength = 0;
+ while((bufferLength = inputStream.read(data,0,DOWNLOAD_BUFFER_SIZE_BYTES)) >= 0){
+ totalBytes += bufferLength;
+ outputStream.write(data,0,bufferLength);
+ status = Status.IN_PROGRESS;
+ LOGGER.trace("Download in progress: " + getDownloadPercent() + "%");
+ if(totalBytes >= remoteSize){
+ volumeFile = srcFile;
+ status = Status.DOWNLOAD_FINISHED;
+ }
+ }
+ outputStream.close();
+ inputStream.close();
+ } catch (IOException e) {
+ LOGGER.error("Exception when downloading from url to staging nfs:" + e.getMessage(), e);
Review comment:
Suggestion:
Improve logging by adding `downloadUrl`, `remoteSize` and `filePath` to the message.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-894600312
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] skattoju4 commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
skattoju4 commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r605325211
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -1180,13 +1210,13 @@ protected Answer registerTemplateOnSwift(DownloadCommand cmd) {
try (FileInputStream fs = new FileInputStream(file)) {
md5sum = DigestUtils.md5Hex(fs);
} catch (IOException e) {
- s_logger.debug("Failed to get md5sum: " + file.getAbsoluteFile());
+ s_logger.error("Failed to get md5sum: " + file.getAbsoluteFile());
Review comment:
thanks will do it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-894601246
Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian. SL-JID 792
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r601774968
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -1137,11 +1167,11 @@ protected File downloadFromUrlToNfs(String url, NfsTO nfs, String path, String n
try (FileOutputStream outputStream = new FileOutputStream(destFile);) {
entity.writeTo(outputStream);
} catch (IOException e) {
- s_logger.debug("downloadFromUrlToNfs:Exception:" + e.getMessage(), e);
+ s_logger.error("downloadFromUrlToNfs:Exception:" + e.getMessage(), e);
}
return new File(destFile.getAbsolutePath());
} catch (IOException e) {
- s_logger.debug("Faild to get url:" + url + ", due to " + e.toString());
+ s_logger.error("Faild to get url:" + url + ", due to " + e.toString());
Review comment:
Suggestion:
Pass the exception as parameter to `s_logger.error`:
```java
s_logger.error("Faild to get url:" + url + ", due to " + e.toString(), e);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#discussion_r704051826
##########
File path: engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java
##########
@@ -146,7 +146,15 @@ private Scope pickCacheScopeForCopy(DataObject srcData, DataObject destData) {
}
protected Answer copyObject(DataObject srcData, DataObject destData, Host destHost) {
- int primaryStorageDownloadWait = StorageManager.PRIMARY_STORAGE_DOWNLOAD_WAIT.value();
+ long dataSize = 0;
+ try{
+ dataSize = srcData.getSize();
+ }catch(NullPointerException e){
Review comment:
```suggestion
} catch(NullPointerException e) {
```
check and correct formatting in the code changes, wherever applicable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4875: Fix data volume import / export functionality when using swift as secondary storage
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4875:
URL: https://github.com/apache/cloudstack/pull/4875#issuecomment-863659785
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org