incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rohit Yadav <bhais...@apache.org>
Subject Re: Duplicate BareMetalTemplateAdapter classes
Date Thu, 14 Feb 2013 05:54:26 GMT
Okay, but the fix is in vim51_win8, do you want to cherry pick on
master/4.1 now or will it come when you'll merge your branch on
master? If you do the latter, it won't be part of 4.1.

Regards.

On Thu, Feb 14, 2013 at 12:22 AM, Min Chen <min.chen@citrix.com> wrote:
> Rohit, it turned out that we were fixing the same bug yesterday:) I also
> ran into template registration issue in vmware testing. I saw that you
> fixed that issue through modifying java code. After discussing with Kelven
> and Alex, we think that a more standard way to fix is to address this in
> componentContext.xml (nonossComponentContext.xml) to set name property and
> remove @Component annotation from BareMetalTemplateAdapater and
> HyervisorTemplateAdapater (here our code has a typo here, this class
> should be named as HypervisorTemplateAdapter). You can see that fix in my
> feature branch
> (https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=commi
> t;h=c0442e2556a7b10cfb363a2d4d55b7fb9c381297).  Kelven is cleaning up code
> to use XML to load all component and remove auto-scanning, this bug fix
> will be addressed there during his cleanup.
>
> Thanks
> -min
>
> On 2/13/13 10:35 AM, "Frank Zhang" <Frank.Zhang@citrix.com> wrote:
>
>>Thanks for filing this bug.
>>I am working on it recently, because new baremetal which is as a single
>>plugin haven't been checked in when javelin refactoring. So there are
>>some old code which is not working still stay in source, I am cleaning
>>up/testing in my local source,
>>will close these bug when fixes are checked in
>>
>>From: rohityadav89@gmail.com [mailto:rohityadav89@gmail.com] On Behalf Of
>>Rohit Yadav
>>Sent: Wednesday, February 13, 2013 2:01 AM
>>To: Frank Zhang
>>Cc: cloudstack
>>Subject: Duplicate BareMetalTemplateAdapter classes
>>
>>Hi Frank,
>>
>>Can you check why we have two BareMetalTemplateAdapter;
>>./plugins/hypervisors/baremetal/src/com/cloud/baremetal/manager/BareMetalT
>>emplateAdapter.java (Found no usage across codebase, removable?)
>>./server/src/com/cloud/baremetal/BareMetalTemplateAdapter.java (only this
>>one is used, as spring would instantiate only this one with @Component
>>annotation)
>>
>>And remove one which is redundant code? Found this while fixing
>>CLOUDSTACK-1237.
>>
>>Regards.
>>On Wed, Feb 13, 2013 at 3:26 PM, Rohit Yadav
>><bhaisaab@apache.org<mailto:bhaisaab@apache.org>> wrote:
>>This is an automatically generated e-mail. To reply, visit:
>>https://reviews.apache.org/r/9420/
>>
>>
>>
>>
>>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
>>
>>
>>On February 13th, 2013, 5:07 a.m., Hongtu Zang wrote:
>>Review request for cloudstack, mice xia, anthony xu, and SrikanteswaraRao
>>Talluri.
>>By Hongtu Zang.
>>
>>Updated Feb. 13, 2013, 5:07 a.m.
>>
>>Description
>>
>>In TemplateManagerImpl.java, function getAdapter(),
>>TemplateAdapterType.Hypervisor.getName() returns "HyervisorAdapter",
>>while it should returns "HyervisorTemplateAdapter". So, in
>>AdapterBase.java function getAdapterByName() returns null.
>>
>>
>>Testing
>>
>>register a template and start a vm.
>>
>>success.
>>
>>Bugs: CLOUDSTACK-1237, CLOUDSTACK-1240
>>Diffs
>>
>> *   server/src/com/cloud/template/TemplateAdapter.java (19cfef0)
>>
>>View Diff<https://reviews.apache.org/r/9420/diff/>
>>
>>
>

Mime
View raw message