Added clarity and doc of snapshot -i option#116
Open
TopView wants to merge 1 commit intokdave:masterfrom
Open
Conversation
_Hi. I have revised this suggestion and am resubmitting. Also I also noticed from usage: that the `-i <qgroupid>` option is now available but not previously documented here. I think these are still important issues to fix. _
-------
Was:
> **snapshot [-r] \<source> \<dest>|[\<dest>/]\<name>**
>
> Create a writable/readonly snapshot of the subvolume \<source> with the name
> \<name> in the \<dest> directory.
>
> If only \<dest> is given, the subvolume will be named the basename of \<source>.
> If \<source> is not a subvolume, btrfs returns an error. If -r is given, the snapshot will be readonly.
--------------------------------------------------------------------
Is:
> **snapshot [-r] [-i <qgroupid>] \<subvolume> { \<subdir>/\<name> | \<subdir> }**
>
> Create a snapshot of a \<subvolume>. Call it \<name> and place it in the \<subdir>.
>
> When only \<subdir> is given, the subvolume will be named using the basename of \<subvolume>.
>
> (\<subvolume> looks like a sub-directory, but is actually a btrfs subvolume rather than a subdirectory.)
>
> **-r**
> Make the new snapshot readonly.
>
> **-i** \<qgroupid>
> Add the new snapshot to a qgroup (quota group). This option can be given multiple times.
--------------------------------------------------------------------
**RATIONALES**
1) First I think there's no reason why the `-r` option usage should be formatted any differently here than for the rest of this documentation. All other options above this are split off and in lines of their own. This is the only option that is spelled out in the text.
2) Also `-i` documentation is added. If you run `$> btrfs subvolume snapshot` you get:
> btrfs subvolume snapshot: too few arguments
> usage: btrfs subvolume snapshot [-r] [-i \<qgroupid>] \<source> \<dest>|[\<dest>/]\<name>
>
> Create a snapshot of the subvolume
>
> Create a writable/readonly snapshot of the subvolume \<source> with
> the name \<name> in the \<dest> directory. If only \<dest> is given,
> the subvolume will be named the basename of \<source>.
>
> -r create a readonly snapshot
> -i \<qgroupid> add the newly created snapshot to a qgroup. This
> option can be given multiple times.
3) The | and [...] leave me to wonder which happens first. Removing the optional [dest] first, and rewriting you get:
> **snapshot [-r] \<source> \<dest>|\<name>**
> **snapshot [-r] \<source> \<dest>|\<dest>/\<name>**
Again removing to remove the OR (|) gives:
> **snapshot [-r] \<source> \<dest>** OR
> **snapshot [-r] \<source> \<name>**
AND
> **snapshot [-r] \<source> \<dest>** OR
> **snapshot [-r] \<source> \<dest>/\<name>**
But these are in conflict with each other.
So for improved clarity, two synopsis lines replace the older more complex, ambiguous synopsis.
----
4) For someone learning this I think using the more exact term "**subvolume**" is more clear than simply "source". Furthermore, this is in agreement with the existing usage: on line 641 in cmds-subvolume.c which says this, "Create a snapshot of the subvolume, ..."
-----------------
5) Likewise "**subdir**" is more clear than "dest". (Check me out, but I don't think it's possible for dest to be anything other than a subdir.) Again in cmds-subvolume.c it says, "<dest> directory". Also a snapshot can't be placed in the root directory, so it has to be in a sub-directory.
----
Thanks
Tip: if you reply remember to backslash any left angle brackets. (\\\<)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Was:
Is:
RATIONALE
First I think there's no reason why the
-roption usage should be formatted any differently here than for the rest of this documentation. All other options above this are split off and in lines of their own. This is the only option that is spelled out in the text.Also
-idocumentation is added. If you run$> btrfs subvolume snapshotyou get: