librbd: ignore EDQUOT errors when releasing the lock

[mxdInspur]

rbd: Add a process when blocking writes or invalidate cache returns with EDQUOT

If the pool is full, the block_write that releases the exclusive lock fails with EDQUOT when we delete the lun, which causes the close object_map to terminate and the object_map object not to be released. Then in the rbd_close func the close() is called, an assert about whether object_map is empty will go wrong and a core error occurs

Signed-off-by: muxiangdong muxiangdong@inspur.com

[sebastian-philipp]

Please sign your work by adding a Signed-off-by to your git commit message. This is mandatory.

[dillaman]

@mxdInspur I am not sure I am following your claim of "data inconsistency and data loss". How does this occur?

[mxdInspur]

Please sign your work by adding a Signed-off-by to your git commit message. This is mandatory.

done

[dillaman]

I am not sure I am following your claim of "data inconsistency and data loss". How does this occur?

@mxdInspur ping

[mxdInspur]

I am not sure I am following your claim of "data inconsistency and data loss". How does this occur?

@mxdInspur ping

@dillaman I restate the issue, check this plesae

[dillaman]

Please add test cases to src/test/librbd/exclusive_lock/test_mock_PreReleaseRequest.cc where you inject a EDQUOT error when attempting to block writes and invalidate the cache. You might also need to cover the case where the journal is in-use and it fails to flush the journal due to to those errors?

[dillaman]

@mxdInspur lgtm -- just needs the new test cases

[mxdInspur]

@mxdInspur lgtm -- just needs the new test cases

actually i add three test cases, please review,thanks.

[dillaman]

@mxdInspur You should be able to combine all three new test cases into a single test case just like the blacklist test case since the errors are ignored.

Also, please prefix your commit subject lines w/ "librbd: ...." (and squash the two commits that stomp on each other) and "test/librbd: ...."

[mxdInspur]

@mxdInspur You should be able to combine all three new test cases into a single test case just like the blacklist test case since the errors are ignored.

Also, please prefix your commit subject lines w/ "librbd: ...." (and squash the two commits that stomp on each other) and "test/librbd: ...."

@dillaman I have updated according to your suggestion. Please review,thanks.

[dillaman]

lgtm (pending the results of make check) -- still need to update the commit subject line to add a "librbd: ..." prefix -- maybe just change it to "librbd: ignore EDQUOT errors when releasing the lock".

[mxdInspur]

fix the signed-off-by issue

[dillaman]

@mxdInspur How do you recreate the case where a full quota is returning a -EDQUOT error from the OSDs? It should only be returned when the try full flag is set (which only happens when removing things), but I cannot recreate the original problem.

[mxdInspur]

@mxdInspur How do you recreate the case where a full quota is returning a -EDQUOT error from the OSDs? It should only be returned when the try full flag is set (which only happens when removing things), but I cannot recreate the original problem.

@dillaman In our client implementation process, release_lock is first called to unlock before deletiing the lun. If the pool is full, an EDQUOT will occur in the release_lock. then an assert in the lun-deleting will go wrong.

[dillaman]

@dillaman In our client implementation process, release_lock is first called to unlock before deletiing the lun. If the pool is full, an EDQUOT will occur in the release_lock. then an assert in the lun-deleting will go wrong.

@mxdInspur Yes, but how did you get the OSDs to return an error instead of blocking? Is this a custom application that is also setting the "try_full" flag in librados?

[stale]

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

[stale]

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

[stale]

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!