Uploaded image for project: 'Lustre'
  1. Lustre
  2. LU-13115

always check object existence after mo_xattr_get()

Details

    • Bug
    • Resolution: Fixed
    • Major
    • Lustre 2.14.0
    • None
    • None
    • 3
    • 9223372036854775807

    Description

      There is specific case with lu_object LOHA_EXISTS flag in DNE environment when it can be dropped if remote object was found non-existent (deleted most likely?) which may cause problems in current MDT code paths.

      Without DNE LOHA_EXISTS is never dropped being set, when object is deleted it is just marked as destroyed (by LU_OBJECT_HEARD_BANSHEE flag). Therefore object being once checked for existence is always 'exists' in rest of code at least until lu_object_put() and requires no extra checks with lu_object_exists()

      With DNE the LOHA_EXISTS flag can be dropped during osp_xattr_get(). I am not sure if that was right decision, probably we have to keep the same semantic and keep LOHA_EXISTS always but just mark object as non-existent remotely, or use LU_OBJECT_HEARD_BANSHEE also like for local objects. Anyway, with DNE it is possible that existent object can become non-existent after mo_xattr_get() call. Therefore a return code of mo_xattr_get() must always be handled and checked for -ENOENT or lu_object_exists() must be performed right after that call.

      The mdt_pack_secctx_in_reply() ignores all return code silently and that opens door for lu_object_exists() assertions in further code path. While this function is about the optional XATTR getting it is still important to check -ENOENT case because it might change object state.

      Also there can be other places in code where result of mo_xattr_get() is not handled and this remains obscured for code writers. I wonder how to make people know about that mo_xattr_get() specific, probably osp_xattr_get() should be revised to don't drop LOHA_EXISTS flag but mark object as non-existent remotely.

      Attachments

        Issue Links

          Activity

            [LU-13115] always check object existence after mo_xattr_get()
            pjones Peter Jones made changes -
            Fix Version/s New: Lustre 2.14.0 [ 14490 ]
            Resolution New: Fixed [ 1 ]
            Status Original: In Progress [ 3 ] New: Resolved [ 5 ]
            pjones Peter Jones added a comment -

            Landed for 2.14

            pjones Peter Jones added a comment - Landed for 2.14

            Oleg Drokin (green@whamcloud.com) merged in patch https://review.whamcloud.com/37148/
            Subject: LU-13115 mdt: handle mdt_pack_sectx_in_reply() errors
            Project: fs/lustre-release
            Branch: master
            Current Patch Set:
            Commit: 732ed22bdb6ca6abe8896f24e7ba46eea3fc379d

            gerrit Gerrit Updater added a comment - Oleg Drokin (green@whamcloud.com) merged in patch https://review.whamcloud.com/37148/ Subject: LU-13115 mdt: handle mdt_pack_sectx_in_reply() errors Project: fs/lustre-release Branch: master Current Patch Set: Commit: 732ed22bdb6ca6abe8896f24e7ba46eea3fc379d
            tappro Mikhail Pershin made changes -
            Status Original: Open [ 1 ] New: In Progress [ 3 ]
            tappro Mikhail Pershin made changes -
            Description Original: There is specific case with {{lu_object LOHA_EXISTS}} flag in DNE environment when it can be dropped if remote object was found non-existent (deleted most likely?) which may cause problems in current MDT code paths.

            Without DNE {{LOHA_EXISTS}} is never dropped being set, when object is deleted it is just marked as destroyed (by {{LU_OBJECT_HEARD_BANSHEE}} flag). Therefore object being once checked for existence is always 'exists' in rest of code at least until {{lu_object_put()}} and requires no extra checks with {{lu_object_exists()}}

            With DNE the {{LOHA_EXISTS}} flag can be dropped during osp_xattr_get(). I am not sure if that was right decision, probably we have to keep the same semantic and keep {{LOHA_EXISTS}} always but just mark object as non-existent remotely, or use {{LU_OBJECT_HEARD_BANSHEE}} also like for local objects. Anyway, with DNE it is possible that existent object can become non-existent after {{md_xattr_get()}} call. Therefore a return code of {{md_xattr_get()}} must always be handled and checked for -ENOENT or {{lu_object_exists()}} must be performed right after that call.

            The {{mdt_pack_secctx_in_reply()}} ignores all return code silently and that opens door for {{lu_object_exists()}} assertions in further code path. While this function is about the optional XATTR getting it is still important to check -ENOENT case because it might change object state.

            Also there can be other places in code where result of md_xattr_get() is not handled and this remains obscured for code writers. I wonder how to make people know about that {{md_xattr_get()}} specific, probably {{osp_xattr_get()}} should be revised to don't drop {{LOHA_EXISTS}} flag but mark object as non-existent remotely.
            New: There is specific case with {{lu_object LOHA_EXISTS}} flag in DNE environment when it can be dropped if remote object was found non-existent (deleted most likely?) which may cause problems in current MDT code paths.

            Without DNE {{LOHA_EXISTS}} is never dropped being set, when object is deleted it is just marked as destroyed (by {{LU_OBJECT_HEARD_BANSHEE}} flag). Therefore object being once checked for existence is always 'exists' in rest of code at least until {{lu_object_put()}} and requires no extra checks with {{lu_object_exists()}}

            With DNE the {{LOHA_EXISTS}} flag can be dropped during {{osp_xattr_get()}}. I am not sure if that was right decision, probably we have to keep the same semantic and keep {{LOHA_EXISTS}} always but just mark object as non-existent remotely, or use {{LU_OBJECT_HEARD_BANSHEE}} also like for local objects. Anyway, with DNE it is possible that existent object can become non-existent after {{mo_xattr_get()}} call. Therefore a return code of {{mo_xattr_get()}} must always be handled and checked for -ENOENT or {{lu_object_exists()}} must be performed right after that call.

            The {{mdt_pack_secctx_in_reply()}} ignores all return code silently and that opens door for {{lu_object_exists()}} assertions in further code path. While this function is about the optional XATTR getting it is still important to check -ENOENT case because it might change object state.

            Also there can be other places in code where result of {{mo_xattr_get()}} is not handled and this remains obscured for code writers. I wonder how to make people know about that {{mo_xattr_get()}} specific, probably {{osp_xattr_get()}} should be revised to don't drop {{LOHA_EXISTS}} flag but mark object as non-existent remotely.
            tappro Mikhail Pershin made changes -
            Summary Original: always check object existence after md_xattr_get() New: always check object existence after mo_xattr_get()

            Mike Pershin (mpershin@whamcloud.com) uploaded a new patch: https://review.whamcloud.com/37148
            Subject: LU-13115 mdt: handle mdt_pack_sectx_in_reply() errors
            Project: fs/lustre-release
            Branch: master
            Current Patch Set: 1
            Commit: d0ea6ecb63e565c41e28e0f22d1b4ef41617bae9

            gerrit Gerrit Updater added a comment - Mike Pershin (mpershin@whamcloud.com) uploaded a new patch: https://review.whamcloud.com/37148 Subject: LU-13115 mdt: handle mdt_pack_sectx_in_reply() errors Project: fs/lustre-release Branch: master Current Patch Set: 1 Commit: d0ea6ecb63e565c41e28e0f22d1b4ef41617bae9
            tappro Mikhail Pershin made changes -
            Description Original: There is specific case with {{lu_object LOHA_EXISTS}} flag in DNE environment when it can be dropped if remote object was found non-existent (deleted most likely?) which may cause problems existent MDT code paths.

            Without DNE {{LOHA_EXISTS}} is never dropped being set, when object is deleted it is just marked as destroyed (by {{LU_OBJECT_HEARD_BANSHEE}} flag). Therefore object being once checked for existence is always 'exists' in rest of code at least until {{lu_object_put()}} and requires no extra checks with {{lu_object_exists()}}

            With DNE the {{LOHA_EXISTS}} flag can be dropped during osp_xattr_get(). I am not sure if that was right decision, probably we have to keep the same semantic and keep {{LOHA_EXISTS}} always but just mark object as non-existent remotely, or use {{LU_OBJECT_HEARD_BANSHEE}} also like for local objects. Anyway, with DNE it is possible that existent object can become non-existent after md_xattr_get() call. Therefore a return code of md_xattr_get() must always be handled and checked for -ENOENT or lu_object_exists() must be performed right after that call.

            The mdt_pack_secctx_in_reply() ignores all return code silently and that opens door for lu_object_exists() assertions in further code path. While this function is about optional xattr getting it is still important to check ENOENT case because it might change object state.

            Also there can be other places in code where result of md_xattr_get() is not handled and this remains obscured for code writers. I wonder how to make people know about that {{md_xattr_get()}} specific, probably {{osp_xattr_get()}} should be revised to don't drop {{LOHA_EXISTS}} flag but mark object as non-existent remotely.
            New: There is specific case with {{lu_object LOHA_EXISTS}} flag in DNE environment when it can be dropped if remote object was found non-existent (deleted most likely?) which may cause problems in current MDT code paths.

            Without DNE {{LOHA_EXISTS}} is never dropped being set, when object is deleted it is just marked as destroyed (by {{LU_OBJECT_HEARD_BANSHEE}} flag). Therefore object being once checked for existence is always 'exists' in rest of code at least until {{lu_object_put()}} and requires no extra checks with {{lu_object_exists()}}

            With DNE the {{LOHA_EXISTS}} flag can be dropped during osp_xattr_get(). I am not sure if that was right decision, probably we have to keep the same semantic and keep {{LOHA_EXISTS}} always but just mark object as non-existent remotely, or use {{LU_OBJECT_HEARD_BANSHEE}} also like for local objects. Anyway, with DNE it is possible that existent object can become non-existent after {{md_xattr_get()}} call. Therefore a return code of {{md_xattr_get()}} must always be handled and checked for -ENOENT or {{lu_object_exists()}} must be performed right after that call.

            The {{mdt_pack_secctx_in_reply()}} ignores all return code silently and that opens door for {{lu_object_exists()}} assertions in further code path. While this function is about the optional XATTR getting it is still important to check -ENOENT case because it might change object state.

            Also there can be other places in code where result of md_xattr_get() is not handled and this remains obscured for code writers. I wonder how to make people know about that {{md_xattr_get()}} specific, probably {{osp_xattr_get()}} should be revised to don't drop {{LOHA_EXISTS}} flag but mark object as non-existent remotely.
            tappro Mikhail Pershin made changes -
            Link New: This issue is related to LU-12895 [ LU-12895 ]
            tappro Mikhail Pershin created issue -

            People

              tappro Mikhail Pershin
              tappro Mikhail Pershin
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: