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