Based on the comments, I'd say you're correct. However, only PSEUDO and GPFS modify the buffer in any way other than setting length in extract_handle(), and those 2 only byte swap them. All the protocols get a handle by running the pair extract_handle() create_handle(). In the majority of FSALs, extract_handle() will only check the handle for validity, but will otherwise be a no-op, so this is effectively just create_handle().
Note that the *only* time extract_handle() is called is before create_handle(). Also note that, at least FSAL_PSEUDO will fail if this sequence is not used, because of endian issues. We can't have the endian fixes in create_handle(), as that's called in a lot of places internally with non-byte-swapped handles, so probably keeping the sequence we have now, and fixing the comments, is the best solution? Daniel On 04/11/2017 07:35 AM, Malahal Naineni wrote: > Take for example nfs3_FhandleToCache(). It gets wire handle and calls > extract_handle() method. What is the job of extract_handle method? > Assuming that it is supposed to return "key" handle as the comments > there say. Then nfs3_FhandleToCache calls create_handle() with the > "key" handle. This is unexpected, right? > > Note that GPFS fsal indeed returns "key" handle in V2.3 and handles > get corrupted due to create_handle() getting "key" handle rather than > the full handle! > > In V2.5, GPFS extract_handle() returns full handle (after some endian > conversions). Note that it doesn't actually return "key" handle as > expected, but mdcache_create_handle() assumes that it is given a "key" > handle. So the "mdcache_create_handle --> mdcache_locate_keyed --> > mdcache_find_keyed" **should** never returns success with GPFS > objects! > > Am I missing something? > > On Mon, Apr 10, 2017 at 11:32 PM, Frank Filz <ffilz...@mindspring.com> wrote: >>> On 04/10/2017 11:57 AM, Frank Filz wrote: >>>>> Hi All, there is usually a 1:1 relationship (ignoring handles across >>>> architectures >>>>> and versions) between nfs handle and object handle. One thing that is >>>>> not clear is the "key" which is used for hashing the objects >>>> (mdcache_entry_t). >>>>> Ganesha 2.5 has handle_to_key() method to take unique bits from the >>>>> handle for acching purpose. How did this work in ganesha 2.3? Ganesha >>>>> 2.3 comments say that extract_handle method is used to get the key, >>>>> but this eventually results in losing the actual handle. >>>>> >>>>> >>>>> Also comment at mdcache_create_handle() indicates that it takes "key" >>>>> but the actual argument "struct gsh_buffdesc *hdl_desc" is written as >>>>> "Buffer descriptor for the "wire" handle". Is this wire handle or key? >>>>> If this is not the "key", then we will be looking for an incorrect >>>>> object, >>>> right? >>>> >>>> Looking over the code, I THINK everything is right... >>>> >>>> There is still a handle_digest method, and for GPFS it uses >>>> gpfs_sizeof_handle to determine the size to copy, so that may well >>>> copy the full handle. >>>> >>>> The code is definitely very confusing... >>>> >>>> There IS a handle_cmp method... >>>> >>>> I suggest we consider re-writing and clarifying all of this handle >>>> processing code. >>>> >>>> I think everywhere, the handle digest should be whatever the FSAL >>>> wants encapsulated. >>>> >>>> The handle_to_key function should be used to extract the key to be >>>> hashed by anything that wants to hash things, and since we will be >>>> using that for hashing, we should drop the handle_cmp method (or have >>>> a default version that just calls handle_to_key on the two handles it >>>> will compare, and then compare memory using the buffer descriptors >>> returned by handle_to_key). >>> >>> This is, in fact, what it does. >> >> Yea, it does look like handle_to_key is passed a gsh_buffdesc which is then >> filled in with the start address and length of what the FSAL would like to >> be the key, so no memcpy occurs. On the other hand, handle_digest is passed >> a gsh_buffdesc which describes where to place the handle, so a memcpy >> occurs. That is probably unavoidable since to create a wire handle we have >> to append the handle to the Ganesha handle header. >> >>>> FSALs having a custom handle_cmp isn't useful because they can't pick >>>> and choose fields in the handle to compare (since a hash of such a >>>> discontinuous handles would hash bytes that aren't part of the >>>> comparison, generating different hashes for handles that are the same >>> key). >>> >>> But it may be able to do this comparison faster, and avoiding copying >> things >>> around. >> >> We shouldn't need to copy the handle bytes around, just form a new buffer >> descriptor that indicates the start of the key and it's length. >> >> If we want keys that are not a contiguous set of bytes, then the hashing >> needs to be done by the FSAL also, we can't hash bytes 0-31 but only compare >> 0-3, 8-11, 16-19, 24-27... Since the hash would include bytes that could be >> different in different versions of the same handle. >> >> So yea, everything works. But it confuses people until they follow through >> all of the code... >> >> Frank >> >>>> I THINK this really only affects GPFS which puts extra stuff into some >>>> of it's handles that may be different at different times for the same >> object. >>>> >>>> Part of why this was never really clear is the folks who did all the >>>> stuff really didn't understand GPFS handles (I remember lots of >>>> confusion before we got it right). >>>> >>>> Frank >>>> >>>> >>> >>> Daniel >>> >>> >>> >> ---------------------------------------------------------------------------- >> -- >>> Check out the vibrant tech community on one of the world's most engaging >>> tech sites, Slashdot.org! http://sdm.link/slashdot >>> _______________________________________________ >>> Nfs-ganesha-devel mailing list >>> Nfs-ganesha-devel@lists.sourceforge.net >>> https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel >> >> >> --- >> This email has been checked for viruses by Avast antivirus software. >> https://www.avast.com/antivirus >> >> >> ------------------------------------------------------------------------------ >> Check out the vibrant tech community on one of the world's most >> engaging tech sites, Slashdot.org! http://sdm.link/slashdot >> _______________________________________________ >> Nfs-ganesha-devel mailing list >> Nfs-ganesha-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Nfs-ganesha-devel mailing list Nfs-ganesha-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel
