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
