From d8554bf1aa0c14712e5064cc8a07d2d32dc880c5 Mon Sep 17 00:00:00 2001 From: Howard Date: Wed, 28 Mar 2018 12:03:33 -0700 Subject: [PATCH] Added clarity and doc of snapshot -i option _Hi. I have revised this suggestion and am resubmitting. Also I also noticed from usage: that the `-i ` option is now available but not previously documented here. I think these are still important issues to fix. _ ------- Was: > **snapshot [-r] \ \|[\/]\** > > Create a writable/readonly snapshot of the subvolume \ with the name > \ in the \ directory. > > If only \ is given, the subvolume will be named the basename of \. > If \ is not a subvolume, btrfs returns an error. If -r is given, the snapshot will be readonly. -------------------------------------------------------------------- Is: > **snapshot [-r] [-i ] \ { \/\ | \ }** > > Create a snapshot of a \. Call it \ and place it in the \. > > When only \ is given, the subvolume will be named using the basename of \. > > (\ looks like a sub-directory, but is actually a btrfs subvolume rather than a subdirectory.) > > **-r** > Make the new snapshot readonly. > > **-i** \ > 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 \] \ \|[\/]\ > > Create a snapshot of the subvolume > > Create a writable/readonly snapshot of the subvolume \ with > the name \ in the \ directory. If only \ is given, > the subvolume will be named the basename of \. > > -r create a readonly snapshot > -i \ 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] \ \|\** > **snapshot [-r] \ \|\/\** Again removing to remove the OR (|) gives: > **snapshot [-r] \ \** OR > **snapshot [-r] \ \** AND > **snapshot [-r] \ \** OR > **snapshot [-r] \ \/\** 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, " 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. (\\\<) --- Documentation/btrfs-subvolume.asciidoc | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/Documentation/btrfs-subvolume.asciidoc b/Documentation/btrfs-subvolume.asciidoc index a8c4af4bce..75e414ca26 100644 --- a/Documentation/btrfs-subvolume.asciidoc +++ b/Documentation/btrfs-subvolume.asciidoc @@ -47,10 +47,11 @@ do not affect the files in the original subvolume. SUBCOMMAND ----------- -*create* [-i ] [/]:: -Create a subvolume in . +*create* [-i ] [/]:: +Create a new subvolume called in . Once created the new subvolume +will look like a subdirectory but will actually be a subvolume. + -If is not given, subvolume will be created in the current +If is not given, the new subvolume will be created in the current directory. + `Options` @@ -157,17 +158,21 @@ The id can be obtained from *btrfs subvolume list*, *btrfs subvolume show* or *show* :: Show information of a given subvolume in the . -*snapshot* [-r] |[/]:: -Create a snapshot of the subvolume with the -name in the directory. +*snapshot* [-r] [-i ] { / | }:: +Create a snapshot of a . Call it and place it in the . ++ +When only is given, the subvolume will be named using the basename of . ++ +( looks like a sub-directory, but is actually a btrfs subvolume rather than a subdirectory.) + -If only is given, the subvolume will be named the basename of . If is not a subvolume, btrfs returns an error. + `Options` + -r:::: Make the new snapshot read only. +-i :::: +Add the new snapshot to a qgroup (quota group). This option can be given multiple times. *sync* [subvolid...]:: Wait until given subvolume(s) are completely removed from the filesystem after