cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Venkata Siva Vijayendra Bhamidipati" <vijayendra.bhamidip...@citrix.com>
Subject Re: Review Request: Provide customizable instance names for guest VMs in cloudstack
Date Fri, 08 Feb 2013 03:27:23 GMT


> On Feb. 1, 2013, 9:50 a.m., Koushik Das wrote:
> > There is another config parameter instance.name based on which a fixed suffix is
appended to the internal name. What happens to this parameter in the context of this fix?
Also as part of this fix you are allowing to suffix the display name to internal name. Sometime
back I saw a request in the cs-user list to allow account name/id to be appended to the internal
instance name. Would it be possible to provide a list of well defined attributes (for e.g.
display name, account etc.) + fixed suffix.
> 
> Wei Zhou wrote:
>     Is it neccessary to add i-<>-<> as a suffix to displayname or internal
name?
>     
>     As a user, I totally agree with the requirements which were posted on https://cwiki.apache.org/confluence/display/CLOUDSTACK/Allow+user+provided+hostname%2C+internal+VM+name+on+hypervisor+for+guest+VMs
>     It seems that the result of this patch does not match the requirements.
>     
>     In setup/db/db/schema-40to410.sql, the field should be 'vm.instancename.flag', not
'vm.hostname.flag'.
> 
> Wei Zhou wrote:
>     Does this patch apply on master? 
>     I fail to apply on master, and change source codes manually. 
>     I have the same testing result which is fine.
>     
>     a suggestion:
>     can you change createDhcpEntryCommand(VirtualRouter, UserVm, NicVO, Commands) in
com.cloud.network.router.VirtualNetworkApplianceManagerImpl, so that the OS hostname of VM
can be set to displayname?

Hi Koushik, Wei,

Thanks for the reviews. Wei, thanks a lot for catching the vm.instancename.flag error in the
sql -- the diffs got mixed up and I will reupload the patch after checking that it applies
on top of tree (master).

The instance.name parameter is a global setting which would apply to all guest VMs. The requirement
was to have a way to easily identify each guest VM using a suffix that would match the display
name. The vm.instancename.flag will let the user set each guest VM's name differently. The
instance.name flag has been preserved to not change existing behavior.

The i-<>-<> notation as a suffix is required for vmsync scripts to identify and
differentiate between cloudstack and non cloudstack VMs, and also between system and guest
VMs. This is why we have an r/s/v/i-<>-<> naming convention for all VMs on cloudstack.
If this naming convention is not followed, scripts implementing vmsync functionality break.

These changes in the requirements were not captured in https://cwiki.apache.org/confluence/display/CLOUDSTACK/Allow+user+provided+hostname%2C+internal+VM+name+on+hypervisor+for+guest+VMs
I will edit the contents of the link to reflect the above points.

Also, it was determined that the hostname should not be changed as part of this feature. Will
update the above link with this as well.

Regarding generic attributes, yes indeed it should be possible to do that. We could have different
flags for the same, or do it in some other way. If the community wants to have it, I could
do it in another patch using a different issue id. In this context, I have a question - I
have set the maximum length of the VM internal instance name to 80 characters - if my memory
is right, I wasn't able to use more than 93 characters for ESX VMs. So I arbitrarily chose
80 as the max length allowed. I do not know what the max lengths are for other hypervisors
- if anyone knows these limits for sure, and provide pointers for the same, it would be helpful
- I will change the code accordingly.

Finally, automated tests for the create VM API command already exist under test/src/com/cloud/test/regression/.
The above changes will be covered by those tests.


Thanks,
Regards,
Vijay


- Venkata Siva Vijayendra


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9202/#review16001
-----------------------------------------------------------


On Jan. 31, 2013, 7:21 p.m., Venkata Siva Vijayendra Bhamidipati wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9202/
> -----------------------------------------------------------
> 
> (Updated Jan. 31, 2013, 7:21 p.m.)
> 
> 
> Review request for cloudstack, Kelven Yang and Frank Zhang.
> 
> 
> Description
> -------
> 
> Patch to allow user to append display name to internal instance name of guest VMs on
cloudstack during guest VM creation.
> 
> 
> This addresses bug CS-778.
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/configuration/Config.java 4ae144e 
>   server/src/com/cloud/vm/UserVmManagerImpl.java ecf1242 
>   server/src/com/cloud/vm/dao/VMInstanceDao.java 8b0a523 
>   server/src/com/cloud/vm/dao/VMInstanceDaoImpl.java 85ad5d0 
>   setup/db/db/schema-40to410.sql ed4946e 
> 
> Diff: https://reviews.apache.org/r/9202/diff/
> 
> 
> Testing
> -------
> 
> Manual Testing:
> Set the global parameter vm.instancename.flag to true, restart mgmt server, create a
guest VM and provide a display Name value when doing so. The vm created will have its internal
name in the form of i-<>-<>-displayname.
> Set the global parameter vm.instancename.flag back to false, restart mgmt server, and
create a guest VM providing the display Name. The vm created will have the internal name in
the form of i-<>-<>
> If no display name is provided during instance creation, the vm internal name will be
in the form of i-<>-<>.
> 
> 
> Thanks,
> 
> Venkata Siva Vijayendra Bhamidipati
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message