incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Animesh Chaturvedi <animesh.chaturv...@citrix.com>
Subject RE: Review Request: Provide customizable instance names for guest VMs in cloudstack
Date Thu, 14 Feb 2013 03:08:20 GMT
Koushik

If you are satisfied with the patch please commit it

Animesh

> -----Original Message-----
> From: Venkata Siva Vijayendra Bhamidipati
> [mailto:noreply@reviews.apache.org] On Behalf Of Venkata Siva Vijayendra
> Bhamidipati
> Sent: Thursday, February 07, 2013 7:27 PM
> To: Frank Zhang; Kelven Yang
> Cc: Wei Zhou; Koushik Das; Vijayendra Bhamidipati; cloudstack
> Subject: Re: Review Request: Provide customizable instance names for guest
> VMs in cloudstack
> 
> 
> 
> > 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+provide
> d+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+provide
> d+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
View raw message