You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Hongtu Zang <ho...@tcloudcomputing.com> on 2013/02/13 06:07:59 UTC
Review Request: fix bug 1237 and 1240 : Register Template fails with "Cannot
find template adapter for XenServer"
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9420/
-----------------------------------------------------------
Review request for cloudstack, mice xia, anthony xu, and SrikanteswaraRao Talluri.
Description
-------
In TemplateManagerImpl.java, function getAdapter(), TemplateAdapterType.Hypervisor.getName() returns "HyervisorAdapter", while it should returns "HyervisorTemplateAdapter". So, in AdapterBase.java function getAdapterByName() returns null.
This addresses bugs CLOUDSTACK-1237 and CLOUDSTACK-1240.
Diffs
-----
server/src/com/cloud/template/TemplateAdapter.java 19cfef0
Diff: https://reviews.apache.org/r/9420/diff/
Testing
-------
register a template and start a vm.
success.
Thanks,
Hongtu Zang
Re: Review Request: fix bug 1237 and 1240 : Register Template fails with
"Cannot find template adapter for XenServer"
Posted by Rohit Yadav <bh...@apache.org>.
> On Feb. 13, 2013, 9:56 a.m., Rohit Yadav wrote:
> > This fix would have worked for Hypervisor but would have failed for baremetal... if we fix like this, there may be other template adapters whose class (simple) names. So, it was better to impl getName() for all implementing template adapters.
> >
> > Hongfu thank you for your patch, I was in middle of working and testing the patch and went ahead to commit the fix.
>
> Hongtu Zang wrote:
> Yes, I haven't note about baremetal.
> It should use "BareMetalTemplateAdapter" instead of "BareMetalAdapter".
> But I cannot test it now. can you test it?
> I will update the patch.
Thanks, but changing the string won't be correct fix. Say there is another class implementation of TemplateAdapter which is of HypervisorAdapter type but has a different impl, in that case that class may have a different class name but we want that the getName() used in BaseAdapter to get the name returns one of these template types, i.e. name which is static var in TemplateAdapter.
I've already fixed this issue on master and 4.1 in this way. You may resend a patch if there is another better way, or submit the review to close it. Thanks.
- Rohit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9420/#review16509
-----------------------------------------------------------
On Feb. 13, 2013, 10:57 a.m., Hongtu Zang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9420/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2013, 10:57 a.m.)
>
>
> Review request for cloudstack, mice xia, anthony xu, and SrikanteswaraRao Talluri.
>
>
> Description
> -------
>
> In TemplateManagerImpl.java, function getAdapter(), TemplateAdapterType.Hypervisor.getName() returns "HyervisorAdapter", while it should returns "HyervisorTemplateAdapter". So, in AdapterBase.java function getAdapterByName() returns null.
>
>
> This addresses bugs CLOUDSTACK-1237 and CLOUDSTACK-1240.
>
>
> Diffs
> -----
>
> server/src/com/cloud/template/TemplateAdapter.java 19cfef0
>
> Diff: https://reviews.apache.org/r/9420/diff/
>
>
> Testing
> -------
>
> register a template and start a vm.
> success.
>
>
> Thanks,
>
> Hongtu Zang
>
>
Re: Review Request: fix bug 1237 and 1240 : Register Template fails with
"Cannot find template adapter for XenServer"
Posted by Hongtu Zang <ho...@tcloudcomputing.com>.
> On Feb. 13, 2013, 9:56 a.m., Rohit Yadav wrote:
> > This fix would have worked for Hypervisor but would have failed for baremetal... if we fix like this, there may be other template adapters whose class (simple) names. So, it was better to impl getName() for all implementing template adapters.
> >
> > Hongfu thank you for your patch, I was in middle of working and testing the patch and went ahead to commit the fix.
Yes, I haven't note about baremetal.
It should use "BareMetalTemplateAdapter" instead of "BareMetalAdapter".
But I cannot test it now. can you test it?
I will update the patch.
- Hongtu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9420/#review16509
-----------------------------------------------------------
On Feb. 13, 2013, 5:07 a.m., Hongtu Zang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9420/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2013, 5:07 a.m.)
>
>
> Review request for cloudstack, mice xia, anthony xu, and SrikanteswaraRao Talluri.
>
>
> Description
> -------
>
> In TemplateManagerImpl.java, function getAdapter(), TemplateAdapterType.Hypervisor.getName() returns "HyervisorAdapter", while it should returns "HyervisorTemplateAdapter". So, in AdapterBase.java function getAdapterByName() returns null.
>
>
> This addresses bugs CLOUDSTACK-1237 and CLOUDSTACK-1240.
>
>
> Diffs
> -----
>
> server/src/com/cloud/template/TemplateAdapter.java 19cfef0
>
> Diff: https://reviews.apache.org/r/9420/diff/
>
>
> Testing
> -------
>
> register a template and start a vm.
> success.
>
>
> Thanks,
>
> Hongtu Zang
>
>
Re: Review Request: fix bug 1237 and 1240 : Register Template fails with
"Cannot find template adapter for XenServer"
Posted by Rohit Yadav <bh...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9420/#review16509
-----------------------------------------------------------
This fix would have worked for Hypervisor but would have failed for baremetal... if we fix like this, there may be other template adapters whose class (simple) names. So, it was better to impl getName() for all implementing template adapters.
Hongfu thank you for your patch, I was in middle of working and testing the patch and went ahead to commit the fix.
- Rohit Yadav
On Feb. 13, 2013, 5:07 a.m., Hongtu Zang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9420/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2013, 5:07 a.m.)
>
>
> Review request for cloudstack, mice xia, anthony xu, and SrikanteswaraRao Talluri.
>
>
> Description
> -------
>
> In TemplateManagerImpl.java, function getAdapter(), TemplateAdapterType.Hypervisor.getName() returns "HyervisorAdapter", while it should returns "HyervisorTemplateAdapter". So, in AdapterBase.java function getAdapterByName() returns null.
>
>
> This addresses bugs CLOUDSTACK-1237 and CLOUDSTACK-1240.
>
>
> Diffs
> -----
>
> server/src/com/cloud/template/TemplateAdapter.java 19cfef0
>
> Diff: https://reviews.apache.org/r/9420/diff/
>
>
> Testing
> -------
>
> register a template and start a vm.
> success.
>
>
> Thanks,
>
> Hongtu Zang
>
>
Re: Review Request: fix bug 1237 and 1240 : Register Template fails with
"Cannot find template adapter for XenServer"
Posted by Hongtu Zang <ho...@tcloudcomputing.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9420/
-----------------------------------------------------------
(Updated Feb. 13, 2013, 10:57 a.m.)
Review request for cloudstack, mice xia, anthony xu, and SrikanteswaraRao Talluri.
Changes
-------
update the patch.
correct "BareMetalAdapter" into "BareMetalTemplateAdapter" for the failure of baremetal.
Description
-------
In TemplateManagerImpl.java, function getAdapter(), TemplateAdapterType.Hypervisor.getName() returns "HyervisorAdapter", while it should returns "HyervisorTemplateAdapter". So, in AdapterBase.java function getAdapterByName() returns null.
This addresses bugs CLOUDSTACK-1237 and CLOUDSTACK-1240.
Diffs (updated)
-----
server/src/com/cloud/template/TemplateAdapter.java 19cfef0
Diff: https://reviews.apache.org/r/9420/diff/
Testing
-------
register a template and start a vm.
success.
Thanks,
Hongtu Zang