On 2017-12-07 06:55, Duncan wrote:
Misono, Tomohiro posted on Thu, 07 Dec 2017 16:15:47 +0900 as excerpted:

On 2017/12/07 11:56, Duncan wrote:
Austin S. Hemmelgarn posted on Wed, 06 Dec 2017 07:39:56 -0500 as
excerpted:

Somewhat OT, but the only operation that's remotely 'instant' is
creating an empty subvolume.  Snapshot creation has to walk the tree
in the subvolume being snapshotted, which can take a long time (and as
a result of it's implementation, also means BTRFS snapshots are _not_
atomic). Subvolume deletion has to do a bunch of cleanup work in the
background (though it may be fairly quick if it was a snapshot and the
source subvolume hasn't changed much).

Indeed, while btrfs in general has taken a strategy of making
/creating/ snapshots and subvolumes fast, snapshot deletion in
particular can take some time[1].

And in that regard a question just occurred to me regarding this whole
very tough problem of a user being able to create but not delete
subvolumes and snapshots:

Given that at least snapshot deletion (not so sure about non-snapshot
subvolume deletion, tho I strongly suspect it would depend on the
number of cross-subvolume reflinks) is already a task that can take
some time, why /not/ just bite the bullet and make the behavior much
more like the directory deletion, given that subvolumes already behave
much like directories.  Yes, for non-root, that /does/ mean tracing the
entire subtree and checking permissions, and yes, that's going to take
time and lower performance somewhat, but subvolume and in particular
snapshot deletion is already an operation that takes time, so this
wouldn't be unduly changing the situation, and it would eliminate the
entire class of security issues that come with either asymmetrically
restricting deletion (but not creation) to root on the one hand,

or possible data loss due to allowing a user to delete a subvolume they
couldn't delete were it an ordinary directory due to not owning stuff
further down the tree.

But, this is also the very reason I'm for "sub del" instead of unlink().
Since snapshot creation won't check the permissions of the containing
files/dirs, it can copy a directory which cannot be deleted by the user.
Therefore if we won't allow "sub del" for the user, he couldn't remove
the snapshot.

Maybe snapshot creation /should/ check all that, in ordered to allow
permissions to allow deletion.

Tho that would unfortunately increase the creation time, and btrfs is
currently optimized for fast creation time.

Hmm... What about creating a "temporary" snapshot if not root, then
walking the tree to check perms and deleting it without ever showing it
to userspace if the perms wouldn't let the user delete it.  That would
retain fast creation logic, tho it wouldn't show up until the perms walk
was completed.

I would argue that it makes more sense to keep snapshot creation as is, keep the subvolume deletion command as is (with some proper permissions checks of course), and just make unlink() work for subvolumes like it does for directories.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to