Created attachment 951446 [details] Patch to correct possible error of md-cache skipping cache validation after successful reads When md-cache is handling a readv callback, its logic is to skip checking the file's iatt (to see if it has changed since the time of the last cache) if "op_ret != 0". I suspect that is a mistake, and the intent was to skip only on error; for other fops, op_ret is 0 on success and nonzero on error, and md-cache has the same code for them. For readv, though, op_ret is the number of bytes read, so it will normally be positive except on EOF. The attached patch changes "op_ret != 0" to "op_ret < 0" to skip the check only when an error was returned. (Or, if the intent was really to check cache validity only on an EOF condition when op_ret is zero, then probably a comment to that effect should be added).
Hey Philip! Many thanks for the patch (and all the others). Are you interested in posting these patches to our Gerrit instance for review? If not, other developers can do that for you, but do let us know what you prefer. If you would like to post this change to Gerrit, there are a couple of steps that you need to do: 1. clone this bug for the "mainline" version, see the clone link in the upper right corner of the bugzilla page (we only include new changes in the "master" branch, and then backport those to other versions) 2. create an account on https://review.gluster.org, upload a public ssh-key 3. checkout the git-repository over ssh: $ git clone ssh://pspencer.org/glusterfs 4. create a branch for your change: $ git checkout -t -b wip/md-cache/validate-read origin/master 5. apply the patch 6. commit the patch with a useful message $ git commit -sam "md-cache: validate iatt after successdul read" 7. post the patch to Gerrit: $ ./rfc.sh Let me know if you have any issues with these steps. You are also most welcome to report success/difficulties on the gluster-devel mailinglist or in #gluster-dev on Freenode IRC. Thanks, Niels
http://review.gluster.org/9129 has been merged in master, and can get backported: - http://www.gluster.org/community/documentation/index.php/Backport_Guidelines
My apologies for not getting back to this sooner ... I have been completely swamped with other things and, now that I had our glusterfs problems more or less sorted out with my patches applied locally, I had to focus on other things and leave this on back burner. Thank you for pushing this fix through for me. I do hope to have some time over the next few weeks to start on some of the other ones by setting up a gerrit account if nobody else has gotten to them by then -- probably starting either next week or the week after.
Thanks for the response, Philip! Let us know if you hit any difficulties or have questions about the process.
This bug is being closed as GlusterFS-3.6 is nearing its End-Of-Life and only important security bugs will be fixed. This bug has been fixed in more recent GlusterFS releases. If you still face this bug with the newer GlusterFS versions, please open a new bug.