diff mbox series

[5/5] btrfs: ioctl: Call btrfs_vol_uevent on subvolume deletion

Message ID 20191024023636.21124-6-marcos.souza.org@gmail.com (mailing list archive)
State New, archived
Headers show
Series btrfs: send uevent on subvolume add/remove | expand

Commit Message

Marcos Paulo de Souza Oct. 24, 2019, 2:36 a.m. UTC 
From: Marcos Paulo de Souza <mpdesouza@suse.com>

Since the function btrfs_ioctl_snap_destroy is used for deleting both
subvolumes and snapshots it was needed call btrfs_is_snapshot,
which checks a giver btrfs_root and returns true if it's a snapshot.
The current code is interested in subvolumes only.

btrfs_vol_uevent will export two environment variables to udev:
BTRFS_VOL_NAME: containing the name of the subvolume deleted
BTRFS_VOL_DEL: will signalize that a volume is being deleted

One can create a udev rule and check for BTRFS_VOL_DEL being set,
these values one could detect whenever a subvolume is deleted, and
take any action based on the subvolume name contained in BTRFS_VOL_NAME.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 fs/btrfs/ioctl.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Graham Cobb Oct. 25, 2019, noon UTC | #1 
On 24/10/2019 03:36, Marcos Paulo de Souza wrote:
> From: Marcos Paulo de Souza <mpdesouza@suse.com>
> 
> Since the function btrfs_ioctl_snap_destroy is used for deleting both
> subvolumes and snapshots it was needed call btrfs_is_snapshot,
> which checks a giver btrfs_root and returns true if it's a snapshot.
> The current code is interested in subvolumes only.

To me, as a user, a snapshot *is* a subvolume. I don't even know what
"is a snapshot" means. Does it mean "was created using the btrfs
subvolume snapshot command"? Does it matter whether the snapshot has
been modified? Whether the originally snapshot subvolume still exists?
Or what?

I note that the man page for "btrfs subvolume" says "A snapshot is a
subvolume like any other, with given initial content.". And I certainly
have some subvolumes which are being used as normal parts of the
filesystem, which were originally created as snapshots (for various
reasons, including reverting changes and going back to an earlier
snapshot, or an easy way to make sure that large common files are
actually sharing blocks).

I would expect this event would be generated for any subvolume deletion.
If it is useful to distinguish subvolumes originally created as
snapshots in some way then export another flag (named to make it clear
what it really indicates, such as BTRFS_VOL_FROM_SNAPSHOT). I don't know
your particular purpose, but my guess is that a more useful flag might
actually be BTRFS_VOL_FROM_READONLY_SNAPSHOT.

Graham
Marcos Paulo de Souza Oct. 25, 2019, 3:44 p.m. UTC | #2 
On Fri, 2019-10-25 at 13:00 +0100, Graham Cobb wrote:
> On 24/10/2019 03:36, Marcos Paulo de Souza wrote:
> > From: Marcos Paulo de Souza <mpdesouza@suse.com>
> > 
> > Since the function btrfs_ioctl_snap_destroy is used for deleting
> both
> > subvolumes and snapshots it was needed call btrfs_is_snapshot,
> > which checks a giver btrfs_root and returns true if it's a
> snapshot.
> > The current code is interested in subvolumes only.
> 
> To me, as a user, a snapshot *is* a subvolume. I don't even know what
> "is a snapshot" means. Does it mean "was created using the btrfs
> subvolume snapshot command"? Does it matter whether the snapshot has
> been modified? Whether the originally snapshot subvolume still
> exists?
> Or what?
> 
> I note that the man page for "btrfs subvolume" says "A snapshot is a
> subvolume like any other, with given initial content.". And I
> certainly
> have some subvolumes which are being used as normal parts of the
> filesystem, which were originally created as snapshots (for various
> reasons, including reverting changes and going back to an earlier
> snapshot, or an easy way to make sure that large common files are
> actually sharing blocks).

Agreed.

> 
> I would expect this event would be generated for any subvolume
> deletion.
> If it is useful to distinguish subvolumes originally created as
> snapshots in some way then export another flag (named to make it
> clear
> what it really indicates, such as BTRFS_VOL_FROM_SNAPSHOT). I don't
> know
> your particular purpose, but my guess is that a more useful flag
> might
> actually be BTRFS_VOL_FROM_READONLY_SNAPSHOT.

As soon as these patches got merged I will send new ones to take care
of snapshoting. Same things: when a snapsoht is created or removed,
send a uevent.

I liked this idea to separate both snapshots and volumes at first to
make things simpler, and get reviews faster. Also, I think it's good to
separate subvolumes and snapshot events, makes easier for one to filter
such events.

  Marcos

> 
> Graham
diff mbox series

Patch

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index c538d3648195..173f2a258508 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2869,6 +2869,7 @@  static noinline int btrfs_ioctl_snap_destroy(struct file *file,
 {
 	struct dentry *parent = file->f_path.dentry;
 	struct btrfs_fs_info *fs_info = btrfs_sb(parent->d_sb);
+	struct block_device *bdev = fs_info->fs_devices->latest_bdev;
 	struct dentry *dentry;
 	struct inode *dir = d_inode(parent);
 	struct inode *inode;
@@ -2962,6 +2963,10 @@  static noinline int btrfs_ioctl_snap_destroy(struct file *file,
 	err = btrfs_delete_subvolume(dir, dentry);
 	inode_unlock(inode);
 	if (!err) {
+		/* send uevent only to subvolume deletion */
+		if (!btrfs_is_snapshot(dest))
+			btrfs_vol_uevent(bdev, false, vol_args->name);
+
 		fsnotify_rmdir(dir, dentry);
 		d_delete(dentry);
 	}