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

lfs_migrate() should try to unlink volatile file

Details

    • Bug
    • Resolution: Fixed
    • Minor
    • Lustre 2.10.0
    • None
    • 3
    • 9223372036854775807

    Description

      lfs_migrate() bypasses llapi_create_volatile_idx() and creates a volatile file directly. (It does this because it needs to create the volatile file with a specific striping and llapi_create_volatile_idx() does not allow us to pass in striping.) It should do as llapi_create_volatile_idx() does and try to unlink the volatile file after opening it (in case volatile file creation is not supported).

      Or we could add O_LOV_DELAY_CREATE to the open flags and use llapi_create_volatile_idx() but then we would need a function to apply the striping described by a struct llapi_stripe_param to an open file handle.

      Attachments

        Issue Links

          Activity

            [LU-8846] lfs_migrate() should try to unlink volatile file
            pjones Peter Jones added a comment -

            Landed for 2.10

            pjones Peter Jones added a comment - Landed for 2.10

            Oleg Drokin (oleg.drokin@intel.com) merged in patch https://review.whamcloud.com/23842/
            Subject: LU-8846 lfs: try to unlink volatile file in lfs_migrate()
            Project: fs/lustre-release
            Branch: master
            Current Patch Set:
            Commit: d15fbc4fd9fbf5b892ea4320771ad40963bfdc69

            gerrit Gerrit Updater added a comment - Oleg Drokin (oleg.drokin@intel.com) merged in patch https://review.whamcloud.com/23842/ Subject: LU-8846 lfs: try to unlink volatile file in lfs_migrate() Project: fs/lustre-release Branch: master Current Patch Set: Commit: d15fbc4fd9fbf5b892ea4320771ad40963bfdc69
            rread Robert Read added a comment -

            Ah, that's a good point.

            rread Robert Read added a comment - Ah, that's a good point.
            jhammond John Hammond added a comment -

            It's also totally conceivable that upstream reviewers will notice volatile file handling and decide that it's a security issue or something.

            jhammond John Hammond added a comment - It's also totally conceivable that upstream reviewers will notice volatile file handling and decide that it's a security issue or something.
            rread Robert Read added a comment -

            The existing API already allows passing O_LOV_DELAY_CREATE), and the copytool uses this so it can set striping on the volatile file.

            But my question was based on the patch comment: "In lfs_migrate() try to unlink the volatile file in case the MDT does not support volatile file creation."

            rread Robert Read added a comment - The existing API already allows passing O_LOV_DELAY_CREATE), and the copytool uses this so it can set striping on the volatile file. But my question was based on the patch comment: "In lfs_migrate() try to unlink the volatile file in case the MDT does not support volatile file creation."

            Robert, I think John is saying that it isn't possible to specify the file layout when creating a volatile file, so that it can't be used when restriping a file during migration.

            I'm thinking it makes sense to create a new API to allow specifying the layout for a volatile file. Probably passing O_LOV_DELAY_CREATE and setting the layout explicitly (within the same llapi_create_volatile_layout() function) would be most straight forward.

            I don't think the current patch is wrong, however, since it shouldn't leave files around if the volatile file create API isn't available.

            adilger Andreas Dilger added a comment - Robert, I think John is saying that it isn't possible to specify the file layout when creating a volatile file, so that it can't be used when restriping a file during migration. I'm thinking it makes sense to create a new API to allow specifying the layout for a volatile file. Probably passing O_LOV_DELAY_CREATE and setting the layout explicitly (within the same llapi_create_volatile_layout() function) would be most straight forward. I don't think the current patch is wrong, however, since it shouldn't leave files around if the volatile file create API isn't available.
            rread Robert Read added a comment -

            I'm curious, what is the situation where volatile files are not supported and we're doing lfs migrate?

            rread Robert Read added a comment - I'm curious, what is the situation where volatile files are not supported and we're doing lfs migrate?

            John L. Hammond (john.hammond@intel.com) uploaded a new patch: http://review.whamcloud.com/23842
            Subject: LU-8846 lfs: try to unlink volatile file in lfs_migrate()
            Project: fs/lustre-release
            Branch: master
            Current Patch Set: 1
            Commit: 222f9b93d7675a53d0b3d5d5e047ffb9f3f50dca

            gerrit Gerrit Updater added a comment - John L. Hammond (john.hammond@intel.com) uploaded a new patch: http://review.whamcloud.com/23842 Subject: LU-8846 lfs: try to unlink volatile file in lfs_migrate() Project: fs/lustre-release Branch: master Current Patch Set: 1 Commit: 222f9b93d7675a53d0b3d5d5e047ffb9f3f50dca

            People

              jhammond John Hammond
              jhammond John Hammond
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: