[OpenAFS-devel] Patch to prepare the client for addition protocols

Felix Frank Felix.Frank@Desy.de
Thu, 02 Jul 2009 12:33:56 +0200


Simon Wilkinson wrote (Thu Jul 02 2009 12:22:08 GMT+0200 (CEST))
> 
> On 2 Jul 2009, at 07:30, Felix Frank wrote:
>> The problem with that being that it does not really simplify anything.
>> What's worse, the fetchOps/storeOps would need being determined before 
>> calling afs_CacheFetchProc/CacheStoreProc. As will be seen in the 
>> upcoming patches, that would not be desirable, I think.
> 
> I have a few points here,
> 
> *) The cache manager already has a cache type indirection mechanism - 
> afs_cacheOps. Things which vary only by cache type should use this 
> indirection layer, rather than adding an additional one.
> *) Having both a cache type layer, and a protocol/cache type layer seems 
> very untidy.
> *) When you have an indirection layer, you should use it. Having the 
> ability to have cache or protocol specific initialisation functions, and 
> then still doing
> if (cacheDiskType == AFS_FCACHE_TYPE_UFS ) {
>   thing
> } else {
>   other thing
> }
> is just untidy programming.

I can see this happening somehow, and it might even end up being quite 
readable. Still, I fear call stacks would compare like this:

Now:
CacheXProc()
	=> appropriate rx_Write/_Read

With current proposal:
CachXProc()
	=> fetchOp/storeOp (from cache-specific ops structure)
		=> appropriate rx calls

With clean seperation:
CacheXProc()
	=> fetchOp/storeOp (from generic ops structure)
		=> rx-wrapper (from struct afs_cacheOps)
			=> appropriate rx calls

That should be manageable, but adds stack overhead.

Cheers
  - Felix