----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9202/#review16681 -----------------------------------------------------------
server/src/com/cloud/configuration/Config.java <https://reviews.apache.org/r/9202/#comment35388> Can you clarify 'set hostname and displayname to the conjoined value'? >From what I understand this feature is only to append display name to internal name. Also what is effect of setting this flag on existing VMs? Is there a possibility of internal name getting changed after performing any operations on the VMs? server/src/com/cloud/vm/UserVmManagerImpl.java <https://reviews.apache.org/r/9202/#comment35389> You had raised a question on this? Is it sorted out, if not discuss on the ML. server/src/com/cloud/vm/UserVmManagerImpl.java <https://reviews.apache.org/r/9202/#comment35387> parseBoolean can handle null, so no need to do null condition check server/src/com/cloud/vm/UserVmManagerImpl.java <https://reviews.apache.org/r/9202/#comment35386> Why do you need to pass member variable _instanceNameFlag to the method? You can access it directly from the method. server/src/com/cloud/vm/UserVmManagerImpl.java <https://reviews.apache.org/r/9202/#comment35390> So as per the code you are either appending 'instance.name' or 'displayname' but not both. This is not clear from the FS. Can you get some feedback from people in the apache list about the interference of these 2 config params - instance.name and vm.instancename.flag. I personally found them to be ambiguous from the names. server/src/com/cloud/vm/UserVmManagerImpl.java <https://reviews.apache.org/r/9202/#comment35391> You are passing the displayName to findVMByInstanceName, should this be the computed instanceName? setup/db/db/schema-40to410.sql <https://reviews.apache.org/r/9202/#comment35392> This feature is no longer part of 4.1. - Koushik Das 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 > >
