[GitHub] blueorangutan commented on issue #2766: kvm: Agent should not check if remaining memory on host is sufficient
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
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.
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
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
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.
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
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
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.
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
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
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
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
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
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
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
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
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
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
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…
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…
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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'
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)
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
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
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)
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
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
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'
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)
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
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)
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
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
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)
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)
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
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
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
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)
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
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
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
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
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
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
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
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
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
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
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