[GitHub] blueorangutan commented on issue #2766: kvm: Agent should not check if remaining memory on host is sufficient

2018-07-26 Thread GitBox
blueorangutan commented on issue #2766: kvm: Agent should not check if 
remaining memory on host is sufficient
URL: https://github.com/apache/cloudstack/pull/2766#issuecomment-408319053
 
 
   @borisstoyanov 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 GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] borisstoyanov commented on issue #2766: kvm: Agent should not check if remaining memory on host is sufficient

2018-07-26 Thread GitBox
borisstoyanov commented on issue #2766: kvm: Agent should not check if 
remaining memory on host is sufficient
URL: https://github.com/apache/cloudstack/pull/2766#issuecomment-408318941
 
 
   @blueorangutan test


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] bennysp commented on issue #2777: Cannot add new host with bonded interface.

2018-07-26 Thread GitBox
bennysp commented on issue #2777: Cannot add new host with bonded interface.
URL: https://github.com/apache/cloudstack/issues/2777#issuecomment-408301929
 
 
   Well, I reinstalled the OS and did this all from scratch... Doesn't look 
like a "bond" issue, since I continue to get the same issue regardless.
   
   Please help.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] mike-tutkowski commented on a change in pull request #2761: Add managed storage pool constraints to MigrateWithVolume API method

2018-07-26 Thread GitBox
mike-tutkowski commented on a change in pull request #2761: Add managed storage 
pool constraints to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#discussion_r205656884
 
 

 ##
 File path: 
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
 ##
 @@ -2279,95 +2279,151 @@ protected void migrate(final VMInstanceVO vm, final 
long srcHostId, final Deploy
 }
 
 /**
- * Create the mapping of volumes and storage pools. If the user did not 
enter a mapping on her/his own, we create one using {@link 
#getDefaultMappingOfVolumesAndStoragePoolForMigration(VirtualMachineProfile, 
Host)}.
- * If the user provided a mapping, we use whatever the user has provided 
(check the method {@link 
#createMappingVolumeAndStoragePoolEnteredByUser(VirtualMachineProfile, Host, 
Map)}).
+ * We create the mapping of volumes and storage pool to migrate the VMs 
according to the information sent by the user.
+ * If the user did not enter a complete mapping, the volumes that were 
left behind will be auto mapped using {@link 
#createStoragePoolMappingsForVolumes(VirtualMachineProfile, Host, Map, List)}
  */
-private Map 
getPoolListForVolumesForMigration(VirtualMachineProfile profile, Host 
targetHost, Map volumeToPool) {
-if (MapUtils.isEmpty(volumeToPool)) {
-return 
getDefaultMappingOfVolumesAndStoragePoolForMigration(profile, targetHost);
-}
+protected Map 
createMappingVolumeAndStoragePool(VirtualMachineProfile profile, Host 
targetHost, Map userDefinedMapOfVolumesAndStoragePools) {
+Map volumeToPoolObjectMap = new HashMap<>();
+buildMapUsingUserInformation(profile, targetHost, 
userDefinedMapOfVolumesAndStoragePools, volumeToPoolObjectMap);
 
-return createMappingVolumeAndStoragePoolEnteredByUser(profile, 
targetHost, volumeToPool);
+List volumesNotMapped = 
findVolumesThatWereNotMappedByTheUser(profile, volumeToPoolObjectMap);
+createStoragePoolMappingsForVolumes(profile, targetHost, 
volumeToPoolObjectMap, volumesNotMapped);
+return volumeToPoolObjectMap;
 }
 
 /**
- * We create the mapping of volumes and storage pool to migrate the VMs 
according to the information sent by the user.
+ *  Given the map of volume to target storage pool entered by the user, we 
check for other volumes that the VM might have and were not configured.
+ *  This map can be then used by CloudStack to find new target storage 
pools according to the target host.
+ */
+private List 
findVolumesThatWereNotMappedByTheUser(VirtualMachineProfile profile, 
Map volumeToStoragePoolObjectMap) {
+List allVolumes = 
_volsDao.findUsableVolumesForInstance(profile.getId());
+List volumesNotMapped = new ArrayList<>();
+for (Volume volume : volumeToStoragePoolObjectMap.keySet()) {
+if (!allVolumes.contains(volume)) {
+volumesNotMapped.add(volume);
+}
+}
+return volumesNotMapped;
+}
+
+/**
+ *  Builds the map of storage pools and volumes with the information 
entered by the user. Before creating the an entry we validate if the migration 
is feasible checking if the migration is allowed and if the target host can 
access the defined target storage pool.
  */
-private Map 
createMappingVolumeAndStoragePoolEnteredByUser(VirtualMachineProfile profile, 
Host host, Map volumeToPool) {
-Map volumeToPoolObjectMap = new HashMap();
-for(Long volumeId: volumeToPool.keySet()) {
+private void buildMapUsingUserInformation(VirtualMachineProfile profile, 
Host targetHost, Map userDefinedVolumeToStoragePoolMap, Map volumeToPoolObjectMap) {
+if (MapUtils.isEmpty(userDefinedVolumeToStoragePoolMap)) {
+return;
+}
+for(Long volumeId: userDefinedVolumeToStoragePoolMap.keySet()) {
 VolumeVO volume = _volsDao.findById(volumeId);
 
-Long poolId = volumeToPool.get(volumeId);
+Long poolId = userDefinedVolumeToStoragePoolMap.get(volumeId);
 StoragePoolVO targetPool = _storagePoolDao.findById(poolId);
 StoragePoolVO currentPool = 
_storagePoolDao.findById(volume.getPoolId());
 
-if (_poolHostDao.findByPoolHost(targetPool.getId(), host.getId()) 
== null) {
-throw new CloudRuntimeException(String.format("Cannot migrate 
the volume [%s] to the storage pool [%s] while migrating VM [%s] to target host 
[%s]. The host does not have access to the storage pool entered.", 
volume.getUuid(), targetPool.getUuid(), profile.getUuid(), host.getUuid()));
+
executeManagedStorageChecksWhenTargetStoragePoolProvided(currentPool, volume, 
targetPool);
+if (_poolHostDao.findByPoolHost(targetPool.getId(), 
targetHost.getId()) == null) {
+throw new CloudRuntimeException(
+String.format("Cannot migrate 

[GitHub] mike-tutkowski commented on a change in pull request #2761: Add managed storage pool constraints to MigrateWithVolume API method

2018-07-26 Thread GitBox
mike-tutkowski commented on a change in pull request #2761: Add managed storage 
pool constraints to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#discussion_r205656884
 
 

 ##
 File path: 
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
 ##
 @@ -2279,95 +2279,151 @@ protected void migrate(final VMInstanceVO vm, final 
long srcHostId, final Deploy
 }
 
 /**
- * Create the mapping of volumes and storage pools. If the user did not 
enter a mapping on her/his own, we create one using {@link 
#getDefaultMappingOfVolumesAndStoragePoolForMigration(VirtualMachineProfile, 
Host)}.
- * If the user provided a mapping, we use whatever the user has provided 
(check the method {@link 
#createMappingVolumeAndStoragePoolEnteredByUser(VirtualMachineProfile, Host, 
Map)}).
+ * We create the mapping of volumes and storage pool to migrate the VMs 
according to the information sent by the user.
+ * If the user did not enter a complete mapping, the volumes that were 
left behind will be auto mapped using {@link 
#createStoragePoolMappingsForVolumes(VirtualMachineProfile, Host, Map, List)}
  */
-private Map 
getPoolListForVolumesForMigration(VirtualMachineProfile profile, Host 
targetHost, Map volumeToPool) {
-if (MapUtils.isEmpty(volumeToPool)) {
-return 
getDefaultMappingOfVolumesAndStoragePoolForMigration(profile, targetHost);
-}
+protected Map 
createMappingVolumeAndStoragePool(VirtualMachineProfile profile, Host 
targetHost, Map userDefinedMapOfVolumesAndStoragePools) {
+Map volumeToPoolObjectMap = new HashMap<>();
+buildMapUsingUserInformation(profile, targetHost, 
userDefinedMapOfVolumesAndStoragePools, volumeToPoolObjectMap);
 
-return createMappingVolumeAndStoragePoolEnteredByUser(profile, 
targetHost, volumeToPool);
+List volumesNotMapped = 
findVolumesThatWereNotMappedByTheUser(profile, volumeToPoolObjectMap);
+createStoragePoolMappingsForVolumes(profile, targetHost, 
volumeToPoolObjectMap, volumesNotMapped);
+return volumeToPoolObjectMap;
 }
 
 /**
- * We create the mapping of volumes and storage pool to migrate the VMs 
according to the information sent by the user.
+ *  Given the map of volume to target storage pool entered by the user, we 
check for other volumes that the VM might have and were not configured.
+ *  This map can be then used by CloudStack to find new target storage 
pools according to the target host.
+ */
+private List 
findVolumesThatWereNotMappedByTheUser(VirtualMachineProfile profile, 
Map volumeToStoragePoolObjectMap) {
+List allVolumes = 
_volsDao.findUsableVolumesForInstance(profile.getId());
+List volumesNotMapped = new ArrayList<>();
+for (Volume volume : volumeToStoragePoolObjectMap.keySet()) {
+if (!allVolumes.contains(volume)) {
+volumesNotMapped.add(volume);
+}
+}
+return volumesNotMapped;
+}
+
+/**
+ *  Builds the map of storage pools and volumes with the information 
entered by the user. Before creating the an entry we validate if the migration 
is feasible checking if the migration is allowed and if the target host can 
access the defined target storage pool.
  */
-private Map 
createMappingVolumeAndStoragePoolEnteredByUser(VirtualMachineProfile profile, 
Host host, Map volumeToPool) {
-Map volumeToPoolObjectMap = new HashMap();
-for(Long volumeId: volumeToPool.keySet()) {
+private void buildMapUsingUserInformation(VirtualMachineProfile profile, 
Host targetHost, Map userDefinedVolumeToStoragePoolMap, Map volumeToPoolObjectMap) {
+if (MapUtils.isEmpty(userDefinedVolumeToStoragePoolMap)) {
+return;
+}
+for(Long volumeId: userDefinedVolumeToStoragePoolMap.keySet()) {
 VolumeVO volume = _volsDao.findById(volumeId);
 
-Long poolId = volumeToPool.get(volumeId);
+Long poolId = userDefinedVolumeToStoragePoolMap.get(volumeId);
 StoragePoolVO targetPool = _storagePoolDao.findById(poolId);
 StoragePoolVO currentPool = 
_storagePoolDao.findById(volume.getPoolId());
 
-if (_poolHostDao.findByPoolHost(targetPool.getId(), host.getId()) 
== null) {
-throw new CloudRuntimeException(String.format("Cannot migrate 
the volume [%s] to the storage pool [%s] while migrating VM [%s] to target host 
[%s]. The host does not have access to the storage pool entered.", 
volume.getUuid(), targetPool.getUuid(), profile.getUuid(), host.getUuid()));
+
executeManagedStorageChecksWhenTargetStoragePoolProvided(currentPool, volume, 
targetPool);
+if (_poolHostDao.findByPoolHost(targetPool.getId(), 
targetHost.getId()) == null) {
+throw new CloudRuntimeException(
+String.format("Cannot migrate 

[GitHub] bennysp commented on issue #2777: Cannot add new host with bonded interface.

2018-07-26 Thread GitBox
bennysp commented on issue #2777: Cannot add new host with bonded interface.
URL: https://github.com/apache/cloudstack/issues/2777#issuecomment-408281916
 
 
   I changed to a single interface and I still see issues...
   
   ```
   DEBUG:root:execute:setenforce 1
   DEBUG:root:execute:hostname -f
   DEBUG:root:execute:selinuxenabled
   DEBUG:root:execute:setenforce 0
   DEBUG:root:cloudbr0 is not a network device, is it down?
   DEBUG:root:execute:route -n|awk '/^0.0.0.0/ {print $2,$8}'
   DEBUG:root:execute:ifconfig em1
   DEBUG:root:[Errno 2] No such file or directory
 File "/usr/lib64/python2.7/site-packages/cloudutils/serviceConfig.py", 
line 38, in configration
   result = self.config()
 File "/usr/lib64/python2.7/site-packages/cloudutils/serviceConfig.py", 
line 309, in config
   super(networkConfigRedhat, self).cfgNetwork()
 File "/usr/lib64/python2.7/site-packages/cloudutils/serviceConfig.py", 
line 108, in cfgNetwork
   device = self.netcfg.getDefaultNetwork()
 File "/usr/lib64/python2.7/site-packages/cloudutils/networkConfig.py", 
line 53, in getDefaultNetwork
   pdi = networkConfig.getDevInfo(dev)
 File "/usr/lib64/python2.7/site-packages/cloudutils/networkConfig.py", 
line 157, in getDevInfo
   elif networkConfig.isBridge(dev) or networkConfig.isOvsBridge(dev):
   
   DEBUG:root:execute:setenforce 1
   ```


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on a change in pull request #2761: Add managed storage pool constraints to MigrateWithVolume API method

2018-07-26 Thread GitBox
rafaelweingartner commented on a change in pull request #2761: Add managed 
storage pool constraints to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#discussion_r205643152
 
 

 ##
 File path: 
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
 ##
 @@ -2279,95 +2279,151 @@ protected void migrate(final VMInstanceVO vm, final 
long srcHostId, final Deploy
 }
 
 /**
- * Create the mapping of volumes and storage pools. If the user did not 
enter a mapping on her/his own, we create one using {@link 
#getDefaultMappingOfVolumesAndStoragePoolForMigration(VirtualMachineProfile, 
Host)}.
- * If the user provided a mapping, we use whatever the user has provided 
(check the method {@link 
#createMappingVolumeAndStoragePoolEnteredByUser(VirtualMachineProfile, Host, 
Map)}).
+ * We create the mapping of volumes and storage pool to migrate the VMs 
according to the information sent by the user.
+ * If the user did not enter a complete mapping, the volumes that were 
left behind will be auto mapped using {@link 
#createStoragePoolMappingsForVolumes(VirtualMachineProfile, Host, Map, List)}
  */
-private Map 
getPoolListForVolumesForMigration(VirtualMachineProfile profile, Host 
targetHost, Map volumeToPool) {
-if (MapUtils.isEmpty(volumeToPool)) {
-return 
getDefaultMappingOfVolumesAndStoragePoolForMigration(profile, targetHost);
-}
+protected Map 
createMappingVolumeAndStoragePool(VirtualMachineProfile profile, Host 
targetHost, Map userDefinedMapOfVolumesAndStoragePools) {
+Map volumeToPoolObjectMap = new HashMap<>();
+buildMapUsingUserInformation(profile, targetHost, 
userDefinedMapOfVolumesAndStoragePools, volumeToPoolObjectMap);
 
-return createMappingVolumeAndStoragePoolEnteredByUser(profile, 
targetHost, volumeToPool);
+List volumesNotMapped = 
findVolumesThatWereNotMappedByTheUser(profile, volumeToPoolObjectMap);
+createStoragePoolMappingsForVolumes(profile, targetHost, 
volumeToPoolObjectMap, volumesNotMapped);
+return volumeToPoolObjectMap;
 }
 
 /**
- * We create the mapping of volumes and storage pool to migrate the VMs 
according to the information sent by the user.
+ *  Given the map of volume to target storage pool entered by the user, we 
check for other volumes that the VM might have and were not configured.
+ *  This map can be then used by CloudStack to find new target storage 
pools according to the target host.
+ */
+private List 
findVolumesThatWereNotMappedByTheUser(VirtualMachineProfile profile, 
Map volumeToStoragePoolObjectMap) {
+List allVolumes = 
_volsDao.findUsableVolumesForInstance(profile.getId());
+List volumesNotMapped = new ArrayList<>();
+for (Volume volume : volumeToStoragePoolObjectMap.keySet()) {
+if (!allVolumes.contains(volume)) {
+volumesNotMapped.add(volume);
+}
+}
+return volumesNotMapped;
+}
+
+/**
+ *  Builds the map of storage pools and volumes with the information 
entered by the user. Before creating the an entry we validate if the migration 
is feasible checking if the migration is allowed and if the target host can 
access the defined target storage pool.
  */
-private Map 
createMappingVolumeAndStoragePoolEnteredByUser(VirtualMachineProfile profile, 
Host host, Map volumeToPool) {
-Map volumeToPoolObjectMap = new HashMap();
-for(Long volumeId: volumeToPool.keySet()) {
+private void buildMapUsingUserInformation(VirtualMachineProfile profile, 
Host targetHost, Map userDefinedVolumeToStoragePoolMap, Map volumeToPoolObjectMap) {
+if (MapUtils.isEmpty(userDefinedVolumeToStoragePoolMap)) {
+return;
+}
+for(Long volumeId: userDefinedVolumeToStoragePoolMap.keySet()) {
 VolumeVO volume = _volsDao.findById(volumeId);
 
-Long poolId = volumeToPool.get(volumeId);
+Long poolId = userDefinedVolumeToStoragePoolMap.get(volumeId);
 StoragePoolVO targetPool = _storagePoolDao.findById(poolId);
 StoragePoolVO currentPool = 
_storagePoolDao.findById(volume.getPoolId());
 
-if (_poolHostDao.findByPoolHost(targetPool.getId(), host.getId()) 
== null) {
-throw new CloudRuntimeException(String.format("Cannot migrate 
the volume [%s] to the storage pool [%s] while migrating VM [%s] to target host 
[%s]. The host does not have access to the storage pool entered.", 
volume.getUuid(), targetPool.getUuid(), profile.getUuid(), host.getUuid()));
+
executeManagedStorageChecksWhenTargetStoragePoolProvided(currentPool, volume, 
targetPool);
+if (_poolHostDao.findByPoolHost(targetPool.getId(), 
targetHost.getId()) == null) {
+throw new CloudRuntimeException(
+String.format("Cannot migrate 

[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method

2018-07-26 Thread GitBox
rafaelweingartner commented on issue #2761: Add managed storage pool 
constraints to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-408278876
 
 
   Ok, no problem, I got the gist.
   
   Those changes created a confusion for me though. 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] bennysp opened a new issue #2777: Cannot add new host with bonded interface.

2018-07-26 Thread GitBox
bennysp opened a new issue #2777: Cannot add new host with bonded interface.
URL: https://github.com/apache/cloudstack/issues/2777
 
 
   
   
   # ISSUE TYPE
   
* Bug Report
   
   # COMPONENT NAME
   
   ~~~
   cloudstack-agent
   ~~~
   
   # CLOUDSTACK VERSION
   
   
   ~~~
   4.11
   ~~~
   
   # CONFIGURATION
   
   1 server (using quicksetup)
   
   # OS / ENVIRONMENT
   
   Centos 7
   
   # SUMMARY
   
   When I am going through the Setup Wizard, it gets to the host install/setup 
and fails.  When I check the agent setup log, this is what I see.
   
   ```
   [root@csserv01 ~]# tail -1000f /var/log/cloudstack/agent/setup.log
   DEBUG:root:execute:hostname -f
   DEBUG:root:execute:selinuxenabled
   DEBUG:root:execute:setenforce 0
   DEBUG:root:cloudbr0 is not a network device, is it down?
   DEBUG:root:execute:route -n|awk '/^0.0.0.0/ {print $2,$8}'
   DEBUG:root:execute:ifconfig bond0
   DEBUG:root:[Errno 2] No such file or directory
 File "/usr/lib64/python2.7/site-packages/cloudutils/serviceConfig.py", 
line 38, in configration
   result = self.config()
 File "/usr/lib64/python2.7/site-packages/cloudutils/serviceConfig.py", 
line 309, in config
   super(networkConfigRedhat, self).cfgNetwork()
 File "/usr/lib64/python2.7/site-packages/cloudutils/serviceConfig.py", 
line 108, in cfgNetwork
   device = self.netcfg.getDefaultNetwork()
 File "/usr/lib64/python2.7/site-packages/cloudutils/networkConfig.py", 
line 53, in getDefaultNetwork
   pdi = networkConfig.getDevInfo(dev)
 File "/usr/lib64/python2.7/site-packages/cloudutils/networkConfig.py", 
line 157, in getDevInfo
   elif networkConfig.isBridge(dev) or networkConfig.isOvsBridge(dev):
   
   DEBUG:root:execute:setenforce 1
   ```
   
   # STEPS TO REPRODUCE
   

   
   
   ~~~
   Setup a bond0 network between 2 or more NICs on the server.
   
   Follow the quickstart guide.  When you get to the step of the wizard 
"Launch", everything will go through except the error on the host setup. 
   ~~~
   
   
   
   # EXPECTED RESULTS
   
   
   ~~~
   That it finds the bond0 interface and continues to setup
   ~~~
   
   # ACTUAL RESULTS
   
   
   
   ~~~
   Host fails and error below is produced...
   
   [root@csserv01 ~]# tail -1000f /var/log/cloudstack/agent/setup.log
   DEBUG:root:execute:hostname -f
   DEBUG:root:execute:selinuxenabled
   DEBUG:root:execute:setenforce 0
   DEBUG:root:cloudbr0 is not a network device, is it down?
   DEBUG:root:execute:route -n|awk '/^0.0.0.0/ {print $2,$8}'
   DEBUG:root:execute:ifconfig bond0
   DEBUG:root:[Errno 2] No such file or directory
 File "/usr/lib64/python2.7/site-packages/cloudutils/serviceConfig.py", 
line 38, in configration
   result = self.config()
 File "/usr/lib64/python2.7/site-packages/cloudutils/serviceConfig.py", 
line 309, in config
   super(networkConfigRedhat, self).cfgNetwork()
 File "/usr/lib64/python2.7/site-packages/cloudutils/serviceConfig.py", 
line 108, in cfgNetwork
   device = self.netcfg.getDefaultNetwork()
 File "/usr/lib64/python2.7/site-packages/cloudutils/networkConfig.py", 
line 53, in getDefaultNetwork
   pdi = networkConfig.getDevInfo(dev)
 File "/usr/lib64/python2.7/site-packages/cloudutils/networkConfig.py", 
line 157, in getDevInfo
   elif networkConfig.isBridge(dev) or networkConfig.isOvsBridge(dev):
   
   DEBUG:root:execute:setenforce 1
   ~~~
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] mike-tutkowski commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method

2018-07-26 Thread GitBox
mike-tutkowski commented on issue #2761: Add managed storage pool constraints 
to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-408271570
 
 
   By the way, I didn't change the test for 
https://github.com/mike-tutkowski/cloudstack/commit/bea1dad5b84bb80ba1f1297a69a1daea1c4ec736,
 so it won't technically compile. I intented it more to give you an idea of 
where we might want to go with the code here.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] mike-tutkowski commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method

2018-07-26 Thread GitBox
mike-tutkowski commented on issue #2761: Add managed storage pool constraints 
to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-408270678
 
 
   @rafaelweingartner Let's say the user provided a (target) managed storage 
pool for the volume in question. If that's the case, then we just want to make 
sure that this managed storage pool is the same as the storage pool of the 
volume. We don't care if it's a zone-wide managed storage pool or a 
cluster-wide managed storage pool in this case (either is acceptable). We just 
want to make sure that it's the same storage pool. Those checks are captured in 
executeManagedStorageChecksWhenTargetStoragePoolProvided (which does not care 
about the zone-wide requirement).
   
   However, if you don't provide a storage pool for the volume in question and 
it is located on managed storage, then we - in case you are doing a migration 
of the VM from one cluster to another - need to only make sure the managed 
storage pool is zone wide. If it turns out that this managed storage pool is 
cluster wide, then the user needs to explicitly pass the volume/target storage 
pool in as a parameter to the API command when performing the migration of the 
VM (with its storage) from one cluster to another.
   
   At least this is how it used to work.
   
   One alternative is to keep a single managed-storage method for checking, but 
to drop the zone-wide requirement. In fact, I think we can get away with only 
calling that check method once then.
   
   Take a look at this proposal:
   
   
https://github.com/mike-tutkowski/cloudstack/commit/bea1dad5b84bb80ba1f1297a69a1daea1c4ec736


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner edited a comment on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method

2018-07-26 Thread GitBox
rafaelweingartner edited a comment on issue #2761: Add managed storage pool 
constraints to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-408264657
 
 
   Thanks mike. I applied your suggestions. I am only a little confused with 
the use of `executeManagedStorageChecksWhenTargetStoragePoolProvided` and 
`executeManagedStorageChecksWhenTargetStoragePoolNotProvided`.
   
   Can't I simply use `executeManagedStorageChecks` in both places?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method

2018-07-26 Thread GitBox
rafaelweingartner commented on issue #2761: Add managed storage pool 
constraints to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-408264657
 
 
   Thanks mike. I applied your suggestions. I am only a little confused with 
the use of `executeManagedStorageChecksWhenTargetStoragePoolProvided` and 
`executeManagedStorageChecksWhenTargetStoragePoolNotProvided`.
   
   Can't I simply use 
`executeManagedStorageChecksWhenTargetStoragePoolProvided` in both places?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] mike-tutkowski commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method

2018-07-26 Thread GitBox
mike-tutkowski commented on issue #2761: Add managed storage pool constraints 
to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-408255118
 
 
   @rafaelweingartner I think we are very close. Take a look at what I did 
here: 
https://github.com/mike-tutkowski/cloudstack/commit/07279b41a3277becdc3b6c79f69f46559958f442
 .
   
   I made a few changes that I felt better reflected what the code used to do 
around checks for managed storage.
   
   To get it to compile, I made a couple changes and commented out some code in 
the JUnit test for now.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] GabrielBrascher commented on issue #2773: CLOUDSTACK-10328: Add Secondary IPv6 address through API

2018-07-26 Thread GitBox
GabrielBrascher commented on issue #2773: CLOUDSTACK-10328: Add Secondary IPv6 
address through API
URL: https://github.com/apache/cloudstack/pull/2773#issuecomment-408215215
 
 
   @rafaelweingartner @rhtyd the Travis is ok now, fixed the missing Apache 
license.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] wido commented on issue #2766: kvm: Agent should not check if remaining memory on host is sufficient

2018-07-26 Thread GitBox
wido commented on issue #2766: kvm: Agent should not check if remaining memory 
on host is sufficient
URL: https://github.com/apache/cloudstack/pull/2766#issuecomment-408214723
 
 
   @rafaelweingartner Ah, yes, I didn't notice the tests yet :) Just wondering 
when we merge PRs into master right now.
   
   But indeed, let's wait for the tests.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on issue #2766: kvm: Agent should not check if remaining memory on host is sufficient

2018-07-26 Thread GitBox
rafaelweingartner commented on issue #2766: kvm: Agent should not check if 
remaining memory on host is sufficient
URL: https://github.com/apache/cloudstack/pull/2766#issuecomment-408210369
 
 
   Yes, I approved it already two days ago.
   
   Shouldn't we wait for the test results though?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] wido commented on issue #2766: kvm: Agent should not check if remaining memory on host is sufficient

2018-07-26 Thread GitBox
wido commented on issue #2766: kvm: Agent should not check if remaining memory 
on host is sufficient
URL: https://github.com/apache/cloudstack/pull/2766#issuecomment-408209932
 
 
   @rafaelweingartner Are you OK with me merging this PR as we have the 
approvals?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] khos2ow commented on issue #2775: Fix typo in ISO url

2018-07-26 Thread GitBox
khos2ow commented on issue #2775: Fix typo in ISO url
URL: https://github.com/apache/cloudstack/pull/2775#issuecomment-408206255
 
 
   I updated the checksum type to `sha1` and double checked that the has is 
actually valid, as well as the URL.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on a change in pull request #2776: Issue 2774: Changed the implementation of isVolumeOnManagedStorage(VolumeInfo) to…

2018-07-26 Thread GitBox
rafaelweingartner commented on a change in pull request #2776: Issue 2774: 
Changed the implementation of isVolumeOnManagedStorage(VolumeInfo) to…
URL: https://github.com/apache/cloudstack/pull/2776#discussion_r205571779
 
 

 ##
 File path: 
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
 ##
 @@ -196,10 +196,16 @@ public StrategyPriority canHandle(DataObject srcData, 
DataObject destData) {
 }
 
 private boolean isVolumeOnManagedStorage(VolumeInfo volumeInfo) {
 
 Review comment:
   What about a unit test here? 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] mike-tutkowski opened a new pull request #2776: Issue 2774: Changed the implementation of isVolumeOnManagedStorage(VolumeInfo) to…

2018-07-26 Thread GitBox
mike-tutkowski opened a new pull request #2776: Issue 2774: Changed the 
implementation of isVolumeOnManagedStorage(VolumeInfo) to…
URL: https://github.com/apache/cloudstack/pull/2776
 
 
   … check if the data store in question is for primary storage
   
   ## Description
   In the StorageSystemDataMotionStrategy.isVolumeOnManagedStorage(VolumeInfo) 
method, we should have been checking if the DataStore associated with the 
VolumeInfo was on primary storage before checking if it is managed storage.
   
   
   
   
   ## 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)
   
   ## GitHub Issue/PRs
   Fixes: #2774
   
   ## How Has This Been Tested?
   Regression testing
   
   ## Checklist:
   
   
   - [ ] I have read the 
[CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md)
 document.
   - [x] My code follows the code style of this project.
   - [ ] My change requires a change to the documentation.
   - [ ] I have updated the documentation accordingly.
   Testing
   - [ ] I have added tests to cover my changes.
   - [ ] All relevant new and existing integration tests have passed.
   - [ ] A full integration testsuite with all test that can run on my 
environment has passed.
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] blueorangutan commented on issue #2769: Fix config drive iso path on Vmware

2018-07-26 Thread GitBox
blueorangutan commented on issue #2769: Fix config drive iso path on Vmware
URL: https://github.com/apache/cloudstack/pull/2769#issuecomment-408197757
 
 
   Trillian test result (tid-2889)
   Environment: vmware-65 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 36675 seconds
   Marvin logs: 
https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2769-t2889-vmware-65.zip
   Intermitten failure detected: 
/marvin/tests/smoke/test_deploy_vm_root_resize.py
   Intermitten failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Smoke tests completed. 66 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_05_rvpc_multi_tiers | `Failure` | 510.15 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 547.57 | test_vpc_redundant.py
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on a change in pull request #2775: Fix typo in ISO url

2018-07-26 Thread GitBox
rafaelweingartner commented on a change in pull request #2775: Fix typo in ISO 
url
URL: https://github.com/apache/cloudstack/pull/2775#discussion_r205509652
 
 

 ##
 File path: tools/appliance/builtin/template.json
 ##
 @@ -23,7 +23,7 @@
   "disk_interface": "virtio",
   "net_device": "virtio-net",
 
-  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1808.iso;,
+  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1804.iso;,
 
 Review comment:
   if the type can be set to sha1 or sha256, please do so. Otherwise, we need 
to configure the right hash value there.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] khos2ow commented on a change in pull request #2747: systemvm: Update ISO URLs to the latest

2018-07-26 Thread GitBox
khos2ow commented on a change in pull request #2747: systemvm: Update ISO URLs 
to the latest
URL: https://github.com/apache/cloudstack/pull/2747#discussion_r205508736
 
 

 ##
 File path: tools/appliance/builtin/template.json
 ##
 @@ -23,8 +23,8 @@
   "disk_interface": "virtio",
   "net_device": "virtio-net",
 
-  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1708.iso;,
-  "iso_checksum": "5848f2fd31c7acf3811ad88eaca6f4aa",
+  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1808.iso;,
 
 Review comment:
   the checksum is _correct_ and based on the official repo sha1 hash, but 
either the type needs to be changed from `md5` to `sha1` or manually calculate 
the checksum based on `md5`. We can follow the discussion in #2775 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] khos2ow commented on a change in pull request #2775: Fix typo in ISO url

2018-07-26 Thread GitBox
khos2ow commented on a change in pull request #2775: Fix typo in ISO url
URL: https://github.com/apache/cloudstack/pull/2775#discussion_r205505855
 
 

 ##
 File path: tools/appliance/builtin/template.json
 ##
 @@ -23,7 +23,7 @@
   "disk_interface": "virtio",
   "net_device": "virtio-net",
 
-  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1808.iso;,
+  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1804.iso;,
 
 Review comment:
   Actually I was just about to send the exact comment. The official repo has 
only `sha1` and `sha256` hashes (which is used in 
`systemvmtemplate/template.json`) but here the `iso_checksum_type` is set to 
`md5`. Either of those needs to be changed.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] khos2ow commented on a change in pull request #2775: Fix typo in ISO url

2018-07-26 Thread GitBox
khos2ow commented on a change in pull request #2775: Fix typo in ISO url
URL: https://github.com/apache/cloudstack/pull/2775#discussion_r205505855
 
 

 ##
 File path: tools/appliance/builtin/template.json
 ##
 @@ -23,7 +23,7 @@
   "disk_interface": "virtio",
   "net_device": "virtio-net",
 
-  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1808.iso;,
+  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1804.iso;,
 
 Review comment:
   Actually I was just about to send the exact comment. The official has only 
`sha1` and `sha256` hashes (which is used in `systemvmtemplate/template.json`) 
but here the `iso_checksum_type` is set to `md5`. Either of those needs to be 
changed.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on a change in pull request #2775: Fix typo in ISO url

2018-07-26 Thread GitBox
rafaelweingartner commented on a change in pull request #2775: Fix typo in ISO 
url
URL: https://github.com/apache/cloudstack/pull/2775#discussion_r205503599
 
 

 ##
 File path: tools/appliance/builtin/template.json
 ##
 @@ -23,7 +23,7 @@
   "disk_interface": "virtio",
   "net_device": "virtio-net",
 
-  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1808.iso;,
+  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1804.iso;,
 
 Review comment:
   Is this hash correct?
   
   I took the hash of that file and it is something else:
   ```
   $ wget 
http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1804.iso
   $ md5sum CentOS-7-x86_64-Minimal-1804.iso
   fabdc67ff3a1674a489953effa285dfd  CentOS-7-x86_64-Minimal-1804.iso
   ```


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on a change in pull request #2747: systemvm: Update ISO URLs to the latest

2018-07-26 Thread GitBox
DaanHoogland commented on a change in pull request #2747: systemvm: Update ISO 
URLs to the latest
URL: https://github.com/apache/cloudstack/pull/2747#discussion_r205500628
 
 

 ##
 File path: tools/appliance/builtin/template.json
 ##
 @@ -23,8 +23,8 @@
   "disk_interface": "virtio",
   "net_device": "virtio-net",
 
-  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1708.iso;,
-  "iso_checksum": "5848f2fd31c7acf3811ad88eaca6f4aa",
+  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1808.iso;,
 
 Review comment:
   just approved the new PR @khos2ow, assuming you did enter the right checksum


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] khos2ow commented on issue #2775: Fix typo in ISO url

2018-07-26 Thread GitBox
khos2ow commented on issue #2775: Fix typo in ISO url
URL: https://github.com/apache/cloudstack/pull/2775#issuecomment-408124262
 
 
   @rhtyd @rafaelweingartner fixed typo.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] khos2ow opened a new pull request #2775: Fix typo in ISO url

2018-07-26 Thread GitBox
khos2ow opened a new pull request #2775: Fix typo in ISO url
URL: https://github.com/apache/cloudstack/pull/2775
 
 
   ## Description
   
   The correct ISO url is 
http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1804.iso 
and not ends with `1808`.
   
   
   
   
   ## Types of changes
   
   - [ ] Breaking change (fix or feature that would cause existing 
functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [ ] 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)
   
   ## GitHub Issue/PRs
   
   
   
   
   
   
   ## Screenshots (if appropriate):
   
   ## How Has This Been Tested?
   
   
   
   
   
   ## Checklist:
   
   
   - [ ] I have read the 
[CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md)
 document.
   - [ ] My code follows the code style of this project.
   - [ ] My change requires a change to the documentation.
   - [ ] I have updated the documentation accordingly.
   Testing
   - [ ] I have added tests to cover my changes.
   - [ ] All relevant new and existing integration tests have passed.
   - [ ] A full integration testsuite with all test that can run on my 
environment has passed.
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] khos2ow commented on a change in pull request #2747: systemvm: Update ISO URLs to the latest

2018-07-26 Thread GitBox
khos2ow commented on a change in pull request #2747: systemvm: Update ISO URLs 
to the latest
URL: https://github.com/apache/cloudstack/pull/2747#discussion_r205486364
 
 

 ##
 File path: tools/appliance/builtin/template.json
 ##
 @@ -23,8 +23,8 @@
   "disk_interface": "virtio",
   "net_device": "virtio-net",
 
-  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1708.iso;,
-  "iso_checksum": "5848f2fd31c7acf3811ad88eaca6f4aa",
+  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1808.iso;,
 
 Review comment:
   oh yes! copy paste error!


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method

2018-07-26 Thread GitBox
rafaelweingartner commented on issue #2761: Add managed storage pool 
constraints to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-408113305
 
 
   @mike-tutkowski I just pushed a new commit with the changes you suggested.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on a change in pull request #2761: Add managed storage pool constraints to MigrateWithVolume API method

2018-07-26 Thread GitBox
rafaelweingartner commented on a change in pull request #2761: Add managed 
storage pool constraints to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#discussion_r205432339
 
 

 ##
 File path: 
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
 ##
 @@ -2292,73 +2292,143 @@ protected void migrate(final VMInstanceVO vm, final 
long srcHostId, final Deploy
 
 /**
  * We create the mapping of volumes and storage pool to migrate the VMs 
according to the information sent by the user.
+ * If the user did not enter a complete mapping, the volumes that were 
left behind will be auto mapped using {@link 
#createStoragePoolMappingsForVolumes(VirtualMachineProfile, Host, Map, List)}
  */
-private Map 
createMappingVolumeAndStoragePoolEnteredByUser(VirtualMachineProfile profile, 
Host host, Map volumeToPool) {
+protected Map 
createMappingVolumeAndStoragePoolEnteredByUser(VirtualMachineProfile profile, 
Host targetHost, Map volumeToPool) {
 Map volumeToPoolObjectMap = new HashMap();
+buildMapUsingUserInformation(profile, targetHost, volumeToPool, 
volumeToPoolObjectMap);
+
+List volumesNotMapped = 
findVolumesThatWereNotMappedByTheUser(profile, volumeToPoolObjectMap);
+createStoragePoolMappingsForVolumes(profile, targetHost, 
volumeToPoolObjectMap, volumesNotMapped);
+return volumeToPoolObjectMap;
+}
+
+/**
+ *  Given the map of volume to target storage pool entered by the user, we 
check for other volumes that the VM might have and were not configured.
+ *  This map can be then used by CloudStack to find new target storage 
pools according to the target host.
+ */
+private List 
findVolumesThatWereNotMappedByTheUser(VirtualMachineProfile profile, 
Map volumeToPoolObjectMap) {
+List allVolumes = 
_volsDao.findUsableVolumesForInstance(profile.getId());
+List volumesNotMapped = new ArrayList<>();
+for (Volume volume : volumeToPoolObjectMap.keySet()) {
+if (!allVolumes.contains(volume)) {
+volumesNotMapped.add(volume);
+}
+}
+return volumesNotMapped;
+}
+
+/**
+ *  Builds the map of storage pools and volumes with the information 
entered by the user. Before creating the an entry we validate if the migration 
is feasible checking if the migration is allowed and if the target host can 
access the defined target storage pool.
+ */
+private void buildMapUsingUserInformation(VirtualMachineProfile profile, 
Host targetHost, Map volumeToPool, Map 
volumeToPoolObjectMap) {
 for(Long volumeId: volumeToPool.keySet()) {
 VolumeVO volume = _volsDao.findById(volumeId);
 
 Long poolId = volumeToPool.get(volumeId);
 StoragePoolVO targetPool = _storagePoolDao.findById(poolId);
 StoragePoolVO currentPool = 
_storagePoolDao.findById(volume.getPoolId());
 
-if (_poolHostDao.findByPoolHost(targetPool.getId(), host.getId()) 
== null) {
-throw new CloudRuntimeException(String.format("Cannot migrate 
the volume [%s] to the storage pool [%s] while migrating VM [%s] to target host 
[%s]. The host does not have access to the storage pool entered.", 
volume.getUuid(), targetPool.getUuid(), profile.getUuid(), host.getUuid()));
+executeManagedStorageChecks(targetHost, currentPool, volume);
+if (_poolHostDao.findByPoolHost(targetPool.getId(), 
targetHost.getId()) == null) {
+throw new CloudRuntimeException(
+String.format("Cannot migrate the volume [%s] to the 
storage pool [%s] while migrating VM [%s] to target host [%s]. The host does 
not have access to the storage pool entered.",
+volume.getUuid(), targetPool.getUuid(), 
profile.getUuid(), targetHost.getUuid()));
 }
 if (currentPool.getId() == targetPool.getId()) {
 s_logger.info(String.format("The volume [%s] is already 
allocated in storage pool [%s].", volume.getUuid(), targetPool.getUuid()));
 }
 volumeToPoolObjectMap.put(volume, targetPool);
 }
-return volumeToPoolObjectMap;
 }
 
 /**
- * We create the default mapping of volumes and storage pools for the 
migration of the VM to the target host.
- * If the current storage pool of one of the volumes is using local 
storage in the host, it then needs to be migrated to a local storage in the 
target host.
- * Otherwise, we do not need to migrate, and the volume can be kept in its 
current storage pool.
+ * We create the default mapping of volumes and storage pools for the 
migration of the VM to the target host. We execute the following checks an 
operations according to some scenarios:
+ * 
+ *  If the current storage pool of one of the volumes is using local 
storage in the host, it then needs 

[GitHub] rafaelweingartner commented on a change in pull request #2761: Add managed storage pool constraints to MigrateWithVolume API method

2018-07-26 Thread GitBox
rafaelweingartner commented on a change in pull request #2761: Add managed 
storage pool constraints to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#discussion_r205469018
 
 

 ##
 File path: 
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
 ##
 @@ -2282,7 +2282,7 @@ protected void migrate(final VMInstanceVO vm, final long 
srcHostId, final Deploy
  * Create the mapping of volumes and storage pools. If the user did not 
enter a mapping on her/his own, we create one using {@link 
#getDefaultMappingOfVolumesAndStoragePoolForMigration(VirtualMachineProfile, 
Host)}.
  * If the user provided a mapping, we use whatever the user has provided 
(check the method {@link 
#createMappingVolumeAndStoragePoolEnteredByUser(VirtualMachineProfile, Host, 
Map)}).
  */
-private Map 
getPoolListForVolumesForMigration(VirtualMachineProfile profile, Host 
targetHost, Map volumeToPool) {
+protected Map 
getPoolListForVolumesForMigration(VirtualMachineProfile profile, Host 
targetHost, Map volumeToPool) {
 if (MapUtils.isEmpty(volumeToPool)) {
 
 Review comment:
   Now it will. I will execute the changes then.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on a change in pull request #2761: Add managed storage pool constraints to MigrateWithVolume API method

2018-07-26 Thread GitBox
rafaelweingartner commented on a change in pull request #2761: Add managed 
storage pool constraints to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#discussion_r205432501
 
 

 ##
 File path: 
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
 ##
 @@ -2367,7 +2437,7 @@ private void 
createVolumeToStoragePoolMappingIfNeeded(VirtualMachineProfile prof
 /**
  * We use {@link StoragePoolAllocator} objects to find local storage pools 
connected to the targetHost where we would be able to allocate the given volume.
  */
-private List 
getCandidateStoragePoolsToMigrateLocalVolume(VirtualMachineProfile profile, 
Host targetHost, VolumeVO volume) {
+private List 
getCandidateStoragePoolsToMigrateLocalVolume(VirtualMachineProfile profile, 
Host targetHost, Volume volume) {
 
 Review comment:
   You are right!


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on a change in pull request #2761: Add managed storage pool constraints to MigrateWithVolume API method

2018-07-26 Thread GitBox
rafaelweingartner commented on a change in pull request #2761: Add managed 
storage pool constraints to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#discussion_r205432339
 
 

 ##
 File path: 
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
 ##
 @@ -2292,73 +2292,143 @@ protected void migrate(final VMInstanceVO vm, final 
long srcHostId, final Deploy
 
 /**
  * We create the mapping of volumes and storage pool to migrate the VMs 
according to the information sent by the user.
+ * If the user did not enter a complete mapping, the volumes that were 
left behind will be auto mapped using {@link 
#createStoragePoolMappingsForVolumes(VirtualMachineProfile, Host, Map, List)}
  */
-private Map 
createMappingVolumeAndStoragePoolEnteredByUser(VirtualMachineProfile profile, 
Host host, Map volumeToPool) {
+protected Map 
createMappingVolumeAndStoragePoolEnteredByUser(VirtualMachineProfile profile, 
Host targetHost, Map volumeToPool) {
 Map volumeToPoolObjectMap = new HashMap();
+buildMapUsingUserInformation(profile, targetHost, volumeToPool, 
volumeToPoolObjectMap);
+
+List volumesNotMapped = 
findVolumesThatWereNotMappedByTheUser(profile, volumeToPoolObjectMap);
+createStoragePoolMappingsForVolumes(profile, targetHost, 
volumeToPoolObjectMap, volumesNotMapped);
+return volumeToPoolObjectMap;
+}
+
+/**
+ *  Given the map of volume to target storage pool entered by the user, we 
check for other volumes that the VM might have and were not configured.
+ *  This map can be then used by CloudStack to find new target storage 
pools according to the target host.
+ */
+private List 
findVolumesThatWereNotMappedByTheUser(VirtualMachineProfile profile, 
Map volumeToPoolObjectMap) {
+List allVolumes = 
_volsDao.findUsableVolumesForInstance(profile.getId());
+List volumesNotMapped = new ArrayList<>();
+for (Volume volume : volumeToPoolObjectMap.keySet()) {
+if (!allVolumes.contains(volume)) {
+volumesNotMapped.add(volume);
+}
+}
+return volumesNotMapped;
+}
+
+/**
+ *  Builds the map of storage pools and volumes with the information 
entered by the user. Before creating the an entry we validate if the migration 
is feasible checking if the migration is allowed and if the target host can 
access the defined target storage pool.
+ */
+private void buildMapUsingUserInformation(VirtualMachineProfile profile, 
Host targetHost, Map volumeToPool, Map 
volumeToPoolObjectMap) {
 for(Long volumeId: volumeToPool.keySet()) {
 VolumeVO volume = _volsDao.findById(volumeId);
 
 Long poolId = volumeToPool.get(volumeId);
 StoragePoolVO targetPool = _storagePoolDao.findById(poolId);
 StoragePoolVO currentPool = 
_storagePoolDao.findById(volume.getPoolId());
 
-if (_poolHostDao.findByPoolHost(targetPool.getId(), host.getId()) 
== null) {
-throw new CloudRuntimeException(String.format("Cannot migrate 
the volume [%s] to the storage pool [%s] while migrating VM [%s] to target host 
[%s]. The host does not have access to the storage pool entered.", 
volume.getUuid(), targetPool.getUuid(), profile.getUuid(), host.getUuid()));
+executeManagedStorageChecks(targetHost, currentPool, volume);
+if (_poolHostDao.findByPoolHost(targetPool.getId(), 
targetHost.getId()) == null) {
+throw new CloudRuntimeException(
+String.format("Cannot migrate the volume [%s] to the 
storage pool [%s] while migrating VM [%s] to target host [%s]. The host does 
not have access to the storage pool entered.",
+volume.getUuid(), targetPool.getUuid(), 
profile.getUuid(), targetHost.getUuid()));
 }
 if (currentPool.getId() == targetPool.getId()) {
 s_logger.info(String.format("The volume [%s] is already 
allocated in storage pool [%s].", volume.getUuid(), targetPool.getUuid()));
 }
 volumeToPoolObjectMap.put(volume, targetPool);
 }
-return volumeToPoolObjectMap;
 }
 
 /**
- * We create the default mapping of volumes and storage pools for the 
migration of the VM to the target host.
- * If the current storage pool of one of the volumes is using local 
storage in the host, it then needs to be migrated to a local storage in the 
target host.
- * Otherwise, we do not need to migrate, and the volume can be kept in its 
current storage pool.
+ * We create the default mapping of volumes and storage pools for the 
migration of the VM to the target host. We execute the following checks an 
operations according to some scenarios:
+ * 
+ *  If the current storage pool of one of the volumes is using local 
storage in the host, it then needs 

[GitHub] rafaelweingartner commented on a change in pull request #2761: Add managed storage pool constraints to MigrateWithVolume API method

2018-07-26 Thread GitBox
rafaelweingartner commented on a change in pull request #2761: Add managed 
storage pool constraints to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#discussion_r205419513
 
 

 ##
 File path: 
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
 ##
 @@ -2302,8 +2302,11 @@ protected void migrate(final VMInstanceVO vm, final 
long srcHostId, final Deploy
 StoragePoolVO targetPool = _storagePoolDao.findById(poolId);
 StoragePoolVO currentPool = 
_storagePoolDao.findById(volume.getPoolId());
 
-if (_poolHostDao.findByPoolHost(targetPool.getId(), host.getId()) 
== null) {
-throw new CloudRuntimeException(String.format("Cannot migrate 
the volume [%s] to the storage pool [%s] while migrating VM [%s] to target host 
[%s]. The host does not have access to the storage pool entered.", 
volume.getUuid(), targetPool.getUuid(), profile.getUuid(), host.getUuid()));
+executeManagedStorageChecks(targetHost, currentPool, volume);
+if (_poolHostDao.findByPoolHost(targetPool.getId(), 
targetHost.getId()) == null) {
+throw new CloudRuntimeException(
+String.format("Cannot migrate the volume [%s] to the 
storage pool [%s] while migrating VM [%s] to target host [%s]. The host does 
not have access to the storage pool entered.",
+volume.getUuid(), targetPool.getUuid(), 
profile.getUuid(), targetHost.getUuid()));
 }
 if (currentPool.getId() == targetPool.getId()) {
 
 Review comment:
   Yes. It was not working with XenServer before because it requires all disks 
to be mapped. Even the ones that will not be migrated (e.g. disks in shared 
storage). 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on a change in pull request #2773: CLOUDSTACK-10328: Add Secondary IPv6 address through API

2018-07-26 Thread GitBox
rafaelweingartner commented on a change in pull request #2773: 
CLOUDSTACK-10328: Add Secondary IPv6 address through API
URL: https://github.com/apache/cloudstack/pull/2773#discussion_r205415478
 
 

 ##
 File path: api/src/main/java/com/cloud/network/NetworkService.java
 ##
 @@ -189,12 +181,16 @@ Network createPrivateNetwork(String networkName, String 
displayText, long physic
 String netmask, long networkOwnerId, Long vpcId, Boolean sourceNat, 
Long networkOfferingId) throws ResourceAllocationException, 
ConcurrentOperationException,
 InsufficientCapacityException;
 
-/* Requests an IP address for the guest nic */
-NicSecondaryIp allocateSecondaryGuestIP(long nicId, String ipaddress) 
throws InsufficientAddressCapacityException;
+/**
+ * Requests an IP address for the guest nic
+ */
+NicSecondaryIp allocateSecondaryGuestIP(long nicId, IpAddresses 
requestedIpPair) throws InsufficientAddressCapacityException;
 
 boolean releaseSecondaryIpFromNic(long ipAddressId);
 
-/* lists the nic informaton */
+/**
+ * lists the nic informaton
 
 Review comment:
   NIC instead of nic


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on a change in pull request #2773: CLOUDSTACK-10328: Add Secondary IPv6 address through API

2018-07-26 Thread GitBox
rafaelweingartner commented on a change in pull request #2773: 
CLOUDSTACK-10328: Add Secondary IPv6 address through API
URL: https://github.com/apache/cloudstack/pull/2773#discussion_r205417670
 
 

 ##
 File path: 
api/src/main/java/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java
 ##
 @@ -169,17 +161,25 @@ public long getEntityOwnerId() {
 
 @Override
 public void create() throws ResourceAllocationException {
-String ip;
 NicSecondaryIp result;
-String secondaryIp = null;
-if ((ip = getIpaddress()) != null) {
-if (!NetUtils.isValidIp4(ip)) {
-throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, 
"Invalid ip address " + ip);
+boolean isIpv4 = true;
+
+if (ipAddr != null) {
+if (!NetUtils.isValidIp4(ipAddr)) {
+isIpv4 = false;
+}
+if (!NetUtils.isValidIp6(ipAddr) && !isIpv4) {
+throw new InvalidParameterValueException("Invalid ip address " 
+ ipAddr);
 }
 }
 
+IpAddresses requestedIpPair = new IpAddresses(ipAddr, null);
 
 Review comment:
   Can I request to allocate an IPV4 and IPV6 at the same time?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on a change in pull request #2773: CLOUDSTACK-10328: Add Secondary IPv6 address through API

2018-07-26 Thread GitBox
rafaelweingartner commented on a change in pull request #2773: 
CLOUDSTACK-10328: Add Secondary IPv6 address through API
URL: https://github.com/apache/cloudstack/pull/2773#discussion_r205415983
 
 

 ##
 File path: 
api/src/main/java/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java
 ##
 @@ -169,17 +161,25 @@ public long getEntityOwnerId() {
 
 @Override
 public void create() throws ResourceAllocationException {
-String ip;
 NicSecondaryIp result;
-String secondaryIp = null;
-if ((ip = getIpaddress()) != null) {
-if (!NetUtils.isValidIp4(ip)) {
-throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, 
"Invalid ip address " + ip);
+boolean isIpv4 = true;
 
 Review comment:
   What about moving this validation to a "service"  layer? This is the 
"view/controller" part of the design. It should only receive the request, get 
the data and dispatch to the service layer where the business gets done.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on a change in pull request #2773: CLOUDSTACK-10328: Add Secondary IPv6 address through API

2018-07-26 Thread GitBox
rafaelweingartner commented on a change in pull request #2773: 
CLOUDSTACK-10328: Add Secondary IPv6 address through API
URL: https://github.com/apache/cloudstack/pull/2773#discussion_r205415515
 
 

 ##
 File path: api/src/main/java/com/cloud/network/NetworkService.java
 ##
 @@ -189,12 +181,16 @@ Network createPrivateNetwork(String networkName, String 
displayText, long physic
 String netmask, long networkOwnerId, Long vpcId, Boolean sourceNat, 
Long networkOfferingId) throws ResourceAllocationException, 
ConcurrentOperationException,
 InsufficientCapacityException;
 
-/* Requests an IP address for the guest nic */
-NicSecondaryIp allocateSecondaryGuestIP(long nicId, String ipaddress) 
throws InsufficientAddressCapacityException;
+/**
+ * Requests an IP address for the guest nic
 
 Review comment:
   NIC


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on a change in pull request #2773: CLOUDSTACK-10328: Add Secondary IPv6 address through API

2018-07-26 Thread GitBox
rafaelweingartner commented on a change in pull request #2773: 
CLOUDSTACK-10328: Add Secondary IPv6 address through API
URL: https://github.com/apache/cloudstack/pull/2773#discussion_r205416708
 
 

 ##
 File path: server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
 ##
 @@ -300,6 +300,8 @@
 Boolean.class, "system.vm.public.ip.reservation.mode.strictness", 
"false",
 "If enabled, the use of System VMs public IP reservation is 
strict, preferred if not.", false, ConfigKey.Scope.Global);
 
+Random rand = new Random(System.currentTimeMillis());
 
 Review comment:
   private?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on a change in pull request #2773: CLOUDSTACK-10328: Add Secondary IPv6 address through API

2018-07-26 Thread GitBox
rafaelweingartner commented on a change in pull request #2773: 
CLOUDSTACK-10328: Add Secondary IPv6 address through API
URL: https://github.com/apache/cloudstack/pull/2773#discussion_r205416258
 
 

 ##
 File path: 
api/src/main/java/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java
 ##
 @@ -147,10 +147,15 @@ public void execute() throws 
InvalidParameterValueException {
 throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Invalid 
IP id is passed");
 }
 
+String secIp = nicSecIp.getIp4Address();
+if (secIp == null) {
+secIp = nicSecIp.getIp6Address();
+}
+
 if (isZoneSGEnabled()) {
 //remove the security group rules for this secondary ip
 boolean success = false;
-success = 
_securityGroupService.securityGroupRulesForVmSecIp(nicSecIp.getNicId(), 
nicSecIp.getIp4Address(), false);
+success = 
_securityGroupService.securityGroupRulesForVmSecIp(nicSecIp.getNicId(), secIp, 
false);
 
 Review comment:
   Instead of sending the parameters what about using the command POJO?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on a change in pull request #2773: CLOUDSTACK-10328: Add Secondary IPv6 address through API

2018-07-26 Thread GitBox
rafaelweingartner commented on a change in pull request #2773: 
CLOUDSTACK-10328: Add Secondary IPv6 address through API
URL: https://github.com/apache/cloudstack/pull/2773#discussion_r205416950
 
 

 ##
 File path: server/src/main/java/com/cloud/network/Ipv6AddressManagerImpl.java
 ##
 @@ -150,4 +164,74 @@ public void revokeDirectIpv6Address(long networkId, 
String ip6Address) {
 _ipv6Dao.remove(ip.getId());
 }
 }
+
+@Override
+public String allocateGuestIpv6(Network network, String requestedIpv6) 
throws InsufficientAddressCapacityException {
+return acquireGuestIpv6Address(network, requestedIpv6);
+}
+
+@Override
+@DB
+public String acquireGuestIpv6Address(Network network, String 
requestedIpv6) throws InsufficientAddressCapacityException {
+if (!_networkModel.isIP6AddressAvailableInNetwork(network.getId())) {
 
 Review comment:
   Do you validate somewhere else if the IP is valid?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on issue #2773: CLOUDSTACK-10328: Add Secondary IPv6 address through API

2018-07-26 Thread GitBox
rafaelweingartner commented on issue #2773: CLOUDSTACK-10328: Add Secondary 
IPv6 address through API
URL: https://github.com/apache/cloudstack/pull/2773#issuecomment-408060351
 
 
   @GabrielBrascher:
   ```
   Unapproved licenses:
 server/src/test/java/com/cloud/network/Ipv6AddressManagerTest.java
   ``` 
   
   Would you mind fixing this?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2660: Macaddresses

2018-07-26 Thread GitBox
DaanHoogland commented on issue #2660: Macaddresses
URL: https://github.com/apache/cloudstack/pull/2660#issuecomment-408058575
 
 
   surely 'squash and merge' @wido 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2593: api: move default response from XML to JSON

2018-07-26 Thread GitBox
DaanHoogland commented on issue #2593: api: move default response from XML to 
JSON
URL: https://github.com/apache/cloudstack/pull/2593#issuecomment-408057489
 
 
   let's do the 5.0 dance


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] blueorangutan commented on issue #2755: Fix migrate vol xen vmware test

2018-07-26 Thread GitBox
blueorangutan commented on issue #2755: Fix migrate vol xen vmware test
URL: https://github.com/apache/cloudstack/pull/2755#issuecomment-408043464
 
 
   @borisstoyanov 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 GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] borisstoyanov commented on issue #2755: Fix migrate vol xen vmware test

2018-07-26 Thread GitBox
borisstoyanov commented on issue #2755: Fix migrate vol xen vmware test
URL: https://github.com/apache/cloudstack/pull/2755#issuecomment-408043295
 
 
   @blueorangutan test


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] wido commented on issue #2593: api: move default response from XML to JSON

2018-07-26 Thread GitBox
wido commented on issue #2593: api: move default response from XML to JSON
URL: https://github.com/apache/cloudstack/pull/2593#issuecomment-408043084
 
 
   I agree with @rafaelweingartner 
   
   This would be a rather big change and I think we should discuss it properly. 
I understand the change, but we will break a lot of things.
   
   Do I hear somebody saying CloudStack 5.0?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] blueorangutan commented on issue #2755: Fix migrate vol xen vmware test

2018-07-26 Thread GitBox
blueorangutan commented on issue #2755: Fix migrate vol xen vmware test
URL: https://github.com/apache/cloudstack/pull/2755#issuecomment-408041738
 
 
   Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2211


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] marcaurele commented on issue #2578: api: add command to list management servers

2018-07-26 Thread GitBox
marcaurele commented on issue #2578: api: add command to list management servers
URL: https://github.com/apache/cloudstack/pull/2578#issuecomment-408041817
 
 
   @rhtyd it was about the sprites, but I found the one I was looking for 
`infrastructure-icons.png` which does not have anything for a management 
server... Due you know what is the source of those images? I might simply use 
the same as the systemVM for the management.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] blueorangutan commented on issue #2755: Fix migrate vol xen vmware test

2018-07-26 Thread GitBox
blueorangutan commented on issue #2755: Fix migrate vol xen vmware test
URL: https://github.com/apache/cloudstack/pull/2755#issuecomment-408033055
 
 
   @borisstoyanov a Jenkins job has been kicked to build packages. I'll keep 
you posted as I make progress.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] borisstoyanov commented on issue #2755: Fix migrate vol xen vmware test

2018-07-26 Thread GitBox
borisstoyanov commented on issue #2755: Fix migrate vol xen vmware test
URL: https://github.com/apache/cloudstack/pull/2755#issuecomment-408032999
 
 
   @blueorangutan package


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland opened a new issue #2774: NPE in isOnVolumeManagedStorage() leads to not being able to extract volumes

2018-07-26 Thread GitBox
DaanHoogland opened a new issue #2774: NPE in isOnVolumeManagedStorage() leads 
to not being able to extract volumes
URL: https://github.com/apache/cloudstack/issues/2774
 
 
   
   
   # ISSUE TYPE
   
* Bug Report
   
   # COMPONENT NAME
   
   ~~~
   Storage Motion
   ~~~
   
   # CLOUDSTACK VERSION
   
   
   ~~~
   4.11.1
   ~~~
   
   # CONFIGURATION
   
   In a mixed environment with several advanced zones, extracting a volume 
failed due to the storage motion strategy 
org.apache.cloudstack.storage.motion.StorageSystemDataMotionStrategy.canHandle(src,dest)
 throwing a NullPointerException. root cause is the isVolumeOnManagedStorage() 
assuming that the dataStore for the destination is Primary while in the case of 
extractVolume it is an image store. In lab and test environments this seems to 
happen to go well as both types of storages probably exist with id==1.
   
   # OS / ENVIRONMENT
   
   
   
   # SUMMARY
   
   
   
   # STEPS TO REPRODUCE
   
   
   
   ~~~
   extract a volume. In the case of a lab environment make sure that no primary 
storage and image stores exist with the same id. 
   ~~~
   
   
   
   # EXPECTED RESULTS
   
   
   ~~~
   user level: the call returns a url for download
   unit level: isVolumeOnMangedStorage should return false or be able to handle 
the image store case (and still return false?)
   ~~~
   
   # ACTUAL RESULTS
   
   
   
   ~~~
   user level: a message 'Failed to copy the volume from the source primary 
storage pool to secondary storage.' is shown in the UI
   unit level: npe
   ~~~
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] blueorangutan commented on issue #2755: Fix migrate vol xen vmware test

2018-07-26 Thread GitBox
blueorangutan commented on issue #2755: Fix migrate vol xen vmware test
URL: https://github.com/apache/cloudstack/pull/2755#issuecomment-408019790
 
 
   Packaging result: ✔centos6 ✖centos7 ✔debian. JID-2210


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] wido commented on issue #2766: kvm: Agent should not check if remaining memory on host is sufficient

2018-07-26 Thread GitBox
wido commented on issue #2766: kvm: Agent should not check if remaining memory 
on host is sufficient
URL: https://github.com/apache/cloudstack/pull/2766#issuecomment-408017277
 
 
   @rhtyd Thanks! As I stated, the Agent shouldn't worry about Memory 
Allocation is that's up to the Management Server and the deployment planners 
there.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] wido commented on issue #2660: Macaddresses

2018-07-26 Thread GitBox
wido commented on issue #2660: Macaddresses
URL: https://github.com/apache/cloudstack/pull/2660#issuecomment-408015501
 
 
   A lot of commits for a fairly small change? Maybe squash and then merge? It 
seems like a code cleanup.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] wido commented on issue #2769: Fix config drive iso path on Vmware

2018-07-26 Thread GitBox
wido commented on issue #2769: Fix config drive iso path on Vmware
URL: https://github.com/apache/cloudstack/pull/2769#issuecomment-408014920
 
 
   In which part of the code is the lowercase 'configdrive' set?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] blueorangutan commented on issue #2766: kvm: Agent should not check if remaining memory on host is sufficient

2018-07-26 Thread GitBox
blueorangutan commented on issue #2766: kvm: Agent should not check if 
remaining memory on host is sufficient
URL: https://github.com/apache/cloudstack/pull/2766#issuecomment-408014240
 
 
   @rhtyd 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 GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] blueorangutan commented on issue #2769: Fix config drive iso path on Vmware

2018-07-26 Thread GitBox
blueorangutan commented on issue #2769: Fix config drive iso path on Vmware
URL: https://github.com/apache/cloudstack/pull/2769#issuecomment-408014232
 
 
   @rhtyd a Trillian-Jenkins test job (centos7 mgmt + vmware-65) 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 GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rhtyd commented on issue #2769: Fix config drive iso path on Vmware

2018-07-26 Thread GitBox
rhtyd commented on issue #2769: Fix config drive iso path on Vmware
URL: https://github.com/apache/cloudstack/pull/2769#issuecomment-408014198
 
 
   @blueorangutan test centos7 vmware-65


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rhtyd commented on issue #2766: kvm: Agent should not check if remaining memory on host is sufficient

2018-07-26 Thread GitBox
rhtyd commented on issue #2766: kvm: Agent should not check if remaining memory 
on host is sufficient
URL: https://github.com/apache/cloudstack/pull/2766#issuecomment-408014090
 
 
   @blueorangutan test 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] blueorangutan commented on issue #2766: kvm: Agent should not check if remaining memory on host is sufficient

2018-07-26 Thread GitBox
blueorangutan commented on issue #2766: kvm: Agent should not check if 
remaining memory on host is sufficient
URL: https://github.com/apache/cloudstack/pull/2766#issuecomment-408013985
 
 
   Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2209


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] blueorangutan commented on issue #2769: Fix config drive iso path on Vmware

2018-07-26 Thread GitBox
blueorangutan commented on issue #2769: Fix config drive iso path on Vmware
URL: https://github.com/apache/cloudstack/pull/2769#issuecomment-408013982
 
 
   Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2208


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] blueorangutan commented on issue #2755: Fix migrate vol xen vmware test

2018-07-26 Thread GitBox
blueorangutan commented on issue #2755: Fix migrate vol xen vmware test
URL: https://github.com/apache/cloudstack/pull/2755#issuecomment-408010366
 
 
   @borisstoyanov a Jenkins job has been kicked to build packages. I'll keep 
you posted as I make progress.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] borisstoyanov commented on issue #2755: Fix migrate vol xen vmware test

2018-07-26 Thread GitBox
borisstoyanov commented on issue #2755: Fix migrate vol xen vmware test
URL: https://github.com/apache/cloudstack/pull/2755#issuecomment-408010160
 
 
   @blueorangutan package


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] borisstoyanov commented on issue #2755: Fix migrate vol xen vmware test

2018-07-26 Thread GitBox
borisstoyanov commented on issue #2755: Fix migrate vol xen vmware test
URL: https://github.com/apache/cloudstack/pull/2755#issuecomment-408009476
 
 
   @rhtyd let me rebase and run tests again to see that we've actually fixed 
the issue.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rhtyd commented on issue #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM and XENSERVER

2018-07-26 Thread GitBox
rhtyd commented on issue #2204: [CLOUDSTACK-10025] Adding Support for NoVNC 
Console for KVM and XENSERVER
URL: https://github.com/apache/cloudstack/pull/2204#issuecomment-408006346
 
 
   Looking forward to it @syed !


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[cloudstack] 01/01: Merge branch '4.11'

2018-07-26 Thread rohit
This is an automated email from the ASF dual-hosted git repository.

rohit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git

commit c8a980b9cfd1361633d1e48a087c23cca3b8a261
Merge: d64f787 52fb426
Author: Rohit Yadav 
AuthorDate: Thu Jul 26 13:08:41 2018 +0530

Merge branch '4.11'

 tools/appliance/builtin/template.json  | 4 ++--
 tools/appliance/systemvmtemplate/template.json | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)



[cloudstack] branch master updated (d64f787 -> c8a980b)

2018-07-26 Thread rohit
This is an automated email from the ASF dual-hosted git repository.

rohit pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git.


from d64f787  Merge branch '4.11'
 add 52fb426  systemvm: Update ISO URLs to the latest (#2747)
 new c8a980b  Merge branch '4.11'

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 tools/appliance/builtin/template.json  | 4 ++--
 tools/appliance/systemvmtemplate/template.json | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)



[GitHub] rhtyd commented on a change in pull request #2747: systemvm: Update ISO URLs to the latest

2018-07-26 Thread GitBox
rhtyd commented on a change in pull request #2747: systemvm: Update ISO URLs to 
the latest
URL: https://github.com/apache/cloudstack/pull/2747#discussion_r205357787
 
 

 ##
 File path: tools/appliance/builtin/template.json
 ##
 @@ -23,8 +23,8 @@
   "disk_interface": "virtio",
   "net_device": "virtio-net",
 
-  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1708.iso;,
-  "iso_checksum": "5848f2fd31c7acf3811ad88eaca6f4aa",
+  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1808.iso;,
 
 Review comment:
   I did not check this, looks like I made a mistake in merging this @khos2ow 
can you send a new PR to fix the iso_url, it ends with `1804` and not 1808.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rhtyd closed pull request #2747: systemvm: Update ISO URLs to the latest

2018-07-26 Thread GitBox
rhtyd closed pull request #2747: systemvm: Update ISO URLs to the latest
URL: https://github.com/apache/cloudstack/pull/2747
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/tools/appliance/builtin/template.json 
b/tools/appliance/builtin/template.json
index c9881c15e5e..2175a70eb6e 100644
--- a/tools/appliance/builtin/template.json
+++ b/tools/appliance/builtin/template.json
@@ -23,8 +23,8 @@
   "disk_interface": "virtio",
   "net_device": "virtio-net",
 
-  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1708.iso;,
-  "iso_checksum": "5848f2fd31c7acf3811ad88eaca6f4aa",
+  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1808.iso;,
+  "iso_checksum": "13675c6f74880e7ff3481b91bdaf925ce81bda8f",
   "iso_checksum_type": "md5",
 
   "vm_name": "builtin",
diff --git a/tools/appliance/systemvmtemplate/template.json 
b/tools/appliance/systemvmtemplate/template.json
index 8fe32309d37..80ceae18441 100644
--- a/tools/appliance/systemvmtemplate/template.json
+++ b/tools/appliance/systemvmtemplate/template.json
@@ -38,8 +38,8 @@
   "disk_interface": "virtio",
   "net_device": "virtio-net",
 
-  "iso_url": 
"https://cdimage.debian.org/debian-cd/current/amd64/iso-cd/debian-9.4.0-amd64-netinst.iso;,
-  "iso_checksum": 
"345c4e674dc10476e8c4f1571fbcdba4ce9788aa5584c5e2590ab3e89e7bb9acb370536f41a3ac740eb92b6aebe3cb8eb9734874dd1658c68875981b8351bc38",
+  "iso_url": 
"https://cdimage.debian.org/debian-cd/current/amd64/iso-cd/debian-9.5.0-amd64-netinst.iso;,
+  "iso_checksum": 
"efe75000c066506326c74a97257163b3050d656a5be8708a6826b0f810208d0a58f413c446de09919c580de8fac6d0a47774534725dd9fdd00c94859e370f373",
   "iso_checksum_type": "sha512",
 
   "vm_name": "systemvmtemplate",


 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[cloudstack] branch 4.11 updated: systemvm: Update ISO URLs to the latest (#2747)

2018-07-26 Thread rohit
This is an automated email from the ASF dual-hosted git repository.

rohit pushed a commit to branch 4.11
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/4.11 by this push:
 new 52fb426  systemvm: Update ISO URLs to the latest (#2747)
52fb426 is described below

commit 52fb426231ad8c4217dd18bc74a4e2e71d51c529
Author: Khosrow Moossavi 
AuthorDate: Thu Jul 26 03:36:08 2018 -0400

systemvm: Update ISO URLs to the latest (#2747)

Update ISO URLs to the latest as the old URLs are broken. Base template 
updated for Debian 9.5.0.
---
 tools/appliance/builtin/template.json  | 4 ++--
 tools/appliance/systemvmtemplate/template.json | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/appliance/builtin/template.json 
b/tools/appliance/builtin/template.json
index c9881c1..2175a70 100644
--- a/tools/appliance/builtin/template.json
+++ b/tools/appliance/builtin/template.json
@@ -23,8 +23,8 @@
   "disk_interface": "virtio",
   "net_device": "virtio-net",
 
-  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1708.iso;,
-  "iso_checksum": "5848f2fd31c7acf3811ad88eaca6f4aa",
+  "iso_url": 
"http://mirror.nbrc.ac.in/centos/7/isos/x86_64/CentOS-7-x86_64-Minimal-1808.iso;,
+  "iso_checksum": "13675c6f74880e7ff3481b91bdaf925ce81bda8f",
   "iso_checksum_type": "md5",
 
   "vm_name": "builtin",
diff --git a/tools/appliance/systemvmtemplate/template.json 
b/tools/appliance/systemvmtemplate/template.json
index 8fe3230..80ceae1 100644
--- a/tools/appliance/systemvmtemplate/template.json
+++ b/tools/appliance/systemvmtemplate/template.json
@@ -38,8 +38,8 @@
   "disk_interface": "virtio",
   "net_device": "virtio-net",
 
-  "iso_url": 
"https://cdimage.debian.org/debian-cd/current/amd64/iso-cd/debian-9.4.0-amd64-netinst.iso;,
-  "iso_checksum": 
"345c4e674dc10476e8c4f1571fbcdba4ce9788aa5584c5e2590ab3e89e7bb9acb370536f41a3ac740eb92b6aebe3cb8eb9734874dd1658c68875981b8351bc38",
+  "iso_url": 
"https://cdimage.debian.org/debian-cd/current/amd64/iso-cd/debian-9.5.0-amd64-netinst.iso;,
+  "iso_checksum": 
"efe75000c066506326c74a97257163b3050d656a5be8708a6826b0f810208d0a58f413c446de09919c580de8fac6d0a47774534725dd9fdd00c94859e370f373",
   "iso_checksum_type": "sha512",
 
   "vm_name": "systemvmtemplate",



[GitHub] rhtyd commented on issue #2747: systemvm: Update ISO URLs to the latest

2018-07-26 Thread GitBox
rhtyd commented on issue #2747: systemvm: Update ISO URLs to the latest
URL: https://github.com/apache/cloudstack/pull/2747#issuecomment-408005444
 
 
   Changes are only in systemvmtemplate appliance build code. I'll merge this 
based on reviews.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] asfgit closed issue #2733: invalid consoleproxy domain after upgrade from 4.5 to 4.11.1

2018-07-26 Thread GitBox
asfgit closed issue #2733: invalid consoleproxy domain after upgrade from 4.5 
to 4.11.1
URL: https://github.com/apache/cloudstack/issues/2733
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[cloudstack] 01/01: Merge branch '4.11'

2018-07-26 Thread rohit
This is an automated email from the ASF dual-hosted git repository.

rohit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git

commit d64f787b6674cf72aa39a75a1ec7c49177f29909
Merge: 1960124 f88d606
Author: Rohit Yadav 
AuthorDate: Thu Jul 26 13:03:46 2018 +0530

Merge branch '4.11'

 .../main/java/com/cloud/info/ConsoleProxyInfo.java | 48 +++---
 ui/scripts/ui-custom/zoneWizard.js |  4 +-
 2 files changed, 28 insertions(+), 24 deletions(-)




[cloudstack] branch master updated (1960124 -> d64f787)

2018-07-26 Thread rohit
This is an automated email from the ASF dual-hosted git repository.

rohit pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git.


from 1960124  server: Make HTTP handler of embedded server to be configured 
(#2762)
 add 4c66908  ui: Fix clicking the previous button to go back to the 
Physical Network screen in the Add Zone Wizard (#2767)
 add f88d606  consoleproxy: Fixes #2733 invalid consoleproxy url after 
upgrade (#2734)
 new d64f787  Merge branch '4.11'

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../main/java/com/cloud/info/ConsoleProxyInfo.java | 48 +++---
 ui/scripts/ui-custom/zoneWizard.js |  4 +-
 2 files changed, 28 insertions(+), 24 deletions(-)



[GitHub] rhtyd closed pull request #2734: Fix invalid consoleproxy url after upgrade

2018-07-26 Thread GitBox
rhtyd closed pull request #2734: Fix invalid consoleproxy url after upgrade
URL: https://github.com/apache/cloudstack/pull/2734
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/core/src/com/cloud/info/ConsoleProxyInfo.java 
b/core/src/com/cloud/info/ConsoleProxyInfo.java
index 16993d71bdc..78cb877e7db 100644
--- a/core/src/com/cloud/info/ConsoleProxyInfo.java
+++ b/core/src/com/cloud/info/ConsoleProxyInfo.java
@@ -35,38 +35,40 @@ public ConsoleProxyInfo(int proxyUrlPort) {
 
 public ConsoleProxyInfo(boolean sslEnabled, String proxyIpAddress, int 
port, int proxyUrlPort, String consoleProxyUrlDomain) {
 this.sslEnabled = sslEnabled;
+this.proxyPort = port;
+this.proxyUrlPort = proxyUrlPort;
+this.proxyAddress = this.formatProxyAddress(consoleProxyUrlDomain, 
proxyIpAddress);
 
 if (sslEnabled) {
-StringBuffer sb = new StringBuffer();
-if (consoleProxyUrlDomain.startsWith("*")) {
-sb.append(proxyIpAddress);
-for (int i = 0; i < proxyIpAddress.length(); i++)
-if (sb.charAt(i) == '.')
-sb.setCharAt(i, '-');
-sb.append(consoleProxyUrlDomain.substring(1));//skip the *
-} else {
-//LB address
-sb.append(consoleProxyUrlDomain);
-}
-proxyAddress = sb.toString();
-proxyPort = port;
-this.proxyUrlPort = proxyUrlPort;
-
 proxyImageUrl = "https://; + proxyAddress;
-if (proxyUrlPort != 443)
+if (proxyUrlPort != 443) {
 proxyImageUrl += ":" + this.proxyUrlPort;
-} else {
-proxyAddress = proxyIpAddress;
-if (StringUtils.isNotBlank(consoleProxyUrlDomain)) {
-proxyAddress = consoleProxyUrlDomain;
 }
-proxyPort = port;
-this.proxyUrlPort = proxyUrlPort;
 
+} else {
 proxyImageUrl = "http://; + proxyAddress;
-if (proxyUrlPort != 80)
+if (proxyUrlPort != 80) {
 proxyImageUrl += ":" + proxyUrlPort;
+}
+}
+}
+
+private String formatProxyAddress(String consoleProxyUrlDomain, String 
proxyIpAddress) {
+StringBuffer sb = new StringBuffer();
+// Domain in format *.example.com, proxy IP is 1.2.3.4 --> 
1-2-3-4.example.com
+if (consoleProxyUrlDomain.startsWith("*")) {
+sb.append(proxyIpAddress.replaceAll("\\.", "-"));
+sb.append(consoleProxyUrlDomain.substring(1)); // skip the *
+
+// Otherwise we assume a valid domain if config not blank
+} else if (StringUtils.isNotBlank(consoleProxyUrlDomain)) {
+sb.append(consoleProxyUrlDomain);
+
+// Blank config, we use the proxy IP
+} else {
+sb.append(proxyIpAddress);
 }
+return sb.toString();
 }
 
 public String getProxyAddress() {


 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[cloudstack] branch 4.11 updated: consoleproxy: Fixes #2733 invalid consoleproxy url after upgrade (#2734)

2018-07-26 Thread rohit
This is an automated email from the ASF dual-hosted git repository.

rohit pushed a commit to branch 4.11
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/4.11 by this push:
 new f88d606  consoleproxy: Fixes #2733 invalid consoleproxy url after 
upgrade (#2734)
f88d606 is described below

commit f88d606acbc566e2f777a18d1b1f90db428162b4
Author: René Moser 
AuthorDate: Thu Jul 26 09:31:23 2018 +0200

consoleproxy: Fixes #2733 invalid consoleproxy url after upgrade (#2734)

Ensures we have a valid console proxy domain for protocol http even a * 
setting is used. See #2733
---
 core/src/com/cloud/info/ConsoleProxyInfo.java | 48 ++-
 1 file changed, 25 insertions(+), 23 deletions(-)

diff --git a/core/src/com/cloud/info/ConsoleProxyInfo.java 
b/core/src/com/cloud/info/ConsoleProxyInfo.java
index 16993d7..78cb877 100644
--- a/core/src/com/cloud/info/ConsoleProxyInfo.java
+++ b/core/src/com/cloud/info/ConsoleProxyInfo.java
@@ -35,38 +35,40 @@ public class ConsoleProxyInfo {
 
 public ConsoleProxyInfo(boolean sslEnabled, String proxyIpAddress, int 
port, int proxyUrlPort, String consoleProxyUrlDomain) {
 this.sslEnabled = sslEnabled;
+this.proxyPort = port;
+this.proxyUrlPort = proxyUrlPort;
+this.proxyAddress = this.formatProxyAddress(consoleProxyUrlDomain, 
proxyIpAddress);
 
 if (sslEnabled) {
-StringBuffer sb = new StringBuffer();
-if (consoleProxyUrlDomain.startsWith("*")) {
-sb.append(proxyIpAddress);
-for (int i = 0; i < proxyIpAddress.length(); i++)
-if (sb.charAt(i) == '.')
-sb.setCharAt(i, '-');
-sb.append(consoleProxyUrlDomain.substring(1));//skip the *
-} else {
-//LB address
-sb.append(consoleProxyUrlDomain);
-}
-proxyAddress = sb.toString();
-proxyPort = port;
-this.proxyUrlPort = proxyUrlPort;
-
 proxyImageUrl = "https://; + proxyAddress;
-if (proxyUrlPort != 443)
+if (proxyUrlPort != 443) {
 proxyImageUrl += ":" + this.proxyUrlPort;
-} else {
-proxyAddress = proxyIpAddress;
-if (StringUtils.isNotBlank(consoleProxyUrlDomain)) {
-proxyAddress = consoleProxyUrlDomain;
 }
-proxyPort = port;
-this.proxyUrlPort = proxyUrlPort;
 
+} else {
 proxyImageUrl = "http://; + proxyAddress;
-if (proxyUrlPort != 80)
+if (proxyUrlPort != 80) {
 proxyImageUrl += ":" + proxyUrlPort;
+}
+}
+}
+
+private String formatProxyAddress(String consoleProxyUrlDomain, String 
proxyIpAddress) {
+StringBuffer sb = new StringBuffer();
+// Domain in format *.example.com, proxy IP is 1.2.3.4 --> 
1-2-3-4.example.com
+if (consoleProxyUrlDomain.startsWith("*")) {
+sb.append(proxyIpAddress.replaceAll("\\.", "-"));
+sb.append(consoleProxyUrlDomain.substring(1)); // skip the *
+
+// Otherwise we assume a valid domain if config not blank
+} else if (StringUtils.isNotBlank(consoleProxyUrlDomain)) {
+sb.append(consoleProxyUrlDomain);
+
+// Blank config, we use the proxy IP
+} else {
+sb.append(proxyIpAddress);
 }
+return sb.toString();
 }
 
 public String getProxyAddress() {



[GitHub] rhtyd commented on issue #2755: Fix migrate vol xen vmware test

2018-07-26 Thread GitBox
rhtyd commented on issue #2755: Fix migrate vol xen vmware test
URL: https://github.com/apache/cloudstack/pull/2755#issuecomment-408004193
 
 
   @borisstoyanov can you comment if this is ready to be merged?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rhtyd closed pull request #2762: Make HTTP handler of embedded server to be configured

2018-07-26 Thread GitBox
rhtyd closed pull request #2762: Make HTTP handler of embedded server to be 
configured
URL: https://github.com/apache/cloudstack/pull/2762
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/client/conf/server.properties.in b/client/conf/server.properties.in
index f255128555c..75505202624 100644
--- a/client/conf/server.properties.in
+++ b/client/conf/server.properties.in
@@ -22,6 +22,7 @@ bind.interface=::
 context.path=/client
 
 # The HTTP port to be used by the management server
+http.enable=true
 http.port=8080
 
 # Max inactivity time in minutes for the session
@@ -33,6 +34,7 @@ session.timeout=30
 # keystore file should exists and be readable by the management server.
 https.enable=false
 https.port=8443
+
 # The keystore and manager passwords are assumed to be same.
 https.keystore=/etc/cloudstack/management/cloud.jks
 https.keystore.password=vmops.com
diff --git a/client/src/main/java/org/apache/cloudstack/ServerDaemon.java 
b/client/src/main/java/org/apache/cloudstack/ServerDaemon.java
index 985b67b755a..1a5e8ff1b1e 100644
--- a/client/src/main/java/org/apache/cloudstack/ServerDaemon.java
+++ b/client/src/main/java/org/apache/cloudstack/ServerDaemon.java
@@ -66,6 +66,7 @@
 private static final String BIND_INTERFACE = "bind.interface";
 private static final String CONTEXT_PATH = "context.path";
 private static final String SESSION_TIMEOUT = "session.timeout";
+private static final String HTTP_ENABLE = "http.enable";
 private static final String HTTP_PORT = "http.port";
 private static final String HTTPS_ENABLE = "https.enable";
 private static final String HTTPS_PORT = "https.port";
@@ -80,6 +81,7 @@
 
 private Server server;
 
+private boolean httpEnable = true;
 private int httpPort = 8080;
 private int httpsPort = 8443;
 private int sessionTimeout = 30;
@@ -105,8 +107,8 @@ public static void main(final String... anArgs) throws 
Exception {
 public void init(final DaemonContext context) {
 final File confFile = 
PropertiesUtil.findConfigFile("server.properties");
 if (confFile == null) {
-LOG.warn(String.format("Server configuration file not found. 
Initializing server daemon on %s:%s, with https.enabled=%s, https.port=%s, 
context.path=%s",
-bindInterface, httpPort, httpsEnable, httpsPort, 
contextPath));
+LOG.warn(String.format("Server configuration file not found. 
Initializing server daemon on %s, with http.enable=%s, http.port=%s, 
https.enable=%s, https.port=%s, context.path=%s",
+bindInterface, httpEnable, httpPort, httpsEnable, 
httpsPort, contextPath));
 return;
 }
 
@@ -119,6 +121,7 @@ public void init(final DaemonContext context) {
 }
 setBindInterface(properties.getProperty(BIND_INTERFACE, ""));
 setContextPath(properties.getProperty(CONTEXT_PATH, "/client"));
+setHttpEnable(Boolean.valueOf(properties.getProperty(HTTP_ENABLE, 
"true")));
 setHttpPort(Integer.valueOf(properties.getProperty(HTTP_PORT, 
"8080")));
 
setHttpsEnable(Boolean.valueOf(properties.getProperty(HTTPS_ENABLE, "false")));
 setHttpsPort(Integer.valueOf(properties.getProperty(HTTPS_PORT, 
"8443")));
@@ -129,9 +132,15 @@ public void init(final DaemonContext context) {
 
setSessionTimeout(Integer.valueOf(properties.getProperty(SESSION_TIMEOUT, 
"30")));
 } catch (final IOException e) {
 LOG.warn("Failed to load configuration from server.properties 
file", e);
+} finally {
+// make sure that at least HTTP is enabled if both of them are set 
to false (misconfiguration)
+if (!httpEnable && !httpsEnable) {
+setHttpEnable(true);
+LOG.warn("Server configuration malformed, neither http nor 
https is enabled, http will be enabled.");
+}
 }
-LOG.info(String.format("Initializing server daemon on %s:%s, with 
https.enabled=%s, https.port=%s, context.path=%s",
-bindInterface, httpPort, httpsEnable, httpsPort, contextPath));
+LOG.info(String.format("Initializing server daemon on %s, with 
http.enable=%s, http.port=%s, https.enable=%s, https.port=%s, context.path=%s",
+bindInterface, httpEnable, httpPort, httpsEnable, httpsPort, 
contextPath));
 }
 
 @Override
@@ -163,11 +172,7 @@ public void start() throws Exception {
 httpConfig.setSendDateHeader(false);
 
 // HTTP Connector
-final ServerConnector httpConnector = new ServerConnector(server, new 
HttpConnectionFactory(httpConfig));
-httpConnector.setPort(httpPort);
-httpConnector.setHost(bindInterface);
-  

[cloudstack] branch master updated: server: Make HTTP handler of embedded server to be configured (#2762)

2018-07-26 Thread rohit
This is an automated email from the ASF dual-hosted git repository.

rohit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/master by this push:
 new 1960124  server: Make HTTP handler of embedded server to be configured 
(#2762)
1960124 is described below

commit 1960124819f0891299d5b4760fa68ba5cfae96fb
Author: Khosrow Moossavi 
AuthorDate: Thu Jul 26 03:29:25 2018 -0400

server: Make HTTP handler of embedded server to be configured (#2762)

This is to have the possibility to completely disable HTTP and only use 
HTTPS.
---
 client/conf/server.properties.in   |  2 +
 .../java/org/apache/cloudstack/ServerDaemon.java   | 79 ++
 2 files changed, 54 insertions(+), 27 deletions(-)

diff --git a/client/conf/server.properties.in b/client/conf/server.properties.in
index f255128..7550520 100644
--- a/client/conf/server.properties.in
+++ b/client/conf/server.properties.in
@@ -22,6 +22,7 @@ bind.interface=::
 context.path=/client
 
 # The HTTP port to be used by the management server
+http.enable=true
 http.port=8080
 
 # Max inactivity time in minutes for the session
@@ -33,6 +34,7 @@ session.timeout=30
 # keystore file should exists and be readable by the management server.
 https.enable=false
 https.port=8443
+
 # The keystore and manager passwords are assumed to be same.
 https.keystore=/etc/cloudstack/management/cloud.jks
 https.keystore.password=vmops.com
diff --git a/client/src/main/java/org/apache/cloudstack/ServerDaemon.java 
b/client/src/main/java/org/apache/cloudstack/ServerDaemon.java
index 985b67b..1a5e8ff 100644
--- a/client/src/main/java/org/apache/cloudstack/ServerDaemon.java
+++ b/client/src/main/java/org/apache/cloudstack/ServerDaemon.java
@@ -66,6 +66,7 @@ public class ServerDaemon implements Daemon {
 private static final String BIND_INTERFACE = "bind.interface";
 private static final String CONTEXT_PATH = "context.path";
 private static final String SESSION_TIMEOUT = "session.timeout";
+private static final String HTTP_ENABLE = "http.enable";
 private static final String HTTP_PORT = "http.port";
 private static final String HTTPS_ENABLE = "https.enable";
 private static final String HTTPS_PORT = "https.port";
@@ -80,6 +81,7 @@ public class ServerDaemon implements Daemon {
 
 private Server server;
 
+private boolean httpEnable = true;
 private int httpPort = 8080;
 private int httpsPort = 8443;
 private int sessionTimeout = 30;
@@ -105,8 +107,8 @@ public class ServerDaemon implements Daemon {
 public void init(final DaemonContext context) {
 final File confFile = 
PropertiesUtil.findConfigFile("server.properties");
 if (confFile == null) {
-LOG.warn(String.format("Server configuration file not found. 
Initializing server daemon on %s:%s, with https.enabled=%s, https.port=%s, 
context.path=%s",
-bindInterface, httpPort, httpsEnable, httpsPort, 
contextPath));
+LOG.warn(String.format("Server configuration file not found. 
Initializing server daemon on %s, with http.enable=%s, http.port=%s, 
https.enable=%s, https.port=%s, context.path=%s",
+bindInterface, httpEnable, httpPort, httpsEnable, 
httpsPort, contextPath));
 return;
 }
 
@@ -119,6 +121,7 @@ public class ServerDaemon implements Daemon {
 }
 setBindInterface(properties.getProperty(BIND_INTERFACE, ""));
 setContextPath(properties.getProperty(CONTEXT_PATH, "/client"));
+setHttpEnable(Boolean.valueOf(properties.getProperty(HTTP_ENABLE, 
"true")));
 setHttpPort(Integer.valueOf(properties.getProperty(HTTP_PORT, 
"8080")));
 
setHttpsEnable(Boolean.valueOf(properties.getProperty(HTTPS_ENABLE, "false")));
 setHttpsPort(Integer.valueOf(properties.getProperty(HTTPS_PORT, 
"8443")));
@@ -129,9 +132,15 @@ public class ServerDaemon implements Daemon {
 
setSessionTimeout(Integer.valueOf(properties.getProperty(SESSION_TIMEOUT, 
"30")));
 } catch (final IOException e) {
 LOG.warn("Failed to load configuration from server.properties 
file", e);
+} finally {
+// make sure that at least HTTP is enabled if both of them are set 
to false (misconfiguration)
+if (!httpEnable && !httpsEnable) {
+setHttpEnable(true);
+LOG.warn("Server configuration malformed, neither http nor 
https is enabled, http will be enabled.");
+}
 }
-LOG.info(String.format("Initializing server daemon on %s:%s, with 
https.enabled=%s, https.port=%s, context.path=%s",
-bindInterface, httpPort, httpsEnable, httpsPort, contextPath));
+LOG.info(String.format("Initializing server daemon on %s, with 
http.enable=%s, http.port=%s, https.enable=%s, https.port=%s, 

[cloudstack] branch master updated: test: Fix test_deploy_virtio_scsi_vm.py smoke test failures (#2752)

2018-07-26 Thread rohit
This is an automated email from the ASF dual-hosted git repository.

rohit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/master by this push:
 new 6156b44  test: Fix test_deploy_virtio_scsi_vm.py smoke test failures 
(#2752)
6156b44 is described below

commit 6156b442ddd8fef2aad088dd314b2e8001ab4aa0
Author: Dingane Hlaluku 
AuthorDate: Thu Jul 26 09:27:39 2018 +0200

test: Fix test_deploy_virtio_scsi_vm.py smoke test failures (#2752)

Fix failing test cases and proper resource cleanup
---
 .../smoke/test_deploy_virtio_scsi_vm.py| 78 ++
 1 file changed, 35 insertions(+), 43 deletions(-)

diff --git a/test/integration/smoke/test_deploy_virtio_scsi_vm.py 
b/test/integration/smoke/test_deploy_virtio_scsi_vm.py
index 260e299..df54c43 100644
--- a/test/integration/smoke/test_deploy_virtio_scsi_vm.py
+++ b/test/integration/smoke/test_deploy_virtio_scsi_vm.py
@@ -25,35 +25,30 @@ from marvin.cloudstackTestCase import cloudstackTestCase
 # base - contains all resources as entities and defines create, delete,
 # list operations on them
 from marvin.lib.base import (Account,
-VirtualMachine,
-ServiceOffering,
-NetworkOffering,
-Network,
-Template,
-DiskOffering,
-StoragePool,
-Volume,
-Host,
-GuestOs)
-
-
+ VirtualMachine,
+ ServiceOffering,
+ Template,
+ DiskOffering,
+ Volume,
+ Host,
+ GuestOs)
 
 # utils - utility classes for common cleanup, external library wrappers etc
 from marvin.lib.utils import cleanup_resources, get_hypervisor_type, 
validateList
 
 # common - commonly used methods for all tests are listed here
-from marvin.lib.common import get_zone, get_domain, get_template, list_hosts, 
get_pod
+from marvin.lib.common import get_zone, get_domain, get_pod
 
 from marvin.sshClient import SshClient
 
-from marvin.codes import FAILED, PASS
+from marvin.codes import FAILED
 
 from nose.plugins.attrib import attr
 
 import xml.etree.ElementTree as ET
-import code
 import logging
 
+
 class Templates:
 """Test data for templates
 """
@@ -75,11 +70,12 @@ class Templates:
 }
 }
 
-class TestDeployVirtioSCSIVM(cloudstackTestCase):
 
+class TestDeployVirtioSCSIVM(cloudstackTestCase):
 """
 Test deploy a kvm virtio scsi template
 """
+
 @classmethod
 def setUpClass(cls):
 cls.logger = logging.getLogger('TestDeployVirtioSCSIVM')
@@ -100,7 +96,6 @@ class TestDeployVirtioSCSIVM(cloudstackTestCase):
 cls.zone = get_zone(cls.apiclient, testClient.getZoneForTests())
 cls.pod = get_pod(cls.apiclient, cls.zone.id)
 cls.services['mode'] = cls.zone.networktype
-cls._cleanup = []
 if cls.hypervisor.lower() not in ['kvm']:
 cls.hypervisorNotSupported = True
 return
@@ -153,41 +148,38 @@ class TestDeployVirtioSCSIVM(cloudstackTestCase):
 
 cls.vmhost = hosts[0]
 
-
+# Stop VM to reset password
+cls.virtual_machine.stop(cls.apiclient)
 
 password = cls.virtual_machine.resetPassword(cls.apiclient)
 cls.virtual_machine.username = "ubuntu"
 cls.virtual_machine.password = password
-cls._cleanup = [
+
+# Start VM after password reset
+cls.virtual_machine.start(cls.apiclient)
+
+cls.cleanup = [
 cls.template,
 cls.service_offering,
 cls.sparse_disk_offering,
 cls.account
 ]
 
-
 @classmethod
 def tearDownClass(cls):
 try:
+cls.apiclient = super(
+TestDeployVirtioSCSIVM,
+cls
+).getClsTestClient().getApiClient()
 # Cleanup resources used
-cleanup_resources(cls.apiclient, cls._cleanup)
+cleanup_resources(cls.apiclient, cls.cleanup)
 except Exception as e:
 raise Exception("Warning: Exception during cleanup : %s" % e)
-return
 
 def setUp(self):
 self.apiclient = self.testClient.getApiClient()
 self.dbclient = self.testClient.getDbConnection()
-self.cleanup = []
-return
-
-def tearDown(self):
-try:
-# Clean up, terminate the created instance, volumes and snapshots
-cleanup_resources(self.apiclient, self.cleanup)
-except Exception as e:
-raise Exception("Warning: Exception during cleanup : %s" % e)
-return
 
 def verifyVirshState(self, 

[GitHub] rhtyd closed pull request #2752: Fix test_deploy_virtio_scsi_vm.py smoke test failures

2018-07-26 Thread GitBox
rhtyd closed pull request #2752: Fix test_deploy_virtio_scsi_vm.py smoke test 
failures
URL: https://github.com/apache/cloudstack/pull/2752
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/test/integration/smoke/test_deploy_virtio_scsi_vm.py 
b/test/integration/smoke/test_deploy_virtio_scsi_vm.py
index 260e299d4f9..df54c4307a9 100644
--- a/test/integration/smoke/test_deploy_virtio_scsi_vm.py
+++ b/test/integration/smoke/test_deploy_virtio_scsi_vm.py
@@ -25,35 +25,30 @@
 # base - contains all resources as entities and defines create, delete,
 # list operations on them
 from marvin.lib.base import (Account,
-VirtualMachine,
-ServiceOffering,
-NetworkOffering,
-Network,
-Template,
-DiskOffering,
-StoragePool,
-Volume,
-Host,
-GuestOs)
-
-
+ VirtualMachine,
+ ServiceOffering,
+ Template,
+ DiskOffering,
+ Volume,
+ Host,
+ GuestOs)
 
 # utils - utility classes for common cleanup, external library wrappers etc
 from marvin.lib.utils import cleanup_resources, get_hypervisor_type, 
validateList
 
 # common - commonly used methods for all tests are listed here
-from marvin.lib.common import get_zone, get_domain, get_template, list_hosts, 
get_pod
+from marvin.lib.common import get_zone, get_domain, get_pod
 
 from marvin.sshClient import SshClient
 
-from marvin.codes import FAILED, PASS
+from marvin.codes import FAILED
 
 from nose.plugins.attrib import attr
 
 import xml.etree.ElementTree as ET
-import code
 import logging
 
+
 class Templates:
 """Test data for templates
 """
@@ -75,11 +70,12 @@ def __init__(self):
 }
 }
 
-class TestDeployVirtioSCSIVM(cloudstackTestCase):
 
+class TestDeployVirtioSCSIVM(cloudstackTestCase):
 """
 Test deploy a kvm virtio scsi template
 """
+
 @classmethod
 def setUpClass(cls):
 cls.logger = logging.getLogger('TestDeployVirtioSCSIVM')
@@ -100,7 +96,6 @@ def setUpClass(cls):
 cls.zone = get_zone(cls.apiclient, testClient.getZoneForTests())
 cls.pod = get_pod(cls.apiclient, cls.zone.id)
 cls.services['mode'] = cls.zone.networktype
-cls._cleanup = []
 if cls.hypervisor.lower() not in ['kvm']:
 cls.hypervisorNotSupported = True
 return
@@ -153,41 +148,38 @@ def setUpClass(cls):
 
 cls.vmhost = hosts[0]
 
-
+# Stop VM to reset password
+cls.virtual_machine.stop(cls.apiclient)
 
 password = cls.virtual_machine.resetPassword(cls.apiclient)
 cls.virtual_machine.username = "ubuntu"
 cls.virtual_machine.password = password
-cls._cleanup = [
+
+# Start VM after password reset
+cls.virtual_machine.start(cls.apiclient)
+
+cls.cleanup = [
 cls.template,
 cls.service_offering,
 cls.sparse_disk_offering,
 cls.account
 ]
 
-
 @classmethod
 def tearDownClass(cls):
 try:
+cls.apiclient = super(
+TestDeployVirtioSCSIVM,
+cls
+).getClsTestClient().getApiClient()
 # Cleanup resources used
-cleanup_resources(cls.apiclient, cls._cleanup)
+cleanup_resources(cls.apiclient, cls.cleanup)
 except Exception as e:
 raise Exception("Warning: Exception during cleanup : %s" % e)
-return
 
 def setUp(self):
 self.apiclient = self.testClient.getApiClient()
 self.dbclient = self.testClient.getDbConnection()
-self.cleanup = []
-return
-
-def tearDown(self):
-try:
-# Clean up, terminate the created instance, volumes and snapshots
-cleanup_resources(self.apiclient, self.cleanup)
-except Exception as e:
-raise Exception("Warning: Exception during cleanup : %s" % e)
-return
 
 def verifyVirshState(self, diskcount):
 host = self.vmhost.ipaddress
@@ -212,14 +204,14 @@ def verifyVirshState(self, diskcount):
 for child in disk:
 if child.tag.lower() == "target":
 dev = child.get("dev")
-self.assert_(dev != None and dev.startswith("sd"), "disk 
dev is invalid")
+self.assert_(dev is not None and dev.startswith("sd"), 
"disk dev is invalid")

[GitHub] rhtyd commented on issue #2773: CLOUDSTACK-10328: Add Secondary IPv6 address through API

2018-07-26 Thread GitBox
rhtyd commented on issue #2773: CLOUDSTACK-10328: Add Secondary IPv6 address 
through API
URL: https://github.com/apache/cloudstack/pull/2773#issuecomment-408003265
 
 
   @GabrielBrascher can you have a look at travis failure


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rhtyd commented on issue #2578: api: add command to list management servers

2018-07-26 Thread GitBox
rhtyd commented on issue #2578: api: add command to list management servers
URL: https://github.com/apache/cloudstack/pull/2578#issuecomment-408002852
 
 
   @marcaurele sorry did not get the question, if you're looking for link/url 
where systemvmtemplates are hosted you can use the 
http://download.cloudstack.org/ server. Or, if you mean by having an icon for 
the tab see if you can find one from sprites.png or add a new one in any free 
space in sprites.png


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[cloudstack] branch master updated: kvm: Fixes #2763 move post-renewal libvirt restart class suitably (#2764)

2018-07-26 Thread rohit
This is an automated email from the ASF dual-hosted git repository.

rohit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/master by this push:
 new 7667846  kvm: Fixes #2763 move post-renewal libvirt restart class 
suitably (#2764)
7667846 is described below

commit 7667846bfa70704cfe785ffe29fc13f0d066a765
Author: Rohit Yadav 
AuthorDate: Thu Jul 26 12:51:24 2018 +0530

kvm: Fixes #2763 move post-renewal libvirt restart class suitably (#2764)

This fixes #2763 by moving a post cert-renewal class for kvm
plugin/hypervisor to src/main/java. The regression is due to change
in file-system layout due to maven standard refactoring on master and
issue was not caught during forward-merging of a PR from 4.11 branch.

Signed-off-by: Rohit Yadav 
---
 .../resource/wrapper/LibvirtPostCertificateRenewalCommandWrapper.java   | 0
 test/integration/smoke/test_vm_life_cycle.py| 2 +-
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPostCertificateRenewalCommandWrapper.java
 
b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPostCertificateRenewalCommandWrapper.java
similarity index 100%
rename from 
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPostCertificateRenewalCommandWrapper.java
rename to 
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPostCertificateRenewalCommandWrapper.java
diff --git a/test/integration/smoke/test_vm_life_cycle.py 
b/test/integration/smoke/test_vm_life_cycle.py
index e9970b6..5906a94 100644
--- a/test/integration/smoke/test_vm_life_cycle.py
+++ b/test/integration/smoke/test_vm_life_cycle.py
@@ -918,7 +918,7 @@ class TestSecuredVmMigration(cloudstackTestCase):
 cmd = provisionCertificate.provisionCertificateCmd()
 cmd.hostid = host.id
 cmd.reconnect = True
-self.apiclient.updateConfiguration(cmd)
+self.apiclient.provisionCertificate(cmd)
 
 for host in self.hosts:
 self.check_connection(secured='true', host=host)



[GitHub] rhtyd closed pull request #2764: kvm: Fixes #2763 move post-renewal libvirt restart class suitably

2018-07-26 Thread GitBox
rhtyd closed pull request #2764: kvm: Fixes #2763 move post-renewal libvirt 
restart class suitably
URL: https://github.com/apache/cloudstack/pull/2764
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git 
a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPostCertificateRenewalCommandWrapper.java
 
b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPostCertificateRenewalCommandWrapper.java
similarity index 100%
rename from 
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPostCertificateRenewalCommandWrapper.java
rename to 
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPostCertificateRenewalCommandWrapper.java
diff --git a/test/integration/smoke/test_vm_life_cycle.py 
b/test/integration/smoke/test_vm_life_cycle.py
index e9970b6f212..5906a94869c 100644
--- a/test/integration/smoke/test_vm_life_cycle.py
+++ b/test/integration/smoke/test_vm_life_cycle.py
@@ -918,7 +918,7 @@ def secure_all_hosts(self):
 cmd = provisionCertificate.provisionCertificateCmd()
 cmd.hostid = host.id
 cmd.reconnect = True
-self.apiclient.updateConfiguration(cmd)
+self.apiclient.provisionCertificate(cmd)
 
 for host in self.hosts:
 self.check_connection(secured='true', host=host)


 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rhtyd closed issue #2763: Host is still unsecured when provisionCertificate api is called

2018-07-26 Thread GitBox
rhtyd closed issue #2763: Host is still unsecured when provisionCertificate api 
is called
URL: https://github.com/apache/cloudstack/issues/2763
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] blueorangutan commented on issue #2766: kvm: Agent should not check if remaining memory on host is sufficient

2018-07-26 Thread GitBox
blueorangutan commented on issue #2766: kvm: Agent should not check if 
remaining memory on host is sufficient
URL: https://github.com/apache/cloudstack/pull/2766#issuecomment-408001606
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted 
as I make progress.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] blueorangutan commented on issue #2769: Fix config drive iso path on Vmware

2018-07-26 Thread GitBox
blueorangutan commented on issue #2769: Fix config drive iso path on Vmware
URL: https://github.com/apache/cloudstack/pull/2769#issuecomment-408001360
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted 
as I make progress.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rhtyd commented on issue #2766: kvm: Agent should not check if remaining memory on host is sufficient

2018-07-26 Thread GitBox
rhtyd commented on issue #2766: kvm: Agent should not check if remaining memory 
on host is sufficient
URL: https://github.com/apache/cloudstack/pull/2766#issuecomment-408001413
 
 
   @blueorangutan package
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rhtyd commented on issue #2769: Fix config drive iso path on Vmware

2018-07-26 Thread GitBox
rhtyd commented on issue #2769: Fix config drive iso path on Vmware
URL: https://github.com/apache/cloudstack/pull/2769#issuecomment-408001236
 
 
   @blueorangutan package
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] mike-tutkowski commented on a change in pull request #2761: Add managed storage pool constraints to MigrateWithVolume API method

2018-07-26 Thread GitBox
mike-tutkowski commented on a change in pull request #2761: Add managed storage 
pool constraints to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#discussion_r205340214
 
 

 ##
 File path: 
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
 ##
 @@ -2302,8 +2302,11 @@ protected void migrate(final VMInstanceVO vm, final 
long srcHostId, final Deploy
 StoragePoolVO targetPool = _storagePoolDao.findById(poolId);
 StoragePoolVO currentPool = 
_storagePoolDao.findById(volume.getPoolId());
 
-if (_poolHostDao.findByPoolHost(targetPool.getId(), host.getId()) 
== null) {
-throw new CloudRuntimeException(String.format("Cannot migrate 
the volume [%s] to the storage pool [%s] while migrating VM [%s] to target host 
[%s]. The host does not have access to the storage pool entered.", 
volume.getUuid(), targetPool.getUuid(), profile.getUuid(), host.getUuid()));
+executeManagedStorageChecks(targetHost, currentPool, volume);
+if (_poolHostDao.findByPoolHost(targetPool.getId(), 
targetHost.getId()) == null) {
+throw new CloudRuntimeException(
+String.format("Cannot migrate the volume [%s] to the 
storage pool [%s] while migrating VM [%s] to target host [%s]. The host does 
not have access to the storage pool entered.",
+volume.getUuid(), targetPool.getUuid(), 
profile.getUuid(), targetHost.getUuid()));
 }
 if (currentPool.getId() == targetPool.getId()) {
 
 Review comment:
   Yeah, in the old code, the volume was not added to the map (even for 
XenServer) unless it needed to be migrated to a new storage pool. It sounds 
like you are saying that logic was incorrect, though?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] mike-tutkowski commented on a change in pull request #2761: Add managed storage pool constraints to MigrateWithVolume API method

2018-07-26 Thread GitBox
mike-tutkowski commented on a change in pull request #2761: Add managed storage 
pool constraints to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#discussion_r205340758
 
 

 ##
 File path: 
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
 ##
 @@ -2367,7 +2437,7 @@ private void 
createVolumeToStoragePoolMappingIfNeeded(VirtualMachineProfile prof
 /**
  * We use {@link StoragePoolAllocator} objects to find local storage pools 
connected to the targetHost where we would be able to allocate the given volume.
  */
-private List 
getCandidateStoragePoolsToMigrateLocalVolume(VirtualMachineProfile profile, 
Host targetHost, VolumeVO volume) {
+private List 
getCandidateStoragePoolsToMigrateLocalVolume(VirtualMachineProfile profile, 
Host targetHost, Volume volume) {
 
 Review comment:
   Not sure if we want to use the word "Local" in this volume name.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] mike-tutkowski commented on a change in pull request #2761: Add managed storage pool constraints to MigrateWithVolume API method

2018-07-26 Thread GitBox
mike-tutkowski commented on a change in pull request #2761: Add managed storage 
pool constraints to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#discussion_r205341947
 
 

 ##
 File path: 
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
 ##
 @@ -2282,7 +2282,7 @@ protected void migrate(final VMInstanceVO vm, final long 
srcHostId, final Deploy
  * Create the mapping of volumes and storage pools. If the user did not 
enter a mapping on her/his own, we create one using {@link 
#getDefaultMappingOfVolumesAndStoragePoolForMigration(VirtualMachineProfile, 
Host)}.
  * If the user provided a mapping, we use whatever the user has provided 
(check the method {@link 
#createMappingVolumeAndStoragePoolEnteredByUser(VirtualMachineProfile, Host, 
Map)}).
  */
-private Map 
getPoolListForVolumesForMigration(VirtualMachineProfile profile, Host 
targetHost, Map volumeToPool) {
+protected Map 
getPoolListForVolumesForMigration(VirtualMachineProfile profile, Host 
targetHost, Map volumeToPool) {
 if (MapUtils.isEmpty(volumeToPool)) {
 
 Review comment:
   Do we even need this?
   
   if (MapUtils.isEmpty(volumeToPool)) {
   return 
getDefaultMappingOfVolumesAndStoragePoolForMigration(profile, targetHost);
   }
   
   Won't the code work just fine without it? It doesn't seem like we need a 
special case for the mapping being empty when it gets here.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] mike-tutkowski commented on a change in pull request #2761: Add managed storage pool constraints to MigrateWithVolume API method

2018-07-26 Thread GitBox
mike-tutkowski commented on a change in pull request #2761: Add managed storage 
pool constraints to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#discussion_r205340486
 
 

 ##
 File path: 
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
 ##
 @@ -2292,73 +2292,143 @@ protected void migrate(final VMInstanceVO vm, final 
long srcHostId, final Deploy
 
 /**
  * We create the mapping of volumes and storage pool to migrate the VMs 
according to the information sent by the user.
+ * If the user did not enter a complete mapping, the volumes that were 
left behind will be auto mapped using {@link 
#createStoragePoolMappingsForVolumes(VirtualMachineProfile, Host, Map, List)}
  */
-private Map 
createMappingVolumeAndStoragePoolEnteredByUser(VirtualMachineProfile profile, 
Host host, Map volumeToPool) {
+protected Map 
createMappingVolumeAndStoragePoolEnteredByUser(VirtualMachineProfile profile, 
Host targetHost, Map volumeToPool) {
 Map volumeToPoolObjectMap = new HashMap();
+buildMapUsingUserInformation(profile, targetHost, volumeToPool, 
volumeToPoolObjectMap);
+
+List volumesNotMapped = 
findVolumesThatWereNotMappedByTheUser(profile, volumeToPoolObjectMap);
+createStoragePoolMappingsForVolumes(profile, targetHost, 
volumeToPoolObjectMap, volumesNotMapped);
+return volumeToPoolObjectMap;
+}
+
+/**
+ *  Given the map of volume to target storage pool entered by the user, we 
check for other volumes that the VM might have and were not configured.
+ *  This map can be then used by CloudStack to find new target storage 
pools according to the target host.
+ */
+private List 
findVolumesThatWereNotMappedByTheUser(VirtualMachineProfile profile, 
Map volumeToPoolObjectMap) {
+List allVolumes = 
_volsDao.findUsableVolumesForInstance(profile.getId());
+List volumesNotMapped = new ArrayList<>();
+for (Volume volume : volumeToPoolObjectMap.keySet()) {
+if (!allVolumes.contains(volume)) {
+volumesNotMapped.add(volume);
+}
+}
+return volumesNotMapped;
+}
+
+/**
+ *  Builds the map of storage pools and volumes with the information 
entered by the user. Before creating the an entry we validate if the migration 
is feasible checking if the migration is allowed and if the target host can 
access the defined target storage pool.
+ */
+private void buildMapUsingUserInformation(VirtualMachineProfile profile, 
Host targetHost, Map volumeToPool, Map 
volumeToPoolObjectMap) {
 for(Long volumeId: volumeToPool.keySet()) {
 VolumeVO volume = _volsDao.findById(volumeId);
 
 Long poolId = volumeToPool.get(volumeId);
 StoragePoolVO targetPool = _storagePoolDao.findById(poolId);
 StoragePoolVO currentPool = 
_storagePoolDao.findById(volume.getPoolId());
 
-if (_poolHostDao.findByPoolHost(targetPool.getId(), host.getId()) 
== null) {
-throw new CloudRuntimeException(String.format("Cannot migrate 
the volume [%s] to the storage pool [%s] while migrating VM [%s] to target host 
[%s]. The host does not have access to the storage pool entered.", 
volume.getUuid(), targetPool.getUuid(), profile.getUuid(), host.getUuid()));
+executeManagedStorageChecks(targetHost, currentPool, volume);
+if (_poolHostDao.findByPoolHost(targetPool.getId(), 
targetHost.getId()) == null) {
+throw new CloudRuntimeException(
+String.format("Cannot migrate the volume [%s] to the 
storage pool [%s] while migrating VM [%s] to target host [%s]. The host does 
not have access to the storage pool entered.",
+volume.getUuid(), targetPool.getUuid(), 
profile.getUuid(), targetHost.getUuid()));
 }
 if (currentPool.getId() == targetPool.getId()) {
 s_logger.info(String.format("The volume [%s] is already 
allocated in storage pool [%s].", volume.getUuid(), targetPool.getUuid()));
 }
 volumeToPoolObjectMap.put(volume, targetPool);
 }
-return volumeToPoolObjectMap;
 }
 
 /**
- * We create the default mapping of volumes and storage pools for the 
migration of the VM to the target host.
- * If the current storage pool of one of the volumes is using local 
storage in the host, it then needs to be migrated to a local storage in the 
target host.
- * Otherwise, we do not need to migrate, and the volume can be kept in its 
current storage pool.
+ * We create the default mapping of volumes and storage pools for the 
migration of the VM to the target host. We execute the following checks an 
operations according to some scenarios:
+ * 
+ *  If the current storage pool of one of the volumes is using local 
storage in the host, it then needs to