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

Clarify '--one-file-system' for btrfs #5391

Merged
merged 3 commits into from
Oct 12, 2020

Conversation

eike-fokken
Copy link
Contributor

This addresses #4009. The documentation now explicitly mentions
btrfs subvolumes.

I'm happy to change the wording if the PR is welcome but the exact wording is not.

This addresses borgbackup#4009. The documentation now explicitly mentions
btrfs subvolumes.
@eike-fokken
Copy link
Contributor Author

rather than adding it to the (short) help here, one could check whether it is documented how -x works (see the check in the code - iirc it is checking for the device number [not] changing) and if that is not documented, add it there.

I don't understand what you mean by "check in the code". I found the code relating to --one-file-system in archiver.py.
To my understanding it explicitely checks the device number from stat.
But is there further documentation on this?

after talking about how borg -x works, one could also point out that some filesystems trigger this change detection:

  • btrfs for subvolumes
  • (more to come?)

I think, borg create is the only command accepting --one-file-system.
So, do you mean, I should add a paragraph in the text part of the borg create page?

@ThomasWaldmann
Copy link
Member

Yes, that is what I meant, add a paragraph to user docs.

Just say what it does so that it is clear how "one filesystem" is defined.

@ThomasWaldmann
Copy link
Member

This PR is changing way to many files.

@ThomasWaldmann
Copy link
Member

Do not run build_usage / build_man (or at least do not commit the result, so it is more clear what the PR is actually changing) - this is done at release time.

@eike-fokken
Copy link
Contributor Author

eike-fokken commented Oct 6, 2020

Oh, sorry, I'm not familiar with github pull requests. I meant to make a different pull request, but it got mashed into this one. I will have look later on or tomorrow.

@eike-fokken
Copy link
Contributor Author

I now inserted a paragraph in the prose part of the docs.

@ThomasWaldmann
Copy link
Member

Looks good.

Can you get some other btrfs user from #4009 to review this also (I am unfamiliar with btrfs specifics)?

@eike-fokken
Copy link
Contributor Author

I will.
What does it mean, that the codecov/project check fails?

@ThomasWaldmann
Copy link
Member

@eike-fokken it means the overall test coverage could be improved. Unrelated to this PR.

The feedback from issue borgbackup#4009 is now included.
@eike-fokken
Copy link
Contributor Author

I got replies from two btrfs users and included their comments, see #4009.

@codecov-io
Copy link

Codecov Report

Merging #5391 into master will decrease coverage by 0.20%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5391      +/-   ##
==========================================
- Coverage   83.61%   83.41%   -0.21%     
==========================================
  Files          37       37              
  Lines        9957     9957              
  Branches     1656     1656              
==========================================
- Hits         8326     8306      -20     
- Misses       1149     1162      +13     
- Partials      482      489       +7     
Impacted Files Coverage Δ
src/borg/archiver.py 81.37% <ø> (+0.10%) ⬆️
src/borg/platform/__init__.py 72.00% <0.00%> (-12.00%) ⬇️
src/borg/platform/xattr.py 82.53% <0.00%> (-6.35%) ⬇️
src/borg/xattr.py 48.83% <0.00%> (-3.49%) ⬇️
src/borg/platform/base.py 77.58% <0.00%> (-3.45%) ⬇️
src/borg/helpers/misc.py 77.02% <0.00%> (-2.03%) ⬇️
src/borg/archive.py 82.36% <0.00%> (-0.28%) ⬇️
src/borg/helpers/parseformat.py 89.00% <0.00%> (-0.17%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f18c4bf...449e07f. Read the comment docs.

1 similar comment
@codecov-commenter
Copy link

Codecov Report

Merging #5391 into master will decrease coverage by 0.20%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5391      +/-   ##
==========================================
- Coverage   83.61%   83.41%   -0.21%     
==========================================
  Files          37       37              
  Lines        9957     9957              
  Branches     1656     1656              
==========================================
- Hits         8326     8306      -20     
- Misses       1149     1162      +13     
- Partials      482      489       +7     
Impacted Files Coverage Δ
src/borg/archiver.py 81.37% <ø> (+0.10%) ⬆️
src/borg/platform/__init__.py 72.00% <0.00%> (-12.00%) ⬇️
src/borg/platform/xattr.py 82.53% <0.00%> (-6.35%) ⬇️
src/borg/xattr.py 48.83% <0.00%> (-3.49%) ⬇️
src/borg/platform/base.py 77.58% <0.00%> (-3.45%) ⬇️
src/borg/helpers/misc.py 77.02% <0.00%> (-2.03%) ⬇️
src/borg/archive.py 82.36% <0.00%> (-0.28%) ⬇️
src/borg/helpers/parseformat.py 89.00% <0.00%> (-0.17%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f18c4bf...449e07f. Read the comment docs.

@ThomasWaldmann
Copy link
Member

Looks good, are you finished?

@eike-fokken
Copy link
Contributor Author

I am!

That was a pleasant experience, thanks for your time!

@ThomasWaldmann ThomasWaldmann merged commit 1f0458d into borgbackup:master Oct 12, 2020
@ThomasWaldmann
Copy link
Member

Thanks for working on this. Guess we better do a backport to 1.1-maint (as the 1.1.x docs are what people usually read).

@eike-fokken
Copy link
Contributor Author

Seems like a good idea, my system's borg is 1.1.13 and I am on Debian testing, so rather up-to-date.

milkey-mouse pushed a commit to milkey-mouse/borg that referenced this pull request Nov 3, 2020
docs: clarify borg create's '--one-file-system' option, borgbackup#4009

The documentation now explicitly mentions btrfs subvolumes and
explains how --one-file-system works.

Co-authored-by: Eike <e.fokken+git@posteo.de>
@ghost ghost mentioned this pull request Aug 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants