libmount: handle btrfs default subvolume mount

PATCH In review Stanislav Brabec 3 participants 18 messages Jan 20, 2016 – Feb 2, 2016 Util-Linux package

Share & tools

Stanislav Brabec

author

2016-01-20 20:34 UTC #0 lore raw

PATCH 1/1 libmount: handle btrfs default subvolume mount

When mounting btrfs volume without subvol= and subvolid=, and the btrfs volume has default subvolume defined, mount() mounts the default subvolume and not the volume root as other filesystems do.

To handle this situation correctly, libmount has to be capable to detect default subvolume.

Add btrfs.c and btrfs.h that implement needed functions.

Known problems not covered by this patch:

- Use of subvolid= in fstab is not yet handled.

- Use of type auto in combination with subvol= in fstab is not yet handled.

- Use of btrfs in loop devices, where image file is specified in fstab is not yet handled (use of /dev/loop0 in fstab works).

- If fstab uses subvol=, and subvol path changes since last "mount -a", subsequent "mount -a" will not recognize that it is already mounted, and it will attempt to mount it second time. To fix it, libmount should remember subvolid in time of mount (subvolid is unique for the subvolume, subvol is not).

- mountinfo contains subvol and subvolid since kernel 4.2. Before kernel 4.2, there is no reasonable way to solve this situation. (One would create temporary mount point, mount the default, call needed ioctl() to determine what was mounted, deduce the default subvolume, compare it with subvolume of mounted volume, unmount and return result.)

How to reproduce:
truncate -s1G btrfs_test.img
mkdir -p btrfs_mnt
/sbin/mkfs.btrfs -f -d single -m single ./btrfs_test.img mount -o loop btrfs_test.img btrfs_mnt
pushd .
cd btrfs_mnt
mkdir -p d0/dd0/ddd0
cd d0/dd0/ddd0
touch file{1..5}
btrfs subvol create s1
cd s1
touch file{1..5}
mkdir -p d1/dd1/ddd1
cd d1/dd1/ddd1
btrfs subvol create s2
rid=$(btrfs inspect rootid s2)
echo new default $rid
btrfs subvol get-default .
btrfs subvol set-default $rid .
popd
umount btrfs_mnt
losetup /dev/loop0 $PWD/btrfs_test.img
echo "/dev/loop0 $PWD/btrfs_mnt btrfs defaults 0 0" >>/etc/fstab mount -a
mount -a
umount btrfs_mnt
sed -i "/\/dev\/loop0/d" /etc/fstab
losetup -d /dev/loop0
rm btrfs_test.img
rmdir btrfs_mnt

Current behavior:
mount: /dev/loop0 is already mounted or /root/btrfs_mnt busy
/dev/loop0 is already mounted on /root/btrfs_mnt

Signed-off-by Stanislav Brabec <[email protected]>

Cc David Štěrba <[email protected]>

---
 libmount/src/Makemodule.am |   2 +
 libmount/src/btrfs.c       | 101 ++++++++++++++++++++++++++++++++
 libmount/src/btrfs.h       | 141 +++++++++++++++++++++++++++++++++++++++++++++
 libmount/src/mountP.h      |   9 +++
 libmount/src/tab.c         |  83 ++++++++++++++++++++++++--
 5 files changed, 332 insertions(+), 4 deletions(-)
 create mode 100644 libmount/src/btrfs.c
 create mode 100644 libmount/src/btrfs.h

libmount/src/Makemodule.am +2 -0

index 11c6324..39d42d5 100644
@@ -27,6 +27,8 @@ libmount_la_SOURCES = \
 
 if LINUX
 libmount_la_SOURCES += \
+	libmount/src/btrfs.c \
+	libmount/src/btrfs.h \
 	libmount/src/context.c \
 	libmount/src/context_loopdev.c \
 	libmount/src/context_mount.c \

libmount/src/btrfs.c new file +101 -0

101 lines not on this page · raw message

libmount/src/btrfs.h new file +141 -0

141 lines not on this page · raw message

libmount/src/mountP.h +9 -0

index 25418a2..5362b2b 100644
@@ -19,6 +19,9 @@
 #include <unistd.h>
 #include <stdio.h>
 #include <stdarg.h>
+#if __linux__
+#include <linux/types.h>
+#endif
 
 #include "c.h"
 #include "list.h"

@@ -40,6 +43,7 @@
 #define MNT_DEBUG_CXT		(1 << 9)
 #define MNT_DEBUG_DIFF		(1 << 10)
 #define MNT_DEBUG_MONITOR	(1 << 11)
+#define MNT_DEBUG_BTRFS		(1 << 12)
 
 #define MNT_DEBUG_ALL		0xFFFF

@@ -411,4 +415,9 @@ extern int mnt_update_set_filename(struct libmnt_update *upd,
 extern int mnt_update_already_done(struct libmnt_update *upd,
 				   struct libmnt_lock *lc);
 
+#if __linux__
+/* btrfs.c */
+extern __u64 btrfs_get_default_subvol_id(const char *path);
+#endif
+
 #endif /* _LIBMOUNT_PRIVATE_H */

libmount/src/tab.c +79 -4

103 lines not on this page · raw message

Signature · 13 lines

2.7.0

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                         e-mail: [email protected]
Lihovarská 1060/12                            tel: +49 911 7405384547
190 00 Praha 9                                 fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

Stanislav Brabec

author

2016-01-20 21:51 UTC #1 lore raw

libmount/src/btrfs.h line 38

Quoted 4 lines · Stanislav Brabec

libmount/src/btrfs.h new file +4 -0

+#ifndef cpu_to_le64
+#if __BYTE_ORDER == __BIG_ENDIAN
+#define cpu_to_le64(x) ((__force __le64)(__u64)(bswap_64(x)))
+#define le64_to_cpu(x) ((__force __u64)(__le64)(bswap_64(x)))

I just found that my patch does not compile on big endian. I will send a new patch that uses bitops.h to fix that.

Signature · 10 lines

Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                         e-mail: [email protected]
Lihovarská 1060/12                            tel: +49 911 7405384547
190 00 Praha 9                                 fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

Stanislav Brabec

author

2016-01-20 21:57 UTC #2 lore raw

PATCH 1/1 libmount: handle btrfs default subvolume mount

When mounting btrfs volume without subvol= and subvolid=, and the btrfs volume has default subvolume defined, mount() mounts the default subvolume and not the volume root as other filesystems do.

To handle this situation correctly, libmount has to be capable to detect default subvolume.

Add btrfs.c and btrfs.h that implement needed functions.

Known problems not covered by this patch:

- Use of subvolid= in fstab is not yet handled.

- Use of type auto in combination with subvol= in fstab is not yet handled.

- Use of btrfs in loop devices, where image file is specified in fstab is not yet handled (use of /dev/loop0 in fstab works).

Current behavior:
mount: /dev/loop0 is already mounted or /root/btrfs_mnt busy
/dev/loop0 is already mounted on /root/btrfs_mnt

Signed-off-by Stanislav Brabec <[email protected]>

Cc David Štěrba <[email protected]>

---
 libmount/src/Makemodule.am |   2 +
 libmount/src/btrfs.c       | 101 ++++++++++++++++++++++++++++++++++
 libmount/src/btrfs.h       | 132 +++++++++++++++++++++++++++++++++++++++++++++
 libmount/src/mountP.h      |   9 ++++
 libmount/src/tab.c         |  83 ++++++++++++++++++++++++++--
 5 files changed, 323 insertions(+), 4 deletions(-)
 create mode 100644 libmount/src/btrfs.c
 create mode 100644 libmount/src/btrfs.h

libmount/src/Makemodule.am +2 -0

index 11c6324..39d42d5 100644
@@ -27,6 +27,8 @@ libmount_la_SOURCES = \
 
 if LINUX
 libmount_la_SOURCES += \
+	libmount/src/btrfs.c \
+	libmount/src/btrfs.h \
 	libmount/src/context.c \
 	libmount/src/context_loopdev.c \
 	libmount/src/context_mount.c \

libmount/src/btrfs.c new file +101 -0

101 lines not on this page · raw message

libmount/src/btrfs.h new file +132 -0

132 lines not on this page · raw message

libmount/src/mountP.h +9 -0

index 25418a2..5362b2b 100644
@@ -19,6 +19,9 @@
 #include <unistd.h>
 #include <stdio.h>
 #include <stdarg.h>
+#if __linux__
+#include <linux/types.h>
+#endif
 
 #include "c.h"
 #include "list.h"

@@ -40,6 +43,7 @@
 #define MNT_DEBUG_CXT		(1 << 9)
 #define MNT_DEBUG_DIFF		(1 << 10)
 #define MNT_DEBUG_MONITOR	(1 << 11)
+#define MNT_DEBUG_BTRFS		(1 << 12)
 
 #define MNT_DEBUG_ALL		0xFFFF

@@ -411,4 +415,9 @@ extern int mnt_update_set_filename(struct libmnt_update *upd,
 extern int mnt_update_already_done(struct libmnt_update *upd,
 				   struct libmnt_lock *lc);
 
+#if __linux__
+/* btrfs.c */
+extern __u64 btrfs_get_default_subvol_id(const char *path);
+#endif
+
 #endif /* _LIBMOUNT_PRIVATE_H */

libmount/src/tab.c +79 -4

103 lines not on this page · raw message

Signature · 13 lines

2.7.0

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                         e-mail: [email protected]
Lihovarská 1060/12                            tel: +49 911 7405384547
190 00 Praha 9                                 fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

Karel Zak

maintainer

2016-01-21 09:48 UTC #3 lore raw

Quoted 3 lines · Stanislav Brabec

+/**
+ * btrfs_get_default_subvol_id:
+ * @path: Path to mounted btrfs volume

this comment formatting indicate API docs, for private library functions we don't use "/**".

Quoted 6 lines

+ * Searches for the btrfs default subvolume id.
+ *
+ * Returns: default subvolume id or -1 in case of no default
+ * subvolume or error. In case of error, errno is set properly.
+ */
+__u64 btrfs_get_default_subvol_id(const char *path)

Please, use standard userspace types, for example uint64_t. This is generic rule, kernel-like types are unacceptable.

libmount/src/btrfs.c line 38

Quoted 8 lines

libmount/src/btrfs.c new file +8 -0

+{
+	int iocret;
+	int fd;
+	DIR *dirstream = NULL;
+	struct btrfs_ioctl_search_args args;
+	struct btrfs_ioctl_search_key *sk = &args.key;
+	struct btrfs_ioctl_search_header *sh;
+	__u64 found = (__u64)-1;

= UINT64_MAX

libmount/src/btrfs.h line 83

Quoted 13 lines

libmount/src/btrfs.h new file +13 -0

+struct btrfs_disk_key {
+	__le64 objectid;
+	__u8 type;
+	__le64 offset;
+} __attribute__ ((__packed__));
+
+struct btrfs_dir_item {
+	struct btrfs_disk_key location;
+	__le64 transid;
+	__le16 data_len;
+	__le16 name_len;
+	__u8 type;
+} __attribute__ ((__packed__));

again, use int64_t, uint8_t, etc.

Quoted 6 lines

/**
+ * mnt_table_find_target_with_option:
+ * @tb: tab pointer
+ * @path: mountpoint directory
+ * @option: option name (e.g "subvol", "subvolid", ...)
+ * @val: option value

Quoted 8 lines

+ * @direction: MNT_ITER_{FORWARD,BACKWARD}
+ *
+ * Try to lookup an entry in the given tab that matches combination of
+ * @path and @option. In difference to mnt_table_find_target(), only
+ * @path iteration is done. No lookup by device name, no canonicalization.
+ *
+ * Returns: a tab entry or NULL.
+ */

if you want to export this function by API then it's necessary to add to libmount.h.in and libmount.sym, otherwise don't use "/**".

libmount/src/tab.c struct libmnt_fs *mnt_table_get_fs_root(struct libmnt_table *tb, · line 1332

Quoted 9 lines

libmount/src/tab.c +8 -1

-		DBG(TAB, ul_debug("setting FS root: btrfs subvol"));
+		{
+			/* If fstab entry does not contain "subvol", we have to
+			 * check, whether btrfs has default subvolume
+			 * defined. */
+
+			__u64 default_id;
+			const char *target;
+			char default_id_str[16]; /* should be safe for u64 */

   char default_id_str[sizeof(stringify_value(UINT64_MAX))];

libmount/src/tab.c struct libmnt_fs *mnt_table_get_fs_root(struct libmnt_table *tb, · line 1345

Quoted 13 lines

libmount/src/tab.c +13 -0

+
+			default_id = btrfs_get_default_subvol_id(mnt_fs_get_target(fs));
+			if (default_id == (__u64)-1)
+				goto dflt;
+
+			/* Volume has default subvolume. Check if it
+			 * matches to the one in mountinfo.
+			 *
+			 * Only kernel >= 4.2 reports subvolid. On older
+			 * kernels, there is no reasonable way to detect which
+			 * subvolume was mounted. */
+			target = mnt_resolve_spec(mnt_fs_get_target(fs), tb->cache);
+			snprintf(default_id_str, 16, "%llu", (unsigned long long int)default_id);

snprintf(default_id_str, sizeof(default_id_str), ....

libmount/src/tab.c struct libmnt_fs *mnt_table_get_fs_root(struct libmnt_table *tb, · line 1358

Quoted 13 lines

libmount/src/tab.c +13 -0

+			DBG(TAB, ul_debug("target = %s subvolid = %s", target, &default_id_str));
+			struct libmnt_fs *f = mnt_table_find_target_with_option(tb, target, "subvolid", default_id_str, MNT_ITER_BACKWARD);
+			if (!f)
+				goto dflt;
+
+			/* Instead of set of BACKREF queries constructing
+			 * subvol path, use the one in mountinfo. Kernel does
+			 * the evaluation for us. */
+			DBG(TAB, ul_debug("setting FS root: btrfs default subvolid = %s", &default_id_str));
+			if (mnt_fs_get_option(f, "subvol", &vol, &volsz))
+				goto dflt;
+		} else
+			DBG(TAB, ul_debug("setting FS root: btrfs subvol"));

Karel

Signature · 2 lines

 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com

Stanislav Brabec

author

2016-01-21 15:24 UTC #4 lore raw

Quoted 8 lines · Karel Zak

Quoted 3 lines · Stanislav Brabec
+/**
+ * btrfs_get_default_subvol_id:
+ * @path: Path to mounted btrfs volume
this comment formatting indicate API docs, for private library functions we don't use "/**".

Should I export this? I think that no.

Well, looking again into libbtrfs, it is also possible to use
btrfs_list_get_default_subvolume() from libbtrfs, as is nearly exactly the same. But I am not sure, whether it is good to introduce such a dependency just for a single small function.

P. S.:

David also wrote btrfs_get_default_subvolume_path(), but then we found, that there is probably better to use subvolid and path saved in procinfo:

+ The tab.c patch would be much smaller and straightforward.
- Kernel evaluates it when creating procinfo entries, this would duplicate it.
x Both subvol and subvolid in procinfo were introduced together in the
mainline kernel, and btrfs_get_default_subvolume_path() needs more ioctl() calls.

Signature · 10 lines

Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                         e-mail: [email protected]
Lihovarská 1060/12                            tel: +49 911 7405384547
190 00 Praha 9                                 fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

Karel Zak

maintainer

2016-01-21 15:37 UTC #5 lore raw

Quoted 10 lines · Stanislav Brabec

Quoted 8 lines · Karel Zak
Quoted 3 lines · Stanislav Brabec
+/**
+ * btrfs_get_default_subvol_id:
+ * @path: Path to mounted btrfs volume
this comment formatting indicate API docs, for private library functions we don't use "/**".
Should I export this? I think that no.

It's private function.

Quoted 5 lines

Well, looking again into libbtrfs, it is also possible to use btrfs_list_get_default_subvolume() from libbtrfs, as is nearly exactly the same. But I am not sure, whether it is good to introduce such a dependency just for a single small function.

I really don't want to depend on libbtrfs.

Quoted 5 lines

P. S.:
David also wrote btrfs_get_default_subvolume_path(), but then we found, that there is probably better to use subvolid and path saved in procinfo:

Do you mean /proc/self/mountinfo ?

Quoted 7 lines

+ The tab.c patch would be much smaller and straightforward.
- Kernel evaluates it when creating procinfo entries, this would duplicate it.
x Both subvol and subvolid in procinfo were introduced together in the
mainline kernel, and btrfs_get_default_subvolume_path() needs more ioctl() calls.

No sure if I follow.

Karel

Signature · 2 lines

 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com

Karel Zak

maintainer

2016-01-21 15:45 UTC #6 lore raw

Quoted 7 lines · Karel Zak

Quoted 3 lines
David also wrote btrfs_get_default_subvolume_path(), but then we found, that there is probably better to use subvolid and path saved in procinfo:
Do you mean /proc/self/mountinfo ?
Quoted 1 line

Quoted 8 lines

Quoted 6 lines
+ The tab.c patch would be much smaller and straightforward.
- Kernel evaluates it when creating procinfo entries, this would duplicate it.
x Both subvol and subvolid in procinfo were introduced together in the
mainline kernel, and btrfs_get_default_subvolume_path() needs more ioctl() calls.
No sure if I follow.

Read it and the patch again, and now it makes sense. Yes, the ioctl and subvolid= from mountinfo seems better than dependence on libbtrfs.

Karel.

Signature · 2 lines

 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com

Stanislav Brabec

author

2016-01-21 17:24 UTC #7 lore raw

Quoted 6 lines · Karel Zak

Quoted 5 lines · Karel Zak
Quoted 3 lines
David also wrote btrfs_get_default_subvolume_path(), but then we found, that there is probably better to use subvolid and path saved in procinfo:
Do you mean /proc/self/mountinfo ?

Yes.

Quoted 2 lines

Read it and the patch again, and now it makes sense. Yes, the ioctl and subvolid= from mountinfo seems better than dependence on libbtrfs.

Well, the function btrfs_get_default_subvolume_path() does not exist in libbtrfs.

Our first implementation did not use subvolid search in mountinfo and constructed the default path by using BACKREF chain evaluation using btrfs specific ioctl().

The required tab.c change was then very small:
if (mnt_fs_get_option(fs, "subvol", &vol, &volsz))
- goto dflt;
+ {
+ vol = btrfs_get_default_subvolume_path(mnt_fs_get_target(fs));
+ if (!vol)
+ goto dflt;
+ volsz = strlen(vol);
+ } else

But btrfs_get_default_subvolume_path() was complicated. It has to been written from scratch, as btfs-progs read the whole subvolume table to memory and performs evaluation in the memory. It is something we really don't want on system with thousands snapshots.

The patch I sent uses ioctl() to get subvolid, and the rest is found in mountinfo. It looks better to me.

If somebody is interested in the function that went to the trash, here it is. Maybe it could be useful for somebody else.

static inline bool prepend_path(char subvol_buffer[PATH_MAX], int *subvol_buffer_index, char *name, int namelen)

{
	if (namelen == 0)
		return true;
	while (name[namelen - 1] == '/') {
		namelen--;
		if (namelen == 0)
			return true;
	}
	/* PATH_MAX - 1 could be fatal here, but it would fail elsewhere anyway. */
	if (*subvol_buffer_index < namelen + 1) {
		printf("PATH_MAX exceeded\n");
		return false;
	}
	/* Is not it the last item? */
	if (*subvol_buffer_index != PATH_MAX - 1) {
		subvol_buffer[*subvol_buffer_index - 1] = '/';
		(*subvol_buffer_index)--;
	}
	*subvol_buffer_index -= namelen;
	memcpy(&subvol_buffer[*subvol_buffer_index], name, namelen);
	return true;
}

/**
* btrfs_get_default_subvolume_path:
* @path: Path to mounted btrfs volume
*
* Evaluates btrfs tree and returns path to the default subvolume
* relative to the volume root.
*
* Returns: default subvolume path or NULL in case of no default
* subvolume or error. In case of error, errno is set properly. */
char *btrfs_get_default_subvolume_path(const char *path)

{
	int iocret;
	int fd = -1;
	DIR *dirstream = NULL;
	struct btrfs_ioctl_search_args args;
	struct btrfs_ioctl_search_key *sk = &args.key;
	struct btrfs_ioctl_search_header *sh;
	u64 found = 0;
	u64 parent = 0;
	u64 dirid = 0;
	struct btrfs_ioctl_ino_lookup_args lookup_args;
	struct btrfs_root_ref *root_ref;
	char subvol_path[PATH_MAX];
	int subvol_path_index = PATH_MAX-1;
	int namelen;
	char *ret_path = NULL;

	errno = 0;

	subvol_path[PATH_MAX - 1] = 0;

	fd = btrfs_open_dir(path, &dirstream, 1);
	if (fd < 0) {
		printf("ERROR: open\n");
		goto out;
	}
	memset(&args, 0, sizeof(args));

	sk->tree_id = BTRFS_ROOT_TREE_OBJECTID;
	sk->min_objectid = BTRFS_ROOT_TREE_DIR_OBJECTID;
	sk->max_objectid = BTRFS_ROOT_TREE_DIR_OBJECTID;
	sk->min_type = BTRFS_DIR_ITEM_KEY;
	sk->max_type = BTRFS_DIR_ITEM_KEY;
	sk->max_offset = (u64)-1;
	sk->max_transid = (u64)-1;
	sk->nr_items = 1;

	iocret = ioctl(fd, BTRFS_IOC_TREE_SEARCH, &args);
	if (iocret < 0) {
		DBG(BTRFS, ul_debug("ioctl() failed for \"%s\" [errno=%d %m]", path, errno));
		goto out;
	}

	/* the ioctl returns the number of items it found in nr_items */
	if (sk->nr_items == 0) {
		DBG(BTRFS, ul_debug("root tree dir object id not found"));
		goto out;
	}
	DBG(BTRFS, ul_debug("found %d root tree dir object id items", sk->nr_items));

	sh = (struct btrfs_ioctl_search_header *)args.buf;

	if (sh->type == BTRFS_DIR_ITEM_KEY) {
		struct btrfs_dir_item *di;
		int name_len;
		char *name;

		di = (struct btrfs_dir_item *)(sh + 1);
		name_len = btrfs_stack_dir_name_len(di);
		name = (char *)(di + 1);

		if (!strncmp("default", name, name_len)) {
			found = btrfs_disk_key_objectid(&di->location);
			DBG(BTRFS, ul_debug("\"default\" id is %llu", (unsigned long long)found));
		} else {
			DBG(BTRFS, ul_debug("\"default\" id not found in tree root"));
			goto out;
		}
	} else {
		DBG(BTRFS, ul_debug("unexpected type found: %d", (int)sh->type));
		goto out;
	}

next_level:

	/* look up parent */
	DBG(BTRFS, ul_debug("lookup BACKREFs for %llu", (unsigned long long)found));
	memset(&args, 0, sizeof(args));

	sk->tree_id = BTRFS_ROOT_TREE_OBJECTID;
	sk->min_objectid = found;
	sk->max_objectid = found;
	sk->min_type = BTRFS_ROOT_BACKREF_KEY;
	sk->max_type = BTRFS_ROOT_BACKREF_KEY;
	sk->min_offset = 0;
	sk->max_offset = (u64)-1;
	sk->max_transid = (u64)-1;
	sk->nr_items = 4096;

	iocret = ioctl(fd, BTRFS_IOC_TREE_SEARCH, &args);
	if (iocret < 0) {
		DBG(BTRFS, ul_debug("ioctl() failed [errno=%d %m]", errno));
		goto out;
	}

	/* the ioctl returns the number of items it found in nr_items */
	if (sk->nr_items == 0) {
		DBG(BTRFS, ul_debug("BACKREF not found"));
		goto out;
	}

	sh = (struct btrfs_ioctl_search_header *)args.buf;

	if (sh->type == BTRFS_ROOT_BACKREF_KEY) {
		char *name;

		parent = sh->offset;
		root_ref = (struct btrfs_root_ref*)(sh + 1);
		dirid = btrfs_stack_root_ref_dirid(root_ref);
		DBG(BTRFS, ul_debug("BACKREF points to %llu dirid %llu",
				    (unsigned long long)sh->offset,
				    (unsigned long long)dirid));
		namelen = btrfs_stack_root_ref_name_len(root_ref);
		name = (char *)(root_ref + 1);
		DBG(BTRFS, ul_debug("path component: %*s", namelen, name));

		/* The current implementation never adds trailing '/'
		   here. But it is an implementation detail and we
		   should not depend on it. */
		if (!prepend_path(subvol_path, &subvol_path_index, name, namelen)) {
			goto out;
		}
	} else {
		DBG(BTRFS, ul_debug("BACKREF key not found: %d", (int)sh->type));
		goto out;
	}

	memset(&lookup_args, 0, sizeof(lookup_args));
	lookup_args.treeid = parent;
	lookup_args.objectid = dirid;
	DBG(BTRFS, ul_debug("DIRID resolve for %llu", (unsigned long long)dirid));
	iocret = ioctl(fd, BTRFS_IOC_INO_LOOKUP, &lookup_args);
	if (iocret < 0) {
		DBG(BTRFS, ul_debug("DIRID search ioctl() failed %llu [errno=%d %m]", (unsigned long long)dirid, errno));
		goto out;
	}
	DBG(BTRFS, ul_debug("path component: %s", lookup_args.name));
	namelen = strlen(lookup_args.name);
	/* The current implementation always adds trailing '/' here.
	   But it is an implementation detail and we should not depend
	   on it. */
		if (!prepend_path(subvol_path, &subvol_path_index, lookup_args.name, namelen)) {
			goto out;
		}
	if (parent != BTRFS_FS_TREE_OBJECTID) {
		DBG(BTRFS, ul_debug("NOT at the toplevel, looping"));
		found = parent;
		goto next_level;
	}

	close_file_or_dir(fd, dirstream);

	return strdup(&subvol_path[subvol_path_index]);
out:
	return NULL;
}

Signature · 10 lines

Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                         e-mail: [email protected]
Lihovarská 1060/12                            tel: +49 911 7405384547
190 00 Praha 9                                 fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

David Sterba

2016-01-22 08:42 UTC #8 lore raw

In reply to Karel Zak

Quoted 15 lines · Karel Zak

Quoted 14 lines · Karel Zak
Quoted 3 lines
David also wrote btrfs_get_default_subvolume_path(), but then we found, that there is probably better to use subvolid and path saved in procinfo:
Do you mean /proc/self/mountinfo ?
Quoted 7 lines
+ The tab.c patch would be much smaller and straightforward.
- Kernel evaluates it when creating procinfo entries, this would duplicate it.
x Both subvol and subvolid in procinfo were introduced together in the
mainline kernel, and btrfs_get_default_subvolume_path() needs more ioctl() calls.

Quoted 4 lines

Quoted 1 line
No sure if I follow.
Read it and the patch again, and now it makes sense. Yes, the ioctl and subvolid= from mountinfo seems better than dependence on libbtrfs.

Also because util-linux is a build dependency for btrfs-progs.

Stanislav Brabec

author

2016-01-21 21:58 UTC #9 lore raw

PATCH 1/1 libmount: handle btrfs default subvolume mount

In reply to Karel Zak

When mounting btrfs volume without subvol= and subvolid=, and the btrfs volume has default subvolume defined, mount() mounts the default subvolume and not the volume root as other filesystems do.

To handle this situation correctly, libmount has to be capable to detect default subvolume.

Add btrfs.c and btrfs.h that implement needed functions.

This patch adds one new function to libmount API. Please update SONAME before release.

Known problems not covered by this patch:

- Use of subvolid= in fstab is not yet handled.

- Use of type auto in combination with subvol= in fstab is not yet handled.

- Use of btrfs in loop devices, where image file is specified in fstab is not yet handled (use of /dev/loop0 in fstab works).

Current behavior:
mount: /dev/loop0 is already mounted or /root/btrfs_mnt busy
/dev/loop0 is already mounted on /root/btrfs_mnt

Signed-off-by Stanislav Brabec <[email protected]>

Cc David Štěrba <[email protected]>

---
 libmount/src/Makemodule.am |   2 +
 libmount/src/btrfs.c       | 102 +++++++++++++++++++++++++++++++++++++
 libmount/src/btrfs.h       | 122 +++++++++++++++++++++++++++++++++++++++++++++
 libmount/src/libmount.h.in |   2 +
 libmount/src/mountP.h      |   6 +++
 libmount/src/tab.c         |  83 ++++++++++++++++++++++++++++--
 6 files changed, 313 insertions(+), 4 deletions(-)
 create mode 100644 libmount/src/btrfs.c
 create mode 100644 libmount/src/btrfs.h

libmount/src/Makemodule.am +2 -0

index 11c6324..39d42d5 100644
@@ -27,6 +27,8 @@ libmount_la_SOURCES = \
 
 if LINUX
 libmount_la_SOURCES += \
+	libmount/src/btrfs.c \
+	libmount/src/btrfs.h \
 	libmount/src/context.c \
 	libmount/src/context_loopdev.c \
 	libmount/src/context_mount.c \

libmount/src/btrfs.c new file +102 -0

102 lines not on this page · raw message

libmount/src/btrfs.h new file +122 -0

122 lines not on this page · raw message

libmount/src/libmount.h.in +2 -0

index 2338cc6..91b1698 100644
@@ -483,6 +483,8 @@ extern struct libmnt_fs *mnt_table_find_srcpath(struct libmnt_table *tb,
 				const char *path, int direction);
 extern struct libmnt_fs *mnt_table_find_tag(struct libmnt_table *tb, const char *tag,
 				const char *val, int direction);
+extern struct libmnt_fs *mnt_table_find_target_with_option(struct libmnt_table *tb, const char *path,
+			const char *option, const char *val, int direction);
 extern struct libmnt_fs *mnt_table_find_source(struct libmnt_table *tb,
 				const char *source, int direction);
 extern struct libmnt_fs *mnt_table_find_pair(struct libmnt_table *tb,

libmount/src/mountP.h +6 -0

index 25418a2..0bc67e5 100644
@@ -40,6 +40,7 @@
 #define MNT_DEBUG_CXT		(1 << 9)
 #define MNT_DEBUG_DIFF		(1 << 10)
 #define MNT_DEBUG_MONITOR	(1 << 11)
+#define MNT_DEBUG_BTRFS		(1 << 12)
 
 #define MNT_DEBUG_ALL		0xFFFF

@@ -411,4 +412,9 @@ extern int mnt_update_set_filename(struct libmnt_update *upd,
 extern int mnt_update_already_done(struct libmnt_update *upd,
 				   struct libmnt_lock *lc);
 
+#if __linux__
+/* btrfs.c */
+extern uint64_t btrfs_get_default_subvol_id(const char *path);
+#endif
+
 #endif /* _LIBMOUNT_PRIVATE_H */

libmount/src/tab.c +79 -4

103 lines not on this page · raw message

Signature · 13 lines

2.7.0

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                         e-mail: [email protected]
Lihovarská 1060/12                            tel: +49 911 7405384547
190 00 Praha 9                                 fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

Karel Zak

maintainer

2016-01-26 10:15 UTC #10 lore raw

Quoted 9 lines · Stanislav Brabec

 libmount/src/Makemodule.am |   2 +
 libmount/src/btrfs.c       | 102 +++++++++++++++++++++++++++++++++++++
 libmount/src/btrfs.h       | 122 +++++++++++++++++++++++++++++++++++++++++++++
 libmount/src/libmount.h.in |   2 +
 libmount/src/mountP.h      |   6 +++
 libmount/src/tab.c         |  83 ++++++++++++++++++++++++++++--
 6 files changed, 313 insertions(+), 4 deletions(-)
 create mode 100644 libmount/src/btrfs.c
 create mode 100644 libmount/src/btrfs.h

Merged with some minor changes. Thanks.

Karel

Signature · 2 lines

 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com

Stanislav Brabec

author

2016-01-28 14:22 UTC #11 lore raw

In reply to Stanislav Brabec

Quoted 3 lines · Stanislav Brabec

Known problems not covered by this patch:
- Use of subvolid= in fstab is not yet handled.

Patch already created, see mails:
[PATCH 1/2] libmount: run btrfs subvol checks for "subvolid" option [PATCH 2/2] libmount: code re-indentation

Quoted 2 lines

- Use of type auto in combination with subvol= in fstab is not yet handled.

Patch already created, see:
[PATCH] libmount: run btrfs subvol checks for "auto" fs type

Quoted 2 lines

- Use of btrfs in loop devices, where image file is specified in fstab is not yet handled (use of /dev/loop0 in fstab works).

With all patches above, I cannot reproduce any more.
Tested situations:
- with and without "loop" option
- with "subvol" or "subvolid" options referring to non-default subvolume
- without "subvol" or "subvolid" referring to default subvolume
- with "btrfs" and "auto" fs type
=> I think that it is now fixed as well.

Quoted 5 lines

- If fstab uses subvol=, and subvol path changes since last "mount -a", subsequent "mount -a" will not recognize that it is already mounted, and it will attempt to mount it second time. To fix it, libmount should remember subvolid in time of mount (subvolid is unique for the subvolume, subvol is not).

I have no plans to write a fix for that now. Too obscure situation.

Quoted 5 lines

- mountinfo contains subvol and subvolid since kernel 4.2. Before kernel 4.2, there is no reasonable way to solve this situation. (One would create temporary mount point, mount the default, call needed ioctl() to determine what was mounted, deduce the default subvolume, compare it with subvolume of mounted volume, unmount and return result.)

There is no reasonable way to detect whether default subvolume was mounted.

In case of mounting with subvolid, there is a way to detect subvolume path even before kernel 4.2: If mountinfo lookup fails, then use btrfs_get_default_subvolume_path(). (See my mail dated Thu, 21 Jan 2016 18:24:59 +0100 in this thread.) However fix is possible here, I have no plan to extent
[PATCH 1/2] libmount: run btrfs subvol checks for "subvolid" option
and implement it. It would be a fix of obscure situation in a kernel with unfixable issues of more common cases.

Summary: After applying of all patches mentioned here, I consider btrfs support as fixed in new kernels.

I strongly discourage advanced use of btrfs in fstab before kernel 4.2.

Signature · 10 lines

Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                         e-mail: [email protected]
Lihovarská 1060/12                            tel: +49 911 7405384547
190 00 Praha 9                                 fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

Karel Zak

maintainer

2016-02-01 12:18 UTC #12 lore raw

Quoted 8 lines · Stanislav Brabec

Quoted 3 lines · Stanislav Brabec
Known problems not covered by this patch:
- Use of subvolid= in fstab is not yet handled.
Patch already created, see mails:
[PATCH 1/2] libmount: run btrfs subvol checks for "subvolid" option [PATCH 2/2] libmount: code re-indentation

I did some changes to the code:

* use mnt_resolve_target() rather than mnt_resolve_spec() for target
* move all to get_btrfs_fs_root()
* make the basic if/else more readable

Please, please, test it.

It would be nice to have tests/ts/mount/btrfs with all the "mount -a" variants you have in commit messages.

Quoted 8 lines

Quoted 5 lines
- mountinfo contains subvol and subvolid since kernel 4.2. Before kernel 4.2, there is no reasonable way to solve this situation. (One would create temporary mount point, mount the default, call needed ioctl() to determine what was mounted, deduce the default subvolume, compare it with subvolume of mounted volume, unmount and return result.)
There is no reasonable way to detect whether default subvolume was mounted.

Quoted 8 lines

In case of mounting with subvolid, there is a way to detect subvolume path even before kernel 4.2: If mountinfo lookup fails, then use btrfs_get_default_subvolume_path(). (See my mail dated
Thu, 21 Jan 2016 18:24:59 +0100 in this thread.) However fix is possible here, I have no plan to extent
[PATCH 1/2] libmount: run btrfs subvol checks for "subvolid" option and implement it. It would be a fix of obscure situation in a kernel with unfixable issues of more common cases.

You know, perfect is the enemy of good :-)

Thanks
Karel

Signature · 2 lines

 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com

Stanislav Brabec

author

2016-02-01 15:38 UTC #13 lore raw

Quoted 5 lines · Karel Zak

I did some changes to the code:
* use mnt_resolve_target() rather than mnt_resolve_spec() for target
* move all to get_btrfs_fs_root()
* make the basic if/else more readable

By the way, I am just fixing next btrfs issue. Now it is in blkid. And it seems, that I will need some changes in the code: move the btrfs default subvol function into the public API, and change it to return the subvolid string.

What do you think? Should I make this function linux or linux+btrfs specific (missing in the API/ABI otherwise, where include file would contain #define MOUNT_HAS_BTRFS_SUPPORT or so), or implement it for all systems. On unsupported systems it would always return NULL (the define could exist in this case as well, but it would be optional).

Quoted 1 line

Please, please, test it.

I'll test it. I think that the attached reproducer should work on any system with btrfs support.

Quoted 2 lines

It would be nice to have tests/ts/mount/btrfs with all the "mount -a" variants you have in commit messages.

Are there any rules/framework for such tests that need root and editing of /etc/fstab?

I think that use of losetup and /dev/loopX can be used for most testcases. I am not sure about kernel partition table support inside loop devices.

And by the way, loop it the only device that can be used for TRIM testing: TRIM is internally converted to making unused blocks sparse.

Here is an example of TRIM testcase using loop:

dd if=/dev/null of=loop.img seek=1000000
mkfs.btrfs /root/loop.img
mount -oloop /root/loop.img /mnt
filefrag -e loop.img
for i in $(seq 0 100) ; do dd if=/dev/urandom of=/mnt/file$i bs=1M count=10 ; done filefrag -e loop.img
rm /mnt/file*
fstrim /mnt

# wait some time to complete
filefrag -e loop.img

Signature · 10 lines

Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                         e-mail: [email protected]
Lihovarská 1060/12                            tel: +49 911 7405384547
190 00 Praha 9                                 fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

Karel Zak

maintainer

2016-02-02 10:11 UTC #14 lore raw

Quoted 4 lines · Stanislav Brabec

Quoted 2 lines · Karel Zak
I did some changes to the code:

Quoted 8 lines

Quoted 3 lines
* use mnt_resolve_target() rather than mnt_resolve_spec() for target
* move all to get_btrfs_fs_root()
* make the basic if/else more readable
By the way, I am just fixing next btrfs issue. Now it is in blkid. And it seems, that I will need some changes in the code: move the btrfs default subvol function into the public API, and change it to return the subvolid string.

Why we need the function in public API?

Add subvolume ID to the probing result is no problem. It's just another NAME=value.

Quoted 5 lines

Quoted 2 lines
It would be nice to have tests/ts/mount/btrfs with all the "mount -a" variants you have in commit messages.
Are there any rules/framework for such tests that need root and editing of /etc/fstab?

See for example tests/ts/mount/fstab-devname,

ts_skip_nonroot -- ignore the test for non-root user
ts_fstab_add -- add line to the fstab

the functions are implemented in tests/functions.sh

Quoted 2 lines

I think that use of losetup and /dev/loopX can be used for most testcases. I am not sure about kernel partition table support inside loop devices.

We use:
ts_device_init -- for loop devies (uses truncate command)
ts_scsi_debug_init -- for devices by scsi_debug kernel module

scsi_debug is better solution, because it's absolutely without any side effects, but it's based on RAM, so create 10GiB disk is bad idea :)

For more details see tests/ts/* with many many examples.

Karel

Signature · 2 lines

 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com

Stanislav Brabec

author

2016-02-02 15:04 UTC #15 lore raw

Quoted 8 lines · Karel Zak

Quoted 4 lines · Stanislav Brabec
By the way, I am just fixing next btrfs issue. Now it is in blkid. And it seems, that I will need some changes in the code: move the btrfs default subvol function into the public API, and change it to return the subvolid string.
Why we need the function in public API?

Because lsblk does not have access to libmount private functions.

Signature · 10 lines

Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                         e-mail: [email protected]
Lihovarská 1060/12                            tel: +49 911 7405384547
190 00 Praha 9                                 fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

Karel Zak

maintainer

2016-02-02 18:43 UTC #16 lore raw

Quoted 11 lines · Stanislav Brabec

Quoted 8 lines · Karel Zak
Quoted 4 lines · Stanislav Brabec
By the way, I am just fixing next btrfs issue. Now it is in blkid. And it seems, that I will need some changes in the code: move the btrfs default subvol function into the public API, and change it to return the subvolid string.
Why we need the function in public API?
Because lsblk does not have access to libmount private functions.

You have talked about blkid, I thought you want to add SUBVOL= to libblkid.

For what purpose do you need it in lsblk? When the same device is mounded on more places?

Anyway, in libmount we can extend mnt_fs_* API and add some additional functions -- for example add mnt_fs_get_default_root().

I definitely don't want btrfs specific functions. The API should be generic.

Karel

Signature · 2 lines

 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com

Stanislav Brabec

author

2016-02-02 19:36 UTC #17 lore raw

Quoted 2 lines · Dne 2.2.2016 v 19:43 Karel Zak wrote:

For what purpose do you need it in lsblk? When the same device is mounded on more places?

When lsblk is called on a device with btrfs mounted, the lsblk heuristic may return sub-optimal (and even inconsistent) result.

Quoted 5 lines

Anyway, in libmount we can extend mnt_fs_* API and add some additional functions -- for example add mnt_fs_get_default_root().
I definitely don't want btrfs specific functions. The API should be generic.

The heuristic code is part of lsblk.c: get_device_mountpoint()

https://git.kernel.org/cgit/utils/util-linux/util-linux.git/tree/misc-utils/lsblk.c?id=4c01c98ca20b404940a4be749e729411d861ae97#n486

It already implements btrfs support, but the heurictic sometimes does not do what one would expect:

1) Search for ANY mount point that refers to device in question.

2) Check whether it is a fsroot if it is equal to "/" (always true for non-btrfs)

3) If not, search through all mounts that match 1).

4) Compare results, and if fsroot is equal to "/", use it.

5) Otherwise return result of 1).

In some installations (e. g. default openSUSE/SLE installation with btrfs root), fsroot is not mounted, 4) fails and 5) returns the sub-optimal guess 1).

The algorithm could be easily improved to search for the default subvolume as well, and only if it is not mounted, use the sub-optimal result.

My idea was:

1) Make btrfs_get_default_subvol_id_str() public

2) Add another loop to get_device_mountpoint() (before existing one) searching for a volume, where subvol_id_str returned by btrfs_get_default_subvol_id_str() matches option in the mountinfo.

Use of mnt_fs_get_default_root() would work as well, with a bit of overhead.

Signature · 10 lines

Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                         e-mail: [email protected]
Lihovarská 1060/12                            tel: +49 911 7405384547
190 00 Praha 9                                 fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76