Could it be just as simple as the canhandle() returns an int and not Boolean. So the ancient would return 1 but if the netapp matches it returns 100. If it does not handle at all you return -1. This effectively gives you prioritization. So the calling code would still loop through all strategies each time looking for the best match. I don't want the priority to be determined at load time as that is less flexible.
Darren > On Oct 3, 2013, at 5:42 PM, "SuichII, Christopher" <chris.su...@netapp.com> > wrote: > > Thanks for the quick reply. > > After talking with Edison, I think what we could do is allow the strategies > to either specify a priority or 'type' and then order them by that when they > are loaded. For example, we could have an enum of types like 'PLUGIN, > HYPERVISOR and DEFAULT' so that we can make sure plugins are asked > canHandle() first, then hypervisor implementations, then finally resort to > the default/ancient implementation. This 'type' or 'category' could be > specified as a bean parameter or as part of the strategy/provider interface. > Any thoughts on which are better? > > The problem with just making canHandle() more intelligent is that we do need > some kind of ordering. Ideally, the non-default implementations should be > asked first, then fall back onto the stock implementations. You saw the > problem yourself - the XenServerSnapshotStrategy just says it will handle all > requests, so if a non-standard strategy wants to be given a chance, it must > be asked before the hypervisor or ancient implementation. > > Alternatively if this matches the same usage of the global configuration > ordering system, we could add in the storage subsystem strategies and > providers. > > The reason for log4j changes is that we may want to enable a different level > of verbosity by default. Our support organization likes to have very verbose > logs to assist them in triaging support issues. The lowest level log4j > categories are 'com.cloud', 'org' and 'net' and those are set to DEBUG and > INFO. Even if we add a line for 'com', the default value should not be > 'TRACE' like we would like ours to be. I'm not all that great with log4j, > though, so maybe I'm missing a simpler solution. > > I'll try to keep an eye on the commands.properties/rbac stuff - that is good > to know. > > Thanks, > Chris > -- > Chris Suich > chris.su...@netapp.com > NetApp Software Engineer > Data Center Platforms – Cloud Solutions > Citrix, Cisco & Red Hat > >> On Oct 3, 2013, at 4:30 PM, Darren Shepherd <darren.s.sheph...@gmail.com> >> wrote: >> >> I forgot to mention about canHandle() ordering. For extensions, it >> should be preferred that we do not rely on loaded or configured order. >> The canHandle() implementation should assume that they're may be any >> order. So having said that, I looked at XenServerSnapshotStrategy and >> its canHandle() is brilliantly implemented as "return true;" >> >> Can we look at making the strategies order independent and not having >> another order configuration parameter? >> >> Darren >> >> On Thu, Oct 3, 2013 at 4:26 PM, Darren Shepherd >> <darren.s.sheph...@gmail.com> wrote: >>> Chris, >>> >>> I've updated the wiki [1]. Copying from the wiki >>> >>> "Extensions are automatically discovered based on the interfaces they >>> implement and which module is their parent. For example, if you place >>> a storage extension in a child module of the network module, it will >>> not be discovered. Additionally, depending on the extension, the >>> ordering may be important in how they extension is used. For the >>> extensions that ordering is important there is also a "Global >>> Configuration" setting for the ordering. The value of the setting is a >>> comma seperated list like SHA256SALT,MD5,LDAP,PLAINTEXT. The values >>> are the names of the extension. The name will be the result of >>> getName(), if the bean implements the Named interface, or the sort >>> class name (ie getClass().getSimpleName())." >>> >>> I setup all the extensible types based on what was in >>> componentContext.xml and also trial an error. The types you have >>> mentioned were not in componetContext.xml so I missed them. It works >>> today, but is not properly extensible. I will update the >>> configuration and wiki today. >>> >>> I've haven't made progress on the commands.properties as I realized it >>> will overlap with the rbac work being done. I will start a thread >>> regarding that so maybe it will be address in the rbac work. >>> >>> Regarding log4j. First, why do you have to change that? Regardless, >>> at the end of the day you are limited to whatever log4j can provide. >>> So log4j AFAIK does not support multiple log4j config files so the >>> best we can do is allow somebody to supply a different log4j config >>> file and configure cloudstack to use that. One of the things I don't >>> particularily like is that ACS forces log4j to its own configuration >>> file and will not allow the regular log4j config discovery. So what I >>> could do is add a JVM property like -Dcloudstack.log4jstandardconfig >>> that will disable the ACS behavior of configuring log4j and then you >>> can use the log4j standard approach [2]. >>> >>> Darren >>> >>> [1] https://cwiki.apache.org/confluence/display/CLOUDSTACK/Extensions >>> [2] http://logging.apache.org/log4j/1.2/manual.html >>> >>> On Thu, Oct 3, 2013 at 3:29 PM, SuichII, Christopher >>> <chris.su...@netapp.com> wrote: >>>> Darren, >>>> >>>> I read through this thread and your docs on the wiki, but I'd appreciate >>>> it if you could answer a couple questions for me: >>>> >>>> -When creating an extension, such as a DataStoreProvider, those items are >>>> currently added into the list of providers for the appropriate bean, such >>>> as: >>>> <bean id="dataStoreProviderManager" >>>> >>>> class="org.apache.cloudstack.storage.datastore.provider.DataStoreProviderManagerImpl"> >>>> <property name="providers"> >>>> <list merge="true"> >>>> <ref bean="cloudStackPrimaryDataStoreProviderImpl"/> >>>> <ref local="cloudStackImageStoreProviderImpl"/> >>>> <ref local="s3ImageStoreProviderImpl"/> >>>> <ref local="swiftImageStoreProviderImpl"/> >>>> <ref local="solidFireDataStoreProvider"/> >>>> </list> >>>> </property> >>>> </bean> >>>> >>>> So, how do we add our bean to that list? >>>> >>>> -There are a number of extensions that are not currently listed, such as >>>> DataMotionStrategy, SnapshotStrategy, etc. Is it a problem that those are >>>> omitted from >>>> https://cwiki.apache.org/confluence/display/CLOUDSTACK/Extensions? >>>> >>>> -I know somewhere in this thread you talked about the order of beans, but >>>> can you document how the ordering or precedence works in the wiki? For >>>> example, if I create a DataMotionStrategy, how do I ensure that my >>>> strategy's canHandle() method is invoked before the >>>> AncientDataStoreStrategy? >>>> >>>> -Is there any progress on modularizing commands.properties and the log4j >>>> configuration? >>>> >>>> Thanks, >>>> Chris >>>> -- >>>> Chris Suich >>>> chris.su...@netapp.com<mailto:chris.su...@netapp.com> >>>> NetApp Software Engineer >>>> Data Center Platforms – Cloud Solutions >>>> Citrix, Cisco & Red Hat >
