[btrfs-tools] Two consecutive identical calls of "btrfs subvolume snapshot" create two different snapshots instead of rejecting the second one

[victormarianpopa]

trafficstars

Usecase:

Create a snapshot of source:/mnt/mount_fs to destination:/mnt/mount_fs/Snapshots with the name: Snapshot_001

Precondition:

folder /mnt/mount_fs/Snapshots/ exists at the time when the command is executed as destination path for the snapshots

Issue

Two consecutive identical calls of "btrfs subvolume snapshot .." create two snapshots instead of rejecting the second call (as occurred with the third command) I don't think this is a right behavior because it can lead to unnecessary identical snapshots under different names in case when the second call is executed by mistake. Tested with btrfs-progs v4.12.0 and btrfs-progs v4.13.3 but I think it occurs with other versions too.

Testcase

First call OK

osboxes@osboxes:~/Working/tests$ sudo btrfs subvolume snapshot /mnt/mount_fs /mnt/mount_fs/Snapshots/Snapshot_001 Create a snapshot of '/mnt/mount_fs' in '/mnt/mount_fs/Snapshots/Snapshot_001' osboxes@osboxes:~/Working/tests$

osboxes@osboxes:~/Working/tests$ sudo btrfs su li /mnt/mount_fs/ ID 310 gen 131 top level 5 path Snapshots/Snapshot_001 osboxes@osboxes:~/Working/tests$

Second identical call => NOT OK !!! (???)

osboxes@osboxes:~/Working/tests$ sudo btrfs subvolume snapshot /mnt/mount_fs /mnt/mount_fs/Snapshots/Snapshot_001 Create a snapshot of '/mnt/mount_fs' in '/mnt/mount_fs/Snapshots/Snapshot_001/mount_fs' osboxes@osboxes:~/Working/tests$

osboxes@osboxes:~/Working/tests$ sudo btrfs su li /mnt/mount_fs/ ID 310 gen 132 top level 5 path Snapshots/Snapshot_001 ID 311 gen 132 top level 310 path Snapshots/Snapshot_001/mount_fs osboxes@osboxes:~/Working/tests$

Third identical call

osboxes@osboxes:~/Working/tests$ sudo btrfs subvolume snapshot /mnt/mount_fs /mnt/mount_fs/Snapshots/Snapshot_001 Create a snapshot of '/mnt/mount_fs' in '/mnt/mount_fs/Snapshots/Snapshot_001/mount_fs' ERROR: cannot snapshot '/mnt/mount_fs': File exists osboxes@osboxes:~/Working/tests$

osboxes@osboxes:~/Working/tests$ sudo btrfs su li /mnt/mount_fs/ ID 310 gen 132 top level 5 path Snapshots/Snapshot_001 ID 311 gen 132 top level 310 path Snapshots/Snapshot_001/mount_fs osboxes@osboxes:~/Working/tests$

[Damenly]

Thanks for your report! A subvolume in btrfs looks like a normal directory. In my opinion, the behavior is normal in logic.

Just like: $cp -r src_dir dst_dir If dst_dir doesn't exist, cp will create a directory named "dst_dir" which is same as src_dir. else if dst_dir exists, cp will cp src_dir to dst_dir.

As for the failure of the third call, there can't be two directories(subvolumes) with the same name in one directory(subvolume).

More information about btrfs subvolume: https://btrfs.wiki.kernel.org/index.php/Manpage/btrfs-subvolume

[victormarianpopa]

Hi, Thanks for your answer, but the issue I wanted to point out is different.

In the the first call, I provide the snapshot name="Snapshot_001" and I get the snapshot in the folder "Snapshots/Snapshot_001" which is OK

In the second identical call I provide the same snapshot name="Snapshot_001" and I get back "Snapshots/Snapshot_001/mount_fs" (see the additional /mount_fs subfolder)

Now my usecase => I need to implement a wrapper around the btrfs-tools. It calls the command and verify back the answer against the expected answer. Below is an extras of execution traces:

First call

osboxes@osboxes:~/Working/tests$ ./a.out COMMAND sudo $(which btrfs) subvolume snapshot /mnt/mount_fs /mnt/mount_fs/Snapshots/Snapshot_001 EXPECTED_ANSWER: Create a snapshot of '/mnt/mount_fs' in '/mnt/mount_fs/Snapshots/Snapshot_001' RECEIVED_ANSWER: Create a snapshot of '/mnt/mount_fs' in '/mnt/mount_fs/Snapshots/Snapshot_001' => OK, as expected

Second call

osboxes@osboxes:~/Working/tests$ ./a.out COMMAND: sudo $(which btrfs) subvolume snapshot /mnt/mount_fs /mnt/mount_fs/Snapshots/Snapshot_001 EXPECTED_ANSWER: Create a snapshot of '/mnt/mount_fs' in '/mnt/mount_fs/Snapshots/Snapshot_001' RECEIVED_ANSWER: Create a snapshot of '/mnt/mount_fs' in '/mnt/mount_fs/Snapshots/Snapshot_001/mount_fs' => not OK, see the mismatch between the expected answer and the received answer

In the second call: 1.) a new unexpected snapshot was created, Snapshot_001/mount_fs 2.) the received answer differs from the expected answer => the wrapper consider it an error case (as it compares the expected vs. received strings and they differs..)

My question is why in the the second call the "/mount_fs" sub-path of the source path "/mnt/mount_fs" is used to create the second snapshot ?

Thanks in advance for supporting..

[victormarianpopa]

To put it simple, the usecase is: create a snapshot and then try to create again the same snapshot => instead of getting an error of type "File exist" actually it creates another snapshot inside the first one

Again the details:

• first call => creates the snapshot [ Snapshot_001 ]
• second call => creates the snapshot [ Snapshot_001/mount_fs ] inside the first one (why ?) and then:
• third call tries to create again [ Snapshot_001/mount_fs ] which leads to a correct error "File exist" because it is the same as the second one.

All three times I used the same command: btrfs subvolume snapshot /mnt/mount_fs /mnt/mount_fs/Snapshots/Snapshot_001 where SRC= /mnt/mount_fs DEST= /mnt/mount_fs/Snapshots SNAPSHOT_NAME = Snapshot_001

Q: Why does it create a snapshot with the name [mount_fs] which was never requested explicitly ? If [mount_fs] is considered from the SOURCE, then why it wasn't created like Snapshot_001/mount_fs from the first call (and are necessary two calls to complete a snapshot ?)

For me it doesn't look OK, either the fist snapshot needs two calls in order to be created (nok) or the second snapshot doesn't have sense.

[Damenly]

I saw...

Some document in man btrfs-subvolume:

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>.

Yes, [mount_fs] is considered as the basename of SOURCE in your second call. However, if dest doesn't exist, btrfs thinks you want to create one directory dest which is same as source.

Before going to create any snapshot, check whether there is a directory with the same name already. It seems you don't happy about it :).

People may feel strange about the logic. But btrfs-subvolume does behave like directory.

[kdave]

The subvolume snapshot command is modelled after cp, so the semantics is similar to something people already know. The behaviour might be ambiguous due to the decision based on the existence of the target path. We'd have to add an option to make this explicit, as we can't simply change the existing behaviour due to backward compatibility.

[victormarianpopa]

Thanks for taking it into consideration. A change based on option is a good idea and it would be nice if you can implement it in one of the upcoming releases. Thank you again to both of you for supporting.

[amstan]

The subvolume snapshot command is modelled after cp, so the semantics is similar to something people already know.

Looks like cp has 3 modes for this:

       cp [OPTION]... [-T] SOURCE DEST
       cp [OPTION]... SOURCE... DIRECTORY
       cp [OPTION]... -t DIRECTORY SOURCE...

The top 2 are like the autodetection stuff from btrfs's current implementation. But.... if you use -T aka --no-target-directory it'll treat dest like the final destination and not make newname=basename(src). cp -t does the opposite, forces dest to be a folder.

Summarized: -T forces the first mode (similar to what this issue wants), -t forces the last mode, if you use neither you get the auto heuristic (similar to what btrfs does now too).

See more at cp's man page.

We'd have to add an option to make this explicit, as we can't simply change the existing behaviour due to backward compatibility.

-T would be a great option to add in the spirit of your requirement.

[amstan]

Did @Zygo just tell me a workaround? Just add a /. as a suffix to src.

alex@alex-framework /mnt/fs $ sudo btrfs subvolume create src
Create subvolume './src'
alex@alex-framework /mnt/fs $ sudo btrfs subvolume snapshot src/. dest/
Create a snapshot of 'src/.' in './dest'
alex@alex-framework /mnt/fs $ sudo btrfs subvolume snapshot src/. dest/
ERROR: invalid snapshot name '.'

It works, but the error is nonsensical, I would have prefered the usual:

Create a snapshot of 'src/' in 'dest/'
ERROR: cannot snapshot ...: File exists

[kdave]

Continued in #747 for implementation.