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/05/10 18:31:13 UTC
[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #4977: Enable creation of vms with solidfire root volumes on vmware67
GutoVeronezi commented on a change in pull request #4977:
URL: https://github.com/apache/cloudstack/pull/4977#discussion_r629581989
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/DatastoreMO.java
##########
@@ -309,16 +309,20 @@ public boolean moveDatastoreFile(String srcFilePath, ManagedObjectReference morS
public boolean fileExists(String fileFullPath) throws Exception {
DatastoreFile file = new DatastoreFile(fileFullPath);
DatastoreFile dirFile = new DatastoreFile(file.getDatastoreName(), file.getDir());
-
- HostDatastoreBrowserMO browserMo = getHostDatastoreBrowserMO();
-
- s_logger.info("Search file " + file.getFileName() + " on " + dirFile.getPath());
- HostDatastoreBrowserSearchResults results = browserMo.searchDatastore(dirFile.getPath(), file.getFileName(), true);
- if (results != null) {
- List<FileInfo> info = results.getFile();
- if (info != null && info.size() > 0) {
- s_logger.info("File " + fileFullPath + " exists on datastore");
- return true;
+ Boolean folderExists = true;
+ if(file.getDir() != ""){
Review comment:
```java
import org.apache.commons.lang3.StringUtils;
```
```java
if (StringUtils.isNotEmpty(file.getDir())){
```
##########
File path: engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
##########
@@ -1390,10 +1390,12 @@ else if (volumeInfo.getFormat() == ImageFormat.OVA) {
verifyCopyCmdAnswer(copyCmdAnswer, templateInfo);
+ // Seems like vmware 6.5 and above does not automatically remount the datastore after rescanning the storage.
+ // Not sure if this is needed.
// If using VMware, have the host rescan its software HBA if dynamic discovery is in use.
- if (HypervisorType.VMware.equals(templateInfo.getHypervisorType())) {
- disconnectHostFromVolume(hostVO, volumeInfo.getPoolId(), volumeInfo.get_iScsiName());
- }
+ //if (HypervisorType.VMware.equals(templateInfo.getHypervisorType())) {
+ // disconnectHostFromVolume(hostVO, volumeInfo.getPoolId(), volumeInfo.get_iScsiName());
+ //}
Review comment:
As git will keep the history, we can remove the code and move the comment from code to commit message.
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/DatastoreMO.java
##########
@@ -309,16 +309,20 @@ public boolean moveDatastoreFile(String srcFilePath, ManagedObjectReference morS
public boolean fileExists(String fileFullPath) throws Exception {
DatastoreFile file = new DatastoreFile(fileFullPath);
DatastoreFile dirFile = new DatastoreFile(file.getDatastoreName(), file.getDir());
-
- HostDatastoreBrowserMO browserMo = getHostDatastoreBrowserMO();
-
- s_logger.info("Search file " + file.getFileName() + " on " + dirFile.getPath());
- HostDatastoreBrowserSearchResults results = browserMo.searchDatastore(dirFile.getPath(), file.getFileName(), true);
- if (results != null) {
- List<FileInfo> info = results.getFile();
- if (info != null && info.size() > 0) {
- s_logger.info("File " + fileFullPath + " exists on datastore");
- return true;
+ Boolean folderExists = true;
+ if(file.getDir() != ""){
+ folderExists = folderExists(String.format("[%s]", file.getDatastoreName()), file.getDir());
+ }
+ if(folderExists){
+ HostDatastoreBrowserMO browserMo = getHostDatastoreBrowserMO();
+ s_logger.info("Search file " + file.getFileName() + " on " + dirFile.getPath());
+ HostDatastoreBrowserSearchResults results = browserMo.searchDatastore(dirFile.getPath(), file.getFileName(), true);
+ if (results != null) {
+ List<FileInfo> info = results.getFile();
+ if (info != null && info.size() > 0) {
Review comment:
```java
import org.apache.commons.collections.CollectionUtils;
```
```java
if (CollectionUtils.isNotEmpty(info)) {
```
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/DatastoreMO.java
##########
@@ -309,16 +309,20 @@ public boolean moveDatastoreFile(String srcFilePath, ManagedObjectReference morS
public boolean fileExists(String fileFullPath) throws Exception {
DatastoreFile file = new DatastoreFile(fileFullPath);
DatastoreFile dirFile = new DatastoreFile(file.getDatastoreName(), file.getDir());
-
- HostDatastoreBrowserMO browserMo = getHostDatastoreBrowserMO();
-
- s_logger.info("Search file " + file.getFileName() + " on " + dirFile.getPath());
- HostDatastoreBrowserSearchResults results = browserMo.searchDatastore(dirFile.getPath(), file.getFileName(), true);
- if (results != null) {
- List<FileInfo> info = results.getFile();
- if (info != null && info.size() > 0) {
- s_logger.info("File " + fileFullPath + " exists on datastore");
- return true;
+ Boolean folderExists = true;
+ if(file.getDir() != ""){
+ folderExists = folderExists(String.format("[%s]", file.getDatastoreName()), file.getDir());
+ }
+ if(folderExists){
Review comment:
```suggestion
if (folderExists){
```
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java
##########
@@ -280,7 +280,8 @@ public ResignatureAnswer resignature(ResignatureCommand cmd) {
}
}
- removeVmfsDatastore(cmd, hyperHost, datastoreName, storageHost, storagePortNumber, trimIqn(iScsiName), lstHosts);
+ //vmware 6.7 does not automatically mount datastores after rescanning once removed
+ //removeVmfsDatastore(cmd, hyperHost, datastoreName, storageHost, storagePortNumber, trimIqn(iScsiName), lstHosts);
Review comment:
As git will keep the history, we can remove the code and move the comment from code to commit 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