On Wed, Jul 16, 2014 at 12:07:10PM +0800, Qu Wenruo wrote:
btrfs uses differnet routine to handle 'subvolid=' and 'subvol=' mount
option.
Given 'subvol=' mount option, btrfs will mount btrfs first and then call
mount_subtree() to mount a subtree of btrfs, making vfs handle the path
searching.
This is good since vfs layer know extactly that a subtree mount is done
and findmnt(8) knows which subtree is mounted.
However when using 'subvolid=' mount option, btrfs will do all the
internal subvolume objectid searching and checking, making VFS unaware
about which subtree is mounted, as result, findmnt(8) can't showing any
useful subtree mount info for end users.
This patch will use the root backref to reverse search the subvolume
path for a given subvolid, making findmnt(8) works again.
Reported-by: Stefan G.Weichinger <li...@xunil.at>
Signed-off-by: Qu Wenruo <quwen...@cn.fujitsu.com>
Ack for unifying the way subvol= and subvolid= are handled, but I don't
like some aspects of the implementation.
The kmalloc/krealloc makes it really complicated and is not imho
necessary. The mount options length is limited to PAGE_SIZE in the vfs
code. Do the same here, allocate a page, filter the options, do the
necessary processing and just check for overflows.
You can drop u64_to_strlen.
+#define CLEAR_SUBVOL 1
+#define CLEAR_SUBVOLID 2
Though they're internal and local to the file, please add BTRFS_ prefix
at least.
/*
- * This will strip out the subvol=%s argument for an argument string and add
- * subvolid=0 to make sure we get the actual tree root for path walking to the
- * subvol we want.
+ * This will strip out the subvol=%s or subvolid=%s argument for an argumen
+ * string and add subvolid=0 to make sure we get the actual tree root for path
+ * walking to the subvol we want.
*/
-static char *setup_root_args(char *args)
+static char *setup_root_args(char *args, int flags, u64 subvol_objectid)
{
- unsigned len = strlen(args) + 2 + 1;
- char *src, *dst, *buf;
+ unsigned len;
+ char *src = NULL, *dst, *buf, *comma;
Please use the recommended style and put each on a separate line. I'm
not sure if you'll need all of them for the implementation witouth the
kmallocs, the comment applies generally.
+ char *subvol_string = "subvolid=";
+ int option_len = 0;
+
+ if (!args) {
+ /* Case 1, not args, all default mounting
+ * just return 'subvolid=<FS_ROOT>' */
Not the preferred style of comments.
+ len = strlen(subvol_string) +
+ u64_to_strlen(subvol_objectid) + 1;
+ dst = kmalloc(len, GFP_NOFS);
+ if (!dst)
+ return NULL;
+ sprintf(dst, "%s%llu", subvol_string, subvol_objectid);
+ return dst;
+ }
- /*
- * We need the same args as before, but with this substitution:
- * s!subvol=[^,]+!subvolid=0!
- *
- * Since the replacement string is up to 2 bytes longer than the
- * original, allocate strlen(args) + 2 + 1 bytes.
- */
+ switch (flags) {
+ case CLEAR_SUBVOL:
+ src = strstr(args, "subvol=");
+ break;
+ case CLEAR_SUBVOLID:
+ src = strstr(args, "subvolid=");
+ break;
+ }
- src = strstr(args, "subvol=");
- /* This shouldn't happen, but just in case.. */
- if (!src)
- return NULL;
+ if (!src) {
+ /* Case 2, some args, default subvolume mounting
+ * just append ',subvolid=<FS_ROOT>' */
+
+ /* 1 for ending '\0', 1 for leading ',' */
+ len = strlen(args) + strlen(subvol_string) +
+ u64_to_strlen(subvol_objectid) + 2;
+ dst = kmalloc(len, GFP_NOFS);
+ if (!dst)
+ return NULL;
+ strcpy(dst, args);
+ sprintf(dst + strlen(args), ",%s%llu", subvol_string,
+ subvol_objectid);
+ return dst;
+ }
+
+ /* Case 3, subvolid=/subvol= mount
+ * repalce the 'subvolid/subvol' options to 'subvolid=<FS_ROOT>' */
+ comma = strchr(src, ',');
+ if (comma)
+ option_len = comma - src;
+ else
+ option_len = strlen(src);
+ len = strlen(args) - option_len + strlen(subvol_string) +
+ u64_to_strlen(subvol_objectid) + 1;
buf = dst = kmalloc(len, GFP_NOFS);
if (!buf)
@@ -1154,28 +1208,126 @@ static char *setup_root_args(char *args)
dst += strlen(args);
}
- strcpy(dst, "subvolid=0");
- dst += strlen("subvolid=0");
+ len = sprintf(dst, "%s%llu", subvol_string, subvol_objectid);
+ dst += len;
/*
* If there is a "," after the original subvol=... string,
* copy that suffix into our buffer. Otherwise, we're done.
*/
- src = strchr(src, ',');
- if (src)
- strcpy(dst, src);
+ if (comma)
+ strcpy(dst, comma);
return buf;
}
-static struct dentry *mount_subvol(const char *subvol_name, int flags,
- const char *device_name, char *data)
+static char *str_append_head(char *dest, char *src)
I think this is called 'prepend' :)
+{
+ memmove(dest + strlen(src), dest, strlen(dest) + 1);
+ memcpy(dest, src, strlen(src));
Yes, prepends src to dest inplace.
+ return dest;
+}
+
+/* Find the path for given subvol_objectid.
+ * Caller needs to readlock the root tree and kzalloc PATH_MAX for
+ * subvol_name and namebuf */
+static char *find_subvol_by_id(struct btrfs_root *root, u64 subvol_objectid)
+{
+ struct btrfs_key key;
+ struct btrfs_key found_key;
+ struct btrfs_root_ref *ref;
+ struct btrfs_path *path;
+ char *namebuf = NULL;
+ char *new_buf = NULL;
+ char *subvol_ret = NULL;
+ int ret = 0;
+ u16 namelen = 0;
+
+ path = btrfs_alloc_path();
+ /* Alloc 1 byte for later strlen() calls */
+ subvol_ret = kzalloc(1, GFP_NOFS);
+ if (!path || !subvol_ret) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ key.objectid = subvol_objectid;
+ key.type = BTRFS_ROOT_BACKREF_KEY;
+ key.offset = 0;
+ /* We don't need to lock the tree_root,
+ * if when we do the backref walking, some one deleted/moved
+ * the subvol, we just return -ENOENT or let mount_subtree
+ * return -ENOENT and no disaster will happen.
+ * User should not modify subvolume when trying to mount it */
+ while (key.objectid != BTRFS_FS_TREE_OBJECTID) {
+ ret = btrfs_search_slot_for_read(root, &key, path, 1, 1);
+ if (ret < 0)
+ goto out;
+ if (ret) {
+ ret = -ENOENT;
+ goto out;
+ }
+ btrfs_item_key_to_cpu(path->nodes[0], &found_key,
+ path->slots[0]);
+ if (found_key.objectid != key.objectid ||
+ found_key.type != BTRFS_ROOT_BACKREF_KEY) {
+ ret = -ENOENT;
+ goto out;
+ }
+ key.objectid = found_key.offset;
+ ref = btrfs_item_ptr(path->nodes[0], path->slots[0],
+ struct btrfs_root_ref);
+ namelen = btrfs_root_ref_name_len(path->nodes[0], ref);
+ /* One for ending '\0' One for '/' */
+ new_buf = krealloc(namebuf, namelen + 2, GFP_NOFS);
+ if (!new_buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ namebuf = new_buf;
+ read_extent_buffer(path->nodes[0], namebuf,
+ (unsigned long)(ref + 1), namelen);
+ btrfs_release_path(path);
+ *(namebuf + namelen) = '/';
+ *(namebuf + namelen + 1) = '\0';
+
+ new_buf = krealloc(subvol_ret, strlen(subvol_ret) + namelen + 2,
+ GFP_NOFS);
+ if (!new_buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ subvol_ret = new_buf;
+ str_append_head(subvol_ret, namebuf);
+ }
+out:
+ kfree(namebuf);
+ btrfs_free_path(path);
+ if (ret < 0) {
+ kfree(subvol_ret);
+ return ERR_PTR(ret);
+ } else
+ return subvol_ret;
+
+}
+
+static struct dentry *mount_subvol(const char *subvol_name, u64
subvol_objectid,
+ int flags, const char *device_name,
+ char *data)
{
struct dentry *root;
struct vfsmount *mnt;
+ struct btrfs_root *tree_root = NULL;
char *newargs;
+ char *subvol_ret = NULL;
+ int ret = 0;
- newargs = setup_root_args(data);
+ if (subvol_name)
+ newargs = setup_root_args(data, CLEAR_SUBVOL,
+ BTRFS_FS_TREE_OBJECTID);
+ else
+ newargs = setup_root_args(data, CLEAR_SUBVOLID,
+ BTRFS_FS_TREE_OBJECTID);
There's no other value passed to setup_root_args than
BTRFS_FS_TREE_OBJECTID? So you can put it directly to setup_root_args,
or I'm missing someting.
