[PATCH 0/5] btrfs: send uevent on subvolume add/remove

[PATCH 0/5] btrfs: send uevent on subvolume add/remove

[Marcos Paulo de Souza]

From: Marcos Paulo de Souza <mpdesouza@suse.com>

Hey guys,

these patches make btrfs to send an uevent to userspace whenever a subvolume is
added/removed. The changes are pretty straightforward. This patchset was based
in btrfs-misc-next.

The first patch adds an additional argument to btrfs_kobject_uevent to receive a
envp, and just forward this argument to kobject_uevent_envp.

Patch number 2 creates a new function that will be called by patches 4 and 5 to
setup the environment variable to be set to userspace using uevent. These two
environment variables are BTRFS_VOL_{NEW,DEL} and BTRFS_VOL_NAME. The first
variable will have the value 1 for subvolume add/remove (only one will be
exported, so udev can distinguish the event), and the second one hold the name
of the subvolume being added/removed.

Feel free to suggest any other useful information to be exported to userspace
when adding/removing a subvolume.

Patches 3 and 5 call btrfs_vol_uevent to send the event on subvolume add/remove.

Patch 4 creates a helper function to distinguish a subvolume from a snapshot,
since the same function is used to delete both. This function is used in patch
5.

Thanks for you reviews!

Marcos Paulo de Souza (5):
  btrfs: sysfs: Add envp argument to btrfs_kobject_uevent
  btrfs: ioctl: Introduce btrfs_vol_uevent
  btrfs: ioctl: Call btrfs_vol_uevent on subvol creation
  btrfs: ctree.h: Add btrfs_is_snapshot function
  btrfs: ioctl: Call btrfs_vol_uevent on subvolume deletion

 fs/btrfs/ctree.h   | 14 ++++++++++++++
 fs/btrfs/ioctl.c   | 39 ++++++++++++++++++++++++++++++++++++++-
 fs/btrfs/sysfs.c   |  7 +++++--
 fs/btrfs/sysfs.h   |  3 ++-
 fs/btrfs/volumes.c |  2 +-
 5 files changed, 60 insertions(+), 5 deletions(-)

-- 
2.23.0

[PATCH 1/5] btrfs: sysfs: Add envp argument to btrfs_kobject_uevent

[Marcos Paulo de Souza]

From: Marcos Paulo de Souza <mpdesouza@suse.com>

This new argument will be used by the next patches for sending uevents
related to subvolumes being added/removed. These uevents will contain
environment data that will be sent to userspace, making it possible to
take actions once these events arrive.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 fs/btrfs/sysfs.c   | 7 +++++--
 fs/btrfs/sysfs.h   | 3 ++-
 fs/btrfs/volumes.c | 2 +-
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index f6d3c80f2e28..93a8ed9e4fe8 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -998,11 +998,14 @@ int btrfs_sysfs_add_device_link(struct btrfs_fs_devices *fs_devices,
 	return error;
 }
 
-void btrfs_kobject_uevent(struct block_device *bdev, enum kobject_action action)
+void btrfs_kobject_uevent(struct block_device *bdev,
+				enum kobject_action action,
+				char *envp[])
 {
 	int ret;
 
-	ret = kobject_uevent(&disk_to_dev(bdev->bd_disk)->kobj, action);
+	ret = kobject_uevent_env(&disk_to_dev(bdev->bd_disk)->kobj,
+				action, envp);
 	if (ret)
 		pr_warn("BTRFS: Sending event '%d' to kobject: '%s' (%p): failed\n",
 			action, kobject_name(&disk_to_dev(bdev->bd_disk)->kobj),
diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
index 610e9c36a94c..5be3953074ad 100644
--- a/fs/btrfs/sysfs.h
+++ b/fs/btrfs/sysfs.h
@@ -26,7 +26,8 @@ void btrfs_sysfs_update_sprout_fsid(struct btrfs_fs_devices *fs_devices,
 				    const u8 *fsid);
 void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
 		u64 bit, enum btrfs_feature_set set);
-void btrfs_kobject_uevent(struct block_device *bdev, enum kobject_action action);
+void btrfs_kobject_uevent(struct block_device *bdev,
+			enum kobject_action action, char *env[]);
 
 int __init btrfs_init_sysfs(void);
 void __cold btrfs_exit_sysfs(void);
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index bdfe4493e43a..7a7a9cae9b80 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -7580,7 +7580,7 @@ void btrfs_scratch_superblocks(struct block_device *bdev, const char *device_pat
 	}
 
 	/* Notify udev that device has changed */
-	btrfs_kobject_uevent(bdev, KOBJ_CHANGE);
+	btrfs_kobject_uevent(bdev, KOBJ_CHANGE, NULL);
 
 	/* Update ctime/mtime for device path for libblkid */
 	update_dev_time(device_path);
-- 
2.23.0

[PATCH 2/5] btrfs: ioctl: Introduce btrfs_vol_uevent

[Marcos Paulo de Souza]

From: Marcos Paulo de Souza <mpdesouza@suse.com>

This new function will be used to send uevents when a subvolume is created
or removed. Two environment variables are being exported:

BTRFS_VOL_NAME: contains the name of the volume being added/removed
BTRFS_VOL_{ADD|DEL}: will signalize whether a subvolume is created/deleted

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

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index de730e56d3f5..174cbe71d6be 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -940,6 +940,33 @@ static inline int btrfs_may_create(struct inode *dir, struct dentry *child)
 	return inode_permission(dir, MAY_WRITE | MAY_EXEC);
 }
 
+static inline int btrfs_vol_uevent(struct block_device *bdev, bool vol_added,
+				const char *name)
+{
+	char *envp[3];
+	int ret = -ENOMEM;
+
+	envp[0] = kasprintf(GFP_KERNEL, "BTRFS_VOL_NAME=%s", name);
+	if (!envp[0])
+		return ret;
+
+	envp[1] = kasprintf(GFP_KERNEL, "BTRFS_VOL_%s=1",
+			vol_added ? "NEW" : "DEL");
+	if (!envp[1])
+		goto free;
+
+	envp[2] = NULL;
+
+	btrfs_kobject_uevent(bdev, KOBJ_CHANGE, envp);
+
+	ret = 0;
+
+	kfree(envp[1]);
+free:
+	kfree(envp[0]);
+	return ret;
+}
+
 /*
  * Create a new subvolume below @parent.  This is largely modeled after
  * sys_mkdirat and vfs_mkdir, but we only do a single component lookup
-- 
2.23.0

[PATCH 3/5] btrfs: ioctl: Call btrfs_vol_uevent on subvol creation

[Marcos Paulo de Souza]

From: Marcos Paulo de Souza <mpdesouza@suse.com>

btrfs_vol_uevent will export two environment variables to udev:
BTRFS_VOL_NAME: containing the name of the volume being created
BTRFS_VOL_ADD: will signalize that a subvol was created

One can create a udev rule and check for BTRFS_VOL_ADD being sent,
these values one could detect whenever a subvolume is created, 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 | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 174cbe71d6be..c538d3648195 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -980,6 +980,7 @@ static noinline int btrfs_mksubvol(const struct path *parent,
 {
 	struct inode *dir = d_inode(parent->dentry);
 	struct btrfs_fs_info *fs_info = btrfs_sb(dir->i_sb);
+	struct block_device *bdev = fs_info->fs_devices->latest_bdev;
 	struct dentry *dentry;
 	int error;
 
@@ -1018,8 +1019,12 @@ static noinline int btrfs_mksubvol(const struct path *parent,
 		error = create_subvol(dir, dentry, name, namelen,
 				      async_transid, inherit);
 	}
-	if (!error)
+	if (!error) {
+		if (!snap_src)
+			btrfs_vol_uevent(bdev, true, name);
+
 		fsnotify_mkdir(dir, dentry);
+	}
 out_up_read:
 	up_read(&fs_info->subvol_sem);
 out_dput:
-- 
2.23.0

[PATCH 4/5] btrfs: ctree.h: Add btrfs_is_snapshot function

[Marcos Paulo de Souza]

From: Marcos Paulo de Souza <mpdesouza@suse.com>

This new function takes a btrfs_root as argument, and returns true is
root_key.offset is bigger than 0, meaning that this tree is a snapshot.

This new function will be used by the next patch.

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

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 19d669d12ca1..8502e9082914 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3411,6 +3411,20 @@ static inline int btrfs_defrag_cancelled(struct btrfs_fs_info *fs_info)
 	return signal_pending(current);
 }
 
+/*
+ * btrfs_is_snapshot() - Verify is a tree is a snapshot
+ * @root: btrfs_root
+ *
+ * When the key.offset field of btrfs_root is bigger than 0 it means the referred
+ * tree is a snapshot.
+ *
+ * Returns true if @root refers to a snapshot.
+ */
+static inline bool btrfs_is_snapshot(struct btrfs_root *root)
+{
+	return root->root_key.offset > 0;
+}
+
 #define in_range(b, first, len) ((b) >= (first) && (b) < (first) + (len))
 
 /* Sanity test specific functions */
-- 
2.23.0

Re: [PATCH 4/5] btrfs: ctree.h: Add btrfs_is_snapshot function

[Filipe Manana]

On Thu, Oct 24, 2019 at 7:56 PM Marcos Paulo de Souza
<marcos.souza.org@gmail.com> wrote:
>
> From: Marcos Paulo de Souza <mpdesouza@suse.com>
>
> This new function takes a btrfs_root as argument, and returns true is
> root_key.offset is bigger than 0, meaning that this tree is a snapshot.
>
> This new function will be used by the next patch.
>
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
>  fs/btrfs/ctree.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 19d669d12ca1..8502e9082914 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -3411,6 +3411,20 @@ static inline int btrfs_defrag_cancelled(struct btrfs_fs_info *fs_info)
>         return signal_pending(current);
>  }
>
> +/*
> + * btrfs_is_snapshot() - Verify is a tree is a snapshot
> + * @root: btrfs_root
> + *
> + * When the key.offset field of btrfs_root is bigger than 0 it means the referred
> + * tree is a snapshot.
> + *
> + * Returns true if @root refers to a snapshot.
> + */
> +static inline bool btrfs_is_snapshot(struct btrfs_root *root)
> +{
> +       return root->root_key.offset > 0;

So this is not true for all roots. For log roots and relocation roots
for example, which are not snapshots,
the offset corresponds to the objectid of the root they are associated to.
So this isn't generic enough to have in ctree.h, and will create
confusion or potential bugs if anyone tries to use it in the future.

Since you only use this function in a later patch at the
subvolume/deletion ioctl, I would suggest using this directly in that
code only,
as there this assumption is true, since user space can't reference a
log or relocation tree in the ioctl call.

Thanks.


> +}
> +
>  #define in_range(b, first, len) ((b) >= (first) && (b) < (first) + (len))
>
>  /* Sanity test specific functions */
> --
> 2.23.0
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

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

[Marcos Paulo de Souza]

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(+)

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);
 	}
-- 
2.23.0

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

[Graham Cobb]

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

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

[Marcos Paulo de Souza]

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

Re: [PATCH 0/5] btrfs: send uevent on subvolume add/remove

[David Sterba]

On Wed, Oct 23, 2019 at 11:36:31PM -0300, Marcos Paulo de Souza wrote:
> From: Marcos Paulo de Souza <mpdesouza@suse.com>
> 
> Hey guys,
> 
> these patches make btrfs to send an uevent to userspace whenever a subvolume is
> added/removed. The changes are pretty straightforward. This patchset was based
> in btrfs-misc-next.
> 
> The first patch adds an additional argument to btrfs_kobject_uevent to receive a
> envp, and just forward this argument to kobject_uevent_envp.

For the reference, this is from the project ideas on wiki
https://btrfs.wiki.kernel.org/index.php/Project_ideas#Send_notifications_about_important_events

There are 2 parts, the "transport" and the events. The device uevents
mechanism is the transport, so all we need here is to extend the
environment pointer with the data. AFAICS, this itself builds on netlink
so there are more possible consumers of the events, namely not just udev
and the like.

The events is more high-level thing and the wiki lists some of them. You
picked the subvolume creation/deletion, which is fine for demonstration
and prototyping. Right now the details of the events need to be defined.

> Patch number 2 creates a new function that will be called by patches 4 and 5 to
> setup the environment variable to be set to userspace using uevent. These two
> environment variables are BTRFS_VOL_{NEW,DEL} and BTRFS_VOL_NAME. The first
> variable will have the value 1 for subvolume add/remove (only one will be
> exported, so udev can distinguish the event), and the second one hold the name
> of the subvolume being added/removed.
> 
> Feel free to suggest any other useful information to be exported to userspace
> when adding/removing a subvolume.

The filesystem id needs to be there, maybe for all events. For the
subvolume in particular there can be the id, parent id and flags. We
need to see and forsee all the events and identify the common event
data, or for same group events (like device manipulation) and decide if
we go specific event "types" for each action. I think something that
follows the naming and granularity of ioctls would be least confusing.

So for subvolumes we can start with SUBVOLUME_CREATE and
SUBVOLUME_DESTROY.