You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Koushik Das <ko...@citrix.com> on 2013/02/01 10:50:16 UTC

Re: Review Request: Provide customizable instance names for guest VMs in cloudstack

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


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.

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


Re: Review Request: Provide customizable instance names for guest VMs in cloudstack

Posted by Wei ZHOU <w....@leaseweb.com>.

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

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


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


Re: Review Request: Provide customizable instance names for guest VMs in cloudstack

Posted by Wei ZHOU <w....@leaseweb.com>.

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

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?


- Wei


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


RE: Review Request: Provide customizable instance names for guest VMs in cloudstack

Posted by Animesh Chaturvedi <an...@citrix.com>.
Koushik

Any updates?

Animesh

> -----Original Message-----
> From: Animesh Chaturvedi [mailto:animesh.chaturvedi@citrix.com]
> Sent: Wednesday, February 13, 2013 7:08 PM
> To: Vijayendra Bhamidipati; Frank Zhang; Kelven Yang
> Cc: Wei Zhou; Koushik Das; cloudstack
> Subject: RE: Review Request: Provide customizable instance names for guest
> VMs in cloudstack
> 
> 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+prov
> > ide
> > 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+prov
> > ide
> > 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
> > >
> > >


RE: Review Request: Provide customizable instance names for guest VMs in cloudstack

Posted by Animesh Chaturvedi <an...@citrix.com>.
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
> >
> >


Re: Review Request: Provide customizable instance names for guest VMs in cloudstack

Posted by Venkata Siva Vijayendra Bhamidipati <vi...@citrix.com>.

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