Review Request 13907: change criteria to set vmware.create.full.clone global flag
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13907/ --- Review request for cloudstack, Alex Huang and Kelven Yang. Bugs: CLOUDSTACK-4539 Repository: cloudstack-git Description --- Changing criteria to set the vmware.create.full.clone global flag in an upgrade scenario from previous versions to 4.2.0 such that the flag will be set to true iff there are no existing data centers in the deployment. For a fresh deployment of 4.2.0, this flag will be set to true. Diffs - engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java 773ad62 setup/db/db/schema-410to420.sql 6eb7534 Diff: https://reviews.apache.org/r/13907/diff/ Testing --- Tested the following scenarios: 1) Upgraded from 3.0.6 that had no data centers created in it, to 4.2, and the flag was set to true post upgrade. 2) Upgraded from 3.0.6 that had data centers created in it, to 4.2, and the flag was set to false post upgrade. 3) Deployed a fresh 4.2.0 setup, and the flag was set to true. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request 13547: Extend listCapabilities API support to verify KVM snapshot enabled or not
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13547/#review25114
---
The build failure is for master, this patch is for 4.2
- Venkata Siva Vijayendra Bhamidipati
On Aug. 14, 2013, 12:46 a.m., Fang Wang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13547/
---
(Updated Aug. 14, 2013, 12:46 a.m.)
Review request for cloudstack and Venkata Siva Vijayendra Bhamidipati.
Bugs: cloudstack-4308
Repository: cloudstack-git
Description
---
For 6.3 KVM, by default, snapshot is not supported. This is managed by a
global setting KVM.snapshot.enabled.
UI needs the API support for listCapabilities response, to stop user to set
recurring snapshot when snapshot is not supported for KVM 6.3.
Diffs
-
api/src/org/apache/cloudstack/api/command/user/config/ListCapabilitiesCmd.java
a30e26c
api/src/org/apache/cloudstack/api/response/CapabilitiesResponse.java
c2996f0
server/src/com/cloud/server/ManagementServerImpl.java 622e167
Diff: https://reviews.apache.org/r/13547/diff/
Testing
---
Before the fix, listCapabilities does not have the KVMSnapshotEnabled field:
listcapabilitiesresponse: {
capability: {
securitygroupsenabled: false,
cloudstackversion: 4.2.0-SNAPSHOT,
userpublictemplateenabled: true,
supportELB: false,
projectinviterequired: false,
allowusercreateprojects: true,
customdiskofferingmaxsize: 1024
}
I tested now with my fix, it shows:
listcapabilitiesresponse : { capability :
{securitygroupsenabled:true,cloudstackversion:4.2.0-SNAPSHOT,userpublictemplateenabled:true,supportELB:false,projectinviterequired:false,allowusercreateprojects:true,customdiskofferingmaxsize:1024,KVMsnapshotenabled:false}
--- default value is false;
I then set the default value to TRUE on MS, and call listCapabilities command
again:
{ listcapabilitiesresponse : { capability :
{securitygroupsenabled:true,cloudstackversion:4.2.0-SNAPSHOT,userpublictemplateenabled:true,supportELB:false,projectinviterequired:false,allowusercreateprojects:true,customdiskofferingmaxsize:1024,KVMsnapshotenabled:true}
} reflect the true value.
Verified the fix through the tests.
Thanks,
Fang Wang
Re: Review Request 13547: Extend listCapabilities API support to verify KVM snapshot enabled or not
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13547/#review25115
---
Ship it!
Ship It!
- Venkata Siva Vijayendra Bhamidipati
On Aug. 14, 2013, 12:46 a.m., Fang Wang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13547/
---
(Updated Aug. 14, 2013, 12:46 a.m.)
Review request for cloudstack and Venkata Siva Vijayendra Bhamidipati.
Bugs: cloudstack-4308
Repository: cloudstack-git
Description
---
For 6.3 KVM, by default, snapshot is not supported. This is managed by a
global setting KVM.snapshot.enabled.
UI needs the API support for listCapabilities response, to stop user to set
recurring snapshot when snapshot is not supported for KVM 6.3.
Diffs
-
api/src/org/apache/cloudstack/api/command/user/config/ListCapabilitiesCmd.java
a30e26c
api/src/org/apache/cloudstack/api/response/CapabilitiesResponse.java
c2996f0
server/src/com/cloud/server/ManagementServerImpl.java 622e167
Diff: https://reviews.apache.org/r/13547/diff/
Testing
---
Before the fix, listCapabilities does not have the KVMSnapshotEnabled field:
listcapabilitiesresponse: {
capability: {
securitygroupsenabled: false,
cloudstackversion: 4.2.0-SNAPSHOT,
userpublictemplateenabled: true,
supportELB: false,
projectinviterequired: false,
allowusercreateprojects: true,
customdiskofferingmaxsize: 1024
}
I tested now with my fix, it shows:
listcapabilitiesresponse : { capability :
{securitygroupsenabled:true,cloudstackversion:4.2.0-SNAPSHOT,userpublictemplateenabled:true,supportELB:false,projectinviterequired:false,allowusercreateprojects:true,customdiskofferingmaxsize:1024,KVMsnapshotenabled:false}
--- default value is false;
I then set the default value to TRUE on MS, and call listCapabilities command
again:
{ listcapabilitiesresponse : { capability :
{securitygroupsenabled:true,cloudstackversion:4.2.0-SNAPSHOT,userpublictemplateenabled:true,supportELB:false,projectinviterequired:false,allowusercreateprojects:true,customdiskofferingmaxsize:1024,KVMsnapshotenabled:true}
} reflect the true value.
Verified the fix through the tests.
Thanks,
Fang Wang
Re: Review Request 13298: Put in upgrade path from 4.1.1 to 4.1.2
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13298/ --- (Updated Aug. 8, 2013, 2:43 a.m.) Review request for cloudstack, Alena Prokharchyk, Chip Childers, and Min Chen. Changes --- Uploading diffs for the new upgrade path for 4.2. The patch has been tested on an upgrade from 4.1 (the latest 4.1 will set the db version at 4.1.2) to 4.2 (which sets the version to 4.2.0). The db version output is pasted below: When at 4.1.2: mysql select * from version; ++-+-+--+ | id | version | updated | step | ++-+-+--+ | 1 | 4.0.0 | 2013-08-07 13:04:04 | Complete | | 2 | 4.1.0 | 2013-08-07 20:05:41 | Complete | | 3 | 4.1.1 | 2013-08-07 20:05:41 | Complete | | 4 | 4.1.2 | 2013-08-07 20:05:41 | Complete | ++-+-+--+ 4 rows in set (0.00 sec) Then, after the upgrade to 4.2: mysql select * from version; ++-+-+--+ | id | version | updated | step | ++-+-+--+ | 1 | 4.0.0 | 2013-08-07 13:04:04 | Complete | | 2 | 4.1.0 | 2013-08-07 20:05:41 | Complete | | 3 | 4.1.1 | 2013-08-07 20:05:41 | Complete | | 4 | 4.1.2 | 2013-08-07 20:05:41 | Complete | | 5 | 4.2.0 | 2013-08-07 20:16:12 | Complete | ++-+-+--+ 5 rows in set (0.00 sec) Bugs: CLOUDSTACK-4091 Repository: cloudstack-git Description --- Putting in upgrade path for db upgrade from 4.1.1 to 4.1.2. Prior to the patch, th mgmt server doesn't start up successfully on 4.1. Diffs (updated) - engine/schema/src/com/cloud/upgrade/DatabaseUpgradeChecker.java 7232058 engine/schema/src/com/cloud/upgrade/dao/Upgrade410to411.java PRE-CREATION engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java 5a6c5b0 engine/schema/src/com/cloud/upgrade/dao/Upgrade411to412.java PRE-CREATION setup/db/db/schema-410to420-cleanup.sql b65717f setup/db/db/schema-410to420.sql fdb53a3 Diff: https://reviews.apache.org/r/13298/diff/ Testing --- Mgmt server comes up successfully upon fresh install of 4.1, and the db version shows the right value: mysql select * from version; ++-+-+--+ | id | version | updated | step | ++-+-+--+ | 1 | 4.0.0 | 2013-08-05 07:40:38 | Complete | | 2 | 4.1.0 | 2013-08-05 14:42:20 | Complete | | 3 | 4.1.1 | 2013-08-05 14:42:20 | Complete | | 4 | 4.1.2 | 2013-08-05 14:42:20 | Complete | ++-+-+--+ 4 rows in set (0.00 sec) mysql Thanks, Venkata Siva Vijayendra Bhamidipati
Review Request 13404: Remove unused global config parameters after upgrade from 4.1 to 4.2.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13404/ --- Review request for cloudstack, Alena Prokharchyk, Alex Huang, Chip Childers, and Kelven Yang. Bugs: CLOUDSTACK-3609 Repository: cloudstack-git Description --- Remove unused vmware global config parameters from db after upgrade from 4.1 to 4.2. Diffs - setup/db/db/schema-410to420.sql 8e2feda Diff: https://reviews.apache.org/r/13404/diff/ Testing --- Before upgrade (when at 4.1.2) : mysql select * from version; ++-+-+--+ | id | version | updated | step | ++-+-+--+ | 1 | 4.0.0 | 2013-08-07 13:04:04 | Complete | | 2 | 4.1.0 | 2013-08-07 20:05:41 | Complete | | 3 | 4.1.1 | 2013-08-07 20:05:41 | Complete | | 4 | 4.1.2 | 2013-08-07 20:05:41 | Complete | ++-+-+--+ 4 rows in set (0.00 sec) mysql select * from configuration where name LIKE %vmware.%.vswitch%; +--+--+---+--+---++ | category | instance | component | name | value | description| +--+--+---+--+---++ | Hidden | DEFAULT | management-server | vmware.guest.vswitch | NULL | Specify the vSwitch on host for guest network | | Hidden | DEFAULT | management-server | vmware.private.vswitch | NULL | Specify the vSwitch on host for private network| | Hidden | DEFAULT | management-server | vmware.public.vswitch| NULL | Specify the vSwitch on host for public network | | Network | DEFAULT | management-server | vmware.use.nexus.vswitch | false | Enable/Disable Cisco Nexus 1000v vSwitch in VMware environment | +--+--+---+--+---++ 4 rows in set (0.00 sec) mysql After upgrade to 4.2: mysql select * from version; ++-+-+--+ | id | version | updated | step | ++-+-+--+ | 1 | 4.0.0 | 2013-08-07 13:04:04 | Complete | | 2 | 4.1.0 | 2013-08-07 20:05:41 | Complete | | 3 | 4.1.1 | 2013-08-07 20:05:41 | Complete | | 4 | 4.1.2 | 2013-08-07 20:05:41 | Complete | | 5 | 4.2.0 | 2013-08-07 20:16:12 | Complete | ++-+-+--+ 5 rows in set (0.00 sec) mysql select * from configuration where name LIKE %vmware.%.vswitch%; +--+--+---+--+---++ | category | instance | component | name | value | description| +--+--+---+--+---++ | Network | DEFAULT | management-server | vmware.use.nexus.vswitch | false | Enable/Disable Cisco Nexus 1000v vSwitch in VMware environment | +--+--+---+--+---++ 1 row in set (0.00 sec) mysql Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request 13008: Fix usage of vm.instancename.flag when generating VM names on ESX hypervisor
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13008/ --- (Updated Aug. 6, 2013, 9:10 p.m.) Review request for cloudstack, Alex Huang, Chip Childers, Kelven Yang, Sateesh Chodapuneedi, and William Chan. Changes --- Updating refactored patch that applies on top of latest 4.2 branch. All testing as described previously was redone with the new patch and all cases passed. Bugs: CLOUDSTACK-3886 Repository: cloudstack-git Description --- The vminstancename flag has been incorrectly used to simply append the displayname to the internal VM name that shows up on vCenter in vmware deployments. It was intended to show the actual name supplied as hostname, on the hypervisor. This helps admins and deployers to quickly identify VMs and resolve issues related to those VMs. Its usage is very limited as it stands now. This fix corrects it to ensure that the name of the VM on the hypervisor matches the hostname if it is supplied, and if the vm.instancename.flag is set to true. Diffs (updated) - engine/orchestration/src/org/apache/cloudstack/platform/orchestration/CloudOrchestrator.java 96fb1d9 plugins/hypervisors/vmware/src/com/cloud/hypervisor/guru/VMwareGuru.java 292f7e9 plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java 8d6bdb8 plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java 6b721de plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java d395425 plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java fe127e3 server/src/com/cloud/configuration/Config.java ad8e3aa server/src/com/cloud/ha/HighAvailabilityManagerImpl.java 25c5a04 server/src/com/cloud/hypervisor/HypervisorGuruBase.java e042eb8 server/src/com/cloud/vm/UserVmManagerImpl.java 4a222c4 server/src/com/cloud/vm/VirtualMachineManagerImpl.java b33ee49 setup/db/db/schema-410to420.sql b3206af vmware-base/src/com/cloud/hypervisor/vmware/mo/ClusterMO.java 04ef0f8 vmware-base/src/com/cloud/hypervisor/vmware/mo/CustomFieldConstants.java 11bc157 vmware-base/src/com/cloud/hypervisor/vmware/mo/HostMO.java 2735fb0 vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java d20eb6f vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java cb97b26 vmware-base/src/com/cloud/hypervisor/vmware/mo/VmwareHypervisorHost.java ac14328 Diff: https://reviews.apache.org/r/13008/diff/ Testing --- Post this change, all major VM operations, namely creation/destruction/expunging/start/stop/reboot of the VM have been tested and observed to work correctly. Part of this patch also puts in a fix for VMSync operations where the CS mgmt server doesn't detect that a guest VM is down, if the guest VM has been shut down/powered off in vCenter. Other operations such as VM snapshot, volume snapshots of disks belonging to the VM, volume migration across primaries, volume attach/detach have also been tested and they are working as expected. This is a functional change, and completely transparent to any of cloudstack's existing functionalities and all the test cases that cover the above code paths and APIs - all existing tests should and do pass - no new tests are necessary. UPDATE for 08/05/2013 = 4.2 code has diverged - need to refactor the patch on top of latest 4.2. Will test out refactored patch and resubmit. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request 13008: Fix usage of vm.instancename.flag when generating VM names on ESX hypervisor
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13008/ --- (Updated Aug. 5, 2013, 9:30 p.m.) Review request for cloudstack, Alex Huang, Chip Childers, Kelven Yang, Sateesh Chodapuneedi, and William Chan. Bugs: CLOUDSTACK-3886 Repository: cloudstack-git Description --- The vminstancename flag has been incorrectly used to simply append the displayname to the internal VM name that shows up on vCenter in vmware deployments. It was intended to show the actual name supplied as hostname, on the hypervisor. This helps admins and deployers to quickly identify VMs and resolve issues related to those VMs. Its usage is very limited as it stands now. This fix corrects it to ensure that the name of the VM on the hypervisor matches the hostname if it is supplied, and if the vm.instancename.flag is set to true. Diffs - engine/orchestration/src/org/apache/cloudstack/platform/orchestration/CloudOrchestrator.java 96fb1d9 plugins/hypervisors/vmware/src/com/cloud/hypervisor/guru/VMwareGuru.java 292f7e9 plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java 8d6bdb8 plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java 10b6de9 plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 1216e17 plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java 112a0cb server/src/com/cloud/configuration/Config.java 0d72694 server/src/com/cloud/ha/HighAvailabilityManagerImpl.java 25c5a04 server/src/com/cloud/hypervisor/HypervisorGuruBase.java e042eb8 server/src/com/cloud/vm/UserVmManagerImpl.java 4a222c4 server/src/com/cloud/vm/VirtualMachineManagerImpl.java 6d35539 setup/db/db/schema-410to420.sql 3f25c3b vmware-base/src/com/cloud/hypervisor/vmware/mo/ClusterMO.java 04ef0f8 vmware-base/src/com/cloud/hypervisor/vmware/mo/CustomFieldConstants.java 11bc157 vmware-base/src/com/cloud/hypervisor/vmware/mo/HostMO.java 2735fb0 vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java dd0f889 vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java e2dd789 vmware-base/src/com/cloud/hypervisor/vmware/mo/VmwareHypervisorHost.java ac14328 Diff: https://reviews.apache.org/r/13008/diff/ Testing (updated) --- Post this change, all major VM operations, namely creation/destruction/expunging/start/stop/reboot of the VM have been tested and observed to work correctly. Part of this patch also puts in a fix for VMSync operations where the CS mgmt server doesn't detect that a guest VM is down, if the guest VM has been shut down/powered off in vCenter. Other operations such as VM snapshot, volume snapshots of disks belonging to the VM, volume migration across primaries, volume attach/detach have also been tested and they are working as expected. This is a functional change, and completely transparent to any of cloudstack's existing functionalities and all the test cases that cover the above code paths and APIs - all existing tests should and do pass - no new tests are necessary. UPDATE for 08/05/2013 = 4.2 code has diverged - need to refactor the patch on top of latest 4.2. Will test out refactored patch and resubmit. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request 13008: Fix usage of vm.instancename.flag when generating VM names on ESX hypervisor
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13008/ --- (Updated Aug. 2, 2013, 12:49 a.m.) Review request for cloudstack, Alex Huang, Chip Childers, Kelven Yang, Sateesh Chodapuneedi, and William Chan. Changes --- Uploading patch rebased against latest 4.2. This patch also corrects the vm.instancename.flag description in Config.java and the 410-420 schema upgrade sql script. Bugs: CLOUDSTACK-3886 Repository: cloudstack-git Description --- The vminstancename flag has been incorrectly used to simply append the displayname to the internal VM name that shows up on vCenter in vmware deployments. It was intended to show the actual name supplied as hostname, on the hypervisor. This helps admins and deployers to quickly identify VMs and resolve issues related to those VMs. Its usage is very limited as it stands now. This fix corrects it to ensure that the name of the VM on the hypervisor matches the hostname if it is supplied, and if the vm.instancename.flag is set to true. Diffs (updated) - engine/orchestration/src/org/apache/cloudstack/platform/orchestration/CloudOrchestrator.java 96fb1d9 plugins/hypervisors/vmware/src/com/cloud/hypervisor/guru/VMwareGuru.java 292f7e9 plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java 8d6bdb8 plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java 10b6de9 plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 1216e17 plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java 112a0cb server/src/com/cloud/configuration/Config.java 0d72694 server/src/com/cloud/ha/HighAvailabilityManagerImpl.java 25c5a04 server/src/com/cloud/hypervisor/HypervisorGuruBase.java e042eb8 server/src/com/cloud/vm/UserVmManagerImpl.java 4a222c4 server/src/com/cloud/vm/VirtualMachineManagerImpl.java 6d35539 setup/db/db/schema-410to420.sql 3f25c3b vmware-base/src/com/cloud/hypervisor/vmware/mo/ClusterMO.java 04ef0f8 vmware-base/src/com/cloud/hypervisor/vmware/mo/CustomFieldConstants.java 11bc157 vmware-base/src/com/cloud/hypervisor/vmware/mo/HostMO.java 2735fb0 vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java dd0f889 vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java e2dd789 vmware-base/src/com/cloud/hypervisor/vmware/mo/VmwareHypervisorHost.java ac14328 Diff: https://reviews.apache.org/r/13008/diff/ Testing --- Post this change, all major VM operations, namely creation/destruction/expunging/start/stop/reboot of the VM have been tested and observed to work correctly. Part of this patch also puts in a fix for VMSync operations where the CS mgmt server doesn't detect that a guest VM is down, if the guest VM has been shut down/powered off in vCenter. Other operations such as VM snapshot, volume snapshots of disks belonging to the VM, volume migration across primaries, volume attach/detach have also been tested and they are working as expected. This is a functional change, and completely transparent to any of cloudstack's existing functionalities and all the test cases that cover the above code paths and APIs - all existing tests should and do pass - no new tests are necessary. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request 13093: Put retry logic to check existence/non-existence of created/deleted files even though vSphere returns successful completion on wait for asynchronous task.
On July 31, 2013, 1:49 a.m., Jenkins Cloudstack.org wrote: Review 13093 PASSED the build test The url of build cloudstack-master-with-patch #90 is : http://jenkins.cloudstack.org/job/cloudstack-master-with-patch/90/ When speaking with Kelven regarding a different bug, Kelven pointed out that waitForTask() is a cloudstack wrapper and its implementation may be causing the premature return of success for the API - this is more plausible since many places in the code are now running into this issue. Will look into this and reupload the patch. - Venkata Siva Vijayendra --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13093/#review24312 --- On July 31, 2013, 12:41 a.m., Venkata Siva Vijayendra Bhamidipati wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13093/ --- (Updated July 31, 2013, 12:41 a.m.) Review request for cloudstack, Alena Prokharchyk, Chip Childers, Kelven Yang, and Sateesh Chodapuneedi. Bugs: CLOUDSTACK-3568 Repository: cloudstack-git Description --- When a large number of VMs are created in parallel in vmware deployments, vSphere can slow down and return wrong results for its APIs. For example, the mgmt server correctly waits on the results of asynchronous tasks like deletion/moving of VM root volume files, but vSphere is returning success for those APIs even when the underlying files aren't yet removed/created from/on the actual storage. So we need to work around this using retry logic. This fix puts that in when creating VMs. Diffs - plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java 112a0cb Diff: https://reviews.apache.org/r/13093/diff/ Testing --- The fix has been tested with parallel creation of ~15 VMs on vmware and the described issues have not been seen. There seem to be some other exceptions with some VMs, but they are not related to this code path. Thanks, Venkata Siva Vijayendra Bhamidipati
Review Request 13093: Put retry logic to check existence/non-existence of created/deleted files even though vSphere returns successful completion on wait for asynchronous task.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13093/ --- Review request for cloudstack, Alena Prokharchyk, Chip Childers, Kelven Yang, and Sateesh Chodapuneedi. Bugs: CLOUDSTACK-3568 Repository: cloudstack-git Description --- When a large number of VMs are created in parallel in vmware deployments, vSphere can slow down and return wrong results for its APIs. For example, the mgmt server correctly waits on the results of asynchronous tasks like deletion/moving of VM root volume files, but vSphere is returning success for those APIs even when the underlying files aren't yet removed/created from/on the actual storage. So we need to work around this using retry logic. This fix puts that in when creating VMs. Diffs - plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java 112a0cb Diff: https://reviews.apache.org/r/13093/diff/ Testing --- The fix has been tested with parallel creation of ~15 VMs on vmware and the described issues have not been seen. There seem to be some other exceptions with some VMs, but they are not related to this code path. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request 13008: Fix usage of vm.instancename.flag when generating VM names on ESX hypervisor
On July 29, 2013, 3:59 a.m., Jenkins Cloudstack.org wrote: Review 13008 failed the build test : FAILURE The url of build cloudstack-master-with-patch #65 is : http://jenkins.cloudstack.org/job/cloudstack-master-with-patch/65/ This patch is for 4.2 and has been tested on 4.2 as described - the patch will not apply on the master branch. The above failure message is for master branch. Requesting reviewers to review patch for 4.2. - Venkata Siva Vijayendra --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13008/#review24078 --- On July 29, 2013, 1:55 a.m., Venkata Siva Vijayendra Bhamidipati wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13008/ --- (Updated July 29, 2013, 1:55 a.m.) Review request for cloudstack, Alex Huang, Chip Childers, Kelven Yang, Sateesh Chodapuneedi, and William Chan. Bugs: CLOUDSTACK-3886 Repository: cloudstack-git Description --- The vminstancename flag has been incorrectly used to simply append the displayname to the internal VM name that shows up on vCenter in vmware deployments. It was intended to show the actual name supplied as hostname, on the hypervisor. This helps admins and deployers to quickly identify VMs and resolve issues related to those VMs. Its usage is very limited as it stands now. This fix corrects it to ensure that the name of the VM on the hypervisor matches the hostname if it is supplied, and if the vm.instancename.flag is set to true. Diffs - engine/orchestration/src/org/apache/cloudstack/platform/orchestration/CloudOrchestrator.java 96fb1d9 plugins/hypervisors/vmware/src/com/cloud/hypervisor/guru/VMwareGuru.java d1392c4 plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java e5c33cc plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 439163a plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java 02e4e64 server/src/com/cloud/ha/HighAvailabilityManagerImpl.java 25c5a04 server/src/com/cloud/hypervisor/HypervisorGuruBase.java ec68529 server/src/com/cloud/vm/UserVmManagerImpl.java 3831f88 server/src/com/cloud/vm/VirtualMachineManagerImpl.java a3187ba vmware-base/src/com/cloud/hypervisor/vmware/mo/ClusterMO.java 04ef0f8 vmware-base/src/com/cloud/hypervisor/vmware/mo/HostMO.java 2735fb0 vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java dd0f889 vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java e2dd789 vmware-base/src/com/cloud/hypervisor/vmware/mo/VmwareHypervisorHost.java ac14328 Diff: https://reviews.apache.org/r/13008/diff/ Testing --- Post this change, all major VM operations, namely creation/destruction/expunging/start/stop/reboot of the VM have been tested and observed to work correctly. Part of this patch also puts in a fix for VMSync operations where the CS mgmt server doesn't detect that a guest VM is down, if the guest VM has been shut down/powered off in vCenter. Other operations such as VM snapshot, volume snapshots of disks belonging to the VM, volume migration across primaries, volume attach/detach have also been tested and they are working as expected. This is a functional change, and completely transparent to any of cloudstack's existing functionalities and all the test cases that cover the above code paths and APIs - all existing tests should and do pass - no new tests are necessary. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request 13008: Fix usage of vm.instancename.flag when generating VM names on ESX hypervisor
On July 29, 2013, 3:59 a.m., Jenkins Cloudstack.org wrote: Review 13008 failed the build test : FAILURE The url of build cloudstack-master-with-patch #65 is : http://jenkins.cloudstack.org/job/cloudstack-master-with-patch/65/ Venkata Siva Vijayendra Bhamidipati wrote: This patch is for 4.2 and has been tested on 4.2 as described - the patch will not apply on the master branch. The above failure message is for master branch. Requesting reviewers to review patch for 4.2. Kelven Yang wrote: It would be better to use existing custom field facility to tag attributes to vCenter objects. Since content of object annotation in vCenter is plain string leterals, we can't store information there in a structural way, which will lead us to add additional logic to enable sharing annotation field from mulitple sources. Another problem with using annotation is that since the field in vCenter is designed to allow vCenter administrators to label object with custom comments, it is editable in vCenter UI, if we use it for special control purpose, it will open a door for people to mess up CloudStack meta information stored in vCenter. I would suggest to change to custom field facility Thanks for the review Kelven! I'll try out the custom field instead of the annotation field and update the request with the new diffs. Regards, Vijay - Venkata Siva Vijayendra --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13008/#review24078 --- On July 29, 2013, 1:55 a.m., Venkata Siva Vijayendra Bhamidipati wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13008/ --- (Updated July 29, 2013, 1:55 a.m.) Review request for cloudstack, Alex Huang, Chip Childers, Kelven Yang, Sateesh Chodapuneedi, and William Chan. Bugs: CLOUDSTACK-3886 Repository: cloudstack-git Description --- The vminstancename flag has been incorrectly used to simply append the displayname to the internal VM name that shows up on vCenter in vmware deployments. It was intended to show the actual name supplied as hostname, on the hypervisor. This helps admins and deployers to quickly identify VMs and resolve issues related to those VMs. Its usage is very limited as it stands now. This fix corrects it to ensure that the name of the VM on the hypervisor matches the hostname if it is supplied, and if the vm.instancename.flag is set to true. Diffs - engine/orchestration/src/org/apache/cloudstack/platform/orchestration/CloudOrchestrator.java 96fb1d9 plugins/hypervisors/vmware/src/com/cloud/hypervisor/guru/VMwareGuru.java d1392c4 plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java e5c33cc plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 439163a plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java 02e4e64 server/src/com/cloud/ha/HighAvailabilityManagerImpl.java 25c5a04 server/src/com/cloud/hypervisor/HypervisorGuruBase.java ec68529 server/src/com/cloud/vm/UserVmManagerImpl.java 3831f88 server/src/com/cloud/vm/VirtualMachineManagerImpl.java a3187ba vmware-base/src/com/cloud/hypervisor/vmware/mo/ClusterMO.java 04ef0f8 vmware-base/src/com/cloud/hypervisor/vmware/mo/HostMO.java 2735fb0 vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java dd0f889 vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java e2dd789 vmware-base/src/com/cloud/hypervisor/vmware/mo/VmwareHypervisorHost.java ac14328 Diff: https://reviews.apache.org/r/13008/diff/ Testing --- Post this change, all major VM operations, namely creation/destruction/expunging/start/stop/reboot of the VM have been tested and observed to work correctly. Part of this patch also puts in a fix for VMSync operations where the CS mgmt server doesn't detect that a guest VM is down, if the guest VM has been shut down/powered off in vCenter. Other operations such as VM snapshot, volume snapshots of disks belonging to the VM, volume migration across primaries, volume attach/detach have also been tested and they are working as expected. This is a functional change, and completely transparent to any of cloudstack's existing functionalities and all the test cases that cover the above code paths and APIs - all existing tests should and do pass - no new tests are necessary. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request 13008: Fix usage of vm.instancename.flag when generating VM names on ESX hypervisor
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13008/ --- (Updated July 30, 2013, 3:46 a.m.) Review request for cloudstack, Alex Huang, Chip Childers, Kelven Yang, Sateesh Chodapuneedi, and William Chan. Changes --- Updating the review request with a patch that now uses a custom field by name CLOUD_INTERNAL_VM_NAME instead of the annotation field. All testing as before has been carried out with these new changes and confirmed to work as expected. Please note that this patch is for 4.2 only and will not apply on master and hence a failure message will show up on this request. Requesting that the committer use git am to apply the patch to ensure that patch application goes through smoothly. Regards, Vijay Bugs: CLOUDSTACK-3886 Repository: cloudstack-git Description --- The vminstancename flag has been incorrectly used to simply append the displayname to the internal VM name that shows up on vCenter in vmware deployments. It was intended to show the actual name supplied as hostname, on the hypervisor. This helps admins and deployers to quickly identify VMs and resolve issues related to those VMs. Its usage is very limited as it stands now. This fix corrects it to ensure that the name of the VM on the hypervisor matches the hostname if it is supplied, and if the vm.instancename.flag is set to true. Diffs (updated) - engine/orchestration/src/org/apache/cloudstack/platform/orchestration/CloudOrchestrator.java 96fb1d9 plugins/hypervisors/vmware/src/com/cloud/hypervisor/guru/VMwareGuru.java d1392c4 plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java 8d6bdb8 plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java 3dece6e plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java a33b94e plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java 6451828 server/src/com/cloud/ha/HighAvailabilityManagerImpl.java 25c5a04 server/src/com/cloud/hypervisor/HypervisorGuruBase.java 478baf3 server/src/com/cloud/vm/UserVmManagerImpl.java 280f7e9 server/src/com/cloud/vm/VirtualMachineManagerImpl.java 45aa50c vmware-base/src/com/cloud/hypervisor/vmware/mo/ClusterMO.java 04ef0f8 vmware-base/src/com/cloud/hypervisor/vmware/mo/CustomFieldConstants.java 11bc157 vmware-base/src/com/cloud/hypervisor/vmware/mo/HostMO.java 2735fb0 vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java dd0f889 vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java e2dd789 vmware-base/src/com/cloud/hypervisor/vmware/mo/VmwareHypervisorHost.java ac14328 Diff: https://reviews.apache.org/r/13008/diff/ Testing --- Post this change, all major VM operations, namely creation/destruction/expunging/start/stop/reboot of the VM have been tested and observed to work correctly. Part of this patch also puts in a fix for VMSync operations where the CS mgmt server doesn't detect that a guest VM is down, if the guest VM has been shut down/powered off in vCenter. Other operations such as VM snapshot, volume snapshots of disks belonging to the VM, volume migration across primaries, volume attach/detach have also been tested and they are working as expected. This is a functional change, and completely transparent to any of cloudstack's existing functionalities and all the test cases that cover the above code paths and APIs - all existing tests should and do pass - no new tests are necessary. Thanks, Venkata Siva Vijayendra Bhamidipati
Review Request 13008: Fix usage of vm.instancename.flag when generating VM names on ESX hypervisor
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13008/ --- Review request for cloudstack, Alex Huang, Chip Childers, Kelven Yang, Sateesh Chodapuneedi, and William Chan. Bugs: CLOUDSTACK-3886 Repository: cloudstack-git Description --- The vminstancename flag has been incorrectly used to simply append the displayname to the internal VM name that shows up on vCenter in vmware deployments. It was intended to show the actual name supplied as hostname, on the hypervisor. This helps admins and deployers to quickly identify VMs and resolve issues related to those VMs. Its usage is very limited as it stands now. This fix corrects it to ensure that the name of the VM on the hypervisor matches the hostname if it is supplied, and if the vm.instancename.flag is set to true. Diffs - engine/orchestration/src/org/apache/cloudstack/platform/orchestration/CloudOrchestrator.java 96fb1d9 plugins/hypervisors/vmware/src/com/cloud/hypervisor/guru/VMwareGuru.java d1392c4 plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java e5c33cc plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 439163a plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java 02e4e64 server/src/com/cloud/ha/HighAvailabilityManagerImpl.java 25c5a04 server/src/com/cloud/hypervisor/HypervisorGuruBase.java ec68529 server/src/com/cloud/vm/UserVmManagerImpl.java 3831f88 server/src/com/cloud/vm/VirtualMachineManagerImpl.java a3187ba vmware-base/src/com/cloud/hypervisor/vmware/mo/ClusterMO.java 04ef0f8 vmware-base/src/com/cloud/hypervisor/vmware/mo/HostMO.java 2735fb0 vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java dd0f889 vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java e2dd789 vmware-base/src/com/cloud/hypervisor/vmware/mo/VmwareHypervisorHost.java ac14328 Diff: https://reviews.apache.org/r/13008/diff/ Testing --- Post this change, all major VM operations, namely creation/destruction/expunging/start/stop/reboot of the VM have been tested and observed to work correctly. Part of this patch also puts in a fix for VMSync operations where the CS mgmt server doesn't detect that a guest VM is down, if the guest VM has been shut down/powered off in vCenter. Other operations such as VM snapshot, volume snapshots of disks belonging to the VM, volume migration across primaries, volume attach/detach have also been tested and they are working as expected. This is a functional change, and completely transparent to any of cloudstack's existing functionalities and all the test cases that cover the above code paths and APIs - all existing tests should and do pass - no new tests are necessary. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request 12612: Fix NPE when attaching vmware-tools iso to guest VM on ESX
On July 17, 2013, 12:24 a.m., Sheng Yang wrote: server/src/com/cloud/template/TemplateManagerImpl.java, line 999 https://reviews.apache.org/r/12612/diff/1/?file=322288#file322288line999 How can this happen?? Add Min Chen in reviewer. Venkata Siva Vijayendra Bhamidipati wrote: This is because vmware tools or xenserver tools ISOs aren't actual ISOs. They're internally provisioned by vmware/xenserver, so the URL will be null. Since it isn't really resident anywhere, the image store will also be absent for it, i.e., null. Min Chen wrote: Vijay, I don't quite understand this. This check is to check if VM is at running state. Original logic is that if VM that we are going to attach ISO to is not at running, we just directly return true and update DB to put iso id for this VM. Why did you change to return false here? Venkata Siva Vijayendra Bhamidipati wrote: Hi Min, VMWare does not allow attaching the vmware tools iso if the VM is not running. Here is the exception response we get from vCenter if true is returned when the VM is not running, and we attempt to carry on with the mount operation: 2013-07-16 09:44:26,158 ERROR [storage.resource.VmwareStorageProcessor] (DirectAgent-7:10.223.74.131) AttachIsoCommand(attach) failed due to Exception: com.vmware.vim25.InvalidState message: [] com.vmware.vim25.InvalidStateFaultMsg: The operation is not allowed in the current state. at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:57) at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) at java.lang.reflect.Constructor.newInstance(Constructor.java:532) at com.sun.xml.internal.ws.fault.SOAPFaultBuilder.createException(SOAPFaultBuilder.java:130) at com.sun.xml.internal.ws.client.sei.SyncMethodHandler.invoke(SyncMethodHandler.java:108) at com.sun.xml.internal.ws.client.sei.SyncMethodHandler.invoke(SyncMethodHandler.java:78) at com.sun.xml.internal.ws.client.sei.SEIStub.invoke(SEIStub.java:107) at sun.proxy.$Proxy95.mountToolsInstaller(Unknown Source) at com.cloud.hypervisor.vmware.mo.VirtualMachineMO.mountToolsInstaller(VirtualMachineMO.java:2152) at com.cloud.storage.resource.VmwareStorageProcessor.attachIso(VmwareStorageProcessor.java:883) at com.cloud.storage.resource.VmwareStorageProcessor.attachIso(VmwareStorageProcessor.java:768) at com.cloud.storage.resource.StorageSubsystemCommandHandlerBase.execute(StorageSubsystemCommandHandlerBase.java:125) at com.cloud.storage.resource.StorageSubsystemCommandHandlerBase.handleStorageCommands(StorageSubsystemCommandHandlerBase.java:55) at com.cloud.hypervisor.vmware.resource.VmwareResource.executeRequest(VmwareResource.java:565) at com.cloud.agent.manager.DirectAgentAttache$Task.run(DirectAgentAttache.java:186) at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471) at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334) at java.util.concurrent.FutureTask.run(FutureTask.java:166) at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$101(ScheduledThreadPoolExecutor.java:165) at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:266) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1146) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615) at java.lang.Thread.run(Thread.java:679) Sheng Yang wrote: In such case, it's VMware specific thing, you shouldn't change the behavior of other hypervisors. And according to Min, the attachment is done after VM boot up, here we just modify DB. Why VMware behavior differently? Turns out it's only a set of VMs in my setup that ran into the above issue - I looked at the logs but couldn't find out why ESX was disallowing iso attaches to them. I'll remove the return false; change because VMware does allow attaching an ISO to a VM in the stopped state. Will submit a new patch. Thanks for your inputs on this! - Venkata Siva Vijayendra --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12612/#review23217 --- On July 17, 2013, 12:39 a.m., Venkata Siva Vijayendra Bhamidipati wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12612
Re: Review Request 12612: Fix NPE when attaching vmware-tools iso to guest VM on ESX
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12612/ --- (Updated July 17, 2013, 11:54 p.m.) Review request for cloudstack, Chip Childers, edison su, Kelven Yang, Sateesh Chodapuneedi, and Sheng Yang. Changes --- Incorporating Min's/Sheng's comments. Bugs: CLOUDSTACK-3554 Repository: cloudstack-git Description --- Fixing NPE that occurs when attaching vmware-tools.iso to a guest VM on ESX. Diffs (updated) - plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java 4113803 Diff: https://reviews.apache.org/r/12612/diff/ Testing --- With the fix in place, vmware-tools.iso attach to a vmware guest VM works as expected. Thanks, Venkata Siva Vijayendra Bhamidipati
Review Request 12716: Fix for NPE
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12716/ --- Review request for cloudstack, Chip Childers, edison su, Min Chen, Sateesh Chodapuneedi, and Sheng Yang. Bugs: CLOUDSTACK-3598 Repository: cloudstack-git Description --- Minor fix for an NPE when setting resource count. Diffs - engine/schema/src/com/cloud/configuration/dao/ResourceCountDaoImpl.java cfd2137 Diff: https://reviews.apache.org/r/12716/diff/ Testing --- This showed up in automation tests in the code path that handles template sync operations. I wasn't able to reproduce this in a manual setup. But the NPE does exist and the code needs to be fixed for that, so I'm submitting the patch. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request 12702: Fix CopyCmdAnswer returned by backupSnapshotCommand for VMware
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12702/#review23354 --- Ship it! Ship It! - Venkata Siva Vijayendra Bhamidipati On July 18, 2013, 4:29 a.m., Sateesh Chodapuneedi wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12702/ --- (Updated July 18, 2013, 4:29 a.m.) Review request for cloudstack, edison su, Fang Wang, and Kelven Yang. Bugs: CLOUDSTACK-3595 Repository: cloudstack-git Description --- Even after successful backup of snapshot to secondary storage, CopyCmdAnswer is sent with failure result. Now sending pass result with full path of backup set to snapshotTO object. Diffs - plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java 4113803 Diff: https://reviews.apache.org/r/12702/diff/ Testing --- Created snapshot from volume in cloudstack deployed over VMware servers. Thanks, Sateesh Chodapuneedi
Review Request 12612: Fix NPE when attaching vmware-tools iso to guest VM on ESX
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12612/ --- Review request for cloudstack, Chip Childers, edison su, Kelven Yang, Sateesh Chodapuneedi, and Sheng Yang. Bugs: CLOUDSTACK-3554 Repository: cloudstack-git Description --- Fixing NPE that occurs when attaching vmware-tools.iso to a guest VM on ESX. Diffs - plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java 4113803 server/src/com/cloud/template/TemplateManagerImpl.java c7cc818 Diff: https://reviews.apache.org/r/12612/diff/ Testing --- With the fix in place, vmware-tools.iso attach to a vmware guest VM works as expected. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request 12517: Fix checks for already existing primary and secondary pvlan id on vmware DVS in pvlan setup
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12517/ --- (Updated July 15, 2013, 10:41 p.m.) Review request for cloudstack, Chip Childers, edison su, Kelven Yang, Sateesh Chodapuneedi, and Sheng Yang. Changes --- +Sheng Bugs: CLOUDSTACK-3311 Repository: cloudstack-git Description --- An incorrect type check for an existing promiscuous pvlan id was causing Virtual Router reboot failure. Fixing the same. Also detected incomplete checking of existing secondary pvlan id on a VMWare DVS, so fixing that as well. Diffs - vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java dc1486a Diff: https://reviews.apache.org/r/12517/diff/ Testing --- Created a pvlan setup in vmware. Created guest instances successfully. Restarted the virtual router of the pvlan shared network. The VR rebooted successfully without erroring out. Created multiple pvlan networks and associated guest VMs and restarted the VRs, all rebooted successfully. Created guest VMs with multiple pvlan networks in each and they also worked as expected. Thanks, Venkata Siva Vijayendra Bhamidipati
Review Request 12517: Fix checks for already existing primary and secondary pvlan id on vmware DVS in pvlan setup
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12517/ --- Review request for cloudstack, Chip Childers, edison su, Kelven Yang, and Sateesh Chodapuneedi. Bugs: CLOUDSTACK-3311 Repository: cloudstack-git Description --- An incorrect type check for an existing promiscuous pvlan id was causing Virtual Router reboot failure. Fixing the same. Also detected incomplete checking of existing secondary pvlan id on a VMWare DVS, so fixing that as well. Diffs - vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java dc1486a Diff: https://reviews.apache.org/r/12517/diff/ Testing --- Created a pvlan setup in vmware. Created guest instances successfully. Restarted the virtual router of the pvlan shared network. The VR rebooted successfully without erroring out. Created multiple pvlan networks and associated guest VMs and restarted the VRs, all rebooted successfully. Created guest VMs with multiple pvlan networks in each and they also worked as expected. Thanks, Venkata Siva Vijayendra Bhamidipati
Review Request 12521: Include vmware in hypervisor list for volume resize support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12521/ --- Review request for cloudstack and edison su. Bugs: CLOUDSTACK-3509 Repository: cloudstack-git Description --- Put in vmware in a check filtering hypervisors supporting volume resize operations. Diffs - server/src/com/cloud/storage/VolumeManagerImpl.java 1618574 Diff: https://reviews.apache.org/r/12521/diff/ Testing --- Verified that volume resize of a non root volume of a guest VM in a vmware deployment works instead of throwing the exception pasted in the bug description. Thanks, Venkata Siva Vijayendra Bhamidipati
Review Request 12369: Ignore network interfaces without uuids when passing extra config to ESX for nvp
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12369/ --- Review request for cloudstack, Chip Childers, Kelven Yang, Sateesh Chodapuneedi, and Hugo Trippaers. Bugs: CLOUDSTACK-3425 Repository: cloudstack-git Description --- The VR gets plumbed with 5 interfaces (passed to vmware resource as nicTOs) but only three of them are initialized. The other two aren't used rightaway until configured later. The extra configs being built and passed on to ESX as part of NVP integration is copying over all the 5 nic configs but two of them don't have uuids, so the VM reconfigure task fails because of unset values of nvp-iface* and VR startup fails. Fixing this by ignoring those unused interfaces when building extra configs. Diffs - plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 372cc1b Diff: https://reviews.apache.org/r/12369/diff/ Testing --- With the fix, the VR comes up successfully, and guest VMs get created. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request 12348: Fix for NPE when listing eligible primary storage pools for volume migration across primary storages
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12348/ --- (Updated July 9, 2013, 10:15 p.m.) Review request for cloudstack, Chip Childers, Kelven Yang, rajeshbabu chintaguntla, and Sateesh Chodapuneedi. Changes --- Observed the same issue on master branch, so uploading the diff for master. Testing showed up other issues in volume migration, but they are unrelated to the issue this patch addresses. The eligible storage pools are listed as expected out of this fix, so requesting that this fix be applied on top of master. Bugs: CLOUDSTACK-3264 Repository: cloudstack-git Description --- When listing zone wide primary storages for volume migration, a null pointer exception is encountered because the listing of the eligible storage pools missed taking into account the absence of cluster id for zone wide primary pools. Fixing the same and putting in appropriate checks for other storage type allocators. Note: This fix is for the master-6-17-stable branch. It is to be seen whether this fix is required for master and if so, needs to be ported/tested on the same. Diffs (updated) - engine/storage/src/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java e16703e engine/storage/src/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java 0933adc engine/storage/src/org/apache/cloudstack/storage/allocator/LocalStoragePoolAllocator.java ef9e84e plugins/storage-allocators/random/src/org/apache/cloudstack/storage/allocator/RandomStoragePoolAllocator.java 76ce663 server/src/com/cloud/server/ManagementServerImpl.java da9d6a2 Diff: https://reviews.apache.org/r/12348/diff/ Testing --- Created multiple zone wide primary storages and confirmed that they show up as eligible destinations when attempting to migrate a volume currently resident on a zone wide primary storage pool. Created mixed zone-wide and cluster-wide primary storages to ensure nothing broke there. Observed that volumes on a zone wide primary can only be moved to other zone wide primary pools and similarly with cluster wide pools. Created new guest VM to confirm that existing primary code paths work correctly with the changes in place. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request 12227: NPE while deploying any instances in kvm/vmware using ZWPS due to capacityIops
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12227/#review22846 --- Ship it! Fix looks good. - Venkata Siva Vijayendra Bhamidipati On July 4, 2013, 12:32 p.m., Rajesh Battala wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12227/ --- (Updated July 4, 2013, 12:32 p.m.) Review request for cloudstack, edison su, Ram Ganesh, and Sateesh Chodapuneedi. Bugs: 3301 Repository: cloudstack-git Description --- Issue: When VM is getting deployed in ZWPS(kvm, vmware), NPE is occuring. Fixed: SolidFire storage had introduced iops, its setting capacityIops on the pool level. Only solidfire is setting and getting it which is causing NPE when checking this value for other type PS. This fixed will resolve the issue for any storage provider which don't set capacityIops. Diffs - server/src/com/cloud/storage/StorageManagerImpl.java bb21afb Diff: https://reviews.apache.org/r/12227/diff/ Testing --- 1. Adding ZWPS, and deployed the VM in KVM. Vm got successfully deployed. 2. Adding CWPS and deployed the VM in KVM. VM got deployed successfully. Thanks, Rajesh Battala
Review Request 12348: Fix for NPE when listing eligible primary storage pools for volume migration across primary storages
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12348/ --- Review request for cloudstack, Chip Childers, Kelven Yang, rajeshbabu chintaguntla, and Sateesh Chodapuneedi. Bugs: CLOUDSTACK-3264 Repository: cloudstack-git Description --- When listing zone wide primary storages for volume migration, a null pointer exception is encountered because the listing of the eligible storage pools missed taking into account the absence of cluster id for zone wide primary pools. Fixing the same and putting in appropriate checks for other storage type allocators. Note: This fix is for the master-6-17-stable branch. It is to be seen whether this fix is required for master and if so, needs to be ported/tested on the same. Diffs - engine/storage/src/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java 5326701 engine/storage/src/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java 5b1f8cd engine/storage/src/org/apache/cloudstack/storage/allocator/LocalStoragePoolAllocator.java 632ba43 plugins/storage-allocators/random/src/org/apache/cloudstack/storage/allocator/RandomStoragePoolAllocator.java cbe6647 server/src/com/cloud/server/ManagementServerImpl.java 682332c Diff: https://reviews.apache.org/r/12348/diff/ Testing --- Created multiple zone wide primary storages and confirmed that they show up as eligible destinations when attempting to migrate a volume currently resident on a zone wide primary storage pool. Created mixed zone-wide and cluster-wide primary storages to ensure nothing broke there. Observed that volumes on a zone wide primary can only be moved to other zone wide primary pools and similarly with cluster wide pools. Created new guest VM to confirm that existing primary code paths work correctly with the changes in place. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request 11984: Fix primary datastore NPE/incorrect db entry/exception propagation for KVM on cloudstack
On June 20, 2013, 10:37 p.m., edison su wrote: engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreLifeCycle.java, line 44 https://reviews.apache.org/r/11984/diff/1/?file=308929#file308929line44 If stoagemanager wants to delete datastore, then just call deletedatastore, don't need add another api. In implementation of deletedatastore method, you can add a conditional check, if the datastore's state is not in ready state, then don't need to send DeleteStoragePoolCommand to hypervisor, delete the db entry instead. Checking for the storage pool state is a better idea, will do that and delete the new API. On June 20, 2013, 10:37 p.m., edison su wrote: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java, line 160 https://reviews.apache.org/r/11984/diff/1/?file=308930#file308930line160 Don't need to check exeception error message here, if adding storage pool failed, then just failed, rethrow the exception to upper layer. I thought checking it would be more granular, but you're right, it's unnecessary here. On June 20, 2013, 10:37 p.m., edison su wrote: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java, line 506 https://reviews.apache.org/r/11984/diff/1/?file=308930#file308930line506 How about log the exception stack trace also? Good idea, will log it. On June 20, 2013, 10:37 p.m., edison su wrote: server/src/com/cloud/storage/StorageManagerImpl.java, line 857 https://reviews.apache.org/r/11984/diff/1/?file=308932#file308932line857 Call lifecycle-deletedatastore Will do that, now that I'll put in the check for the storage pool state. - Venkata Siva Vijayendra --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11984/#review22205 --- On June 20, 2013, 2:42 a.m., Venkata Siva Vijayendra Bhamidipati wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11984/ --- (Updated June 20, 2013, 2:42 a.m.) Review request for cloudstack, Chip Childers, edison su, and Min Chen. Bugs: CLOUDSTACK-1510 Repository: cloudstack-git Description --- Patch for fixes for issues detected while working on bug CLOUDSTACK-1510 (https://issues.apache.org/jira/browse/CLOUDSTACK-1510). Diffs - api/src/org/apache/cloudstack/api/command/admin/storage/CreateStoragePoolCmd.java 74eb2b9 engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreLifeCycle.java cb46709 plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java 89e22c8 plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java fb37e8f server/src/com/cloud/storage/StorageManagerImpl.java 20b435c Diff: https://reviews.apache.org/r/11984/diff/ Testing --- Deploy KVM cluster in cloudstack. Attempt to add a primary NFS datastore using an invalid path. NPE is not encountered anymore. If KVM host is down or the cloud-agent on the KVM host is down, the primary datastore (whether valid or otherwise) is not logged to the db's storage_pool table. So invalid datastores do not show up in the GUI when listing the primary datastores available. Also, exception is propagated to GUI. Thanks, Venkata Siva Vijayendra Bhamidipati
Review Request 12180: Fix NPE when adding invalid primary datastore
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12180/ --- Review request for cloudstack, Chip Childers, Devdeep Singh, Kelven Yang, and Sateesh Chodapuneedi. Bugs: CLOUDSTACK-3110 Repository: cloudstack-git Description --- Fixing an NPE when an attempt is made to add a primary datastore by providing an unreachable/nonexistent datastore/host. Diffs - engine/storage/src/org/apache/cloudstack/storage/datastore/DataStoreManagerImpl.java b92f92f engine/storage/volume/src/org/apache/cloudstack/storage/datastore/manager/PrimaryDataStoreProviderManagerImpl.java 06b54e0 Diff: https://reviews.apache.org/r/12180/diff/ Testing --- With fix in place, an attempt to add an invalid primary datastore will fail with a GUI error dialog instead of failing silently with an NPE in the mgmt server log. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request 12180: Fix NPE when adding invalid primary datastore
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12180/ --- (Updated June 29, 2013, 12:48 a.m.) Review request for cloudstack, Chip Childers, Devdeep Singh, Kelven Yang, and Sateesh Chodapuneedi. Changes --- Attaching refactored patch for the master-6-17-stable branch. Bugs: CLOUDSTACK-3110 Repository: cloudstack-git Description --- Fixing an NPE when an attempt is made to add a primary datastore by providing an unreachable/nonexistent datastore/host. Diffs - engine/storage/src/org/apache/cloudstack/storage/datastore/DataStoreManagerImpl.java b92f92f engine/storage/volume/src/org/apache/cloudstack/storage/datastore/manager/PrimaryDataStoreProviderManagerImpl.java 06b54e0 Diff: https://reviews.apache.org/r/12180/diff/ Testing --- With fix in place, an attempt to add an invalid primary datastore will fail with a GUI error dialog instead of failing silently with an NPE in the mgmt server log. File Attachments (updated) Refactored patch for master-6-17-stable branch https://reviews.apache.org/media/uploaded/files/2013/06/29/0001-CLOUDSTACK-3110-VMWARE-NPE-while-adding-primary-stor.patch Thanks, Venkata Siva Vijayendra Bhamidipati
Review Request: Fix primary datastore NPE/incorrect db entry/exception propagation for KVM on cloudstack
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11984/ --- Review request for cloudstack, Chip Childers, edison su, and Min Chen. Description --- Patch for fixes for issues detected while working on bug CLOUDSTACK-1510 (https://issues.apache.org/jira/browse/CLOUDSTACK-1510). This addresses bug CLOUDSTACK-1510. Diffs - api/src/org/apache/cloudstack/api/command/admin/storage/CreateStoragePoolCmd.java 74eb2b9 engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreLifeCycle.java cb46709 plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java 89e22c8 plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java fb37e8f server/src/com/cloud/storage/StorageManagerImpl.java 20b435c Diff: https://reviews.apache.org/r/11984/diff/ Testing --- Deploy KVM cluster in cloudstack. Attempt to add a primary NFS datastore using an invalid path. NPE is not encountered anymore. If KVM host is down or the cloud-agent on the KVM host is down, the primary datastore (whether valid or otherwise) is not logged to the db's storage_pool table. So invalid datastores do not show up in the GUI when listing the primary datastores available. Also, exception is propagated to GUI. Thanks, Venkata Siva Vijayendra Bhamidipati
Review Request: Fix for NPE during ip range creation
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11850/ --- Review request for cloudstack, Chip Childers, Kelven Yang, bharat kumar, and Min Chen. Description --- Fixing an NPE encountered when creating advanced zone. Exception stack trace provided in https://issues.apache.org/jira/browse/CLOUDSTACK-2969 This addresses bug CLOUDSTACK-2969. Diffs - server/src/com/cloud/configuration/ConfigurationManagerImpl.java 111586d Diff: https://reviews.apache.org/r/11850/diff/ Testing --- Advanced zone creation proceeds correctly with fix in place. Thanks, Venkata Siva Vijayendra Bhamidipati
Review Request: Windows 8 (64 bit) guest OS enablement for KVM deployments in cloudstack
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11611/ --- Review request for cloudstack, edison su and Min Chen. Description --- Putting in Windows 8 (64 bit) as one of the options for guest OSes for KVM in cloudstack. This addresses bug CS-2827. Diffs - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/KVMGuestOsMapper.java bd1573c setup/db/templates.sql 1685dce Diff: https://reviews.apache.org/r/11611/diff/ Testing --- Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: PVLAN provisioning support for vmware Distributed Virtual Switch deployments on cloudstack.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11019/ --- (Updated May 14, 2013, 10:35 p.m.) Review request for cloudstack, Chip Childers, Sheng Yang, Sateesh Chodapuneedi, Kelven Yang, and Animesh Chaturvedi. Changes --- Uploading fully tested vmware changes for pvlan support. Tests carried out: == Step 0. Provision PVLAN primary/isolated secondary VLAN on physical switch that the physical hosts will connect to, following appropriate manuals of the switch vendor. Step 1. Set the vmware.use.dvswitch global flag to true, restart mgmt server. Step 2. Create vmware cluster in vCenter. Enable cluster for vMotion. Step 3. Create vmware DVS on the vmware datacenter that the cluster belongs to. Step 4. Create cloudstack zone/vmware cluster on mgmt server. Step 5. Login as admin to the mgmt server, and under - Infrastructure - Zones view all - your zone - Physical network tab - Physical network name - Guest configure - Network tab - add Guest Network : create a pvlan enabled advanced shared network (you can use the Offering for shared networks network offering available in the drop down menu). Specify the primary (promiscuous vlan id) and the secondary isolated pvlan id. Specify valid routable IP gateway/ip range/netmask values. Step 6. Create a guest VM having the above network. It should come up with an eth0 interface plumbed with a DHCP IP picked from the ip address range provided in step 5. Ping the router VM's ip (on the same subnet) - it should go through. Step 7. Create another guest VM having the same network. It should also come up with an IP picked from the ip address range in step 5. This VM should be able to ping the router VM ip, but not the guest VM created in step 6. Step 8. Create an isolated network under Network - Add Guest network. Create a new guest VM having both this new isolated network and the shared pvlan network created above. The VM should come up fine with two eth interfaces, each having an ip range from the specified ranges. Same ping behavior must be observed for interfaces between guest VMs in the pvlan network. Step 9. vMotion the guest VMs from one physical host to another. The migration must be smooth, the VM up, with no change seen in network configuration or behavior. Description --- Please find attached the diffs for pvlan support for vmware DVSwitch deployments on cloudstack. You will find two diffs - the parent diff is Sateesh's fix for CLOUSTACK-2316 which is needed to be cherry-picked on the pvlan branch from the master. The other diff contains the changes for pvlan support. These diffs do not contain changes for pvlan provisioning on the Cisco Nexus 1000v distributed virtual switch. This addresses bug CLOUDSTACK-1456. Diffs (updated) - api/src/com/cloud/agent/api/PlugNicCommand.java b896e45 plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 99ad1ca server/src/com/cloud/network/NetworkManagerImpl.java 7a09eb5 server/src/com/cloud/network/NetworkModelImpl.java bd62886 server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java bdfac06 server/src/com/cloud/vm/UserVmManagerImpl.java 683f0da server/src/com/cloud/vm/VirtualMachineManagerImpl.java b0d6378 vmware-base/src/com/cloud/hypervisor/vmware/mo/DistributedVirtualSwitchMO.java 247be2a vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java 7f323c5 Diff: https://reviews.apache.org/r/11019/diff/ Testing --- The code has been tested on the Vmware DVSwitch for advanced shared networks on vmware cluster deployments on cloudstack. Unit tests will be the same as those provided by Sheng as part of the overall PVLAN support for XenServer and KVM, and will exercise the vmware pvlan code path when user VMs are created with vNICs sitting on advanced shared networks that have the optional Private VLAN value set during their creation. VM live migration using vmware vMotion has also been tested with these changes on vmware and it works as expected. Further testing will be carried out and this review request will be updated accordingly. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: PVLAN provisioning support for vmware Distributed Virtual Switch deployments on cloudstack.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11019/ --- (Updated May 9, 2013, 5:47 p.m.) Review request for cloudstack, Chip Childers, Sateesh Chodapuneedi, Kelven Yang, and Animesh Chaturvedi. Changes --- +Animesh to list of reviewers. Description --- Please find attached the diffs for pvlan support for vmware DVSwitch deployments on cloudstack. You will find two diffs - the parent diff is Sateesh's fix for CLOUSTACK-2316 which is needed to be cherry-picked on the pvlan branch from the master. The other diff contains the changes for pvlan support. These diffs do not contain changes for pvlan provisioning on the Cisco Nexus 1000v distributed virtual switch. This addresses bug CLOUDSTACK-1456. Diffs - plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 99ad1ca server/src/com/cloud/network/NetworkManagerImpl.java 7a09eb5 server/src/com/cloud/network/NetworkModelImpl.java bd62886 server/src/com/cloud/vm/UserVmManagerImpl.java 683f0da server/src/com/cloud/vm/VirtualMachineManagerImpl.java b0d6378 vmware-base/src/com/cloud/hypervisor/vmware/mo/DistributedVirtualSwitchMO.java 247be2a vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java 7f323c5 Diff: https://reviews.apache.org/r/11019/diff/ Testing --- The code has been tested on the Vmware DVSwitch for advanced shared networks on vmware cluster deployments on cloudstack. Unit tests will be the same as those provided by Sheng as part of the overall PVLAN support for XenServer and KVM, and will exercise the vmware pvlan code path when user VMs are created with vNICs sitting on advanced shared networks that have the optional Private VLAN value set during their creation. VM live migration using vmware vMotion has also been tested with these changes on vmware and it works as expected. Further testing will be carried out and this review request will be updated accordingly. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: PVLAN provisioning support for vmware Distributed Virtual Switch deployments on cloudstack.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11019/ --- (Updated May 9, 2013, 7:09 p.m.) Review request for cloudstack, Chip Childers, Sheng Yang, Sateesh Chodapuneedi, Kelven Yang, and Animesh Chaturvedi. Changes --- +Sheng Description --- Please find attached the diffs for pvlan support for vmware DVSwitch deployments on cloudstack. You will find two diffs - the parent diff is Sateesh's fix for CLOUSTACK-2316 which is needed to be cherry-picked on the pvlan branch from the master. The other diff contains the changes for pvlan support. These diffs do not contain changes for pvlan provisioning on the Cisco Nexus 1000v distributed virtual switch. This addresses bug CLOUDSTACK-1456. Diffs - plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 99ad1ca server/src/com/cloud/network/NetworkManagerImpl.java 7a09eb5 server/src/com/cloud/network/NetworkModelImpl.java bd62886 server/src/com/cloud/vm/UserVmManagerImpl.java 683f0da server/src/com/cloud/vm/VirtualMachineManagerImpl.java b0d6378 vmware-base/src/com/cloud/hypervisor/vmware/mo/DistributedVirtualSwitchMO.java 247be2a vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java 7f323c5 Diff: https://reviews.apache.org/r/11019/diff/ Testing --- The code has been tested on the Vmware DVSwitch for advanced shared networks on vmware cluster deployments on cloudstack. Unit tests will be the same as those provided by Sheng as part of the overall PVLAN support for XenServer and KVM, and will exercise the vmware pvlan code path when user VMs are created with vNICs sitting on advanced shared networks that have the optional Private VLAN value set during their creation. VM live migration using vmware vMotion has also been tested with these changes on vmware and it works as expected. Further testing will be carried out and this review request will be updated accordingly. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: PVLAN provisioning support for vmware Distributed Virtual Switch deployments on cloudstack.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11019/ --- (Updated May 10, 2013, 2:32 a.m.) Review request for cloudstack, Chip Childers, Sheng Yang, Sateesh Chodapuneedi, Kelven Yang, and Animesh Chaturvedi. Changes --- Incorporating changes following Sheng's comments. Description --- Please find attached the diffs for pvlan support for vmware DVSwitch deployments on cloudstack. You will find two diffs - the parent diff is Sateesh's fix for CLOUSTACK-2316 which is needed to be cherry-picked on the pvlan branch from the master. The other diff contains the changes for pvlan support. These diffs do not contain changes for pvlan provisioning on the Cisco Nexus 1000v distributed virtual switch. This addresses bug CLOUDSTACK-1456. Diffs (updated) - plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 99ad1ca server/src/com/cloud/network/NetworkManagerImpl.java 7a09eb5 server/src/com/cloud/network/NetworkModelImpl.java bd62886 server/src/com/cloud/vm/UserVmManagerImpl.java 683f0da server/src/com/cloud/vm/VirtualMachineManagerImpl.java b0d6378 vmware-base/src/com/cloud/hypervisor/vmware/mo/DistributedVirtualSwitchMO.java 247be2a vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java 7f323c5 Diff: https://reviews.apache.org/r/11019/diff/ Testing --- The code has been tested on the Vmware DVSwitch for advanced shared networks on vmware cluster deployments on cloudstack. Unit tests will be the same as those provided by Sheng as part of the overall PVLAN support for XenServer and KVM, and will exercise the vmware pvlan code path when user VMs are created with vNICs sitting on advanced shared networks that have the optional Private VLAN value set during their creation. VM live migration using vmware vMotion has also been tested with these changes on vmware and it works as expected. Further testing will be carried out and this review request will be updated accordingly. Thanks, Venkata Siva Vijayendra Bhamidipati
Review Request: PVLAN support for vmware Distributed Virtual switch deployments on cloudstack
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11018/ --- Review request for cloudstack, Chip Childers, Sateesh Chodapuneedi, and Kelven Yang. Description --- Please find attached the diffs for pvlan support for vmware DVSwitch deployments on cloudstack. You will find two diffs - the parent diff is Sateesh's fix for CLOUSTACK-2316 which is needed to be cherry-picked on the pvlan branch from the master. The other diff contains the changes for pvlan support. This addresses bug CLOUDSTACK-1456. Diffs - engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java 25e7423 plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 99ad1ca server/src/com/cloud/network/NetworkManagerImpl.java 7a09eb5 server/src/com/cloud/network/NetworkModelImpl.java bd62886 server/src/com/cloud/vm/UserVmManagerImpl.java 683f0da server/src/com/cloud/vm/VirtualMachineManagerImpl.java b0d6378 vmware-base/src/com/cloud/hypervisor/vmware/mo/DatacenterMO.java 0a3e20b vmware-base/src/com/cloud/hypervisor/vmware/mo/DistributedVirtualSwitchMO.java 247be2a vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java 7f323c5 Diff: https://reviews.apache.org/r/11018/diff/ Testing --- The code has been tested on the Vmware DVSwitch for advanced shared networks on vmware cluster deployments on cloudstack. Unit tests will be the same as those provided by Sheng as part of the overall PVLAN support for XenServer and KVM, and will exercise the vmware pvlan code path when user VMs are created with vNICs sitting on advanced shared networks that have the optional Private VLAN value set during their creation. VM live migration using vmware vMotion has also been tested with these changes on vmware and it works as expected. Further testing will be carried out and this review request will be updated accordingly. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: PVLAN support for vmware Distributed Virtual switch deployments on cloudstack
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11018/ --- (Updated May 9, 2013, 2:08 a.m.) Review request for cloudstack, Chip Childers, Sateesh Chodapuneedi, and Kelven Yang. Description --- Please find attached the diffs for pvlan support for vmware DVSwitch deployments on cloudstack. You will find two diffs - the parent diff is Sateesh's fix for CLOUSTACK-2316 which is needed to be cherry-picked on the pvlan branch from the master. The other diff contains the changes for pvlan support. This addresses bug CLOUDSTACK-1456. Diffs - engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java 25e7423 plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 99ad1ca server/src/com/cloud/network/NetworkManagerImpl.java 7a09eb5 server/src/com/cloud/network/NetworkModelImpl.java bd62886 server/src/com/cloud/vm/UserVmManagerImpl.java 683f0da server/src/com/cloud/vm/VirtualMachineManagerImpl.java b0d6378 vmware-base/src/com/cloud/hypervisor/vmware/mo/DatacenterMO.java 0a3e20b vmware-base/src/com/cloud/hypervisor/vmware/mo/DistributedVirtualSwitchMO.java 247be2a vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java 7f323c5 Diff: https://reviews.apache.org/r/11018/diff/ Testing --- The code has been tested on the Vmware DVSwitch for advanced shared networks on vmware cluster deployments on cloudstack. Unit tests will be the same as those provided by Sheng as part of the overall PVLAN support for XenServer and KVM, and will exercise the vmware pvlan code path when user VMs are created with vNICs sitting on advanced shared networks that have the optional Private VLAN value set during their creation. VM live migration using vmware vMotion has also been tested with these changes on vmware and it works as expected. Further testing will be carried out and this review request will be updated accordingly. Thanks, Venkata Siva Vijayendra Bhamidipati
Review Request: PVLAN provisioning support for vmware Distributed Virtual Switch deployments on cloudstack.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11019/ --- Review request for cloudstack, Chip Childers, Sateesh Chodapuneedi, and Kelven Yang. Description --- Please find attached the diffs for pvlan support for vmware DVSwitch deployments on cloudstack. You will find two diffs - the parent diff is Sateesh's fix for CLOUSTACK-2316 which is needed to be cherry-picked on the pvlan branch from the master. The other diff contains the changes for pvlan support. These diffs do not contain changes for pvlan provisioning on the Cisco Nexus 1000v distributed virtual switch. This addresses bug CLOUDSTACK-1456. Diffs - plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 99ad1ca server/src/com/cloud/network/NetworkManagerImpl.java 7a09eb5 server/src/com/cloud/network/NetworkModelImpl.java bd62886 server/src/com/cloud/vm/UserVmManagerImpl.java 683f0da server/src/com/cloud/vm/VirtualMachineManagerImpl.java b0d6378 vmware-base/src/com/cloud/hypervisor/vmware/mo/DistributedVirtualSwitchMO.java 247be2a vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java 7f323c5 Diff: https://reviews.apache.org/r/11019/diff/ Testing --- The code has been tested on the Vmware DVSwitch for advanced shared networks on vmware cluster deployments on cloudstack. Unit tests will be the same as those provided by Sheng as part of the overall PVLAN support for XenServer and KVM, and will exercise the vmware pvlan code path when user VMs are created with vNICs sitting on advanced shared networks that have the optional Private VLAN value set during their creation. VM live migration using vmware vMotion has also been tested with these changes on vmware and it works as expected. Further testing will be carried out and this review request will be updated accordingly. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: Remove 2k limitation for user data on a deployVMCmd issued as an HTTP POST request
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/ --- (Updated April 23, 2013, 1:52 a.m.) Review request for cloudstack, Chip Childers, Hugo Trippaers, Kelven Yang, and Min Chen. Changes --- Script to create a VM using a GET request to send user data on integration port. Description --- Please refer to https://cwiki.apache.org/confluence/display/CLOUDSTACK/DeployVirtualMachine+userdata+enhancements for a background on the requirements driving this patch. This patch hasn't been extensively tested yet, and I will update this request with more info. I am uploading a first diff for initial review/comments. This addresses bug CLOUDSTACK-1086. Diffs - api/src/com/cloud/vm/UserVmService.java aa21136 api/src/org/apache/cloudstack/api/BaseCmd.java 42c0680 api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java 70c0159 api/src/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java ff8fff1 core/src/com/cloud/vm/UserVmVO.java a16eaf9 server/src/com/cloud/api/ApiDispatcher.java 925d90a server/src/com/cloud/api/ApiServer.java d842819 server/src/com/cloud/api/ApiServerService.java 12d8b52 server/src/com/cloud/api/ApiServlet.java 03bfb5f server/src/com/cloud/vm/UserVmManagerImpl.java 1843f60 server/test/com/cloud/vm/MockUserVmManagerImpl.java 0d0a8f4 server/test/com/cloud/vm/dao/UserVmDaoImplTest.java 0936180 server/test/com/cloud/vm/dao/UserVmDaoTestConfiguration.java PRE-CREATION server/test/resources/UserVMDaoTestContext.xml PRE-CREATION setup/db/db/schema-410to420.sql 10cdbba test/integration/component/test_deploy_vm_with_userdata.py PRE-CREATION Diff: https://reviews.apache.org/r/10294/diff/ Testing --- Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: Remove 2k limitation for user data on a deployVMCmd issued as an HTTP POST request
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/ --- (Updated April 23, 2013, 1:55 a.m.) Review request for cloudstack, Chip Childers, Hugo Trippaers, Kelven Yang, and Min Chen. Changes --- Uploading testing scripts. Description --- Please refer to https://cwiki.apache.org/confluence/display/CLOUDSTACK/DeployVirtualMachine+userdata+enhancements for a background on the requirements driving this patch. This patch hasn't been extensively tested yet, and I will update this request with more info. I am uploading a first diff for initial review/comments. This addresses bug CLOUDSTACK-1086. Diffs - api/src/com/cloud/vm/UserVmService.java aa21136 api/src/org/apache/cloudstack/api/BaseCmd.java 42c0680 api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java 70c0159 api/src/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java ff8fff1 core/src/com/cloud/vm/UserVmVO.java a16eaf9 server/src/com/cloud/api/ApiDispatcher.java 925d90a server/src/com/cloud/api/ApiServer.java d842819 server/src/com/cloud/api/ApiServerService.java 12d8b52 server/src/com/cloud/api/ApiServlet.java 03bfb5f server/src/com/cloud/vm/UserVmManagerImpl.java 1843f60 server/test/com/cloud/vm/MockUserVmManagerImpl.java 0d0a8f4 server/test/com/cloud/vm/dao/UserVmDaoImplTest.java 0936180 server/test/com/cloud/vm/dao/UserVmDaoTestConfiguration.java PRE-CREATION server/test/resources/UserVMDaoTestContext.xml PRE-CREATION setup/db/db/schema-410to420.sql 10cdbba test/integration/component/test_deploy_vm_with_userdata.py PRE-CREATION Diff: https://reviews.apache.org/r/10294/diff/ Testing --- Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: Remove 2k limitation for user data on a deployVMCmd issued as an HTTP POST request
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/ --- (Updated April 23, 2013, 1:57 a.m.) Review request for cloudstack, Chip Childers, Hugo Trippaers, Kelven Yang, and Min Chen. Description --- Please refer to https://cwiki.apache.org/confluence/display/CLOUDSTACK/DeployVirtualMachine+userdata+enhancements for a background on the requirements driving this patch. This patch hasn't been extensively tested yet, and I will update this request with more info. I am uploading a first diff for initial review/comments. This addresses bug CLOUDSTACK-1086. Diffs - api/src/com/cloud/vm/UserVmService.java aa21136 api/src/org/apache/cloudstack/api/BaseCmd.java 42c0680 api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java 70c0159 api/src/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java ff8fff1 core/src/com/cloud/vm/UserVmVO.java a16eaf9 server/src/com/cloud/api/ApiDispatcher.java 925d90a server/src/com/cloud/api/ApiServer.java d842819 server/src/com/cloud/api/ApiServerService.java 12d8b52 server/src/com/cloud/api/ApiServlet.java 03bfb5f server/src/com/cloud/vm/UserVmManagerImpl.java 1843f60 server/test/com/cloud/vm/MockUserVmManagerImpl.java 0d0a8f4 server/test/com/cloud/vm/dao/UserVmDaoImplTest.java 0936180 server/test/com/cloud/vm/dao/UserVmDaoTestConfiguration.java PRE-CREATION server/test/resources/UserVMDaoTestContext.xml PRE-CREATION setup/db/db/schema-410to420.sql 10cdbba test/integration/component/test_deploy_vm_with_userdata.py PRE-CREATION Diff: https://reviews.apache.org/r/10294/diff/ Testing (updated) --- Manual testing done with scripts generating large/small userdata during creation of VMs, with both GET and POST requests confirmed to work correctly, on both integration port and the 8080 port. Basic Marvin test to create a VM with user data has been written. Since marvin doesn't yet support POSTable APIs, as soon as that is made available for create/update VM operations, marvin tests will be written for the same. Requesting that this be noted as an AI for the future. Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: Remove 2k limitation for user data on a deployVMCmd issued as an HTTP POST request
On April 16, 2013, 4:31 a.m., Prasanna Santhanam wrote: tools/marvin/marvin/cloudstackConnection.py, line 61 https://reviews.apache.org/r/10294/diff/4/?file=281521#file281521line61 How and when does marvin decide to use POST? Which commands will call the POST httpmethod? May be you can provide a more complete example when your test is ready. I saw that make_request() was calling into make_request_with_auth(), so I put in the default value for httpmethod as GET, but missed the fact that make_request_with_auth() is calling itself when retrying - thanks for catching that - I've put in a default value of GET for this parameter. When a unittest calls this function, it would be expected to pass in all arguments in case it wants to use POST, but existing tests that simply use GET don't need to change.. does that look ok? Or is there a better way for me to implement this? On April 16, 2013, 4:31 a.m., Prasanna Santhanam wrote: tools/marvin/marvin/cloudstackConnection.py, line 77 https://reviews.apache.org/r/10294/diff/4/?file=281521#file281521line77 Are you sure you don't want to urlencode the POST data? Yes that is definitely better than doing requestUrl = .join([=.join([r[0], urllib.quote_plus(str(r[1]))]) for r in request]) , which won't take care of spaces either.. have made the change here and even in make_request_without_auth().. On April 16, 2013, 4:31 a.m., Prasanna Santhanam wrote: tools/marvin/marvin/cloudstackConnection.py, line 79 https://reviews.apache.org/r/10294/diff/4/?file=281521#file281521line79 Minor nitpick: urllib2.Request() can be used for both GET and POST and you can set the data argument only when you intend to use POST. That will reduce some code duplication here. I changed both cases to use urllib2.Request(), but still it's still the same number of lines :) Is there a way I can merge those two separate calls to urllib2.Request(url) and urllib2.Request(url, requestUrl) into a single call? - Venkata Siva Vijayendra --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/#review19245 --- On April 16, 2013, 2:53 a.m., Venkata Siva Vijayendra Bhamidipati wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/ --- (Updated April 16, 2013, 2:53 a.m.) Review request for cloudstack, Chip Childers, Hugo Trippaers, Kelven Yang, and Min Chen. Description --- Please refer to https://cwiki.apache.org/confluence/display/CLOUDSTACK/DeployVirtualMachine+userdata+enhancements for a background on the requirements driving this patch. This patch hasn't been extensively tested yet, and I will update this request with more info. I am uploading a first diff for initial review/comments. This addresses bug CLOUDSTACK-1086. Diffs - api/src/com/cloud/vm/UserVmService.java d963b74 api/src/org/apache/cloudstack/api/BaseCmd.java 42c0680 api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java 77ba9fe core/src/com/cloud/vm/UserVmVO.java a16eaf9 server/src/com/cloud/api/ApiDispatcher.java 925d90a server/src/com/cloud/api/ApiServer.java d842819 server/src/com/cloud/api/ApiServerService.java 12d8b52 server/src/com/cloud/api/ApiServlet.java 03bfb5f server/src/com/cloud/vm/UserVmManagerImpl.java d281e5b server/test/com/cloud/vm/MockUserVmManagerImpl.java fd826d9 server/test/com/cloud/vm/dao/UserVmDaoImplTest.java 0936180 server/test/com/cloud/vm/dao/UserVmDaoTestConfiguration.java PRE-CREATION server/test/resources/UserVMDaoTestContext.xml PRE-CREATION setup/db/db/schema-410to420.sql fb760bf tools/marvin/marvin/cloudstackConnection.py 1caeef3 Diff: https://reviews.apache.org/r/10294/diff/ Testing --- Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: Remove 2k limitation for user data on a deployVMCmd issued as an HTTP POST request
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/ --- (Updated April 17, 2013, 5:44 a.m.) Review request for cloudstack, Chip Childers, Hugo Trippaers, Kelven Yang, and Min Chen. Changes --- Incorporating changes suggested by Prasanna. Have tested the code for both GET and POST HTTP calls. GET fails as expected when 2K of userdata is provided, and succeeds when less than 2K is passed in. POST succeeds when upto 32K of userdata is supplied and fails if the userdata decoded size exceeds 32K. Random userdata was generated using userdata = b'%s'%(os.urandom(x*1024)) where x is the desired number of KB. I'm having a few issues with marvin in my development environment, and am working on putting in the marvin test. The standalone test scripts worked, it's just a matter of getting the marvin test in place. Description --- Please refer to https://cwiki.apache.org/confluence/display/CLOUDSTACK/DeployVirtualMachine+userdata+enhancements for a background on the requirements driving this patch. This patch hasn't been extensively tested yet, and I will update this request with more info. I am uploading a first diff for initial review/comments. This addresses bug CLOUDSTACK-1086. Diffs (updated) - api/src/com/cloud/vm/UserVmService.java d963b74 api/src/org/apache/cloudstack/api/BaseCmd.java 42c0680 api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java 77ba9fe core/src/com/cloud/vm/UserVmVO.java a16eaf9 server/src/com/cloud/api/ApiDispatcher.java 925d90a server/src/com/cloud/api/ApiServer.java d842819 server/src/com/cloud/api/ApiServerService.java 12d8b52 server/src/com/cloud/api/ApiServlet.java 03bfb5f server/src/com/cloud/vm/UserVmManagerImpl.java 53a57c8 server/test/com/cloud/vm/MockUserVmManagerImpl.java fd826d9 server/test/com/cloud/vm/dao/UserVmDaoImplTest.java 0936180 server/test/com/cloud/vm/dao/UserVmDaoTestConfiguration.java PRE-CREATION server/test/resources/UserVMDaoTestContext.xml PRE-CREATION setup/db/db/schema-410to420.sql 6a7a72c tools/marvin/marvin/cloudstackConnection.py 1caeef3 Diff: https://reviews.apache.org/r/10294/diff/ Testing --- Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: Remove 2k limitation for user data on a deployVMCmd issued as an HTTP POST request
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/ --- (Updated April 16, 2013, 2:53 a.m.) Review request for cloudstack, Chip Childers, Hugo Trippaers, Kelven Yang, and Min Chen. Changes --- Please find the latest diff attached with the following changes: - Marvin client API changes to support POST requests by clients - Inclusion of unittest files missing from earlier patch (UserVmDaoTestConfiguration.java and UserVMDaoTestContext.xml) - Fixed a bug discovered during testing w.r.t httpmethod value being populated in the parameter map in the integration port code path The marvin HTTP API itself is still to be put in. I'm working on it, will put it in at the earliest. Tests already exist for DeployVirtualMachineCmd. I will need to make appropriate changes to add to those tests, a case where userdata 2K is fired using a POST request. Description --- Please refer to https://cwiki.apache.org/confluence/display/CLOUDSTACK/DeployVirtualMachine+userdata+enhancements for a background on the requirements driving this patch. This patch hasn't been extensively tested yet, and I will update this request with more info. I am uploading a first diff for initial review/comments. This addresses bug CLOUDSTACK-1086. Diffs (updated) - api/src/com/cloud/vm/UserVmService.java d963b74 api/src/org/apache/cloudstack/api/BaseCmd.java 42c0680 api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java 77ba9fe core/src/com/cloud/vm/UserVmVO.java a16eaf9 server/src/com/cloud/api/ApiDispatcher.java 925d90a server/src/com/cloud/api/ApiServer.java d842819 server/src/com/cloud/api/ApiServerService.java 12d8b52 server/src/com/cloud/api/ApiServlet.java 03bfb5f server/src/com/cloud/vm/UserVmManagerImpl.java d281e5b server/test/com/cloud/vm/MockUserVmManagerImpl.java fd826d9 server/test/com/cloud/vm/dao/UserVmDaoImplTest.java 0936180 server/test/com/cloud/vm/dao/UserVmDaoTestConfiguration.java PRE-CREATION server/test/resources/UserVMDaoTestContext.xml PRE-CREATION setup/db/db/schema-410to420.sql fb760bf tools/marvin/marvin/cloudstackConnection.py 1caeef3 Diff: https://reviews.apache.org/r/10294/diff/ Testing --- Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: Remove 2k limitation for user data on a deployVMCmd issued as an HTTP POST request
On April 11, 2013, 8:55 p.m., Min Chen wrote: In your description, you mentioned that it is not extensively tested yet. Is this still true? If not, please revise your description to avoid confusing people. Not true anymore - apologies for the confusion. I'll put in details of all testing done when I upload the new diffs (wip). - Venkata Siva Vijayendra --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/#review19032 --- On April 16, 2013, 2:53 a.m., Venkata Siva Vijayendra Bhamidipati wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/ --- (Updated April 16, 2013, 2:53 a.m.) Review request for cloudstack, Chip Childers, Hugo Trippaers, Kelven Yang, and Min Chen. Description --- Please refer to https://cwiki.apache.org/confluence/display/CLOUDSTACK/DeployVirtualMachine+userdata+enhancements for a background on the requirements driving this patch. This patch hasn't been extensively tested yet, and I will update this request with more info. I am uploading a first diff for initial review/comments. This addresses bug CLOUDSTACK-1086. Diffs - api/src/com/cloud/vm/UserVmService.java d963b74 api/src/org/apache/cloudstack/api/BaseCmd.java 42c0680 api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java 77ba9fe core/src/com/cloud/vm/UserVmVO.java a16eaf9 server/src/com/cloud/api/ApiDispatcher.java 925d90a server/src/com/cloud/api/ApiServer.java d842819 server/src/com/cloud/api/ApiServerService.java 12d8b52 server/src/com/cloud/api/ApiServlet.java 03bfb5f server/src/com/cloud/vm/UserVmManagerImpl.java d281e5b server/test/com/cloud/vm/MockUserVmManagerImpl.java fd826d9 server/test/com/cloud/vm/dao/UserVmDaoImplTest.java 0936180 server/test/com/cloud/vm/dao/UserVmDaoTestConfiguration.java PRE-CREATION server/test/resources/UserVMDaoTestContext.xml PRE-CREATION setup/db/db/schema-410to420.sql fb760bf tools/marvin/marvin/cloudstackConnection.py 1caeef3 Diff: https://reviews.apache.org/r/10294/diff/ Testing --- Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: Remove 2k limitation for user data on a deployVMCmd issued as an HTTP POST request
On April 11, 2013, 8:54 p.m., Min Chen wrote: server/src/com/cloud/vm/UserVmManagerImpl.java, line 2581 https://reviews.apache.org/r/10294/diff/3/?file=280319#file280319line2581 Based on your comment, for GET, maxBytes = 4K, for POST, maxBytes = 32K. Why not define these constants instead of an intermediate MAX_USER_DATA_LENGTH_BYTES and do some math calculation here? This code is hard to maintain later in case that http length standard changes later. I just used the existing constant value, but yes, defining explicit constants for userdata length for different http methods will be cleaner. Will do that. On April 11, 2013, 8:54 p.m., Min Chen wrote: server/test/com/cloud/vm/dao/UserVmDaoImplTest.java, line 61 https://reviews.apache.org/r/10294/diff/3/?file=280321#file280321line61 This test only tests database persistence. We may need another automated test to issue a POST request. Will add a marvin test for that. On April 11, 2013, 8:54 p.m., Min Chen wrote: server/test/com/cloud/vm/dao/UserVmDaoTestConfiguration.java, line 1 https://reviews.apache.org/r/10294/diff/3/?file=280322#file280322line1 Where are UserVmDaoTestConfiguration used in your test? You are missing some Spring Junit annotation in your UserVmDaoImplTest. Thanks for catching this - looks like the Testconfiguration.java and textcontext.xml files slipped out of the diffs - when uploading the new diffs, I'll ensure that they're present. - Venkata Siva Vijayendra --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/#review19030 --- On April 10, 2013, 9:59 p.m., Venkata Siva Vijayendra Bhamidipati wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/ --- (Updated April 10, 2013, 9:59 p.m.) Review request for cloudstack, Chip Childers, Hugo Trippaers, Kelven Yang, and Min Chen. Description --- Please refer to https://cwiki.apache.org/confluence/display/CLOUDSTACK/DeployVirtualMachine+userdata+enhancements for a background on the requirements driving this patch. This patch hasn't been extensively tested yet, and I will update this request with more info. I am uploading a first diff for initial review/comments. This addresses bug CLOUDSTACK-1086. Diffs - api/src/com/cloud/vm/UserVmService.java 2c33d41 api/src/org/apache/cloudstack/api/BaseCmd.java 8fef422 api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java 21a45f8 core/src/com/cloud/vm/UserVmVO.java a16eaf9 server/src/com/cloud/api/ApiDispatcher.java 925d90a server/src/com/cloud/api/ApiServer.java d842819 server/src/com/cloud/api/ApiServerService.java 12d8b52 server/src/com/cloud/api/ApiServlet.java 03bfb5f server/src/com/cloud/vm/UserVmManagerImpl.java 1c3764a server/test/com/cloud/vm/MockUserVmManagerImpl.java dd8dd83 server/test/com/cloud/vm/dao/UserVmDaoImplTest.java 0936180 server/test/com/cloud/vm/dao/UserVmDaoTestConfiguration.java PRE-CREATION server/test/resources/UserVMDaoTestContext.xml PRE-CREATION setup/db/db/schema-410to420.sql c7c8b5b Diff: https://reviews.apache.org/r/10294/diff/ Testing --- Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: Remove 2k limitation for user data on a deployVMCmd issued as an HTTP POST request
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/ --- (Updated April 10, 2013, 9:59 p.m.) Review request for cloudstack, Chip Childers, Hugo Trippaers, Kelven Yang, and Min Chen. Changes --- Adding unit tests. Description --- Please refer to https://cwiki.apache.org/confluence/display/CLOUDSTACK/DeployVirtualMachine+userdata+enhancements for a background on the requirements driving this patch. This patch hasn't been extensively tested yet, and I will update this request with more info. I am uploading a first diff for initial review/comments. This addresses bug CLOUDSTACK-1086. Diffs (updated) - api/src/com/cloud/vm/UserVmService.java 2c33d41 api/src/org/apache/cloudstack/api/BaseCmd.java 8fef422 api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java 21a45f8 core/src/com/cloud/vm/UserVmVO.java a16eaf9 server/src/com/cloud/api/ApiDispatcher.java 925d90a server/src/com/cloud/api/ApiServer.java d842819 server/src/com/cloud/api/ApiServerService.java 12d8b52 server/src/com/cloud/api/ApiServlet.java 03bfb5f server/src/com/cloud/vm/UserVmManagerImpl.java 1c3764a server/test/com/cloud/vm/MockUserVmManagerImpl.java dd8dd83 server/test/com/cloud/vm/dao/UserVmDaoImplTest.java 0936180 server/test/com/cloud/vm/dao/UserVmDaoTestConfiguration.java PRE-CREATION server/test/resources/UserVMDaoTestContext.xml PRE-CREATION setup/db/db/schema-410to420.sql c7c8b5b Diff: https://reviews.apache.org/r/10294/diff/ Testing --- Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: Remove 2k limitation for user data on a deployVMCmd issued as an HTTP POST request
On April 5, 2013, 5:46 p.m., Rohit Yadav wrote: As long as we have query based API, the overall base64 encoded payload in the url cannot exceed this limit as per a RFC (I recall discussing this issue on ML when I applied this feature from previous ports). If we handle POST request method on the ApiServlet class and accept userdate from POST request, this would work. Cheers. Venkata Siva Vijayendra Bhamidipati wrote: Yes, the POST will be handled by ApiServlet's doPost() only, and the userdata will need to be put into the POST body. I've put in a check to allow the 2k limit to be exceeded only for userdata that comes in via a post request in the deployVMcmd codepath. I think the length limit that a POST body can reach is specified in the webserver configuration and independent of CS as such, unless that limit is configured too low - in that case I'm assuming the webserver wouldn't even pass on the request to reach the ApiServlet in CS - is that the case? Rohit Yadav wrote: No, it would the trick is to first understand how we parse and create apiname and args from the GET query and I would prefer that we don't look for only userdata in POST request, but create a map of apicmd and list of args from the POST request generically so not just userdata any API can be called or becomes POST-able HTTP request. Summary: Make every API POST-able, that way any param could be sent via a POST request. Cheers. Actually any POST command can be handled by the ApiServlet code - the map of api command and args list is created and supplied to the servlet by the web server. API calls issued as POST requests currently work with the mgmt server, so no changes are required to that part of the code. - Venkata Siva Vijayendra --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/#review18713 --- On April 5, 2013, 10:22 p.m., Venkata Siva Vijayendra Bhamidipati wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/ --- (Updated April 5, 2013, 10:22 p.m.) Review request for cloudstack, Chip Childers, Hugo Trippaers, Kelven Yang, and Min Chen. Description --- Please refer to https://cwiki.apache.org/confluence/display/CLOUDSTACK/DeployVirtualMachine+userdata+enhancements for a background on the requirements driving this patch. This patch hasn't been extensively tested yet, and I will update this request with more info. I am uploading a first diff for initial review/comments. This addresses bug CLOUDSTACK-1086. Diffs - api/src/com/cloud/vm/UserVmService.java 2c33d41 api/src/org/apache/cloudstack/api/BaseCmd.java 78a2af3 api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java 21a45f8 core/src/com/cloud/vm/UserVmVO.java a16eaf9 server/src/com/cloud/api/ApiDispatcher.java 925d90a server/src/com/cloud/api/ApiServer.java d842819 server/src/com/cloud/api/ApiServerService.java 12d8b52 server/src/com/cloud/api/ApiServlet.java 03bfb5f server/src/com/cloud/vm/UserVmManagerImpl.java 24bce8b server/test/com/cloud/vm/MockUserVmManagerImpl.java dd8dd83 setup/db/db/schema-410to420.sql ab9df05 Diff: https://reviews.apache.org/r/10294/diff/ Testing --- Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: Remove 2k limitation for user data on a deployVMCmd issued as an HTTP POST request
On April 5, 2013, 4:03 p.m., Chip Childers wrote: Is there any automated testing of this feature today? Marvin tests of the HTTP service? Unit tests? If not, now seems like a good time to add some. I realize you are just updating the size of the data field, but IMO we should strive to improve automated tests with everything we touch. There probably are automated tests already for the deployvmcmd itself, and that should cover some of the functionality, for example, current GET requests should continue to work with the expanded userdata field. I will take a look though to see if more tests can be added. On April 5, 2013, 4:03 p.m., Chip Childers wrote: setup/db/create-schema.sql, line 1131 https://reviews.apache.org/r/10294/diff/1/?file=278165#file278165line1131 I don't believe we should be modifying create-schema.sql for this. See: https://cwiki.apache.org/confluence/display/CLOUDSTACK/Database+Creator It says: WE NEVER CHANGE schema files such as create-schema.sql Thanks for catching that - I'll remove it. - Venkata Siva Vijayendra --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/#review18704 --- On April 5, 2013, 1:56 a.m., Venkata Siva Vijayendra Bhamidipati wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/ --- (Updated April 5, 2013, 1:56 a.m.) Review request for cloudstack, Chip Childers, Hugo Trippaers, Kelven Yang, and Min Chen. Description --- Please refer to https://cwiki.apache.org/confluence/display/CLOUDSTACK/DeployVirtualMachine+userdata+enhancements for a background on the requirements driving this patch. This patch hasn't been extensively tested yet, and I will update this request with more info. I am uploading a first diff for initial review/comments. This addresses bug CLOUDSTACK-1086. Diffs - api/src/com/cloud/vm/UserVmService.java 2c33d41 api/src/org/apache/cloudstack/api/BaseCmd.java 78a2af3 api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java 21a45f8 core/src/com/cloud/vm/UserVmVO.java a16eaf9 server/src/com/cloud/api/ApiDispatcher.java 925d90a server/src/com/cloud/api/ApiServer.java d842819 server/src/com/cloud/api/ApiServerService.java 12d8b52 server/src/com/cloud/api/ApiServlet.java 03bfb5f server/src/com/cloud/vm/UserVmManagerImpl.java 24bce8b server/test/com/cloud/vm/MockUserVmManagerImpl.java dd8dd83 setup/db/create-schema.sql b1feb02 setup/db/db/schema-410to420.sql ab9df05 Diff: https://reviews.apache.org/r/10294/diff/ Testing --- Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: Remove 2k limitation for user data on a deployVMCmd issued as an HTTP POST request
On April 5, 2013, 5:46 p.m., Rohit Yadav wrote: As long as we have query based API, the overall base64 encoded payload in the url cannot exceed this limit as per a RFC (I recall discussing this issue on ML when I applied this feature from previous ports). If we handle POST request method on the ApiServlet class and accept userdate from POST request, this would work. Cheers. Yes, the POST will be handled by ApiServlet's doPost() only, and the userdata will need to be put into the POST body. I've put in a check to allow the 2k limit to be exceeded only for userdata that comes in via a post request in the deployVMcmd codepath. I think the length limit that a POST body can reach is specified in the webserver configuration and independent of CS as such, unless that limit is configured too low - in that case I'm assuming the webserver wouldn't even pass on the request to reach the ApiServlet in CS - is that the case? - Venkata Siva Vijayendra --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/#review18713 --- On April 5, 2013, 1:56 a.m., Venkata Siva Vijayendra Bhamidipati wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/ --- (Updated April 5, 2013, 1:56 a.m.) Review request for cloudstack, Chip Childers, Hugo Trippaers, Kelven Yang, and Min Chen. Description --- Please refer to https://cwiki.apache.org/confluence/display/CLOUDSTACK/DeployVirtualMachine+userdata+enhancements for a background on the requirements driving this patch. This patch hasn't been extensively tested yet, and I will update this request with more info. I am uploading a first diff for initial review/comments. This addresses bug CLOUDSTACK-1086. Diffs - api/src/com/cloud/vm/UserVmService.java 2c33d41 api/src/org/apache/cloudstack/api/BaseCmd.java 78a2af3 api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java 21a45f8 core/src/com/cloud/vm/UserVmVO.java a16eaf9 server/src/com/cloud/api/ApiDispatcher.java 925d90a server/src/com/cloud/api/ApiServer.java d842819 server/src/com/cloud/api/ApiServerService.java 12d8b52 server/src/com/cloud/api/ApiServlet.java 03bfb5f server/src/com/cloud/vm/UserVmManagerImpl.java 24bce8b server/test/com/cloud/vm/MockUserVmManagerImpl.java dd8dd83 setup/db/create-schema.sql b1feb02 setup/db/db/schema-410to420.sql ab9df05 Diff: https://reviews.apache.org/r/10294/diff/ Testing --- Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: Remove 2k limitation for user data on a deployVMCmd issued as an HTTP POST request
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/ --- (Updated April 5, 2013, 10:22 p.m.) Review request for cloudstack, Chip Childers, Hugo Trippaers, Kelven Yang, and Min Chen. Changes --- Removing column datatype modification in schema-create.sql. Adding a lazy fetch annotation to the user_data column in the UserVMVO class to improve read performance. Having this will cause ehcache to not read in the user_data column every time it loads in the VO object from the db. This helps especially if the userdata is 32k in size and we don't need to work with it. If we do need to access it, ehcache will pull it in dynamically. Description --- Please refer to https://cwiki.apache.org/confluence/display/CLOUDSTACK/DeployVirtualMachine+userdata+enhancements for a background on the requirements driving this patch. This patch hasn't been extensively tested yet, and I will update this request with more info. I am uploading a first diff for initial review/comments. This addresses bug CLOUDSTACK-1086. Diffs (updated) - api/src/com/cloud/vm/UserVmService.java 2c33d41 api/src/org/apache/cloudstack/api/BaseCmd.java 78a2af3 api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java 21a45f8 core/src/com/cloud/vm/UserVmVO.java a16eaf9 server/src/com/cloud/api/ApiDispatcher.java 925d90a server/src/com/cloud/api/ApiServer.java d842819 server/src/com/cloud/api/ApiServerService.java 12d8b52 server/src/com/cloud/api/ApiServlet.java 03bfb5f server/src/com/cloud/vm/UserVmManagerImpl.java 24bce8b server/test/com/cloud/vm/MockUserVmManagerImpl.java dd8dd83 setup/db/db/schema-410to420.sql ab9df05 Diff: https://reviews.apache.org/r/10294/diff/ Testing --- Thanks, Venkata Siva Vijayendra Bhamidipati
Review Request: Remove 2k limitation for user data on a deployVMCmd issued as an HTTP POST request
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/ --- Review request for cloudstack, Chip Childers, Hugo Trippaers, Kelven Yang, and Min Chen. Description --- Please refer to https://cwiki.apache.org/confluence/display/CLOUDSTACK/DeployVirtualMachine+userdata+enhancements for a background on the requirements driving this patch. This patch hasn't been extensively tested yet, and I will update this request with more info. I am uploading a first diff for initial review/comments. This addresses bug CLOUDSTACK-1086. Diffs - api/src/com/cloud/vm/UserVmService.java 2c33d41 api/src/org/apache/cloudstack/api/BaseCmd.java 78a2af3 api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java 21a45f8 core/src/com/cloud/vm/UserVmVO.java a16eaf9 server/src/com/cloud/api/ApiDispatcher.java 925d90a server/src/com/cloud/api/ApiServer.java d842819 server/src/com/cloud/api/ApiServerService.java 12d8b52 server/src/com/cloud/api/ApiServlet.java 03bfb5f server/src/com/cloud/vm/UserVmManagerImpl.java 24bce8b server/test/com/cloud/vm/MockUserVmManagerImpl.java dd8dd83 setup/db/create-schema.sql b1feb02 setup/db/db/schema-410to420.sql ab9df05 Diff: https://reviews.apache.org/r/10294/diff/ Testing --- Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: Make SHA256Salt the default password encoding and authentication mechanism for cloudstack
On March 29, 2013, 5:22 p.m., Min Chen wrote: api/src/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java, line 66 https://reviews.apache.org/r/10039/diff/1/?file=272336#file272336line66 If password is default hashed to SHA256SALT, then it should not be clear text password. This description is contradictory to me. Hi Min, this is the password that needs to be sent in by the client - it will need to be the clear text password itself - even if plaintext authentication is being used and the password passed in is itself is a hash, technically it would still be a clear text password that matches the password stored in the db. That is what is reflected in the description above.. The sha encoding kicks in only on the server side. If the cleartext is to be protected, SSL via HTTPS would serve that purpose. On March 29, 2013, 5:22 p.m., Min Chen wrote: developer/developer-prefill.sql, line 39 https://reviews.apache.org/r/10039/diff/1/?file=272341#file272341line39 Should this password be encoded using SHA if that is the default way? The pre-filled one is still MD5. If the admin user is created as enabled, the mgmt server will leave the password as is and not change it. However, if created as disabled, the mgmt server code, when creating the admin user, will replace the password for the admin user as hash(password) where the hash function is the default function configured. - Venkata Siva Vijayendra --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10039/#review18519 --- On March 28, 2013, 8:26 p.m., Venkata Siva Vijayendra Bhamidipati wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10039/ --- (Updated March 28, 2013, 8:26 p.m.) Review request for cloudstack, Hugo Trippaers, Kelven Yang, and Min Chen. Description --- Changing default password encoding mechanism from MD5 to SHA256Salted. This addresses bug CS-1734. Diffs - api/src/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java 89673ea api/src/org/apache/cloudstack/api/command/admin/user/CreateUserCmd.java fb29e1a api/src/org/apache/cloudstack/api/command/admin/user/UpdateUserCmd.java 1f31662 client/tomcatconf/componentContext.xml.in 016df0a client/tomcatconf/nonossComponentContext.xml.in 8f8dae5 developer/developer-prefill.sql 6300d35 plugins/user-authenticators/ldap/src/com/cloud/server/auth/LDAPUserAuthenticator.java 61eebe5 plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java 026125e plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java 52e7cb3 plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java 1b29f69 server/src/com/cloud/server/ManagementServerImpl.java b689f93 server/src/com/cloud/user/AccountManagerImpl.java b69f314 Diff: https://reviews.apache.org/r/10039/diff/ Testing --- Manual testing done for both oss and nonoss components. Both admin and users added later are encoded according to the scheme configured, and authenticated by the same scheme. To change the order of the schemes, modify the following list properties in client/tomcatconf/nonossComponentContext.xml.in or client/tomcatconf/componentContext.xml.in as applicable, to the desired order: property name=UserAuthenticators list ref bean=SHA256SaltedUserAuthenticator/ ref bean=MD5UserAuthenticator/ ref bean=LDAPUserAuthenticator/ ref bean=PlainTextUserAuthenticator/ /list /property property name=UserPasswordEncoders list ref bean=SHA256SaltedUserAuthenticator/ ref bean=MD5UserAuthenticator/ ref bean=LDAPUserAuthenticator/ ref bean=PlainTextUserAuthenticator/ /list Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: Make SHA256Salt the default password encoding and authentication mechanism for cloudstack
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10039/ --- (Updated April 2, 2013, 5:42 a.m.) Review request for cloudstack, Hugo Trippaers, Kelven Yang, and Min Chen. Changes --- Attaching latest diffs on top of master, incorporating changes suggested by Min. Description --- Changing default password encoding mechanism from MD5 to SHA256Salted. This addresses bug CS-1734. Diffs (updated) - api/src/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java 89673ea api/src/org/apache/cloudstack/api/command/admin/user/CreateUserCmd.java fb29e1a api/src/org/apache/cloudstack/api/command/admin/user/UpdateUserCmd.java 1f31662 client/tomcatconf/applicationContext.xml.in 636eac2 client/tomcatconf/componentContext.xml.in fea1d0f client/tomcatconf/nonossComponentContext.xml.in 0b02eb6 developer/developer-prefill.sql 6300d35 plugins/user-authenticators/ldap/src/com/cloud/server/auth/LDAPUserAuthenticator.java 61eebe5 plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java 026125e plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java 52e7cb3 plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java 1b29f69 server/src/com/cloud/server/ManagementServerImpl.java d0904e1 server/src/com/cloud/user/AccountManagerImpl.java 40db4ed Diff: https://reviews.apache.org/r/10039/diff/ Testing --- Manual testing done for both oss and nonoss components. Both admin and users added later are encoded according to the scheme configured, and authenticated by the same scheme. To change the order of the schemes, modify the following list properties in client/tomcatconf/nonossComponentContext.xml.in or client/tomcatconf/componentContext.xml.in as applicable, to the desired order: property name=UserAuthenticators list ref bean=SHA256SaltedUserAuthenticator/ ref bean=MD5UserAuthenticator/ ref bean=LDAPUserAuthenticator/ ref bean=PlainTextUserAuthenticator/ /list /property property name=UserPasswordEncoders list ref bean=SHA256SaltedUserAuthenticator/ ref bean=MD5UserAuthenticator/ ref bean=LDAPUserAuthenticator/ ref bean=PlainTextUserAuthenticator/ /list Thanks, Venkata Siva Vijayendra Bhamidipati
Re: Review Request: Temporarily disabling baremetal functionality in CS 4.1
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10071/ --- (Updated March 22, 2013, 11:55 p.m.) Review request for cloudstack, Kelven Yang and Frank Zhang. Changes --- Hi Chip! Reuploading a consolidated patch with the sql changes as well, plus another change in the plugins/pom.xml file - this is on top of the 4.1 branch - generated using git format-patch.. can you please check and let me if it applies? Thanks! Regards, Vijay Description --- Temporarily disabling baremetal functionality in CS 4.1 This addresses bug CS-1773. Diffs (updated) - client/pom.xml 38ba405 client/tomcatconf/componentContext.xml.in ff7376e client/tomcatconf/nonossComponentContext.xml.in 35e1b28 plugins/pom.xml 02459b4 server/src/com/cloud/configuration/Config.java 17fe62b setup/db/db/schema-40to410.sql 754bfb6 Diff: https://reviews.apache.org/r/10071/diff/ Testing --- Baremetal and Cisco UCS related APIs confirmed to not be exposed to clients. Thanks, Venkata Siva Vijayendra Bhamidipati
