Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Treat extended attribute ltfs.mediaPool.name like a virtual extended attribute. Allow both reading and writing, and store in MAM data. #388

Conversation

richard42
Copy link
Contributor

@richard42 richard42 commented Mar 27, 2023

Summary of changes

This pull request includes a fix for the following problem: attempting to read the extended attribute "ltfs.mediaPool.name" will fail with an LTFS_NO_XATTR error:

rgoedeken@fk-rg-03 TapeTools % xattr -p ltfs.mediaPool.name /Volumes/tape000002
xattr: /Volumes/tape000002: No such xattr: ltfs.mediaPool.name

Description

Previously, this attribute could be written via some special-case code in xattr_set(), and presumably it was designed this way so that the attribute's value could be stored in the LTFS index. But this appears to be unnecessary as I don't see anywhere that this value gets written to the XML index.

Previously, this attribute could not be read at all, and any attempt would fail with LTFS_NO_XATTR. With this change, it can be read as well as written, as it is treated like all other virtual extended attributes.

I have tested this on MacOS 10.15.7 and it works as intended. With a build including this change, I am able to read the attribute:

rgoedeken@fk-rg-03 TapeTools % xattr -p ltfs.mediaPool.name /Volumes/tape000003
PROJECT=BADF17F7-496B-402E-8647-BD5B0B813955

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • This change requires a documentation update

Checklist:

  • [* ] My code follows the style guidelines of this project
  • [* ] I have performed a self-review of my own code
  • [* ] I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • [* ] My changes generate no new warnings
  • [ *] I have confirmed my fix is effective or that my feature works

…attribute. Allow both reading and writing, and store in MAM data.
Copy link
Member

@piste-jp piste-jp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much. It looks good to me.

@piste-jp
Copy link
Member

@juliocelon

Could you merge this after review?

@juliocelon juliocelon requested a review from perezle April 17, 2023 17:57
@richard42 richard42 force-pushed the fix-mediapool-name-virtual-xattr branch from 4a88641 to d1c99a4 Compare May 3, 2023 17:45
@richard42
Copy link
Contributor Author

Sorry for the force push; this merge request is now as it was before. I have fixed another bug and accidentally pushed it into this branch, therefore requiring a force push to remove it. I will submit a new pull request for the other bug fix.

@piste-jp
Copy link
Member

@juliocelon ,

Please make a Squash and merge this into the master branch.

@piste-jp piste-jp added the to master Merge to master branch label Jun 23, 2023
@juliocelon juliocelon merged commit 66f0b28 into LinearTapeFileSystem:master Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
to master Merge to master branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants