[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15892015#comment-15892015
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user priyankparihar commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1302#discussion_r103892827
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -2226,17 +2227,33 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 return new String[] {datastoreDiskPath};
 }
 
-// Pair
-private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+/**
+ * This method generates VM name for Vcenter and Cloudstack( when 
Hypervisor is VMware).
+ * It generates VM name according to  _instanceNameFlag setting.
+ *
+ * @param VirtualMachineTO
+ *vmSpec
+ * @return Pair. A pair which 
contain 'internal CS name'  and
+ * 'vCenter display name'(vCenter knows VM by this name).
+ **/
+protected Pair composeVmNames(VirtualMachineTO vmSpec) 
{
+
+String vmInternalCSName = "";
+String vmNameOnVcenter  = "";
+if(vmSpec != null)
+{
+vmInternalCSName = vmNameOnVcenter = vmSpec.getName();
+if (_instanceNameFlag == true && vmSpec.getType() == 
VirtualMachine.Type.User) {
+String[] tokens = vmInternalCSName.split("-");
+if(tokens.length<3) throw new 
InvalidParameterValueException("Invalid vmInternalCSName format: 
"+vmInternalCSName+" expected format : *-*-* (e.g -> i-x-y)"); // 
vmInternalCSName has format i-x-y-
+vmNameOnVcenter = String.format("%s-%s-%s-%s", tokens[0], 
tokens[1], tokens[2], vmSpec.getHostName());
+}
+
 }
 return new Pair(vmInternalCSName, vmNameOnVcenter);
 }
 
-protected void configNestedHVSupport(VirtualMachineMO vmMo, 
VirtualMachineTO vmSpec, VirtualMachineConfigSpec vmConfigSpec) throws 
Exception {
+protected static void configNestedHVSupport(VirtualMachineMO vmMo, 
VirtualMachineTO vmSpec, VirtualMachineConfigSpec vmConfigSpec) throws 
Exception {
--- End diff --

Removed.


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>Assignee: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15892012#comment-15892012
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user priyankparihar commented on the issue:

https://github.com/apache/cloudstack/pull/1302
  
@rhtyd I think now all of your questions are answered. Please provide your 
LGTM comment. 


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>Assignee: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15891902#comment-15891902
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user priyankparihar commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1302#discussion_r103880795
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -2226,17 +2227,33 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 return new String[] {datastoreDiskPath};
 }
 
-// Pair
-private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+/**
+ * This method generates VM name for Vcenter and Cloudstack( when 
Hypervisor is VMware).
+ * It generates VM name according to  _instanceNameFlag setting.
+ *
+ * @param VirtualMachineTO
+ *vmSpec
+ * @return Pair. A pair which 
contain 'internal CS name'  and
+ * 'vCenter display name'(vCenter knows VM by this name).
+ **/
+protected Pair composeVmNames(VirtualMachineTO vmSpec) 
{
+
+String vmInternalCSName = "";
+String vmNameOnVcenter  = "";
+if(vmSpec != null)
+{
+vmInternalCSName = vmNameOnVcenter = vmSpec.getName();
+if (_instanceNameFlag == true && vmSpec.getType() == 
VirtualMachine.Type.User) {
+String[] tokens = vmInternalCSName.split("-");
+if(tokens.length<3) throw new 
InvalidParameterValueException("Invalid vmInternalCSName format: 
"+vmInternalCSName+" expected format : *-*-* (e.g -> i-x-y)"); // 
vmInternalCSName has format i-x-y-
+vmNameOnVcenter = String.format("%s-%s-%s-%s", tokens[0], 
tokens[1], tokens[2], vmSpec.getHostName());
--- End diff --

>As per previous code, vmNameOnVcenter = vmSpec.getHostName(), which will 
introduce backward compatibility issues for older 4.3 environments. Is there 
any code that handles for such old VMs.

Yes, vm.instancename.flag will handle backward compatibility issue. 

Case (vm.instancename.flag = True):
User can create the VM with same name form different account. 
Case (vm.instancename.flag = False):
User can not create the VM with same name form different account.
Note - But in both the cases, VM with same name are not allowed on a 
particular network. And only for the User VM's name format will be like 
"I--" if "vm.instancename.flag = True". It will not 
cause any impact on other VM names( like Router VM).


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>Assignee: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15887787#comment-15887787
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user priyankparihar commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1302#discussion_r103427642
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -2226,17 +2227,33 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 return new String[] {datastoreDiskPath};
 }
 
-// Pair
-private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+/**
+ * This method generates VM name for Vcenter and Cloudstack( when 
Hypervisor is VMware).
+ * It generates VM name according to  _instanceNameFlag setting.
+ *
+ * @param VirtualMachineTO
+ *vmSpec
+ * @return Pair. A pair which 
contain 'internal CS name'  and
+ * 'vCenter display name'(vCenter knows VM by this name).
+ **/
+protected Pair composeVmNames(VirtualMachineTO vmSpec) 
{
+
+String vmInternalCSName = "";
+String vmNameOnVcenter  = "";
+if(vmSpec != null)
+{
+vmInternalCSName = vmNameOnVcenter = vmSpec.getName();
+if (_instanceNameFlag == true && vmSpec.getType() == 
VirtualMachine.Type.User) {
--- End diff --

@rhtyd Good point. Code will be modified. 


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>Assignee: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15887784#comment-15887784
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user priyankparihar commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1302#discussion_r103427150
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -2226,17 +2227,33 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 return new String[] {datastoreDiskPath};
 }
 
-// Pair
-private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+/**
+ * This method generates VM name for Vcenter and Cloudstack( when 
Hypervisor is VMware).
+ * It generates VM name according to  _instanceNameFlag setting.
+ *
+ * @param VirtualMachineTO
+ *vmSpec
+ * @return Pair. A pair which 
contain 'internal CS name'  and
+ * 'vCenter display name'(vCenter knows VM by this name).
+ **/
+protected Pair composeVmNames(VirtualMachineTO vmSpec) 
{
+
--- End diff --

>Why is the method attribute changed from private to protected?

Otherwise Unit testing will be difficult. 

>If vmspec is null, this method should throw an exception, to avoid a 
situation where empty vmnames are returned.

In CS vmspec can not be null. But still code will be  modified according 
your suggestion.



> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>Assignee: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15887764#comment-15887764
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user priyankparihar commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1302#discussion_r103423690
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -2226,17 +2227,33 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 return new String[] {datastoreDiskPath};
 }
 
-// Pair
-private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+/**
+ * This method generates VM name for Vcenter and Cloudstack( when 
Hypervisor is VMware).
+ * It generates VM name according to  _instanceNameFlag setting.
+ *
+ * @param VirtualMachineTO
+ *vmSpec
+ * @return Pair. A pair which 
contain 'internal CS name'  and
+ * 'vCenter display name'(vCenter knows VM by this name).
+ **/
+protected Pair composeVmNames(VirtualMachineTO vmSpec) 
{
+
+String vmInternalCSName = "";
+String vmNameOnVcenter  = "";
+if(vmSpec != null)
+{
+vmInternalCSName = vmNameOnVcenter = vmSpec.getName();
+if (_instanceNameFlag == true && vmSpec.getType() == 
VirtualMachine.Type.User) {
+String[] tokens = vmInternalCSName.split("-");
+if(tokens.length<3) throw new 
InvalidParameterValueException("Invalid vmInternalCSName format: 
"+vmInternalCSName+" expected format : *-*-* (e.g -> i-x-y)"); // 
vmInternalCSName has format i-x-y-
+vmNameOnVcenter = String.format("%s-%s-%s-%s", tokens[0], 
tokens[1], tokens[2], vmSpec.getHostName());
--- End diff --

@sateesh-chodapuneedi what is your opinion ?


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>Assignee: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15887616#comment-15887616
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user rhtyd commented on the issue:

https://github.com/apache/cloudstack/pull/1302
  
I've left some outstanding comments, I'm -1 on the PR.


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>Assignee: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15887610#comment-15887610
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user rhtyd commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1302#discussion_r103405279
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -2226,17 +2227,33 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 return new String[] {datastoreDiskPath};
 }
 
-// Pair
-private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+/**
+ * This method generates VM name for Vcenter and Cloudstack( when 
Hypervisor is VMware).
+ * It generates VM name according to  _instanceNameFlag setting.
+ *
+ * @param VirtualMachineTO
+ *vmSpec
+ * @return Pair. A pair which 
contain 'internal CS name'  and
+ * 'vCenter display name'(vCenter knows VM by this name).
+ **/
+protected Pair composeVmNames(VirtualMachineTO vmSpec) 
{
+
+String vmInternalCSName = "";
+String vmNameOnVcenter  = "";
+if(vmSpec != null)
+{
+vmInternalCSName = vmNameOnVcenter = vmSpec.getName();
+if (_instanceNameFlag == true && vmSpec.getType() == 
VirtualMachine.Type.User) {
+String[] tokens = vmInternalCSName.split("-");
+if(tokens.length<3) throw new 
InvalidParameterValueException("Invalid vmInternalCSName format: 
"+vmInternalCSName+" expected format : *-*-* (e.g -> i-x-y)"); // 
vmInternalCSName has format i-x-y-
+vmNameOnVcenter = String.format("%s-%s-%s-%s", tokens[0], 
tokens[1], tokens[2], vmSpec.getHostName());
--- End diff --

The token length comparison may break backward compatibility for existing 
VMs that may not conform to this scheme. Also, assuming the token length is 3 
the line can be written as:
`vmNameOnVcenter = String.format("%s-%s", vmInternalCSName, 
vmSpec.getHostName());`

As per previous code, `vmNameOnVcenter = vmSpec.getHostName()`, which will 
introduce backward compatibility issues for older 4.3 environments. Is there 
any code that handles for such old VMs. If a 4.3 user decides to upgrade to 
4.10+, their VMs might not start the next time someone restarts. Please comment 
on how the change addresses backward compatibility issue.


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>Assignee: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15887612#comment-15887612
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user rhtyd commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1302#discussion_r103405341
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -2226,17 +2227,33 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 return new String[] {datastoreDiskPath};
 }
 
-// Pair
-private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+/**
+ * This method generates VM name for Vcenter and Cloudstack( when 
Hypervisor is VMware).
+ * It generates VM name according to  _instanceNameFlag setting.
+ *
+ * @param VirtualMachineTO
+ *vmSpec
+ * @return Pair. A pair which 
contain 'internal CS name'  and
+ * 'vCenter display name'(vCenter knows VM by this name).
+ **/
+protected Pair composeVmNames(VirtualMachineTO vmSpec) 
{
+
+String vmInternalCSName = "";
+String vmNameOnVcenter  = "";
+if(vmSpec != null)
+{
+vmInternalCSName = vmNameOnVcenter = vmSpec.getName();
+if (_instanceNameFlag == true && vmSpec.getType() == 
VirtualMachine.Type.User) {
+String[] tokens = vmInternalCSName.split("-");
+if(tokens.length<3) throw new 
InvalidParameterValueException("Invalid vmInternalCSName format: 
"+vmInternalCSName+" expected format : *-*-* (e.g -> i-x-y)"); // 
vmInternalCSName has format i-x-y-
+vmNameOnVcenter = String.format("%s-%s-%s-%s", tokens[0], 
tokens[1], tokens[2], vmSpec.getHostName());
+}
+
 }
 return new Pair(vmInternalCSName, vmNameOnVcenter);
 }
 
-protected void configNestedHVSupport(VirtualMachineMO vmMo, 
VirtualMachineTO vmSpec, VirtualMachineConfigSpec vmConfigSpec) throws 
Exception {
+protected static void configNestedHVSupport(VirtualMachineMO vmMo, 
VirtualMachineTO vmSpec, VirtualMachineConfigSpec vmConfigSpec) throws 
Exception {
--- End diff --

Please explain why `static` was added, thanks.


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>Assignee: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15887604#comment-15887604
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user rhtyd commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1302#discussion_r103404645
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -2226,17 +2227,33 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 return new String[] {datastoreDiskPath};
 }
 
-// Pair
-private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+/**
+ * This method generates VM name for Vcenter and Cloudstack( when 
Hypervisor is VMware).
+ * It generates VM name according to  _instanceNameFlag setting.
+ *
+ * @param VirtualMachineTO
+ *vmSpec
+ * @return Pair. A pair which 
contain 'internal CS name'  and
+ * 'vCenter display name'(vCenter knows VM by this name).
+ **/
+protected Pair composeVmNames(VirtualMachineTO vmSpec) 
{
+
+String vmInternalCSName = "";
+String vmNameOnVcenter  = "";
+if(vmSpec != null)
+{
+vmInternalCSName = vmNameOnVcenter = vmSpec.getName();
+if (_instanceNameFlag == true && vmSpec.getType() == 
VirtualMachine.Type.User) {
--- End diff --

If _instanceNameFlag is boolean, so there is no need of an explicit `== 
true`. 


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>Assignee: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15887601#comment-15887601
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user rhtyd commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1302#discussion_r103404351
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -2226,17 +2227,33 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 return new String[] {datastoreDiskPath};
 }
 
-// Pair
-private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+/**
+ * This method generates VM name for Vcenter and Cloudstack( when 
Hypervisor is VMware).
+ * It generates VM name according to  _instanceNameFlag setting.
+ *
+ * @param VirtualMachineTO
+ *vmSpec
+ * @return Pair. A pair which 
contain 'internal CS name'  and
+ * 'vCenter display name'(vCenter knows VM by this name).
+ **/
+protected Pair composeVmNames(VirtualMachineTO vmSpec) 
{
+
--- End diff --

- Why is the method attribute changed from private to protected?
- If vmspec is null, this method should throw an exception, to avoid a 
situation where empty vmnames are returned. Discuss?


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>Assignee: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15887565#comment-15887565
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user blueorangutan commented on the issue:

https://github.com/apache/cloudstack/pull/1302
  
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-543


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>Assignee: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15887514#comment-15887514
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user rhtyd commented on the issue:

https://github.com/apache/cloudstack/pull/1302
  
@blueorangutan package


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>Assignee: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15887518#comment-15887518
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user blueorangutan commented on the issue:

https://github.com/apache/cloudstack/pull/1302
  
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you 
posted as I make progress.


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>Assignee: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15886924#comment-15886924
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user karuturi commented on the issue:

https://github.com/apache/cloudstack/pull/1302
  
@blueorangutan @rhtyd @borisroman vmware tests please


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>Assignee: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15878309#comment-15878309
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user priyankparihar commented on the issue:

https://github.com/apache/cloudstack/pull/1302
  
Hi @borisstoyanov Please check.


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>Assignee: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15358371#comment-15358371
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user priyankparihar commented on the issue:

https://github.com/apache/cloudstack/pull/1302
  
Hi @rhtyd and @sateesh-chodapuneedi,
I have made modification, according to your suggestion. Please take a look. 


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15251748#comment-15251748
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user bhaisaab commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1302#discussion_r60565861
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -2030,12 +2030,29 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 return new String[] {datastoreDiskPath};
 }
 
-// Pair
-private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+
+/**
+ * This method generates VM name for Vcenter and Cloudstack( when 
Hypervisor is VMware).
+ * It generates VM name according to  _instanceNameFlag setting.
+ *
+ * @param VirtualMachineTO
+ *vmSpec
+ * @return Pair. A pair which 
contain 'internal CS name'  and
+ * 'vCenter display name'(vCenter knows VM by this name).
+ **/
+private Pair composeVmNatmes(VirtualMachineTO vmSpec) {
+
+String vmInternalCSName = null;
+String vmNameOnVcenter  = null;
+if(vmSpec != null)
+{
+vmInternalCSName = vmNameOnVcenter = vmSpec.getName();
+if (_instanceNameFlag == true && 
vmSpec.getType()==VirtualMachine.Type.User) {
+String[] tokens = vmInternalCSName.split("-");
+assert (tokens.length >= 3); // vmInternalCSName has 
format i-x-y-
+vmNameOnVcenter = String.format("%s-%s-%s-%s", tokens[0], 
tokens[1], tokens[2], vmSpec.getHostName());
--- End diff --

what if one of the tokens are null/empty? why not simply add the hostname 
(if only it's not null/empty) at the end of vmInternalCSName; simply do a 
`vmInternalCSName + "-" + vmSpace.getHostName()` (iff 
!Strings.isNullOrEmpty(getHostName()))

Using the same internal CS name on vmware should be alright and ideal, 
solve the issue of inconsistent and unique naming.


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15251746#comment-15251746
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/1302#issuecomment-212873785
  
@priyankparihar we'll need a unit test that shows that this change is 
backward compatible. I like that we're considering both default instance name 
with host name here (i.e. the names should be globally unique) but we'll need 
few more units tests to make sure the contract is not broken again in future.

We've changed how internal/external vmware vm names are composed 3-4 times 
since 4.0, can you explain why we want to do this again.


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15251723#comment-15251723
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1302#discussion_r60562805
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -2030,12 +2030,29 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 return new String[] {datastoreDiskPath};
 }
 
-// Pair
-private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+
+/**
+ * This method generates VM name for Vcenter and Cloudstack( when 
Hypervisor is VMware).
+ * It generates VM name according to  _instanceNameFlag setting.
+ *
+ * @param VirtualMachineTO
+ *vmSpec
+ * @return Pair. A pair which 
contain 'internal CS name'  and
+ * 'vCenter display name'(vCenter knows VM by this name).
+ **/
+private Pair composeVmNatmes(VirtualMachineTO vmSpec) {
+
+String vmInternalCSName = null;
+String vmNameOnVcenter  = null;
+if(vmSpec != null)
+{
+vmInternalCSName = vmNameOnVcenter = vmSpec.getName();
+if (_instanceNameFlag == true && 
vmSpec.getType()==VirtualMachine.Type.User) {
+String[] tokens = vmInternalCSName.split("-");
+assert (tokens.length >= 3); // vmInternalCSName has 
format i-x-y-
--- End diff --

Guava's ``Preconditions.checkState`` is terse way to implement such checks.


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15251719#comment-15251719
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user sateesh-chodapuneedi commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1302#discussion_r60562370
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -2030,12 +2030,29 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 return new String[] {datastoreDiskPath};
 }
 
-// Pair
-private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+
+/**
+ * This method generates VM name for Vcenter and Cloudstack( when 
Hypervisor is VMware).
+ * It generates VM name according to  _instanceNameFlag setting.
+ *
+ * @param VirtualMachineTO
+ *vmSpec
+ * @return Pair. A pair which 
contain 'internal CS name'  and
+ * 'vCenter display name'(vCenter knows VM by this name).
+ **/
+private Pair composeVmNatmes(VirtualMachineTO vmSpec) {
--- End diff --

@priyankparihar 
Can you rename composeVmNatmes to composeVmNames?


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15251716#comment-15251716
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user sateesh-chodapuneedi commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1302#discussion_r60562268
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -2030,12 +2030,29 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 return new String[] {datastoreDiskPath};
 }
 
-// Pair
-private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+
+/**
+ * This method generates VM name for Vcenter and Cloudstack( when 
Hypervisor is VMware).
+ * It generates VM name according to  _instanceNameFlag setting.
+ *
+ * @param VirtualMachineTO
+ *vmSpec
+ * @return Pair. A pair which 
contain 'internal CS name'  and
+ * 'vCenter display name'(vCenter knows VM by this name).
+ **/
+private Pair composeVmNatmes(VirtualMachineTO vmSpec) {
--- End diff --

@priyankparihar 
Can you please add a unit test for composeVmNatmes?


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15251714#comment-15251714
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user sateesh-chodapuneedi commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1302#discussion_r60562111
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -2030,12 +2030,29 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 return new String[] {datastoreDiskPath};
 }
 
-// Pair
-private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+
+/**
+ * This method generates VM name for Vcenter and Cloudstack( when 
Hypervisor is VMware).
+ * It generates VM name according to  _instanceNameFlag setting.
+ *
+ * @param VirtualMachineTO
+ *vmSpec
+ * @return Pair. A pair which 
contain 'internal CS name'  and
+ * 'vCenter display name'(vCenter knows VM by this name).
+ **/
+private Pair composeVmNatmes(VirtualMachineTO vmSpec) {
+
+String vmInternalCSName = null;
+String vmNameOnVcenter  = null;
+if(vmSpec != null)
+{
+vmInternalCSName = vmNameOnVcenter = vmSpec.getName();
+if (_instanceNameFlag == true && 
vmSpec.getType()==VirtualMachine.Type.User) {
--- End diff --

add a whitespace before and after '=='


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15251707#comment-15251707
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user sateesh-chodapuneedi commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1302#discussion_r60561591
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -2030,12 +2030,29 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 return new String[] {datastoreDiskPath};
 }
 
-// Pair
-private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+
+/**
+ * This method generates VM name for Vcenter and Cloudstack( when 
Hypervisor is VMware).
+ * It generates VM name according to  _instanceNameFlag setting.
+ *
+ * @param VirtualMachineTO
+ *vmSpec
+ * @return Pair. A pair which 
contain 'internal CS name'  and
+ * 'vCenter display name'(vCenter knows VM by this name).
+ **/
+private Pair composeVmNatmes(VirtualMachineTO vmSpec) {
+
+String vmInternalCSName = null;
+String vmNameOnVcenter  = null;
+if(vmSpec != null)
+{
+vmInternalCSName = vmNameOnVcenter = vmSpec.getName();
+if (_instanceNameFlag == true && 
vmSpec.getType()==VirtualMachine.Type.User) {
+String[] tokens = vmInternalCSName.split("-");
+assert (tokens.length >= 3); // vmInternalCSName has 
format i-x-y-
--- End diff --

Because assertions may be disabled, programs must not assume that the 
boolean expression contained in an assertion will be evaluated. Try to check it 
as well.


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15249517#comment-15249517
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user priyankparihar commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1302#discussion_r60372700
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -2030,12 +2030,29 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 return new String[] {datastoreDiskPath};
 }
 
-// Pair
-private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+
+/**
+ * This method gemerate VM name for Vcenter and Cloudstack( when 
Hypervisor is VMware).
--- End diff --

Hi  @alexandrelimassantana,
Now, i think there is no typo.

--
Thanks for your careful observation.


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15155715#comment-15155715
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user alexandrelimassantana commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1302#discussion_r53554647
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -2030,12 +2030,29 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 return new String[] {datastoreDiskPath};
 }
 
-// Pair
-private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+
+/**
+ * This method gemerate VM name for Vcenter and Cloudstack( when 
Hypervisor is VMware).
--- End diff --

Just a little typo in line 2035, "gemerate".

Thanks for taking the time to write the javadoc.


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15129858#comment-15129858
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user priyankparihar commented on the pull request:

https://github.com/apache/cloudstack/pull/1302#issuecomment-179028659
  
Hi, 
@alexandrelimassantana thanks for notifying. I have made modification,  
according to your suggestion. 



> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15114735#comment-15114735
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user alexandrelimassantana commented on the pull request:

https://github.com/apache/cloudstack/pull/1302#issuecomment-174396396
  
@priyankparihar, since you are modifying this method, you could also 
generate it's javadoc. It would be a nice upgrade from that single line comment 
placed just before the method signature ( line 2033 ). You could even state 
your parameter null check in the @parameter field in it.


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15088965#comment-15088965
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user priyankparihar commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1302#discussion_r49171092
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -2032,11 +2032,20 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 
 // Pair
 private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+
+String vmInternalCSName = null;
+String vmNameOnVcenter  = null;
+if(vmSpec != null)
--- End diff --

@rafaelweingartner Currently. it is not possible. Because user does not 
generate it. Cloud stack generates it only if it has all required info. 
otherwise it does not call this function.


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15089128#comment-15089128
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user rafaelweingartner commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1302#discussion_r49183061
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -2032,11 +2032,20 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 
 // Pair
 private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+
+String vmInternalCSName = null;
+String vmNameOnVcenter  = null;
+if(vmSpec != null)
--- End diff --

If today there is no way a null value is received in that method, I see no 
reason to have that check in 2038. 


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15080715#comment-15080715
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


GitHub user priyankparihar opened a pull request:

https://github.com/apache/cloudstack/pull/1302

CLOUDSTACK-9104: VM naming convention in case vmware is used

I have reverted all the changes. Now functionality is same as it was in 
earlier version. 
ACS  Bug Link is as follow ==>
https://issues.apache.org/jira/browse/CLOUDSTACK-9104




You can merge this pull request into a Git repository by running:

$ git pull https://github.com/priyankparihar/cloudstack CLOUDSTACK-9104

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1302.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1302


commit fd2cba99b354d54f1a101591455b39f78bab2e3f
Author: Priyank Parihar 
Date:   2015-11-05T15:18:00Z

CLOUDSTACK-9104: VM naming convention in case vmware is used




> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15080714#comment-15080714
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user priyankparihar closed the pull request at:

https://github.com/apache/cloudstack/pull/1165


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15055768#comment-15055768
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/1165#issuecomment-164401688
  
@priyankparihar I won't be able to give a LGTM without the null check, 
split string related code and a unit test that breaks if someone tries to 
change the convention in future


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15055698#comment-15055698
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user priyankparihar commented on the pull request:

https://github.com/apache/cloudstack/pull/1165#issuecomment-164388358
  
@bhaisaab I think null check is not required. I have verified( because in 
case of null if automatically picks  some random value). But if you still think 
it may break something then please notify.


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15055605#comment-15055605
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user bhaisaab commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1165#discussion_r47469773
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -1889,11 +1889,20 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 
 // Pair
 private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+
+String vmInternalCSName = null;
+String vmNameOnVcenter  = null;
+if(vmSpec != null)
+{
+vmInternalCSName = vmNameOnVcenter = vmSpec.getName();
+if (_instanceNameFlag == true) {
+String[] tokens = vmInternalCSName.split("-");
+assert (tokens.length >= 3); // vmInternalCSName has 
format i-x-y-
+vmNameOnVcenter = String.format("%s-%s-%s-%s", tokens[0], 
tokens[1], tokens[2], vmSpec.getHostName());
--- End diff --

@priyankparihar alright, even if we go with this change. If 
vmSpec.getHostName() was null then we've a problem with the vm name (if 
_instanceNameFlag, i.e. the global setting was true).

Once you put a null check, it should be good to go.


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15055607#comment-15055607
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/1165#issuecomment-164374286
  
@priyankparihar Fair enough, please see a code review comment. Once you fix 
the null check, I think your PR is good to go. Also, write a unit test to 
enforce the changes; i.e. the unit test should break in future if someone tries 
to change the VM naming convention in future.


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15055608#comment-15055608
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user bhaisaab commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1165#discussion_r47469908
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -1889,11 +1889,20 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 
 // Pair
 private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+
+String vmInternalCSName = null;
+String vmNameOnVcenter  = null;
+if(vmSpec != null)
+{
+vmInternalCSName = vmNameOnVcenter = vmSpec.getName();
+if (_instanceNameFlag == true) {
+String[] tokens = vmInternalCSName.split("-");
+assert (tokens.length >= 3); // vmInternalCSName has 
format i-x-y-
+vmNameOnVcenter = String.format("%s-%s-%s-%s", tokens[0], 
tokens[1], tokens[2], vmSpec.getHostName());
--- End diff --

Do something like;
String hostString = vmSpec.getHostName();
if (hostString == null) {
  hostString = // get value from the vm suffix global setting (by default 
"VM");
}


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15055462#comment-15055462
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user priyankparihar commented on the pull request:

https://github.com/apache/cloudstack/pull/1165#issuecomment-164348885
  
@bhaisaab 
User with different account cannot create VMs with the same name, which was 
possible earlier (I am not sure in which CCP version). That time naming 
convention used was like this “I--”
Currently if vm.instancename.flag is set to true the VM name will be 
exactly as display name given.



> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15054584#comment-15054584
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1165#issuecomment-164178352
  
@bhaisaab @priyankparihar what is the verdict on this change?


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15044091#comment-15044091
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1165#issuecomment-162341273
  
@priyankparihar I do not know any vmware users
@bhaisaab Do you have any insight on this change? or do you know any 
candidate reviewer for this?


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15044382#comment-15044382
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user bhaisaab commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1165#discussion_r46787171
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -1889,11 +1889,20 @@ int getReservedCpuMHZ(VirtualMachineTO vmSpec) {
 
 // Pair
 private Pair composeVmNames(VirtualMachineTO vmSpec) {
-String vmInternalCSName = vmSpec.getName();
-String vmNameOnVcenter = vmSpec.getName();
-if (_instanceNameFlag && vmSpec.getHostName() != null) {
-vmNameOnVcenter = vmSpec.getHostName();
+
+String vmInternalCSName = null;
+String vmNameOnVcenter  = null;
+if(vmSpec != null)
+{
+vmInternalCSName = vmNameOnVcenter = vmSpec.getName();
+if (_instanceNameFlag == true) {
+String[] tokens = vmInternalCSName.split("-");
+assert (tokens.length >= 3); // vmInternalCSName has 
format i-x-y-
+vmNameOnVcenter = String.format("%s-%s-%s-%s", tokens[0], 
tokens[1], tokens[2], vmSpec.getHostName());
--- End diff --

@priyankparihar can you comment on the issues and motivation around this? 
Do you think we have users who would want to keep hostname as VM names on 
vCenter side if the global setting is enabled (the _instanceNameFlag is true).

If this is not true, by default the name (as returned by the vmSpec) is 
like: i-[0-9]*-[0-9]*-VM. (The suffix "VM", is taken from another global 
setting where the default value is "VM").


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15044386#comment-15044386
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/1165#issuecomment-162413895
  
@DaanHoogland yes I can do that, I'm personally dealt with a lot of 
vmware/cloudstack naming issues. The syntax/pattern has changes several times 
in the past between 4.0, 4.2, 4.3 causing VM sync issues when users upgrade.


> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-9104) VM naming convention in case vmware is used

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15040811#comment-15040811
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9104:


GitHub user priyankparihar opened a pull request:

https://github.com/apache/cloudstack/pull/1165

CLOUDSTACK-9104: VM naming convention in case vmware is used

I have reverted all the changes. Now functionality is same as it was  in 
earlier version. 

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/priyankparihar/cloudstack CLOUDSTACK-9104

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1165.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1165


commit 2588925e9c82a9914b76e06204a04f4a5ff1ddb1
Author: Priyank Parihar 
Date:   2015-11-05T15:18:00Z

CLOUDSTACK-9104: VM naming convention in case vmware is used




> VM naming convention in case vmware is used
> ---
>
> Key: CLOUDSTACK-9104
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9104
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>Reporter: Priyank Parihar
>
> ISSUE
> ==
> VM naming convention in case vmware is used.
> Description
> ==
> User with different account cannot create VMs with the same name, which was 
> possible earlier (I am not sure in which CCP version). That time naming 
> convention used was like this “I--”
> Currently if vm.instancename.flag is set to true the VM name will be exactly 
> as display name given. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)