> 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.

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


- Kelven


-----------------------------------------------------------
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
> 
>

Reply via email to