[PATCH 16/24] btrfs: return actual error on btrfs_init_sysfs
From: Jie Liu jeff@oracle.com Return the actual error code if call kset_create_and_add() failed Cc: Chris Mason c...@fb.com Cc: Josef Bacik jba...@fb.com Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/sysfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c index df39458..8f7dfa7 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -705,8 +705,8 @@ int btrfs_init_sysfs(void) int ret; btrfs_kset = kset_create_and_add(btrfs, NULL, fs_kobj); - if (!btrfs_kset) - return -ENOMEM; + if (IS_ERR(btrfs_kset)) + return PTR_ERR(btrfs_kset); ret = btrfs_init_debugfs(); if (ret) -- 1.8.3.2 -- 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
Re: [PATCH 16/24] btrfs: return actual error on btrfs_init_sysfs
Hi, On 06/17/2014 22:32 PM, Jeff Liu wrote: From: Jie Liu jeff@oracle.com Return the actual error code if call kset_create_and_add() failed Cc: Chris Mason c...@fb.com Cc: Josef Bacik jba...@fb.com Signed-off-by: Jie Liu jeff@oracle.com Please ignore this patch because Greg think that is incorrect. Cheers, -Jeff -- 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
Re: [PATCH] xfstests: generic/315: allow a little tolerance for our used check
On 07/30/2013 02:55 AM, Josef Bacik wrote: So df in btrfs is tricky at best, and relying on it for accurate information is not great, but it's the best way to verify this test. To get around btrfs being inconsistent sometimes just use _within_tolerance to check our new df value to make sure that our truncate did something. With this patch I no longer see transient failures of this test. Thanks, Signed-off-by: Josef Bacik jba...@fusionio.com --- tests/generic/315 |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/tests/generic/315 b/tests/generic/315 index 7cfc40d..9c01b5e 100644 --- a/tests/generic/315 +++ b/tests/generic/315 @@ -73,7 +73,8 @@ sync # Preallocated disk space should be released avail_done=`df -P $TEST_DIR | awk 'END {print $4}'` -[ $avail_done -eq $avail_begin ] || _fail Available disk space ($avail_done KiB) +_within_tolerance df $avail_done $avail_begin 1% +[ $? -eq 0 ] || _fail Available disk space ($avail_done KiB) wanted ($avail_begin KiB) Looks good to me. Reviewed-by: Jie Liu jeff@oracle.com Thanks, -Jeff -- 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
Re: [PATCH] btrfs: fix file truncation if FALLOC_FL_KEEP_SIZE is specified
On 06/28/2013 08:41 PM, Josef Bacik wrote: On Fri, Jun 28, 2013 at 01:15:52PM +0800, Jeff Liu wrote: From: Jie Liu jeff@oracle.com Create a small file and fallocate it to a big size with FALLOC_FL_KEEP_SIZE option, then truncate it back to the small size again, the disk free space is not changed back in this case. i.e, # dd if=/dev/zero of=/mnt/test bs=512 count=1 # ls -l /mnt total 4 -rw-r--r-- 1 root root 512 Jun 28 11:35 test # df -h Filesystem Size Used Avail Use% Mounted on /dev/sdb1 8.0G 56K 7.2G 1% /mnt # xfs_io -c 'falloc -k 512 5G' /mnt/test # ls -l /mnt/test -rw-r--r-- 1 root root 512 Jun 28 11:35 /mnt/test # sync; df -h Filesystem Size Used Avail Use% Mounted on /dev/sdb1 8.0G 5.1G 2.2G 70% /mnt # xfs_io -c 'truncate 512' /mnt/test # sync; df -h Filesystem Size Used Avail Use% Mounted on /dev/sdb1 8.0G 5.1G 2.2G 70% /mnt With this fix, the truncated up space is back as: # sync; df -h Filesystem Size Used Avail Use% Mounted on /dev/sdb1 8.0G 56K 7.2G 1% /mnt Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/inode.c |3 --- 1 file changed, 3 deletions(-) diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 4f9d16b..7e1a5ff 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -4509,9 +4509,6 @@ static int btrfs_setsize(struct inode *inode, struct iattr *attr) int mask = attr-ia_valid; int ret; -if (newsize == oldsize) -return 0; - /* * The regular truncate() case without ATTR_CTIME and ATTR_MTIME is a * special case where we need to update the times despite not having Cc'ing a few people on this since I'd like their opinion. Looking at other fs's it looks like ext4 does the same thing we do and would leave the prealloc'ed space, but it appears that xfs will truncate it. What do we think is the correct behavior? I'm inclined to take this patch, but I'd like to have an xfstest made for it so other file systems can be made to be consistent, and I'd like to make sure we all agree what is the correct behavior before we wander down that road. Thanks, Looks Ext4 does the same thing to XFS in this case :), but OCFS2 does not. I'd like to write a test case for xfstest if we reach an agreement. Thanks, -Jeff -- 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
Re: [PATCH] btrfs: fix file truncation if FALLOC_FL_KEEP_SIZE is specified
On 06/29/2013 10:22 AM, Dave Chinner wrote: On Fri, Jun 28, 2013 at 08:41:00AM -0400, Josef Bacik wrote: On Fri, Jun 28, 2013 at 01:15:52PM +0800, Jeff Liu wrote: From: Jie Liu jeff@oracle.com Create a small file and fallocate it to a big size with FALLOC_FL_KEEP_SIZE option, then truncate it back to the small size again, the disk free space is not changed back in this case. i.e, # dd if=/dev/zero of=/mnt/test bs=512 count=1 # ls -l /mnt total 4 -rw-r--r-- 1 root root 512 Jun 28 11:35 test # df -h Filesystem Size Used Avail Use% Mounted on /dev/sdb1 8.0G 56K 7.2G 1% /mnt # xfs_io -c 'falloc -k 512 5G' /mnt/test # ls -l /mnt/test -rw-r--r-- 1 root root 512 Jun 28 11:35 /mnt/test # sync; df -h Filesystem Size Used Avail Use% Mounted on /dev/sdb1 8.0G 5.1G 2.2G 70% /mnt # xfs_io -c 'truncate 512' /mnt/test # sync; df -h Filesystem Size Used Avail Use% Mounted on /dev/sdb1 8.0G 5.1G 2.2G 70% /mnt With this fix, the truncated up space is back as: # sync; df -h Filesystem Size Used Avail Use% Mounted on /dev/sdb1 8.0G 56K 7.2G 1% /mnt Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/inode.c |3 --- 1 file changed, 3 deletions(-) diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 4f9d16b..7e1a5ff 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -4509,9 +4509,6 @@ static int btrfs_setsize(struct inode *inode, struct iattr *attr) int mask = attr-ia_valid; int ret; - if (newsize == oldsize) - return 0; - /* * The regular truncate() case without ATTR_CTIME and ATTR_MTIME is a * special case where we need to update the times despite not having Cc'ing a few people on this since I'd like their opinion. Looking at other fs's it looks like ext4 does the same thing we do and would leave the prealloc'ed space, but it appears that xfs will truncate it. What do we think is the correct behavior? XFS has had this truncate behaviour since at least the start of the git tree history (2005). Given that these fallocate() prealloc-blocks-beyond-EOF behaviours are modelled on what XFS has historically provided, I think y'all can see what i think should be done... I'm inclined to take this patch, but I'd like to have an xfstest made for it so other file systems can be made to be consistent, and I'd like to make sure we all agree what is the correct behavior before we wander down that road. Thanks, I couldn't have said it better myself. Jeff, can you take care of this, please? Yes, I'll take care of this. Thanks, -Jeff -- 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
[PATCH] btrfs: fix file truncation if FALLOC_FL_KEEP_SIZE is specified
From: Jie Liu jeff@oracle.com Create a small file and fallocate it to a big size with FALLOC_FL_KEEP_SIZE option, then truncate it back to the small size again, the disk free space is not changed back in this case. i.e, # dd if=/dev/zero of=/mnt/test bs=512 count=1 # ls -l /mnt total 4 -rw-r--r-- 1 root root 512 Jun 28 11:35 test # df -h Filesystem Size Used Avail Use% Mounted on /dev/sdb1 8.0G 56K 7.2G 1% /mnt # xfs_io -c 'falloc -k 512 5G' /mnt/test # ls -l /mnt/test -rw-r--r-- 1 root root 512 Jun 28 11:35 /mnt/test # sync; df -h Filesystem Size Used Avail Use% Mounted on /dev/sdb1 8.0G 5.1G 2.2G 70% /mnt # xfs_io -c 'truncate 512' /mnt/test # sync; df -h Filesystem Size Used Avail Use% Mounted on /dev/sdb1 8.0G 5.1G 2.2G 70% /mnt With this fix, the truncated up space is back as: # sync; df -h Filesystem Size Used Avail Use% Mounted on /dev/sdb1 8.0G 56K 7.2G 1% /mnt Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/inode.c |3 --- 1 file changed, 3 deletions(-) diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 4f9d16b..7e1a5ff 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -4509,9 +4509,6 @@ static int btrfs_setsize(struct inode *inode, struct iattr *attr) int mask = attr-ia_valid; int ret; - if (newsize == oldsize) - return 0; - /* * The regular truncate() case without ATTR_CTIME and ATTR_MTIME is a * special case where we need to update the times despite not having -- 1.7.9.5 -- 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
[PATCH] btrfs: fix the code comments for LZO compression workspace
From: Jie Liu jeff@oracle.com Fix the code comments for lzo compression workspace. The buf item is used to store the decompressed data and cbuf is used to store the compressed data. Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/lzo.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c index 743b86f..f93151a 100644 --- a/fs/btrfs/lzo.c +++ b/fs/btrfs/lzo.c @@ -31,8 +31,8 @@ struct workspace { void *mem; - void *buf; /* where compressed data goes */ - void *cbuf; /* where decompressed data goes */ + void *buf; /* where decompressed data goes */ + void *cbuf; /* where compressed data goes */ struct list_head list; }; -- 1.7.9.5 -- 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
[PATCH v2 1/2] btrfs-progs: refactor check_label()
Refactor check_label(). - Make it be static at first, this is a preparation step since we'll remove btrfslabel.[c|h] and move those functions from there to utils.[c|h], we can do pre-checking against the input label string with it. - Fix the label length check up from BTRFS_LABEL_SIZE to BTRFS_LABEL_SIZE - 1. - Kill the check of label contains an invalid character, see below commits for detail: 79e0e445fc2365e47fc7f060d5a4445d37e184b8 btrfs-progs: kill check for /'s in labels. Signed-off-by: Jie Liu jeff@oracle.com CC: David Sterba dste...@suse.cz CC: Gene Czarcinski g...@czarc.net --- utils.c | 14 -- utils.h |1 - 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/utils.c b/utils.c index d59bca3..9dc688a 100644 --- a/utils.c +++ b/utils.c @@ -1120,23 +1120,17 @@ char *pretty_sizes(u64 size) * Returns: 0if everything is safe and usable -1if the label is too long - -2if the label contains an invalid character */ -int check_label(char *input) +static int check_label(const char *input) { - int i; int len = strlen(input); - if (len BTRFS_LABEL_SIZE) { + if (len BTRFS_LABEL_SIZE - 1) { + fprintf(stderr, ERROR: Label %s is too long (max %d)\n, + input, BTRFS_LABEL_SIZE - 1); return -1; } - for (i = 0; i len; i++) { - if (input[i] == '/' || input[i] == '\\') { - return -2; - } - } - return 0; } diff --git a/utils.h b/utils.h index 8750f28..a0b782b 100644 --- a/utils.h +++ b/utils.h @@ -42,7 +42,6 @@ int check_mounted_where(int fd, const char *file, char *where, int size, int btrfs_device_already_in_root(struct btrfs_root *root, int fd, int super_offset); char *pretty_sizes(u64 size); -int check_label(char *input); int get_mountpt(char *dev, char *mntpt, size_t size); int btrfs_scan_block_devices(int run_ioctl); -- 1.7.9.5 -- 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
[PATCH v2 2/2] btrfs-progs: move btrfslabel.[c|h] stuff to utils.[c|h]
Clean btrfslabel.[c|h] out of the source tree and move those related functions to utils.[c|h]. Signed-off-by: Jie Liu jeff@oracle.com CC: David Sterba dste...@suse.cz CC: Gene Czarcinski g...@czarc.net --- Makefile |4 +- btrfslabel.c | 178 - btrfslabel.h |5 -- cmds-filesystem.c |1 - utils.c | 130 ++ utils.h |2 + 6 files changed, 134 insertions(+), 186 deletions(-) delete mode 100644 btrfslabel.c delete mode 100644 btrfslabel.h diff --git a/Makefile b/Makefile index 4894903..e54b21e 100644 --- a/Makefile +++ b/Makefile @@ -4,8 +4,8 @@ CFLAGS = -g -O1 objects = ctree.o disk-io.o radix-tree.o extent-tree.o print-tree.o \ root-tree.o dir-item.o file-item.o inode-item.o \ inode-map.o crc32c.o rbtree.o extent-cache.o extent_io.o \ - volumes.o utils.o btrfs-list.o btrfslabel.o repair.o \ - send-stream.o send-utils.o qgroup.o + volumes.o utils.o btrfs-list.o repair.o send-stream.o \ + send-utils.o qgroup.o cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \ cmds-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \ cmds-quota.o cmds-qgroup.o diff --git a/btrfslabel.c b/btrfslabel.c deleted file mode 100644 index 2826050..000 --- a/btrfslabel.c +++ /dev/null @@ -1,178 +0,0 @@ -/* - * Copyright (C) 2008 Morey Roof. All rights reserved. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public - * License v2 as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public - * License along with this program; if not, write to the - * Free Software Foundation, Inc., 59 Temple Place - Suite 330, - * Boston, MA 021110-1307, USA. - */ - -#define _GNU_SOURCE - -#ifndef __CHECKER__ -#include sys/ioctl.h -#include sys/mount.h -#include ioctl.h -#endif /* __CHECKER__ */ - -#include stdio.h -#include stdlib.h -#include sys/types.h -#include sys/stat.h -#include dirent.h -#include fcntl.h -#include unistd.h -#include linux/fs.h -#include linux/limits.h -#include ctype.h -#include kerncompat.h -#include ctree.h -#include utils.h -#include version.h -#include disk-io.h -#include transaction.h - -#define MOUNTED1 -#define UNMOUNTED 2 -#define GET_LABEL 3 -#define SET_LABEL 4 - -static int set_label_unmounted(const char *dev, const char *label) -{ - struct btrfs_trans_handle *trans; - struct btrfs_root *root; - int ret; - - ret = check_mounted(dev); - if (ret 0) { - fprintf(stderr, FATAL: error checking %s mount status\n, dev); - return -1; - } - if (ret 0) { - fprintf(stderr, ERROR: dev %s is mounted, use mount point\n, - dev); - return -1; - } - - if (strlen(label) BTRFS_LABEL_SIZE - 1) { - fprintf(stderr, ERROR: Label %s is too long (max %d)\n, - label, BTRFS_LABEL_SIZE - 1); - return -1; - } - - /* Open the super_block at the default location -* and as read-write. -*/ - root = open_ctree(dev, 0, 1); - if (!root) /* errors are printed by open_ctree() */ - return -1; - - trans = btrfs_start_transaction(root, 1); - snprintf(root-fs_info-super_copy.label, BTRFS_LABEL_SIZE, %s, -label); - btrfs_commit_transaction(trans, root); - - /* Now we close it since we are done. */ - close_ctree(root); - return 0; -} - -static int set_label_mounted(const char *mount_path, const char *label) -{ - int fd; - - fd = open(mount_path, O_RDONLY | O_NOATIME); - if (fd 0) { - fprintf(stderr, ERROR: unable access to '%s'\n, mount_path); - return -1; - } - - if (ioctl(fd, BTRFS_IOC_SET_FSLABEL, label) 0) { - fprintf(stderr, ERROR: unable to set label %s\n, - strerror(errno)); - close(fd); - return -1; - } - - return 0; -} - -static int get_label_unmounted(const char *dev) -{ - struct btrfs_root *root; - int ret; - - ret = check_mounted(dev); - if (ret 0) { - fprintf(stderr, FATAL: error checking %s mount status\n, dev); - return -1; - } - if (ret 0) { - fprintf(stderr, ERROR: dev %s is mounted, use mount point\n, -
Re: [PATCH 2/2] btrfs-progs: remove btrfslabel.[c|h]
On 01/29/2013 06:26 PM, Stefan Behrens wrote: On Tue, 29 Jan 2013 14:24:13 +0800, Jeff Liu wrote: Clean btrfslabel.[c|h] out of the source tree and move those related functions to utils.[c|h]. Signed-off-by: Jie Liu jeff@oracle.com CC: David Sterba dste...@suse.cz CC: Gene Czarcinski g...@czarc.net --- Makefile |4 +- btrfslabel.c | 178 - btrfslabel.h |5 -- cmds-filesystem.c |1 - utils.c | 129 ++ utils.h |2 + 6 files changed, 133 insertions(+), 186 deletions(-) delete mode 100644 btrfslabel.c delete mode 100644 btrfslabel.h diff --git a/Makefile b/Makefile index 4894903..e54b21e 100644 --- a/Makefile +++ b/Makefile @@ -4,8 +4,8 @@ CFLAGS = -g -O1 objects = ctree.o disk-io.o radix-tree.o extent-tree.o print-tree.o \ root-tree.o dir-item.o file-item.o inode-item.o \ inode-map.o crc32c.o rbtree.o extent-cache.o extent_io.o \ - volumes.o utils.o btrfs-list.o btrfslabel.o repair.o \ - send-stream.o send-utils.o qgroup.o + volumes.o utils.o btrfs-list.o repair.o send-stream.o \ + send-utils.o qgroup.o cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \ cmds-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \ cmds-quota.o cmds-qgroup.o diff --git a/btrfslabel.c b/btrfslabel.c deleted file mode 100644 index 2826050..000 --- a/btrfslabel.c +++ /dev/null @@ -1,178 +0,0 @@ -/* - * Copyright (C) 2008 Morey Roof. All rights reserved. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public - * License v2 as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public - * License along with this program; if not, write to the - * Free Software Foundation, Inc., 59 Temple Place - Suite 330, - * Boston, MA 021110-1307, USA. - */ - -#define _GNU_SOURCE - -#ifndef __CHECKER__ -#include sys/ioctl.h -#include sys/mount.h -#include ioctl.h -#endif /* __CHECKER__ */ - -#include stdio.h -#include stdlib.h -#include sys/types.h -#include sys/stat.h -#include dirent.h -#include fcntl.h -#include unistd.h -#include linux/fs.h -#include linux/limits.h -#include ctype.h -#include kerncompat.h -#include ctree.h -#include utils.h -#include version.h -#include disk-io.h -#include transaction.h - -#define MOUNTED1 -#define UNMOUNTED 2 -#define GET_LABEL 3 -#define SET_LABEL 4 - -static int set_label_unmounted(const char *dev, const char *label) -{ -struct btrfs_trans_handle *trans; -struct btrfs_root *root; -int ret; - -ret = check_mounted(dev); -if (ret 0) { - fprintf(stderr, FATAL: error checking %s mount status\n, dev); - return -1; -} -if (ret 0) { -fprintf(stderr, ERROR: dev %s is mounted, use mount point\n, -dev); -return -1; -} - -if (strlen(label) BTRFS_LABEL_SIZE - 1) { -fprintf(stderr, ERROR: Label %s is too long (max %d)\n, -label, BTRFS_LABEL_SIZE - 1); -return -1; -} - -/* Open the super_block at the default location - * and as read-write. - */ -root = open_ctree(dev, 0, 1); -if (!root) /* errors are printed by open_ctree() */ -return -1; - -trans = btrfs_start_transaction(root, 1); -snprintf(root-fs_info-super_copy.label, BTRFS_LABEL_SIZE, %s, - label); -btrfs_commit_transaction(trans, root); - -/* Now we close it since we are done. */ -close_ctree(root); -return 0; -} - -static int set_label_mounted(const char *mount_path, const char *label) -{ -int fd; - -fd = open(mount_path, O_RDONLY | O_NOATIME); -if (fd 0) { -fprintf(stderr, ERROR: unable access to '%s'\n, mount_path); -return -1; -} - -if (ioctl(fd, BTRFS_IOC_SET_FSLABEL, label) 0) { -fprintf(stderr, ERROR: unable to set label %s\n, -strerror(errno)); -close(fd); -return -1; -} - -return 0; -} - -static int get_label_unmounted(const char *dev) -{ -struct btrfs_root *root; -int ret; - -ret = check_mounted(dev); -if (ret 0) { - fprintf(stderr, FATAL: error checking %s mount status\n, dev); - return -1; -} -if (ret 0) { -fprintf
Re: [PATCH 1/2] btrfs-progs: refactor check_label()
On 01/29/2013 11:19 PM, David Sterba wrote: On Tue, Jan 29, 2013 at 02:24:12PM +0800, Jeff Liu wrote: --- a/utils.c +++ b/utils.c @@ -1122,17 +1122,21 @@ char *pretty_sizes(u64 size) -1if the label is too long -2if the label contains an invalid character */ -int check_label(char *input) +static int check_label(char *input) { int i; int len = strlen(input); - if (len BTRFS_LABEL_SIZE) { + if (len BTRFS_LABEL_SIZE - 1) { +fprintf(stderr, ERROR: Label %s is too long (max %d)\n, +input, BTRFS_LABEL_SIZE - 1); return -1; } for (i = 0; i len; i++) { if (input[i] == '/' || input[i] == '\\') { +fprintf(stderr, ERROR: Label %s contains invalid +characters\n, input); return -2; } Plase drop this check, see http://repo.or.cz/w/btrfs-progs-unstable/devel.git/commit/79e0e445fc2365e47fc7f060d5a4445d37e184b8 (also function comment and maybe the callers) btrfs-progs: kill check for /'s in labels This patch kills a check in mkfs's label stuff which doesn't allow labels that have /'s in them. This causes problems for Anaconda which try to label volumes with their mountpoints. (mkfs.c) Ok, so looks we can safely clean this routine out of the code base since there is no other users call it if am not missing anything. Thanks, -Jeff -- 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
Re: [PATCH] Btrfs-progs: v2 Fix minor problems in btrfslabel.c
Hi David, On 01/29/2013 12:16 AM, David Sterba wrote: On Sat, Jan 26, 2013 at 01:09:56PM -0500, Gene Czarcinski wrote: Fixed indentation, replace spaces with tabs. Signed-off-by: Danny Kukawka danny.kuka...@bisect.de Rebased; fixed compiler warnings; added space after if Signed-off-by: Gene Czarcinski g...@czarc.net --- btrfslabel.c | 57 + Jeff Liu has sent a series that updates the code and also fixes the formatting. And I'd prefer to take his series as a more extensive update. Also, btrfslabel.c is another file to be removed and its contents placed into the right locations, like cmds-filesystem.c or util.[ch] . I didn't realize that during reviewing the label series. Jeff, I suggest to add a new patch that moves the functions out of btrfslabel.c, so you don't have to rework the whole series again. I'll deal with it later, Thanks for your kind suggestion. -Jeff -- 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
[PATCH 0/2] btrfs-progs: btrfslabel source code consolidation
Hello, As per David's suggestions, we should remove btrfslabel source code and move the related functions to cmds-filesystems or utils. I prefer to move them into utils since there already has a check_label() function which can be used for the pre-checkup against the input label. This patch set is based on my previous v5 patches of add get/set label support against a mounted filesystem: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/21713 Thanks, -Jeff -- 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
[PATCH 1/2] btrfs-progs: refactor check_label()
Refactor check_label(). Make it be static at first, this is a preparation step since we'll remove btrfslabel.[c|h] and move those functions at them to utils.[c|h], we can do pre-checking against the input label string with it. Also, fix the input lable length verfication from BTRFS_LABEL_SIZE to BTRFS_LABEL_SIZE - 1. Signed-off-by: Jie Liu jeff@oracle.com CC: David Sterba dste...@suse.cz CC: Gene Czarcinski g...@czarc.net --- utils.c |8 ++-- utils.h |1 - 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/utils.c b/utils.c index d59bca3..034da8f 100644 --- a/utils.c +++ b/utils.c @@ -1122,17 +1122,21 @@ char *pretty_sizes(u64 size) -1if the label is too long -2if the label contains an invalid character */ -int check_label(char *input) +static int check_label(char *input) { int i; int len = strlen(input); - if (len BTRFS_LABEL_SIZE) { + if (len BTRFS_LABEL_SIZE - 1) { + fprintf(stderr, ERROR: Label %s is too long (max %d)\n, + input, BTRFS_LABEL_SIZE - 1); return -1; } for (i = 0; i len; i++) { if (input[i] == '/' || input[i] == '\\') { + fprintf(stderr, ERROR: Label %s contains invalid + characters\n, input); return -2; } } diff --git a/utils.h b/utils.h index 8750f28..a0b782b 100644 --- a/utils.h +++ b/utils.h @@ -42,7 +42,6 @@ int check_mounted_where(int fd, const char *file, char *where, int size, int btrfs_device_already_in_root(struct btrfs_root *root, int fd, int super_offset); char *pretty_sizes(u64 size); -int check_label(char *input); int get_mountpt(char *dev, char *mntpt, size_t size); int btrfs_scan_block_devices(int run_ioctl); -- 1.7.9.5 -- 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
[PATCH 2/2] btrfs-progs: remove btrfslabel.[c|h]
Clean btrfslabel.[c|h] out of the source tree and move those related functions to utils.[c|h]. Signed-off-by: Jie Liu jeff@oracle.com CC: David Sterba dste...@suse.cz CC: Gene Czarcinski g...@czarc.net --- Makefile |4 +- btrfslabel.c | 178 - btrfslabel.h |5 -- cmds-filesystem.c |1 - utils.c | 129 ++ utils.h |2 + 6 files changed, 133 insertions(+), 186 deletions(-) delete mode 100644 btrfslabel.c delete mode 100644 btrfslabel.h diff --git a/Makefile b/Makefile index 4894903..e54b21e 100644 --- a/Makefile +++ b/Makefile @@ -4,8 +4,8 @@ CFLAGS = -g -O1 objects = ctree.o disk-io.o radix-tree.o extent-tree.o print-tree.o \ root-tree.o dir-item.o file-item.o inode-item.o \ inode-map.o crc32c.o rbtree.o extent-cache.o extent_io.o \ - volumes.o utils.o btrfs-list.o btrfslabel.o repair.o \ - send-stream.o send-utils.o qgroup.o + volumes.o utils.o btrfs-list.o repair.o send-stream.o \ + send-utils.o qgroup.o cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \ cmds-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \ cmds-quota.o cmds-qgroup.o diff --git a/btrfslabel.c b/btrfslabel.c deleted file mode 100644 index 2826050..000 --- a/btrfslabel.c +++ /dev/null @@ -1,178 +0,0 @@ -/* - * Copyright (C) 2008 Morey Roof. All rights reserved. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public - * License v2 as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public - * License along with this program; if not, write to the - * Free Software Foundation, Inc., 59 Temple Place - Suite 330, - * Boston, MA 021110-1307, USA. - */ - -#define _GNU_SOURCE - -#ifndef __CHECKER__ -#include sys/ioctl.h -#include sys/mount.h -#include ioctl.h -#endif /* __CHECKER__ */ - -#include stdio.h -#include stdlib.h -#include sys/types.h -#include sys/stat.h -#include dirent.h -#include fcntl.h -#include unistd.h -#include linux/fs.h -#include linux/limits.h -#include ctype.h -#include kerncompat.h -#include ctree.h -#include utils.h -#include version.h -#include disk-io.h -#include transaction.h - -#define MOUNTED1 -#define UNMOUNTED 2 -#define GET_LABEL 3 -#define SET_LABEL 4 - -static int set_label_unmounted(const char *dev, const char *label) -{ - struct btrfs_trans_handle *trans; - struct btrfs_root *root; - int ret; - - ret = check_mounted(dev); - if (ret 0) { - fprintf(stderr, FATAL: error checking %s mount status\n, dev); - return -1; - } - if (ret 0) { - fprintf(stderr, ERROR: dev %s is mounted, use mount point\n, - dev); - return -1; - } - - if (strlen(label) BTRFS_LABEL_SIZE - 1) { - fprintf(stderr, ERROR: Label %s is too long (max %d)\n, - label, BTRFS_LABEL_SIZE - 1); - return -1; - } - - /* Open the super_block at the default location -* and as read-write. -*/ - root = open_ctree(dev, 0, 1); - if (!root) /* errors are printed by open_ctree() */ - return -1; - - trans = btrfs_start_transaction(root, 1); - snprintf(root-fs_info-super_copy.label, BTRFS_LABEL_SIZE, %s, -label); - btrfs_commit_transaction(trans, root); - - /* Now we close it since we are done. */ - close_ctree(root); - return 0; -} - -static int set_label_mounted(const char *mount_path, const char *label) -{ - int fd; - - fd = open(mount_path, O_RDONLY | O_NOATIME); - if (fd 0) { - fprintf(stderr, ERROR: unable access to '%s'\n, mount_path); - return -1; - } - - if (ioctl(fd, BTRFS_IOC_SET_FSLABEL, label) 0) { - fprintf(stderr, ERROR: unable to set label %s\n, - strerror(errno)); - close(fd); - return -1; - } - - return 0; -} - -static int get_label_unmounted(const char *dev) -{ - struct btrfs_root *root; - int ret; - - ret = check_mounted(dev); - if (ret 0) { - fprintf(stderr, FATAL: error checking %s mount status\n, dev); - return -1; - } - if (ret 0) { - fprintf(stderr, ERROR: dev %s is mounted, use mount point\n, -
[PATCH] Btrfs: fix sparse warning related to drop_delayed_ref()
Fix a sparse warning: fs/btrfs/delayed-ref.c:236:1: warning: ‘inline’ is not at beginning of declaration [-Wold-style-declaration] Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/delayed-ref.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c index ae94117..c7280b4 100644 --- a/fs/btrfs/delayed-ref.c +++ b/fs/btrfs/delayed-ref.c @@ -233,7 +233,7 @@ int btrfs_delayed_ref_lock(struct btrfs_trans_handle *trans, return 0; } -static void inline drop_delayed_ref(struct btrfs_trans_handle *trans, +static inline void drop_delayed_ref(struct btrfs_trans_handle *trans, struct btrfs_delayed_ref_root *delayed_refs, struct btrfs_delayed_ref_node *ref) { -- 1.7.9.5 -- 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
[PATCH] Btrfs: remove meanlingless owner as well as it related stuff in btrfs_new_inode
Remove the owner variable as well as it related stuff. owner is meaningless in btrfs_new_inode() context, clean it out. Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/inode.c |6 -- 1 file changed, 6 deletions(-) diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 95542a1..f56eae1 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -4673,7 +4673,6 @@ static struct inode *btrfs_new_inode(struct btrfs_trans_handle *trans, u32 sizes[2]; unsigned long ptr; int ret; - int owner; path = btrfs_alloc_path(); if (!path) @@ -4719,11 +4718,6 @@ static struct inode *btrfs_new_inode(struct btrfs_trans_handle *trans, */ set_bit(BTRFS_INODE_NEEDS_FULL_SYNC, BTRFS_I(inode)-runtime_flags); - if (S_ISDIR(mode)) - owner = 0; - else - owner = 1; - key[0].objectid = objectid; btrfs_set_key_type(key[0], BTRFS_INODE_ITEM_KEY); key[0].offset = 0; -- 1.7.9.5 -- 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
Re: [PATCH] Btrfs: replace simple_strtoull() with kstrtoull() in btrfs_ioctl_resize()
On 01/08/2013 12:03 AM, David Sterba wrote: On Sun, Jan 06, 2013 at 11:53:41AM +0800, Jeff Liu wrote: -devid = simple_strtoull(devstr, end, 10); +ret = kstrtoull(devstr, 10, devid); +if (ret) { +pr_err(btrfs: resizer unable to parse device %s\n, +devstr); Code looks ok, I would prefer the message error text to say: pr_err(btrfs: resize unable to parse device id %s\n, devstr); but that's maybe just me :) Thanks for the review, yours is better :) The revised patch was shown as following: simple_strtoull() is obsolete, use kstrtoull() instead. Signed-off-by: Jie Liu jeff@oracle.com Cc: David Sterba dste...@suse.cz --- fs/btrfs/ioctl.c |9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 8fcf9a5..d9045eb 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -1331,11 +1331,16 @@ static noinline int btrfs_ioctl_resize(struct btrfs_root *root, sizestr = vol_args-name; devstr = strchr(sizestr, ':'); if (devstr) { - char *end; sizestr = devstr + 1; *devstr = '\0'; devstr = vol_args-name; - devid = simple_strtoull(devstr, end, 10); + ret = kstrtoull(devstr, 10, devid); + if (ret) { + pr_err(btrfs: resizer unable to parse device id %s\n, + devstr); + ret = -EINVAL; + goto out_free; + } printk(KERN_INFO btrfs: resizing devid %llu\n, (unsigned long long)devid); } -- 1.7.9.5 -- 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
Re: [RFC PATCH v9 1/2] Btrfs: Add a new ioctl to get the label of a mounted file system
Hi Goffredo, On 01/08/2013 01:32 AM, Goffredo Baroncelli wrote: Hi Jeff, On 01/07/2013 07:24 AM, Jeff Liu wrote: Hi, On 01/07/2013 02:44 AM, Goffredo Baroncelli wrote: Hi Jeff, On 01/05/2013 03:48 AM, Jeff Liu wrote: Add a new ioctl(2) BTRFS_IOC_GET_FSLABLE, so that we can get the label upon a mounted filesystem. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com Cc: Miao Xie mi...@cn.fujitsu.com Cc: Goffredo Baroncelli kreij...@inwind.it Cc: David Sterba dste...@suse.cz --- fs/btrfs/ioctl.c | 21 + fs/btrfs/ioctl.h |2 ++ 2 files changed, 23 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 8fcf9a5..ef2f55a 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3699,6 +3699,25 @@ out: return ret; } May be that it was already discussed, and I am missing something, but if we check the label length we should terminate the string with a zero in case this is too long... However this is a minor bug, please push this patch forward.. I don't think so. Why we need to terminate the string with a zero? We have already truncated the label string up to maximum 255 bytes if it was too long. Consider the normal conditions, i.e. the label string is less than 256, we just copy it back to the user without a terminated NUL. Sorry I don't understand the reason to terminate the string up to 255 chars without adding a zero. Or we copy all the buffer (BTRFS_LABEL_SIZE characters without furthers checks) or we copy BTRFS_LABEL_SIZE-1 characters, adding a zero at the end. For normal cases, there should already has a NUL at the end of the input label string when performing btrfs_ioc_set_fslabel(), and we have also added a NUL for unmounted label set in btrfs-progs. In both cases, put it again on btrfs_ioc_get_fslabel() is redundant IMO. If we ran into a too long label, return the first 255 bytes is fine for both mounted and unmounted get_label() routines in btrfs-progs since I did memset(label, 0, sizeof(label)) at first for fetching mounted fs label. Also, I have not found any existing kernel code does similar things by adding a NUL to end up a char array, could you show me an example? Or we copy all the buffer(BTRFS_LABEL_SIZE) characters without furthers checks If an array is capable of N chars, we can only full it with N - 1 chars. Thanks, -Jeff +static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) +{ + struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root; + const char *label = root-fs_info-super_copy-label; + size_t len = strnlen(label, BTRFS_LABEL_SIZE); + int ret; + int label_is_too_long = 0; bool label_is_too_long = false; + + if (len == BTRFS_LABEL_SIZE) { + pr_warn(btrfs: label is too long, return the first %zu bytes\n, + --len); + label_is_too_long = 1; label_is_too_long = true; + } + + mutex_lock(root-fs_info-volume_mutex); + ret = copy_to_user(arg, label, len); + if (!ret label_is_too_long) + ret = copy_to_user(arg+BTRFS_LABEL_SIZE, , 1); Hmm? ret = copy_to_user(arg + len, , 1); My idea was to put a zero at the end of the string, but I did a mistake with the cutpaste.. sorry :-) + mutex_unlock(root-fs_info-volume_mutex); If you or anyone has objection to this patch, please just post yours, I won't follow it up again. Sorry I don't want to bother anybody, I know that reviewing a patch nine times is a very havvy work. I (and I think more other peoples) really appreciate your efforts. BR G.Baroncelli + + return ret ? -EFAULT : 0; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3797,6 +3816,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_create(root, argp); case BTRFS_IOC_QGROUP_LIMIT: return btrfs_ioctl_qgroup_limit(root, argp); + case BTRFS_IOC_GET_FSLABEL: + return btrfs_ioctl_get_fslabel(file, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 731e287..5b2cbef 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -451,6 +451,8 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_create_args) #define BTRFS_IOC_QGROUP_LIMIT _IOR(BTRFS_IOCTL_MAGIC, 43, \ struct btrfs_ioctl_qgroup_limit_args) +#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ + char[BTRFS_LABEL_SIZE]) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) #endif -- 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
Re: [RFC PATCH v9 1/2] Btrfs: Add a new ioctl to get the label of a mounted file system
Hi, On 01/07/2013 02:44 AM, Goffredo Baroncelli wrote: Hi Jeff, On 01/05/2013 03:48 AM, Jeff Liu wrote: Add a new ioctl(2) BTRFS_IOC_GET_FSLABLE, so that we can get the label upon a mounted filesystem. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com Cc: Miao Xie mi...@cn.fujitsu.com Cc: Goffredo Baroncelli kreij...@inwind.it Cc: David Sterba dste...@suse.cz --- fs/btrfs/ioctl.c | 21 + fs/btrfs/ioctl.h |2 ++ 2 files changed, 23 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 8fcf9a5..ef2f55a 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3699,6 +3699,25 @@ out: return ret; } May be that it was already discussed, and I am missing something, but if we check the label length we should terminate the string with a zero in case this is too long... However this is a minor bug, please push this patch forward.. I don't think so. Why we need to terminate the string with a zero? We have already truncated the label string up to maximum 255 bytes if it was too long. Consider the normal conditions, i.e. the label string is less than 256, we just copy it back to the user without a terminated NUL. +static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) +{ +struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root; +const char *label = root-fs_info-super_copy-label; +size_t len = strnlen(label, BTRFS_LABEL_SIZE); +int ret; + int label_is_too_long = 0; bool label_is_too_long = false; + +if (len == BTRFS_LABEL_SIZE) { +pr_warn(btrfs: label is too long, return the first %zu bytes\n, +--len); + label_is_too_long = 1; label_is_too_long = true; +} + +mutex_lock(root-fs_info-volume_mutex); +ret = copy_to_user(arg, label, len); + if (!ret label_is_too_long) + ret = copy_to_user(arg+BTRFS_LABEL_SIZE, , 1); Hmm?ret = copy_to_user(arg + len, , 1); +mutex_unlock(root-fs_info-volume_mutex); If you or anyone has objection to this patch, please just post yours, I won't follow it up again. + +return ret ? -EFAULT : 0; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3797,6 +3816,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_create(root, argp); case BTRFS_IOC_QGROUP_LIMIT: return btrfs_ioctl_qgroup_limit(root, argp); +case BTRFS_IOC_GET_FSLABEL: +return btrfs_ioctl_get_fslabel(file, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 731e287..5b2cbef 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -451,6 +451,8 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_create_args) #define BTRFS_IOC_QGROUP_LIMIT _IOR(BTRFS_IOCTL_MAGIC, 43, \ struct btrfs_ioctl_qgroup_limit_args) +#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ + char[BTRFS_LABEL_SIZE]) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) #endif -- 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
[PATCH] Btrfs: replace simple_strtoull() with kstrtoull() in btrfs_ioctl_resize()
simple_strtoull() is obsolete, use kstrtoull() instead. Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/ioctl.c |9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 8fcf9a5..b147424 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -1331,11 +1331,16 @@ static noinline int btrfs_ioctl_resize(struct btrfs_root *root, sizestr = vol_args-name; devstr = strchr(sizestr, ':'); if (devstr) { - char *end; sizestr = devstr + 1; *devstr = '\0'; devstr = vol_args-name; - devid = simple_strtoull(devstr, end, 10); + ret = kstrtoull(devstr, 10, devid); + if (ret) { + pr_err(btrfs: resizer unable to parse device %s\n, + devstr); + ret = -EINVAL; + goto out_free; + } printk(KERN_INFO btrfs: resizing devid %llu\n, (unsigned long long)devid); } -- 1.7.9.5 -- 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
[RFC PATCH v9 1/2] Btrfs: Add a new ioctl to get the label of a mounted file system
Add a new ioctl(2) BTRFS_IOC_GET_FSLABLE, so that we can get the label upon a mounted filesystem. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com Cc: Miao Xie mi...@cn.fujitsu.com Cc: Goffredo Baroncelli kreij...@inwind.it Cc: David Sterba dste...@suse.cz --- fs/btrfs/ioctl.c | 21 + fs/btrfs/ioctl.h |2 ++ 2 files changed, 23 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 8fcf9a5..ef2f55a 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3699,6 +3699,25 @@ out: return ret; } +static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) +{ + struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root; + const char *label = root-fs_info-super_copy-label; + size_t len = strnlen(label, BTRFS_LABEL_SIZE); + int ret; + + if (len == BTRFS_LABEL_SIZE) { + pr_warn(btrfs: label is too long, return the first %zu bytes\n, + --len); + } + + mutex_lock(root-fs_info-volume_mutex); + ret = copy_to_user(arg, label, len); + mutex_unlock(root-fs_info-volume_mutex); + + return ret ? -EFAULT : 0; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3797,6 +3816,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_create(root, argp); case BTRFS_IOC_QGROUP_LIMIT: return btrfs_ioctl_qgroup_limit(root, argp); + case BTRFS_IOC_GET_FSLABEL: + return btrfs_ioctl_get_fslabel(file, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 731e287..5b2cbef 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -451,6 +451,8 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_create_args) #define BTRFS_IOC_QGROUP_LIMIT _IOR(BTRFS_IOCTL_MAGIC, 43, \ struct btrfs_ioctl_qgroup_limit_args) +#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ + char[BTRFS_LABEL_SIZE]) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) #endif -- 1.7.9.5 -- 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
[RFC PATCH v9 2/2] Btrfs: set/change the label of a mounted file system
With this new ioctl(2) BTRFS_IOC_SET_FSLABEL, we can set/change the label of a mounted file system. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com Reviewed-by: Miao Xie mi...@cn.fujitsu.com Reviewed-by: Goffredo Baroncelli kreij...@inwind.it Reviewed-by: David Sterba dste...@suse.cz --- fs/btrfs/ioctl.c | 42 ++ fs/btrfs/ioctl.h |2 ++ 2 files changed, 44 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index ef2f55a..8e676c3 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3718,6 +3718,46 @@ static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) return ret ? -EFAULT : 0; } +static int btrfs_ioctl_set_fslabel(struct file *file, void __user *arg) +{ + struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root; + struct btrfs_super_block *super_block = root-fs_info-super_copy; + struct btrfs_trans_handle *trans; + char label[BTRFS_LABEL_SIZE]; + int ret; + + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + + if (copy_from_user(label, arg, sizeof(label))) + return -EFAULT; + + if (strnlen(label, BTRFS_LABEL_SIZE) == BTRFS_LABEL_SIZE) { + pr_err(btrfs: unable to set label with more than %d bytes\n, + BTRFS_LABEL_SIZE - 1); + return -EINVAL; + } + + ret = mnt_want_write_file(file); + if (ret) + return ret; + + mutex_lock(root-fs_info-volume_mutex); + trans = btrfs_start_transaction(root, 0); + if (IS_ERR(trans)) { + ret = PTR_ERR(trans); + goto out_unlock; + } + + strcpy(super_block-label, label); + ret = btrfs_end_transaction(trans, root); + +out_unlock: + mutex_unlock(root-fs_info-volume_mutex); + mnt_drop_write_file(file); + return ret; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3818,6 +3858,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_limit(root, argp); case BTRFS_IOC_GET_FSLABEL: return btrfs_ioctl_get_fslabel(file, argp); + case BTRFS_IOC_SET_FSLABEL: + return btrfs_ioctl_set_fslabel(file, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 5b2cbef..2abe239 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -453,6 +453,8 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_limit_args) #define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ char[BTRFS_LABEL_SIZE]) +#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \ + char[BTRFS_LABEL_SIZE]) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) #endif -- 1.7.9.5 -- 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
[RFC PATCH v9 0/2] Btrfs: get/set label for a mounted file system
Hello, There have a couple of changes in this version which were shown as following: - Don't reserve an unit of transactions for set filesystem label. - Replace the old warning message to btrfs: label is too long, return the first %zu bytes in btrfs_ioc_get_fslabel() is the existing label length is longer than than BTRFS_LABEL_SIZE - 1. Otherwise, checkpatch.pl would give a warning as: WARNING: quoted string split across lines. - btrfs_ioc_set_fslabel(): drop an error message if the input label length is too long for debugging purpose. The old versions can be found at: v8: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/21906 v7: http://www.spinics.net/lists/linux-btrfs/msg20994.html v6: http://www.spinics.net/lists/linux-btrfs/msg20922.html v5: http://www.spinics.net/lists/linux-btrfs/msg20888.html v4: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/21618 v3: https://patchwork.kernel.org/patch/1124642/ v2: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/12877 v1: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/12872 Thanks, -Jeff -- 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
Re: [RFC PATCH v8 2/2] Btrfs: Add a new ioctl to set/change the label of a mounted file system
On 12/28/2012 08:03 PM, David Sterba wrote: On Fri, Dec 28, 2012 at 11:42:40AM +0800, Jeff Liu wrote: +static int btrfs_ioctl_set_fslabel(struct file *file, void __user *arg) +{ +struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root; +struct btrfs_super_block *super_block = root-fs_info-super_copy; +struct btrfs_trans_handle *trans; +char label[BTRFS_LABEL_SIZE]; +int ret; + +if (!capable(CAP_SYS_ADMIN)) +return -EPERM; + +if (copy_from_user(label, arg, sizeof(label))) +return -EFAULT; + +if (strnlen(label, BTRFS_LABEL_SIZE) == BTRFS_LABEL_SIZE) I would expect a message if this happens, similar to the 'get_fslabel' one, otherwise it's difficult to find out the reason. That's sounds make sense. +return -EINVAL; + +ret = mnt_want_write_file(file); +if (ret) +return ret; + +mutex_lock(root-fs_info-volume_mutex); +trans = btrfs_start_transaction(root, 1); Do we need to reserve 1 unit here? This does not touch any non-superblock metadata, modifies superblock in place. This could lead to an ENOSPC (changing label on a full fs), altghouh it need not happen. Don't need, I can not recalled why I did that before, will fix it. Thanks, -Jeff +if (IS_ERR(trans)) { +ret = PTR_ERR(trans); +goto out_unlock; +} + +strcpy(super_block-label, label); +ret = btrfs_end_transaction(trans, root); + +out_unlock: +mutex_unlock(root-fs_info-volume_mutex); +mnt_drop_write_file(file); +return ret; +} -- 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
[RFC PATCH v8 1/2] Btrfs: Add a new ioctl to get the label of a mounted file system
Add a new ioctl(2) BTRFS_IOC_GET_FSLABLE, so that we can get the label upon a mounted file system. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com Cc: Miao Xie mi...@cn.fujitsu.com Cc: Goffredo Baroncelli kreij...@inwind.it Cc: David Sterba dste...@suse.cz --- fs/btrfs/ioctl.c | 22 ++ fs/btrfs/ioctl.h |2 ++ 2 files changed, 24 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 8fcf9a5..4be5d15 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3699,6 +3699,26 @@ out: return ret; } +static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) +{ + struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root; + const char *label = root-fs_info-super_copy-label; + size_t len = strnlen(label, BTRFS_LABEL_SIZE); + int ret; + + if (len == BTRFS_LABEL_SIZE) { + len--; + WARN(1, btrfs: device label is not zero terminated, + it will be truncated to %zu bytes.\n, len); + } + + mutex_lock(root-fs_info-volume_mutex); + ret = copy_to_user(arg, label, len); + mutex_unlock(root-fs_info-volume_mutex); + + return ret ? -EFAULT : 0; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3797,6 +3817,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_create(root, argp); case BTRFS_IOC_QGROUP_LIMIT: return btrfs_ioctl_qgroup_limit(root, argp); + case BTRFS_IOC_GET_FSLABEL: + return btrfs_ioctl_get_fslabel(file, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 731e287..5b2cbef 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -451,6 +451,8 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_create_args) #define BTRFS_IOC_QGROUP_LIMIT _IOR(BTRFS_IOCTL_MAGIC, 43, \ struct btrfs_ioctl_qgroup_limit_args) +#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ + char[BTRFS_LABEL_SIZE]) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) #endif -- 1.7.9.5 -- 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
[RFC PATCH v8 0/2] Btrfs: get/set label of a mounted file system
Hello, Here is the updated version of introduce get/set_or_change label upon a mounted filesystem. v8-v7: btrfs_ioc_fs_getlabel(): - Examine the label length with strnlen(). - Tweak up warn() info if the label is not NUL terminated according to Goffredo's suggestions. The old versions can be found at: v7: http://www.spinics.net/lists/linux-btrfs/msg20994.html v6: http://www.spinics.net/lists/linux-btrfs/msg20922.html v5: http://www.spinics.net/lists/linux-btrfs/msg20888.html v4: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/21618 v3: https://patchwork.kernel.org/patch/1124642/ v2: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/12877 v1: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/12872 Thanks, -Jeff -- 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
[RFC PATCH v8 2/2] Btrfs: Add a new ioctl to set/change the label of a mounted file system
With this new ioctl(2) BTRFS_IOC_SET_FSLABEL, we can set/change the label of a mounted file system. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com Reviewed-by: Miao Xie mi...@cn.fujitsu.com Reviewed-by: Goffredo Baroncelli kreij...@inwind.it Reviewed-by: David Sterba dste...@suse.cz --- fs/btrfs/ioctl.c | 39 +++ fs/btrfs/ioctl.h |2 ++ 2 files changed, 41 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 4be5d15..7383f49 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3719,6 +3719,43 @@ static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) return ret ? -EFAULT : 0; } +static int btrfs_ioctl_set_fslabel(struct file *file, void __user *arg) +{ + struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root; + struct btrfs_super_block *super_block = root-fs_info-super_copy; + struct btrfs_trans_handle *trans; + char label[BTRFS_LABEL_SIZE]; + int ret; + + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + + if (copy_from_user(label, arg, sizeof(label))) + return -EFAULT; + + if (strnlen(label, BTRFS_LABEL_SIZE) == BTRFS_LABEL_SIZE) + return -EINVAL; + + ret = mnt_want_write_file(file); + if (ret) + return ret; + + mutex_lock(root-fs_info-volume_mutex); + trans = btrfs_start_transaction(root, 1); + if (IS_ERR(trans)) { + ret = PTR_ERR(trans); + goto out_unlock; + } + + strcpy(super_block-label, label); + ret = btrfs_end_transaction(trans, root); + +out_unlock: + mutex_unlock(root-fs_info-volume_mutex); + mnt_drop_write_file(file); + return ret; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3819,6 +3856,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_limit(root, argp); case BTRFS_IOC_GET_FSLABEL: return btrfs_ioctl_get_fslabel(file, argp); + case BTRFS_IOC_SET_FSLABEL: + return btrfs_ioctl_set_fslabel(file, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 5b2cbef..2abe239 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -453,6 +453,8 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_limit_args) #define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ char[BTRFS_LABEL_SIZE]) +#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \ + char[BTRFS_LABEL_SIZE]) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) #endif -- 1.7.9.5 -- 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
Re: [RFC PATCH v7 1/2] Btrfs: Add a new ioctl to get the label of a mounted file system
On 12/22/2012 01:36 AM, Goffredo Baroncelli wrote: On 12/21/2012 07:42 AM, Jeff Liu wrote: Hi Goffredo, On 12/21/2012 04:18 AM, Goffredo Baroncelli wrote: HI Jeff, On 12/20/2012 09:43 AM, Jeff Liu wrote: With the new ioctl(2) BTRFS_IOC_GET_FSLABEL we can fetch the label of a mounted file system. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com Cc: Miao Xie mi...@cn.fujitsu.com Cc: Goffredo Baroncelli kreij...@inwind.it Cc: David Sterba dste...@suse.cz [...] +static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) +{ + struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root; + const char *label = root-fs_info-super_copy-label; + int ret; + + mutex_lock(root-fs_info-volume_mutex); + ret = copy_to_user(arg, label, strlen(label)); Sorry for pointing out my doubt too late, but should we trust super_copy-label ? An user could insert a usb-key with a btrfs filesystem with a label without zero. In this case strlen() could access outside super_copy-label[]. Thank you for letting me be aware of this situation. First of all, if the user set label via btrfs tools, he can not make it length exceeding BTRFS_LABLE_SIZE - 1. If the user does that through codes wrote by himself like: btrfslabel.c-set_label_unmounted(), he can do that. However, it most likely he did that for evil purpose or any other reasons? I think the most likely case is the evil purpose. I think that it should be quite easy to alter artificially a filesystem to crash the kernel. So I not consider this as big problem. However *in case* of a further cycle of this patch I suggest to replace strlen() with strnlen(). I don't think we should replace strlen() with strnlen() since it's totally wrong if the length of label is more than BTRFS_LABEL_SIZE -1, we can not just truncating the label and return it in this case. This for me is sufficient, or we could copy all the label buffer, without further check: copy_to_user(arg, label, BTRFS_LABEL_SIZE) That sounds ok to me, but it's better to limit the length to be 'BTRFS_LABEL_SIZE - 1' bytes, or else, it would copy 256 bytes back to the user space if the label length is beyond or equal to the array limits. Add BUG_ON(strlen(label) BTRFS_LABEL_SIZE - 1) is reasonable instead. I agree with Stefan, this is not a correct use of BUG_ON; a warning is sufficient (there is un-correct data read from disk). Maybe like following? size_t len = strlen(label); if (len BTRFS_LABEL_SIZE - 1) { WARN(1, btrfs: device label has weird length %zu bytes, it will be truncated to %d bytes.\n, len, BTRFS_LABEL_SIZE - 1); len = BTRFS_LABEL_SIZE - 1; } I'll post a new patch with those changes if no other objections. Happy Christmas. -Jeff Thanks, -Jeff + mutex_unlock(root-fs_info-volume_mutex); + + return ret ? -EFAULT : 0; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3797,6 +3810,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_create(root, argp); case BTRFS_IOC_QGROUP_LIMIT: return btrfs_ioctl_qgroup_limit(root, argp); + case BTRFS_IOC_GET_FSLABEL: + return btrfs_ioctl_get_fslabel(file, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 731e287..5b2cbef 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -451,6 +451,8 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_create_args) #define BTRFS_IOC_QGROUP_LIMIT _IOR(BTRFS_IOCTL_MAGIC, 43, \ struct btrfs_ioctl_qgroup_limit_args) +#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ + char[BTRFS_LABEL_SIZE]) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) #endif -- 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 -- 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
Re: [RFC PATCH v7 1/2] Btrfs: Add a new ioctl to get the label of a mounted file system
On 12/24/2012 09:46 PM, Goffredo Baroncelli wrote: Hi Jeff, On 12/24/2012 09:07 AM, Jeff Liu wrote: On 12/22/2012 01:36 AM, Goffredo Baroncelli wrote: On 12/21/2012 07:42 AM, Jeff Liu wrote: [...] I don't think we should replace strlen() with strnlen() since it's totally wrong if the length of label is more than BTRFS_LABEL_SIZE -1, we can not just truncating the label and return it in this case. This for me is sufficient, or we could copy all the label buffer, without further check: copy_to_user(arg, label, BTRFS_LABEL_SIZE) That sounds ok to me, but it's better to limit the length to be 'BTRFS_LABEL_SIZE - 1' bytes, or else, it would copy 256 bytes back to the user space if the label length is beyond or equal to the array limits. Sorry I don't understand your reply. The ioctl has as argument an array of BTRFS_LABEL_SIZE characters, so it should not be any problem of page fault (with the exception of an user who passes a shorter array). So it wouldn't be any problem to copy BTRFS_LABEL_SIZE character, or I am missing something ? We can copy label length up to BTRFS_LABEL_SIZE back to the user space, but please see my response which were shown as following. The question is another. Is a kernel responsibility to assure that the returned string is zero terminated ? If yes (and I think so) we should truncate the string before the copy: On both the old btrfslabel.c-change_label_unmounted() and the new ioctl(BTRFS_IOC_SET_FSLABE), we all made the terminated character to be NUL(i.e. actually, the maximum length is 255), so that it's better to assure that the returned string to be same as well IMO. label[BTRFS_LABEL_SIZE-1] = 0; copy_to_user(arg, label, BTRFS_LABEL_SIZE); This definitely works, please see my response below again. Add BUG_ON(strlen(label) BTRFS_LABEL_SIZE - 1) is reasonable instead. I agree with Stefan, this is not a correct use of BUG_ON; a warning is sufficient (there is un-correct data read from disk). Maybe like following? size_t len = strlen(label); As we have already evaluated the real length of the label, why not just use it to indicate the length that will be copied back? If label (who is an array read from the disk) is not zero terminated, this would lead to a possible page fault. I suggest to use strnlen() or similar. Good point, that would make code better, i.e. avoid page fault. :) if (len BTRFS_LABEL_SIZE - 1) { WARN(1, btrfs: device label has weird length %zu bytes, it will be truncated to %d bytes.\n, len, BTRFS_LABEL_SIZE - 1); len = BTRFS_LABEL_SIZE - 1; } As message I suggest: WARN(1, btrfs: device label is not zero terminated, it will be truncated to %d bytes.\n, BTRFS_LABEL-1 ); Ok, I'd like to follow up with your suggestions as am not english native. NOTE: it is suggested to not span the string on more lines, to help searching in the source a messages with grep. It is an exception of the rule of the max 80 columns text width. That's why I splitted the warning info into two lines. Thanks, -Jeff Happy Christmas you too GB I'll post a new patch with those changes if no other objections. Happy Christmas. -Jeff Thanks, -Jeff +mutex_unlock(root-fs_info-volume_mutex); + +return ret ? -EFAULT : 0; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3797,6 +3810,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_create(root, argp); case BTRFS_IOC_QGROUP_LIMIT: return btrfs_ioctl_qgroup_limit(root, argp); +case BTRFS_IOC_GET_FSLABEL: +return btrfs_ioctl_get_fslabel(file, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 731e287..5b2cbef 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -451,6 +451,8 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_create_args) #define BTRFS_IOC_QGROUP_LIMIT _IOR(BTRFS_IOCTL_MAGIC, 43, \ struct btrfs_ioctl_qgroup_limit_args) +#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ + char[BTRFS_LABEL_SIZE]) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) #endif -- 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 -- 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
Re: [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system
On 12/19/2012 10:21 PM, David Sterba wrote: On Tue, Dec 18, 2012 at 11:06:07AM +0800, Jeff Liu wrote: +static int btrfs_ioctl_set_fslabel(struct file *file, void __user *arg) +{ +struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root; +struct btrfs_super_block *super_block = root-fs_info-super_copy; +struct btrfs_trans_handle *trans; +char label[BTRFS_LABEL_SIZE]; +int ret; + +if (!capable(CAP_SYS_ADMIN)) +return -EPERM; + +if (copy_from_user(label, arg, sizeof(label))) +return -EFAULT; + +if (strnlen(label, BTRFS_LABEL_SIZE) == BTRFS_LABEL_SIZE) +return -EINVAL; + +ret = mnt_want_write_file(file); +if (ret) +return ret; + +mutex_lock(root-fs_info-volume_mutex); +trans = btrfs_start_transaction(root, 1); +if (IS_ERR(trans)) { +ret = PTR_ERR(trans); +goto out_unlock; +} + +strcpy(super_block-label, label); +btrfs_end_transaction(trans, root); If this fails, eg. with EIO, it will not be reported back to the user ret = btrfs_end_transaction(trans, root); should fix it. Thank you, I'll send out a revised version after a little while. -Jeff + +out_unlock: +mutex_unlock(root-fs_info-volume_mutex); +mnt_drop_write_file(file); +return ret; +} -- 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 -- 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
[RFC PATCH v7 1/2] Btrfs: Add a new ioctl to get the label of a mounted file system
With the new ioctl(2) BTRFS_IOC_GET_FSLABEL we can fetch the label of a mounted file system. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com Cc: Miao Xie mi...@cn.fujitsu.com Cc: Goffredo Baroncelli kreij...@inwind.it Cc: David Sterba dste...@suse.cz --- fs/btrfs/ioctl.c | 15 +++ fs/btrfs/ioctl.h |2 ++ 2 files changed, 17 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 8fcf9a5..6a2488a 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3699,6 +3699,19 @@ out: return ret; } +static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) +{ + struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root; + const char *label = root-fs_info-super_copy-label; + int ret; + + mutex_lock(root-fs_info-volume_mutex); + ret = copy_to_user(arg, label, strlen(label)); + mutex_unlock(root-fs_info-volume_mutex); + + return ret ? -EFAULT : 0; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3797,6 +3810,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_create(root, argp); case BTRFS_IOC_QGROUP_LIMIT: return btrfs_ioctl_qgroup_limit(root, argp); + case BTRFS_IOC_GET_FSLABEL: + return btrfs_ioctl_get_fslabel(file, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 731e287..5b2cbef 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -451,6 +451,8 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_create_args) #define BTRFS_IOC_QGROUP_LIMIT _IOR(BTRFS_IOCTL_MAGIC, 43, \ struct btrfs_ioctl_qgroup_limit_args) +#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ + char[BTRFS_LABEL_SIZE]) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) #endif -- 1.7.9.5 -- 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
[RFC PATCH v7 2/2] Btrfs: Add a new ioctl to set/change the label of a mounted file system
With the new ioctl(2) BTRFS_IOC_SET_FSLABEL, we can set/change the label of a mounted file system. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com Cc: Miao Xie mi...@cn.fujitsu.com Cc: Goffredo Baroncelli kreij...@inwind.it Cc: David Sterba dste...@suse.cz --- fs/btrfs/ioctl.c | 39 +++ fs/btrfs/ioctl.h |2 ++ 2 files changed, 41 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 6a2488a..99aa812 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3712,6 +3712,43 @@ static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) return ret ? -EFAULT : 0; } +static int btrfs_ioctl_set_fslabel(struct file *file, void __user *arg) +{ + struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root; + struct btrfs_super_block *super_block = root-fs_info-super_copy; + struct btrfs_trans_handle *trans; + char label[BTRFS_LABEL_SIZE]; + int ret; + + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + + if (copy_from_user(label, arg, sizeof(label))) + return -EFAULT; + + if (strnlen(label, BTRFS_LABEL_SIZE) == BTRFS_LABEL_SIZE) + return -EINVAL; + + ret = mnt_want_write_file(file); + if (ret) + return ret; + + mutex_lock(root-fs_info-volume_mutex); + trans = btrfs_start_transaction(root, 1); + if (IS_ERR(trans)) { + ret = PTR_ERR(trans); + goto out_unlock; + } + + strcpy(super_block-label, label); + ret = btrfs_end_transaction(trans, root); + +out_unlock: + mutex_unlock(root-fs_info-volume_mutex); + mnt_drop_write_file(file); + return ret; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3812,6 +3849,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_limit(root, argp); case BTRFS_IOC_GET_FSLABEL: return btrfs_ioctl_get_fslabel(file, argp); + case BTRFS_IOC_SET_FSLABEL: + return btrfs_ioctl_set_fslabel(file, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 5b2cbef..2abe239 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -453,6 +453,8 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_limit_args) #define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ char[BTRFS_LABEL_SIZE]) +#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \ + char[BTRFS_LABEL_SIZE]) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) #endif -- 1.7.9.5 -- 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
[RFC PATCH v7 0/2] Btrfs: get/set label of a mounted file system
Hello, Per David's comments upon v6, I missed a check up against the return value of btrfs_end_transaction for btrfs_ioctl_fs_setlabel(), it was fixed in this version. v7-v6: - take care of btrfs_end_transaction() in btrfs_ioctl_fs_setlabel() rather than keeping silence. The old versions can be found at: v6: http://www.spinics.net/lists/linux-btrfs/msg20922.html v5: http://www.spinics.net/lists/linux-btrfs/msg20888.html v4: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/21618 v3: https://patchwork.kernel.org/patch/1124642/ v2: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/12877 v1: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/12872 Thanks, -Jeff -- 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
Re: [RFC PATCH v7 1/2] Btrfs: Add a new ioctl to get the label of a mounted file system
Hi Goffredo, On 12/21/2012 04:18 AM, Goffredo Baroncelli wrote: HI Jeff, On 12/20/2012 09:43 AM, Jeff Liu wrote: With the new ioctl(2) BTRFS_IOC_GET_FSLABEL we can fetch the label of a mounted file system. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com Cc: Miao Xie mi...@cn.fujitsu.com Cc: Goffredo Baroncelli kreij...@inwind.it Cc: David Sterba dste...@suse.cz [...] +static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) +{ +struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root; +const char *label = root-fs_info-super_copy-label; +int ret; + +mutex_lock(root-fs_info-volume_mutex); +ret = copy_to_user(arg, label, strlen(label)); Sorry for pointing out my doubt too late, but should we trust super_copy-label ? An user could insert a usb-key with a btrfs filesystem with a label without zero. In this case strlen() could access outside super_copy-label[]. Thank you for letting me be aware of this situation. First of all, if the user set label via btrfs tools, he can not make it length exceeding BTRFS_LABLE_SIZE - 1. If the user does that through codes wrote by himself like: btrfslabel.c-set_label_unmounted(), he can do that. However, it most likely he did that for evil purpose or any other reasons? I think that it should be quite easy to alter artificially a filesystem to crash the kernel. So I not consider this as big problem. However *in case* of a further cycle of this patch I suggest to replace strlen() with strnlen(). I don't think we should replace strlen() with strnlen() since it's totally wrong if the length of label is more than BTRFS_LABEL_SIZE -1, we can not just truncating the label and return it in this case. Add BUG_ON(strlen(label) BTRFS_LABEL_SIZE - 1) is reasonable instead. Thanks, -Jeff +mutex_unlock(root-fs_info-volume_mutex); + +return ret ? -EFAULT : 0; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3797,6 +3810,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_create(root, argp); case BTRFS_IOC_QGROUP_LIMIT: return btrfs_ioctl_qgroup_limit(root, argp); +case BTRFS_IOC_GET_FSLABEL: +return btrfs_ioctl_get_fslabel(file, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 731e287..5b2cbef 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -451,6 +451,8 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_create_args) #define BTRFS_IOC_QGROUP_LIMIT _IOR(BTRFS_IOCTL_MAGIC, 43, \ struct btrfs_ioctl_qgroup_limit_args) +#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ + char[BTRFS_LABEL_SIZE]) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) #endif -- 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
Re: [RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system
Hi Goffredo, Thanks for your review. On 12/19/2012 02:00 AM, Goffredo Baroncelli wrote: Hi Jeff, On 12/18/2012 04:31 AM, Miao Xie wrote: [...] diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c [...] +static int btrfs_ioctl_set_fslabel(struct file *file, void __user *arg) +{ + struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root; + struct btrfs_super_block *super_block = root-fs_info-super_copy; + struct btrfs_trans_handle *trans; + char label[BTRFS_LABEL_SIZE]; + int ret; + + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + + if (copy_from_user(label, arg, sizeof(label))) + return -EFAULT; + + if (strnlen(label, BTRFS_LABEL_SIZE) == BTRFS_LABEL_SIZE) + return -EINVAL; + + ret = mnt_want_write_file(file); + if (ret) + return ret; + + mutex_lock(root-fs_info-volume_mutex); + trans = btrfs_start_transaction(root, 1); + if (IS_ERR(trans)) { + ret = PTR_ERR(trans); + goto out_unlock; + } + + strcpy(super_block-label, label); I think that you removed for mistake the following line + label[BTRFS_LABEL_SIZE - 1] = '\0'; I removed it since it was used to cut the label string off the max array size but now we have the previous strnlen(). In the V5 patch it was present. May be we could replace strcpy() with strlcpy(super_block-label, label, BTRFS_LABEL_SIZE-1) ? That is ok to me. However, it should be strlcpy(super_block-label, label, BTRFS_LABEL_SIZE) ranther than 'BTRFS_LABREL_SIZE -1' because strlcpy() does size - 1 internally. i.e. strlcpy(char *d, const char *s, size_t size) { size_t ret = strlen(s); . size_t len = (ret = size) ? size - 1 : ret; } But does the current implementation make anything wrong? :) Thanks, -Jeff BR G.Baroncelli -- 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
[RFC PATCH V5 0/2] Btrfs: get/set label of mounted file system
Hello, This patch set is trying to make Btrfs support get/set label for a mounted file sytem via ioctl(2). There are a couple of changes according to Miao's comments which were shown as following. Changes of V5-V4 in kernel: - Revise the ioctl number of BTRFS_IOC_GET_FSLABEL/BTRFS_IOC_SET_FSLABEL to 49/50 separately. - Replace btrfs_root_readonly() check up with mnt_want_write_file(). - Validate the input label length, return -EINVAL if the specified length is exceeding BTRFS_LABEL_SIZE -1. Changes of V5-V4 in user space: - Revise the ioctl number accordingly. - Don't proceed to get/set the label upon a mounted file system if the dev patch is specified, i.e. btrfs filesystem label /dev/sdaX [new label] Instead, alert the user to execute this command against the mount point, i.e. btrfs filesystem label /btrfs_mount_path [new label] - Validate the input label length for changing the label upon an unmounted file system as well. We does not check it up currently, the command just keeping silent and truncate the label characters which are beyond BTRFS_LABEL_SIZE - 1. Tests: == /dev/sda6 on /btrfs type btrfs (rw) jeff@koala:~/oss/btrfs-progs$ sudo ./btrfs filesystem label /btrfs btrfs_label Failure if the length is exceeding 255. jeff@koala:~/oss/btrfs-progs$ sudo ./btrfs filesystem label /btrfs `perl -e 'print Ax256'` ERROR: unable to set label Invalid argument Otherwise, that's ok. jeff@koala:~/oss/btrfs-progs$ sudo ./btrfs filesystem label /btrfs `perl -e 'print Ax255'` jeff@koala:~/oss/btrfs-progs$ sudo ./btrfs filesystem label /btrfs AA AA AAA The old versions can be found at: v4: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/21618 v3: https://patchwork.kernel.org/patch/1124642/ v2: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/12877 v1: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/12872 Thanks, -Jeff -- 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
[RFC PATCH V5 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system
Introduce a new ioctl BTRFS_IOC_SET_FSLABEL to change the label of a mounted file system. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com Cc: Miao Xie mi...@cn.fujitsu.com --- fs/btrfs/ioctl.c | 40 fs/btrfs/ioctl.h |2 ++ 2 files changed, 42 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 6a2488a..0186651 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3712,6 +3712,44 @@ static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) return ret ? -EFAULT : 0; } +static int btrfs_ioctl_set_fslabel(struct file *file, void __user *arg) +{ + struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root; + struct btrfs_super_block *super_block = root-fs_info-super_copy; + char label[BTRFS_LABEL_SIZE]; + int ret; + + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + + if (copy_from_user(label, arg, sizeof(label))) + return -EFAULT; + + if (strlen(label) BTRFS_LABEL_SIZE - 1) + return -EINVAL; + + ret = mnt_want_write_file(file); + if (ret) + return ret; + + mutex_lock(root-fs_info-volume_mutex); + trans = btrfs_start_transaction(root, 1); + if (IS_ERR(trans)) { + ret = PTR_ERR(trans); + goto out_unlock; + } + + label[BTRFS_LABEL_SIZE - 1] = '\0'; + strcpy(super_block-label, label); + btrfs_end_transaction(trans, root); + +out_unlock: + mutex_unlock(root-fs_info-volume_mutex); + mnt_drop_write_file(file); + return ret; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3812,6 +3850,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_limit(root, argp); case BTRFS_IOC_GET_FSLABEL: return btrfs_ioctl_get_fslabel(file, argp); + case BTRFS_IOC_SET_FSLABEL: + return btrfs_ioctl_set_fslabel(file, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 5b2cbef..2abe239 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -453,6 +453,8 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_limit_args) #define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ char[BTRFS_LABEL_SIZE]) +#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \ + char[BTRFS_LABEL_SIZE]) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) #endif -- 1.7.9.5 -- 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
[RFC PATCH V5 1/2] Btrfs: Add a new ioctl to get the label of a mounted filesystem
Introduce a new ioctl BTRFS_IOC_GET_FSLABEL to fetch the label of a mounted file system. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com Cc: Miao Xie mi...@cn.fujitsu.com --- fs/btrfs/ioctl.c | 15 +++ fs/btrfs/ioctl.h |2 ++ 2 files changed, 17 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 8fcf9a5..6a2488a 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3699,6 +3699,19 @@ out: return ret; } +static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) +{ + struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root; + const char *label = root-fs_info-super_copy-label; + int ret; + + mutex_lock(root-fs_info-volume_mutex); + ret = copy_to_user(arg, label, strlen(label)); + mutex_unlock(root-fs_info-volume_mutex); + + return ret ? -EFAULT : 0; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3797,6 +3810,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_create(root, argp); case BTRFS_IOC_QGROUP_LIMIT: return btrfs_ioctl_qgroup_limit(root, argp); + case BTRFS_IOC_GET_FSLABEL: + return btrfs_ioctl_get_fslabel(file, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 731e287..5b2cbef 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -451,6 +451,8 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_create_args) #define BTRFS_IOC_QGROUP_LIMIT _IOR(BTRFS_IOCTL_MAGIC, 43, \ struct btrfs_ioctl_qgroup_limit_args) +#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ + char[BTRFS_LABEL_SIZE]) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) #endif -- 1.7.9.5 -- 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
[PATCH v5 1/4] Btrfs-progs: get label of a mounted file system
With this new ioctl(2), we can get the label for a mounted file system. It still does normal process to fetch label if the specified file system is unmounted. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com --- btrfslabel.c | 74 ++ ioctl.h |2 ++ utils.c |2 +- utils.h |1 + 4 files changed, 53 insertions(+), 26 deletions(-) diff --git a/btrfslabel.c b/btrfslabel.c index cb142b0..443eff6 100644 --- a/btrfslabel.c +++ b/btrfslabel.c @@ -67,45 +67,69 @@ static void change_label_unmounted(char *dev, char *nLabel) close_ctree(root); } -int get_label_unmounted(char *dev) +static int get_label_unmounted(const char *dev) { - struct btrfs_root *root; + struct btrfs_root *root; + int ret; - /* Open the super_block at the default location -* and as read-only. -*/ - root = open_ctree(dev, 0, 0); + ret = check_mounted(dev); + if (ret 0) { + fprintf(stderr, FATAL: error checking %s mount status\n, dev); + return -1; + } + if (ret 0) { + fprintf(stderr, ERROR: dev %s is mounted, use mount point\n, + dev); + return -1; + } - if(!root) - return -1; + /* Open the super_block at the default location +* and as read-only. +*/ + root = open_ctree(dev, 0, 0); + if(!root) + return -1; - fprintf(stdout, %s\n, root-fs_info-super_copy.label); + fprintf(stdout, %s\n, root-fs_info-super_copy.label); - /* Now we close it since we are done. */ - close_ctree(root); - return 0; + /* Now we close it since we are done. */ + close_ctree(root); + return 0; } -int get_label(char *btrfs_dev) +/* + * If a partition is mounted, try to get the filesystem label via its + * mounted path rather than device. Return the corresponding error + * the user specified the device path. + */ +static int get_label_mounted(const char *mount_path) { + char label[BTRFS_LABEL_SIZE]; + int fd; - int ret; - ret = check_mounted(btrfs_dev); - if (ret 0) - { - fprintf(stderr, FATAL: error checking %s mount status\n, btrfs_dev); - return -1; + fd = open(mount_path, O_RDONLY | O_NOATIME); + if (fd 0) { + fprintf(stderr, ERROR: unable access to '%s'\n, mount_path); + return -1; } - if(ret != 0) - { - fprintf(stderr, FATAL: the filesystem has to be unmounted\n); - return -2; + memset(label, '\0', sizeof(label)); + if (ioctl(fd, BTRFS_IOC_GET_FSLABEL, label) 0) { + fprintf(stderr, ERROR: unable get label %s\n, strerror(errno)); + close(fd); + return -1; } - ret = get_label_unmounted(btrfs_dev); - return ret; + + fprintf(stdout, %s\n, label); + return 0; } +int get_label(const char *btrfs_dev) +{ + return is_existing_blk_or_reg_file(btrfs_dev) ? + get_label_unmounted(btrfs_dev) : + get_label_mounted(btrfs_dev); +} int set_label(char *btrfs_dev, char *nLabel) { diff --git a/ioctl.h b/ioctl.h index 6fda3a1..0807b05 100644 --- a/ioctl.h +++ b/ioctl.h @@ -432,4 +432,6 @@ struct btrfs_ioctl_clone_range_args { struct btrfs_ioctl_qgroup_create_args) #define BTRFS_IOC_QGROUP_LIMIT _IOR(BTRFS_IOCTL_MAGIC, 43, \ struct btrfs_ioctl_qgroup_limit_args) +#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ + char[BTRFS_LABEL_SIZE]) #endif diff --git a/utils.c b/utils.c index 205e667..d59bca3 100644 --- a/utils.c +++ b/utils.c @@ -811,7 +811,7 @@ int check_mounted(const char* file) return -errno; } - ret = check_mounted_where(fd, file, NULL, 0, NULL); + ret = check_mounted_where(fd, file, NULL, 0, NULL); close(fd); return ret; diff --git a/utils.h b/utils.h index 3a0368b..8750f28 100644 --- a/utils.h +++ b/utils.h @@ -46,4 +46,5 @@ int check_label(char *input); int get_mountpt(char *dev, char *mntpt, size_t size); int btrfs_scan_block_devices(int run_ioctl); +int is_existing_blk_or_reg_file(const char* filename); #endif -- 1.7.9.5 -- 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
[PATCH V5 2/4] Btrfs-progs: Change the label of a mounted file system
With this new ioctl(2), we can set/change the label for a mounted file system. It still does normal process for an umounted file system. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com --- btrfslabel.c | 90 +++--- ioctl.h |2 ++ 2 files changed, 57 insertions(+), 35 deletions(-) diff --git a/btrfslabel.c b/btrfslabel.c index 443eff6..938e8b4 100644 --- a/btrfslabel.c +++ b/btrfslabel.c @@ -46,25 +46,58 @@ #define GET_LABEL 3 #define SET_LABEL 4 -static void change_label_unmounted(char *dev, char *nLabel) +static int set_label_unmounted(const char *dev, const char *label) { - struct btrfs_root *root; - struct btrfs_trans_handle *trans; - - /* Open the super_block at the default location -* and as read-write. -*/ - root = open_ctree(dev, 0, 1); - if (!root) /* errors are printed by open_ctree() */ - return; - - trans = btrfs_start_transaction(root, 1); - strncpy(root-fs_info-super_copy.label, nLabel, BTRFS_LABEL_SIZE); - root-fs_info-super_copy.label[BTRFS_LABEL_SIZE-1] = 0; - btrfs_commit_transaction(trans, root); - - /* Now we close it since we are done. */ - close_ctree(root); + struct btrfs_trans_handle *trans; + struct btrfs_root *root; + int ret; + + ret = check_mounted(dev); + if (ret 0) { + fprintf(stderr, FATAL: error checking %s mount status\n, dev); + return -1; + } + if (ret 0) { + fprintf(stderr, ERROR: dev %s is mounted, use mount point\n, + dev); + return -1; + } + + /* Open the super_block at the default location +* and as read-write. +*/ + root = open_ctree(dev, 0, 1); + if (!root) /* errors are printed by open_ctree() */ + return -1; + + trans = btrfs_start_transaction(root, 1); + strncpy(root-fs_info-super_copy.label, label, BTRFS_LABEL_SIZE); + root-fs_info-super_copy.label[BTRFS_LABEL_SIZE-1] = 0; + btrfs_commit_transaction(trans, root); + + /* Now we close it since we are done. */ + close_ctree(root); + return 0; +} + +static int set_label_mounted(const char *mount_path, const char *label) +{ + int fd; + + fd = open(mount_path, O_RDONLY | O_NOATIME); + if (fd 0) { + fprintf(stderr, ERROR: unable access to '%s'\n, mount_path); + return -1; + } + + if (ioctl(fd, BTRFS_IOC_SET_FSLABEL, label) 0) { + fprintf(stderr, ERROR: unable to set label %s\n, + strerror(errno)); + close(fd); + return -1; + } + + return 0; } static int get_label_unmounted(const char *dev) @@ -131,22 +164,9 @@ int get_label(const char *btrfs_dev) get_label_mounted(btrfs_dev); } -int set_label(char *btrfs_dev, char *nLabel) +int set_label(char *btrfs_dev, char *label) { - - int ret; - ret = check_mounted(btrfs_dev); - if (ret 0) - { - fprintf(stderr, FATAL: error checking %s mount status\n, btrfs_dev); - return -1; - } - - if(ret != 0) - { - fprintf(stderr, FATAL: the filesystem has to be unmounted\n); - return -2; - } - change_label_unmounted(btrfs_dev, nLabel); - return 0; + return is_existing_blk_or_reg_file(btrfs_dev) ? + set_label_unmounted(btrfs_dev, label) : + set_label_mounted(btrfs_dev, label); } diff --git a/ioctl.h b/ioctl.h index 0807b05..0882d6c 100644 --- a/ioctl.h +++ b/ioctl.h @@ -434,4 +434,6 @@ struct btrfs_ioctl_clone_range_args { struct btrfs_ioctl_qgroup_limit_args) #define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ char[BTRFS_LABEL_SIZE]) +#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \ + char[BTRFS_LABEL_SIZE]) #endif -- 1.7.9.5 -- 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
[PATCH V5 3/4] Btrfs-progs: Fix set_label_unmounted() with label length validation
Currently, we keeping silent if the label length is exceeding BTRFS_LABEL_SIZE - 1, and just truncating the characters beyond that. This patch make it return error and exit in this situation. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com --- btrfslabel.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/btrfslabel.c b/btrfslabel.c index 938e8b4..2826050 100644 --- a/btrfslabel.c +++ b/btrfslabel.c @@ -63,6 +63,12 @@ static int set_label_unmounted(const char *dev, const char *label) return -1; } + if (strlen(label) BTRFS_LABEL_SIZE - 1) { + fprintf(stderr, ERROR: Label %s is too long (max %d)\n, + label, BTRFS_LABEL_SIZE - 1); + return -1; + } + /* Open the super_block at the default location * and as read-write. */ @@ -71,8 +77,8 @@ static int set_label_unmounted(const char *dev, const char *label) return -1; trans = btrfs_start_transaction(root, 1); - strncpy(root-fs_info-super_copy.label, label, BTRFS_LABEL_SIZE); - root-fs_info-super_copy.label[BTRFS_LABEL_SIZE-1] = 0; + snprintf(root-fs_info-super_copy.label, BTRFS_LABEL_SIZE, %s, +label); btrfs_commit_transaction(trans, root); /* Now we close it since we are done. */ -- 1.7.9.5 -- 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
[PATCH v5 4/4] Btrfs-progs: fix cmd_label_usage to reflect this change.
Fix the command usage of btrfs filesystem label to reflect this change. i.e. so that we can get/set the label of a mounted filesystem against the mountpoint. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com --- cmds-filesystem.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmds-filesystem.c b/cmds-filesystem.c index 9c43d35..5770d8b 100644 --- a/cmds-filesystem.c +++ b/cmds-filesystem.c @@ -517,8 +517,8 @@ static int cmd_resize(int argc, char **argv) } static const char * const cmd_label_usage[] = { - btrfs filesystem label device [newlabel], - Get or change the label of an unmounted filesystem, + btrfs filesystem label [device|mountpoint] [newlabel], + Get or change the label of a filesystem, With one argument, get the label of filesystem on device., If newlabel is passed, set the filesystem label to newlabel., NULL -- 1.7.9.5 -- 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
Re: [RFC PATCH V5 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system
On 12/17/2012 07:57 PM, Miao Xie wrote: Onmon, 17 Dec 2012 19:22:11 +0800, Jeff Liu wrote: Introduce a new ioctl BTRFS_IOC_SET_FSLABEL to change the label of a mounted file system. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com Cc: Miao Xie mi...@cn.fujitsu.com --- fs/btrfs/ioctl.c | 40 fs/btrfs/ioctl.h |2 ++ 2 files changed, 42 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 6a2488a..0186651 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3712,6 +3712,44 @@ static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) return ret ? -EFAULT : 0; } +static int btrfs_ioctl_set_fslabel(struct file *file, void __user *arg) +{ +struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root; +struct btrfs_super_block *super_block = root-fs_info-super_copy; +char label[BTRFS_LABEL_SIZE]; +int ret; + +if (!capable(CAP_SYS_ADMIN)) +return -EPERM; + +if (copy_from_user(label, arg, sizeof(label))) +return -EFAULT; + +if (strlen(label) BTRFS_LABEL_SIZE - 1) +return -EINVAL; I think we should use strnlen() AFAICS, strnlen() is better only if the caller need to get the length of a length-limited string and make use of it proceeding, which means that the procedure would not return an error even if the length is beyond the limit. Or if the caller need to examine if a length-limited string is nul-terminated or not in a manner below, if (strnlen(buf, MAX_BUF_SIZE) == MAX_BUF_SIZE) { } I don't think it really needed here since the logic is clear with strlen(), or Am I miss anything? Thanks, -Jeff Thanks Miao + +ret = mnt_want_write_file(file); +if (ret) +return ret; + +mutex_lock(root-fs_info-volume_mutex); +trans = btrfs_start_transaction(root, 1); +if (IS_ERR(trans)) { +ret = PTR_ERR(trans); +goto out_unlock; +} + +label[BTRFS_LABEL_SIZE - 1] = '\0'; +strcpy(super_block-label, label); +btrfs_end_transaction(trans, root); + +out_unlock: +mutex_unlock(root-fs_info-volume_mutex); +mnt_drop_write_file(file); +return ret; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3812,6 +3850,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_limit(root, argp); case BTRFS_IOC_GET_FSLABEL: return btrfs_ioctl_get_fslabel(file, argp); +case BTRFS_IOC_SET_FSLABEL: +return btrfs_ioctl_set_fslabel(file, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 5b2cbef..2abe239 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -453,6 +453,8 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_limit_args) #define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ char[BTRFS_LABEL_SIZE]) +#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \ + char[BTRFS_LABEL_SIZE]) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) #endif -- 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 -- 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
Re: [RFC PATCH V5 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system
On 12/18/2012 01:34 AM, Goffredo Baroncelli wrote: On 12/17/2012 02:30 PM, Jeff Liu wrote: On 12/17/2012 07:57 PM, Miao Xie wrote: On mon, 17 Dec 2012 19:22:11 +0800, Jeff Liu wrote: Introduce a new ioctl BTRFS_IOC_SET_FSLABEL to change the label of a mounted file system. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com Cc: Miao Xie mi...@cn.fujitsu.com [...] + + if (strlen(label) BTRFS_LABEL_SIZE - 1) + return -EINVAL; I think we should use strnlen() AFAICS, strnlen() is better only if the caller need to get the length of a length-limited string and make use of it proceeding, which means that the procedure would not return an error even if the length is beyond the limit. Or if the caller need to examine if a length-limited string is nul-terminated or not in a manner below, if (strnlen(buf, MAX_BUF_SIZE) == MAX_BUF_SIZE) { } I don't think it really needed here since the logic is clear with strlen(), or Am I miss anything? I think that Miao fears strlen() searching a zero could go beyond the page limit touching an un-mapped page and raising an segmentation fault I think that we should change the code as + label[BTRFS_LABEL_SIZE - 1] = 0; Ah, I moved above line for strcpy()... + + if (strlen(label) BTRFS_LABEL_SIZE - 1) + return -EINVAL; That's right, thank you! -Jeff My 2¢ Ciao G.Baroncelli Thanks, -Jeff Thanks Miao + + ret = mnt_want_write_file(file); + if (ret) + return ret; + + mutex_lock(root-fs_info-volume_mutex); + trans = btrfs_start_transaction(root, 1); + if (IS_ERR(trans)) { + ret = PTR_ERR(trans); + goto out_unlock; + } + + label[BTRFS_LABEL_SIZE - 1] = '\0'; + strcpy(super_block-label, label); + btrfs_end_transaction(trans, root); + +out_unlock: + mutex_unlock(root-fs_info-volume_mutex); + mnt_drop_write_file(file); + return ret; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3812,6 +3850,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_limit(root, argp); case BTRFS_IOC_GET_FSLABEL: return btrfs_ioctl_get_fslabel(file, argp); + case BTRFS_IOC_SET_FSLABEL: + return btrfs_ioctl_set_fslabel(file, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 5b2cbef..2abe239 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -453,6 +453,8 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_limit_args) #define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ char[BTRFS_LABEL_SIZE]) +#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \ + char[BTRFS_LABEL_SIZE]) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) #endif -- 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 -- 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 -- 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
Re: [RFC PATCH V5 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system
On 12/18/2012 10:21 AM, Miao Xie wrote: Onmon, 17 Dec 2012 18:34:41 +0100, Goffredo Baroncelli wrote: On 12/17/2012 02:30 PM, Jeff Liu wrote: On 12/17/2012 07:57 PM, Miao Xie wrote: On mon, 17 Dec 2012 19:22:11 +0800, Jeff Liu wrote: Introduce a new ioctl BTRFS_IOC_SET_FSLABEL to change the label of a mounted file system. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com Cc: Miao Xie mi...@cn.fujitsu.com [...] + + if (strlen(label) BTRFS_LABEL_SIZE - 1) + return -EINVAL; I think we should use strnlen() AFAICS, strnlen() is better only if the caller need to get the length of a length-limited string and make use of it proceeding, which means that the procedure would not return an error even if the length is beyond the limit. Or if the caller need to examine if a length-limited string is nul-terminated or not in a manner below, if (strnlen(buf, MAX_BUF_SIZE) == MAX_BUF_SIZE) { } I don't think it really needed here since the logic is clear with strlen(), or Am I miss anything? I think that Miao fears strlen() searching a zero could go beyond the page limit touching an un-mapped page and raising an segmentation fault Yes, so I think the following check is better. if (strnlen(buf, BTRFS_LABEL_SIZE) == BTRFS_LABEL_SIZE) return -EINVAL; Generally speaking, the user would not input a large string for normal purpose, so strnlen() will always have a bit waste(can be ignore here) with the counter self-check. i.e. for (; count--, ;). Thanks Miao I think that we should change the code as +label[BTRFS_LABEL_SIZE - 1] = 0; + +if (strlen(label) BTRFS_LABEL_SIZE - 1) +return -EINVAL; Both suggestion are fine to me, but I prefer to above approach. Thanks, -Jeff -- 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
Re: [RFC PATCH V5 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system
On 12/18/2012 10:33 AM, Jeff Liu wrote: On 12/18/2012 10:21 AM, Miao Xie wrote: On mon, 17 Dec 2012 18:34:41 +0100, Goffredo Baroncelli wrote: On 12/17/2012 02:30 PM, Jeff Liu wrote: On 12/17/2012 07:57 PM, Miao Xie wrote: Onmon, 17 Dec 2012 19:22:11 +0800, Jeff Liu wrote: Introduce a new ioctl BTRFS_IOC_SET_FSLABEL to change the label of a mounted file system. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com Cc: Miao Xie mi...@cn.fujitsu.com [...] + +if (strlen(label) BTRFS_LABEL_SIZE - 1) +return -EINVAL; I think we should use strnlen() AFAICS, strnlen() is better only if the caller need to get the length of a length-limited string and make use of it proceeding, which means that the procedure would not return an error even if the length is beyond the limit. Or if the caller need to examine if a length-limited string is nul-terminated or not in a manner below, if (strnlen(buf, MAX_BUF_SIZE) == MAX_BUF_SIZE) { } I don't think it really needed here since the logic is clear with strlen(), or Am I miss anything? I think that Miao fears strlen() searching a zero could go beyond the page limit touching an un-mapped page and raising an segmentation fault Yes, so I think the following check is better. if (strnlen(buf, BTRFS_LABEL_SIZE) == BTRFS_LABEL_SIZE) return -EINVAL; Generally speaking, the user would not input a large string for normal purpose, so strnlen() will always have a bit waste(can be ignore here) with the counter self-check. i.e. for (; count--, ;). Thanks Miao I think that we should change the code as + label[BTRFS_LABEL_SIZE - 1] = 0; + + if (strlen(label) BTRFS_LABEL_SIZE - 1) + return -EINVAL; Both suggestion are fine to me, but I prefer to above approach. Oh No, Miao is right. We can not perform the check as above because we have already made the last character of label to NUL, hence strlen(label) BTRFS_LABEL_SIZE -1 will be an invalid checking even if the input string is longer than BTRFS_LABEL_SIZE -1. Thanks, -Jeff Thanks, -Jeff -- 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 -- 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
[RFC PATCH V6 0/2] Btrfs: get/set label of a mounted file system
Hello, Here is quick fix for btrfs set lable ioctl(2) based on v5, it does label length check up with strnlen() to avoid searching a zero could go beyond the page limit touching an un-mapped page and raising an segmentation fault with strlen(). V6-v5: - Improve the input label string validation with strnlen(). The old versions can be found at: v5: http://www.spinics.net/lists/linux-btrfs/msg20888.html v4: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/21618 v3: https://patchwork.kernel.org/patch/1124642/ v2: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/12877 v1: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/12872 Thanks, -Jeff -- 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
[RFC PATCH V6 2/1] Btrfs: Add a new ioctl to get the label of a mounted file system
Introduce a new ioctl BTRFS_IOC_GET_FSLABEL to fetch the label of a mounted file system. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com Reviewed-by: Miao Xie mi...@cn.fujitsu.com --- fs/btrfs/ioctl.c | 15 +++ fs/btrfs/ioctl.h |2 ++ 2 files changed, 17 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 8fcf9a5..6a2488a 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3699,6 +3699,19 @@ out: return ret; } +static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) +{ + struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root; + const char *label = root-fs_info-super_copy-label; + int ret; + + mutex_lock(root-fs_info-volume_mutex); + ret = copy_to_user(arg, label, strlen(label)); + mutex_unlock(root-fs_info-volume_mutex); + + return ret ? -EFAULT : 0; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3797,6 +3810,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_create(root, argp); case BTRFS_IOC_QGROUP_LIMIT: return btrfs_ioctl_qgroup_limit(root, argp); + case BTRFS_IOC_GET_FSLABEL: + return btrfs_ioctl_get_fslabel(file, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 731e287..5b2cbef 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -451,6 +451,8 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_create_args) #define BTRFS_IOC_QGROUP_LIMIT _IOR(BTRFS_IOCTL_MAGIC, 43, \ struct btrfs_ioctl_qgroup_limit_args) +#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ + char[BTRFS_LABEL_SIZE]) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) #endif -- 1.7.9.5 -- 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
[RFC PATCH V6 2/2] Btrfs: Add a new ioctl to change the label of a mounted file system
Introduce a new ioctl BTRFS_IOC_SET_FSLABEL to change the label of a mounted file system. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com Cc: Miao Xie mi...@cn.fujitsu.com Cc: Goffredo Baroncelli kreij...@inwind.it --- fs/btrfs/ioctl.c | 39 +++ fs/btrfs/ioctl.h |2 ++ 2 files changed, 41 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 6a2488a..2bb037e 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3712,6 +3712,43 @@ static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg) return ret ? -EFAULT : 0; } +static int btrfs_ioctl_set_fslabel(struct file *file, void __user *arg) +{ + struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root; + struct btrfs_super_block *super_block = root-fs_info-super_copy; + struct btrfs_trans_handle *trans; + char label[BTRFS_LABEL_SIZE]; + int ret; + + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + + if (copy_from_user(label, arg, sizeof(label))) + return -EFAULT; + + if (strnlen(label, BTRFS_LABEL_SIZE) == BTRFS_LABEL_SIZE) + return -EINVAL; + + ret = mnt_want_write_file(file); + if (ret) + return ret; + + mutex_lock(root-fs_info-volume_mutex); + trans = btrfs_start_transaction(root, 1); + if (IS_ERR(trans)) { + ret = PTR_ERR(trans); + goto out_unlock; + } + + strcpy(super_block-label, label); + btrfs_end_transaction(trans, root); + +out_unlock: + mutex_unlock(root-fs_info-volume_mutex); + mnt_drop_write_file(file); + return ret; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3812,6 +3849,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_limit(root, argp); case BTRFS_IOC_GET_FSLABEL: return btrfs_ioctl_get_fslabel(file, argp); + case BTRFS_IOC_SET_FSLABEL: + return btrfs_ioctl_set_fslabel(file, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 5b2cbef..2abe239 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -453,6 +453,8 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_limit_args) #define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ char[BTRFS_LABEL_SIZE]) +#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \ + char[BTRFS_LABEL_SIZE]) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) #endif -- 1.7.9.5 -- 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
Re: [RESEND PATCH V4 2/2] Btrfs: Add a new ioctl to change the label of a mounted filesystem
Sorry for my late response, I missed your feedback somehow. On 12/12/2012 12:15 PM, Miao Xie wrote: On wed, 12 Dec 2012 11:23:00 +0800, Jeff Liu wrote: Add a new ioctl BTRFS_FS_SETLABEL to change the label of a mounted filesystem. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com --- fs/btrfs/ioctl.c | 34 ++ fs/btrfs/ioctl.h |2 ++ 2 files changed, 36 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index b0a5e17..ebbf634 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3711,6 +3711,38 @@ static int btrfs_ioctl_get_fslabel(struct btrfs_root *root, void __user *arg) return ret ? -EFAULT : 0; } +static int btrfs_ioctl_set_fslabel(struct btrfs_root *root, void __user *arg) +{ +struct btrfs_super_block *super_block = root-fs_info-super_copy; +struct btrfs_trans_handle *trans; +char label[BTRFS_LABEL_SIZE]; +int ret = 0; + +if (!capable(CAP_SYS_ADMIN)) +return -EPERM; + +if (btrfs_root_readonly(root)) +return -EROFS; Since this label is a global parameter, we needn't check the root is R/O or not. The check we need is the write access of the fs, so we need call mnt_want_write_file() Yes, you are right. + +if (copy_from_user(label, arg, sizeof(label))) +return -EFAULT; + +label[BTRFS_LABEL_SIZE - 1] = '\0'; I don't think setting the last byte to '\0' is a good way, because the user would be very strange that the new label is truncated, but the program told him that it was done successfully. Good point, will revise it later. Thanks, -Jeff + +mutex_lock(root-fs_info-volume_mutex); +trans = btrfs_start_transaction(root, 1); +if (IS_ERR(trans)) { +ret = PTR_ERR(trans); +goto out_unlock; +} +strcpy(super_block-label, label); +btrfs_end_transaction(trans, root); + +out_unlock: +mutex_unlock(root-fs_info-volume_mutex); +return ret; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3811,6 +3843,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_limit(root, argp); case BTRFS_IOC_GET_FSLABEL: return btrfs_ioctl_get_fslabel(root, argp); +case BTRFS_IOC_SET_FSLABEL: +return btrfs_ioctl_set_fslabel(root, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 98f896f..cc4e657 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -455,4 +455,6 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_get_dev_stats) #define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 53, \ char[BTRFS_LABEL_SIZE]) +#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 54, \ + char[BTRFS_LABEL_SIZE]) #endif number 50 has been reserved for this feature. Thanks Miao -- 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
[PATCH V4 1/2] Btrfs: Add a new ioctl to get the label of a mounted filesystem
Introduce a new ioctl BTRFS_IOC_GET_FSLABEL, which would be used to fetch the label of a mounted filesystem. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com --- fs/btrfs/ioctl.c | 14 ++ fs/btrfs/ioctl.h |2 ++ 2 files changed, 16 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 8fcf9a5..b0a5e17 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3699,6 +3699,18 @@ out: return ret; } +static int btrfs_ioctl_get_fslabel(struct btrfs_root *root, void __user *arg) +{ + const char *label = root-fs_info-super_copy-label; + int ret; + + mutex_lock(root-fs_info-volume_mutex); + ret = copy_to_user(arg, label, strlen(label)); + mutex_unlock(root-fs_info-volume_mutex); + + return ret ? -EFAULT : 0; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3797,6 +3809,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_create(root, argp); case BTRFS_IOC_QGROUP_LIMIT: return btrfs_ioctl_qgroup_limit(root, argp); + case BTRFS_IOC_GET_FSLABEL: + return btrfs_ioctl_get_fslabel(root, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 731e287..98f896f 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -453,4 +453,6 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_limit_args) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) +#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 53, \ + char[BTRFS_LABEL_SIZE]) #endif -- 1.7.9.5 -- 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
[PATCH V4 2/2] Btrfs: Add a new ioctl to change the label of a mounted filesystem
Add a new ioctl BTRFS_FS_SETLABEL to change the label of a mounted filesystem. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com --- fs/btrfs/ioctl.c | 14 ++ fs/btrfs/ioctl.h |2 ++ 2 files changed, 16 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 8fcf9a5..b0a5e17 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3699,6 +3699,18 @@ out: return ret; } +static int btrfs_ioctl_get_fslabel(struct btrfs_root *root, void __user *arg) +{ + const char *label = root-fs_info-super_copy-label; + int ret; + + mutex_lock(root-fs_info-volume_mutex); + ret = copy_to_user(arg, label, strlen(label)); + mutex_unlock(root-fs_info-volume_mutex); + + return ret ? -EFAULT : 0; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3797,6 +3809,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_create(root, argp); case BTRFS_IOC_QGROUP_LIMIT: return btrfs_ioctl_qgroup_limit(root, argp); + case BTRFS_IOC_GET_FSLABEL: + return btrfs_ioctl_get_fslabel(root, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 731e287..98f896f 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -453,4 +453,6 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_limit_args) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) +#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 53, \ + char[BTRFS_LABEL_SIZE]) #endif -- 1.7.9.5 jeff@koala:/usr/src/linux$ cat btrfs_label_v3/0002-set-label.patch Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/ioctl.c | 34 ++ fs/btrfs/ioctl.h |2 ++ 2 files changed, 36 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index b0a5e17..ebbf634 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3711,6 +3711,38 @@ static int btrfs_ioctl_get_fslabel(struct btrfs_root *root, void __user *arg) return ret ? -EFAULT : 0; } +static int btrfs_ioctl_set_fslabel(struct btrfs_root *root, void __user *arg) +{ + struct btrfs_super_block *super_block = root-fs_info-super_copy; + struct btrfs_trans_handle *trans; + char label[BTRFS_LABEL_SIZE]; + int ret = 0; + + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + + if (btrfs_root_readonly(root)) + return -EROFS; + + if (copy_from_user(label, arg, sizeof(label))) + return -EFAULT; + + label[BTRFS_LABEL_SIZE - 1] = '\0'; + + mutex_lock(root-fs_info-volume_mutex); + trans = btrfs_start_transaction(root, 1); + if (IS_ERR(trans)) { + ret = PTR_ERR(trans); + goto out_unlock; + } + strcpy(super_block-label, label); + btrfs_end_transaction(trans, root); + +out_unlock: + mutex_unlock(root-fs_info-volume_mutex); + return ret; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3811,6 +3843,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_limit(root, argp); case BTRFS_IOC_GET_FSLABEL: return btrfs_ioctl_get_fslabel(root, argp); + case BTRFS_IOC_SET_FSLABEL: + return btrfs_ioctl_set_fslabel(root, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 98f896f..cc4e657 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -455,4 +455,6 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_get_dev_stats) #define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 53, \ char[BTRFS_LABEL_SIZE]) +#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 54, \ + char[BTRFS_LABEL_SIZE]) #endif -- 1.7.9.5 -- 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
[RFC PATCH V4 0/2] Btrfs: get/set mounted filesystem label support
Hello, This is an improved tiny patch set to add get/set the label upon a mounted file system via ioctl(2). I'd like to add the Signed-off-by with Anand for the credit since He has posted a similar patch set a few month ago. Changes of V4-V3: - Add a new ioctl to fetch the label of a mounted file system. - Replace struct btrfs_ioctl_fs_label_args with char label[BTRFS_LABEL_SIZE]. - ioctl(2) number changed from 50 to 53/54. I'll update the btrfs wiki page with regard to the number allocation to reflect this change if it is ok. The old versions can be found at: v3: https://patchwork.kernel.org/patch/1124642/ v2: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/12877 v1: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/12872 Thanks, -Jeff -- 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
[PATCH v4 3/3] Btrfs-progs: fix cmd_label_usage to reflect this change.
Fix the command usage of btrfs filesystem label to reflect this change. i.e. so that we can get/set the label of a mounted filesystem against the mountpoint. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com --- cmds-filesystem.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmds-filesystem.c b/cmds-filesystem.c index 9c43d35..5770d8b 100644 --- a/cmds-filesystem.c +++ b/cmds-filesystem.c @@ -517,8 +517,8 @@ static int cmd_resize(int argc, char **argv) } static const char * const cmd_label_usage[] = { - btrfs filesystem label device [newlabel], - Get or change the label of an unmounted filesystem, + btrfs filesystem label [device|mountpoint] [newlabel], + Get or change the label of a filesystem, With one argument, get the label of filesystem on device., If newlabel is passed, set the filesystem label to newlabel., NULL -- 1.7.9.5 -- 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
[PATCH v4 1/3] Btrfs-progs: get the label of a mounted filesystem
Get the label of a mounted filesystem through the new ioctl BTRFS_IOCTL_FS_GETLABEL. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com --- btrfslabel.c | 68 +- ioctl.h |2 ++ utils.c |2 +- utils.h |1 + 4 files changed, 47 insertions(+), 26 deletions(-) diff --git a/btrfslabel.c b/btrfslabel.c index cb142b0..761a312 100644 --- a/btrfslabel.c +++ b/btrfslabel.c @@ -67,45 +67,63 @@ static void change_label_unmounted(char *dev, char *nLabel) close_ctree(root); } -int get_label_unmounted(char *dev) +static int get_label_unmounted(const char *dev) { - struct btrfs_root *root; + struct btrfs_root *root; + int ret; - /* Open the super_block at the default location -* and as read-only. -*/ - root = open_ctree(dev, 0, 0); + ret = check_mounted(dev); + if (ret 0) { + fprintf(stderr, FATAL: error checking %s mount status\n, dev); + return -1; + } - if(!root) - return -1; + /* Open the super_block at the default location +* and as read-only. +*/ + root = open_ctree(dev, 0, 0); + if(!root) + return -1; - fprintf(stdout, %s\n, root-fs_info-super_copy.label); + fprintf(stdout, %s\n, root-fs_info-super_copy.label); - /* Now we close it since we are done. */ - close_ctree(root); - return 0; + /* Now we close it since we are done. */ + close_ctree(root); + return 0; } -int get_label(char *btrfs_dev) +/* + * If a partition is mounted, try to get the filesystem label via its + * mounted path rather than device. Return the corresponding error + * the user specified the device path. + */ +static int get_label_mounted(const char *mount_path) { + char label[BTRFS_LABEL_SIZE]; + int fd; - int ret; - ret = check_mounted(btrfs_dev); - if (ret 0) - { - fprintf(stderr, FATAL: error checking %s mount status\n, btrfs_dev); - return -1; + fd = open(mount_path, O_RDONLY | O_NOATIME); + if (fd 0) { + fprintf(stderr, ERROR: unable access to '%s'\n, mount_path); + return -1; } - if(ret != 0) - { - fprintf(stderr, FATAL: the filesystem has to be unmounted\n); - return -2; + if (ioctl(fd, BTRFS_IOC_GET_FSLABEL, label) 0) { + fprintf(stderr, ERROR: unable get label %s\n, strerror(errno)); + close(fd); + return -1; } - ret = get_label_unmounted(btrfs_dev); - return ret; + + fprintf(stdout, %s\n, label); + return 0; } +int get_label(const char *btrfs_dev) +{ + return is_existing_blk_or_reg_file(btrfs_dev) ? + get_label_unmounted(btrfs_dev) : + get_label_mounted(btrfs_dev); +} int set_label(char *btrfs_dev, char *nLabel) { diff --git a/ioctl.h b/ioctl.h index 6fda3a1..ed6784d 100644 --- a/ioctl.h +++ b/ioctl.h @@ -432,4 +432,6 @@ struct btrfs_ioctl_clone_range_args { struct btrfs_ioctl_qgroup_create_args) #define BTRFS_IOC_QGROUP_LIMIT _IOR(BTRFS_IOCTL_MAGIC, 43, \ struct btrfs_ioctl_qgroup_limit_args) +#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 53, \ + char[BTRFS_LABEL_SIZE]) #endif diff --git a/utils.c b/utils.c index 205e667..d59bca3 100644 --- a/utils.c +++ b/utils.c @@ -811,7 +811,7 @@ int check_mounted(const char* file) return -errno; } - ret = check_mounted_where(fd, file, NULL, 0, NULL); + ret = check_mounted_where(fd, file, NULL, 0, NULL); close(fd); return ret; diff --git a/utils.h b/utils.h index 3a0368b..8750f28 100644 --- a/utils.h +++ b/utils.h @@ -46,4 +46,5 @@ int check_label(char *input); int get_mountpt(char *dev, char *mntpt, size_t size); int btrfs_scan_block_devices(int run_ioctl); +int is_existing_blk_or_reg_file(const char* filename); #endif -- 1.7.9.5 -- 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
[PATCH v4 2/3] Btrfs-progs: change the label of a mounted filesystem
Change the label of a mounted file system through the new ioctl BTRFS_IOCTL_FS_SETLABEL. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com --- btrfslabel.c | 84 ++ ioctl.h |2 ++ 2 files changed, 51 insertions(+), 35 deletions(-) diff --git a/btrfslabel.c b/btrfslabel.c index 761a312..65388e5 100644 --- a/btrfslabel.c +++ b/btrfslabel.c @@ -46,25 +46,52 @@ #define GET_LABEL 3 #define SET_LABEL 4 -static void change_label_unmounted(char *dev, char *nLabel) +static int set_label_unmounted(const char *dev, const char *label) { - struct btrfs_root *root; - struct btrfs_trans_handle *trans; - - /* Open the super_block at the default location -* and as read-write. -*/ - root = open_ctree(dev, 0, 1); - if (!root) /* errors are printed by open_ctree() */ - return; - - trans = btrfs_start_transaction(root, 1); - strncpy(root-fs_info-super_copy.label, nLabel, BTRFS_LABEL_SIZE); - root-fs_info-super_copy.label[BTRFS_LABEL_SIZE-1] = 0; - btrfs_commit_transaction(trans, root); - - /* Now we close it since we are done. */ - close_ctree(root); + struct btrfs_trans_handle *trans; + struct btrfs_root *root; + int ret; + + ret = check_mounted(dev); + if (ret 0) { + fprintf(stderr, FATAL: error checking %s mount status\n, dev); + return -1; + } + + /* Open the super_block at the default location +* and as read-write. +*/ + root = open_ctree(dev, 0, 1); + if (!root) /* errors are printed by open_ctree() */ + return -1; + + trans = btrfs_start_transaction(root, 1); + strncpy(root-fs_info-super_copy.label, label, BTRFS_LABEL_SIZE); + root-fs_info-super_copy.label[BTRFS_LABEL_SIZE-1] = 0; + btrfs_commit_transaction(trans, root); + + /* Now we close it since we are done. */ + close_ctree(root); + return 0; +} + +static int set_label_mounted(const char *mount_path, const char *label) +{ + int fd; + + fd = open(mount_path, O_RDONLY | O_NOATIME); + if (fd 0) { + fprintf(stderr, ERROR: unable access to '%s'\n, mount_path); + return -1; + } + + if (ioctl(fd, BTRFS_IOC_SET_FSLABEL, label) 0) { + fprintf(stderr, ERROR: unable get label %s\n, strerror(errno)); + close(fd); + return -1; + } + + return 0; } static int get_label_unmounted(const char *dev) @@ -125,22 +152,9 @@ int get_label(const char *btrfs_dev) get_label_mounted(btrfs_dev); } -int set_label(char *btrfs_dev, char *nLabel) +int set_label(char *btrfs_dev, char *label) { - - int ret; - ret = check_mounted(btrfs_dev); - if (ret 0) - { - fprintf(stderr, FATAL: error checking %s mount status\n, btrfs_dev); - return -1; - } - - if(ret != 0) - { - fprintf(stderr, FATAL: the filesystem has to be unmounted\n); - return -2; - } - change_label_unmounted(btrfs_dev, nLabel); - return 0; + return is_existing_blk_or_reg_file(btrfs_dev) ? + set_label_unmounted(btrfs_dev, label) : + set_label_mounted(btrfs_dev, label); } diff --git a/ioctl.h b/ioctl.h index ed6784d..07259f9 100644 --- a/ioctl.h +++ b/ioctl.h @@ -434,4 +434,6 @@ struct btrfs_ioctl_clone_range_args { struct btrfs_ioctl_qgroup_limit_args) #define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 53, \ char[BTRFS_LABEL_SIZE]) +#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 54, \ + char[BTRFS_LABEL_SIZE]) #endif -- 1.7.9.5 -- 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
Re: [PATCH V4 2/2] Btrfs: Add a new ioctl to change the label of a mounted filesystem
Hi Miao, On 12/12/2012 11:03 AM, Miao Xie wrote: Hi, Liu On wed, 12 Dec 2012 08:22:28 +0800, Jeff Liu wrote: Add a new ioctl BTRFS_FS_SETLABEL to change the label of a mounted filesystem. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com --- fs/btrfs/ioctl.c | 14 ++ fs/btrfs/ioctl.h |2 ++ 2 files changed, 16 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 8fcf9a5..b0a5e17 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3699,6 +3699,18 @@ out: return ret; } +static int btrfs_ioctl_get_fslabel(struct btrfs_root *root, void __user *arg) +{ +const char *label = root-fs_info-super_copy-label; +int ret; + +mutex_lock(root-fs_info-volume_mutex); +ret = copy_to_user(arg, label, strlen(label)); +mutex_unlock(root-fs_info-volume_mutex); + +return ret ? -EFAULT : 0; +} You sent out a wrong patch. Ah! I should not send patches out during my sleepy state, will re-send them a little while, thank you. :) -Jeff Thanks Miao + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3797,6 +3809,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_create(root, argp); case BTRFS_IOC_QGROUP_LIMIT: return btrfs_ioctl_qgroup_limit(root, argp); +case BTRFS_IOC_GET_FSLABEL: +return btrfs_ioctl_get_fslabel(root, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 731e287..98f896f 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -453,4 +453,6 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_limit_args) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) +#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 53, \ + char[BTRFS_LABEL_SIZE]) #endif -- 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
[RESEND PATCH V4 1/2] Btrfs: Add a new ioctl to get the label of a mounted filesystem
Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com --- fs/btrfs/ioctl.c | 14 ++ fs/btrfs/ioctl.h |2 ++ 2 files changed, 16 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 8fcf9a5..b0a5e17 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3699,6 +3699,18 @@ out: return ret; } +static int btrfs_ioctl_get_fslabel(struct btrfs_root *root, void __user *arg) +{ + const char *label = root-fs_info-super_copy-label; + int ret; + + mutex_lock(root-fs_info-volume_mutex); + ret = copy_to_user(arg, label, strlen(label)); + mutex_unlock(root-fs_info-volume_mutex); + + return ret ? -EFAULT : 0; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3797,6 +3809,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_create(root, argp); case BTRFS_IOC_QGROUP_LIMIT: return btrfs_ioctl_qgroup_limit(root, argp); + case BTRFS_IOC_GET_FSLABEL: + return btrfs_ioctl_get_fslabel(root, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 731e287..98f896f 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -453,4 +453,6 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_limit_args) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) +#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 53, \ + char[BTRFS_LABEL_SIZE]) #endif -- 1.7.9.5 -- 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
[RESEND PATCH V4 2/2] Btrfs: Add a new ioctl to change the label of a mounted filesystem
Add a new ioctl BTRFS_FS_SETLABEL to change the label of a mounted filesystem. Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com --- fs/btrfs/ioctl.c | 34 ++ fs/btrfs/ioctl.h |2 ++ 2 files changed, 36 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index b0a5e17..ebbf634 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3711,6 +3711,38 @@ static int btrfs_ioctl_get_fslabel(struct btrfs_root *root, void __user *arg) return ret ? -EFAULT : 0; } +static int btrfs_ioctl_set_fslabel(struct btrfs_root *root, void __user *arg) +{ + struct btrfs_super_block *super_block = root-fs_info-super_copy; + struct btrfs_trans_handle *trans; + char label[BTRFS_LABEL_SIZE]; + int ret = 0; + + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + + if (btrfs_root_readonly(root)) + return -EROFS; + + if (copy_from_user(label, arg, sizeof(label))) + return -EFAULT; + + label[BTRFS_LABEL_SIZE - 1] = '\0'; + + mutex_lock(root-fs_info-volume_mutex); + trans = btrfs_start_transaction(root, 1); + if (IS_ERR(trans)) { + ret = PTR_ERR(trans); + goto out_unlock; + } + strcpy(super_block-label, label); + btrfs_end_transaction(trans, root); + +out_unlock: + mutex_unlock(root-fs_info-volume_mutex); + return ret; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3811,6 +3843,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_limit(root, argp); case BTRFS_IOC_GET_FSLABEL: return btrfs_ioctl_get_fslabel(root, argp); + case BTRFS_IOC_SET_FSLABEL: + return btrfs_ioctl_set_fslabel(root, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 98f896f..cc4e657 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -455,4 +455,6 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_get_dev_stats) #define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 53, \ char[BTRFS_LABEL_SIZE]) +#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 54, \ + char[BTRFS_LABEL_SIZE]) #endif -- 1.7.9.5 -- 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
Re: [RESEND PATCH V4 1/2] Btrfs: Add a new ioctl to get the label of a mounted filesystem
On 12/12/2012 11:50 AM, Miao Xie wrote: On Wed, 12 Dec 2012 11:22:49 +0800, Jeff Liu wrote: Signed-off-by: Jie Liu jeff@oracle.com Signed-off-by: Anand Jain anand.j...@oracle.com --- fs/btrfs/ioctl.c | 14 ++ fs/btrfs/ioctl.h |2 ++ 2 files changed, 16 insertions(+) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 8fcf9a5..b0a5e17 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3699,6 +3699,18 @@ out: return ret; } +static int btrfs_ioctl_get_fslabel(struct btrfs_root *root, void __user *arg) +{ +const char *label = root-fs_info-super_copy-label; +int ret; + +mutex_lock(root-fs_info-volume_mutex); +ret = copy_to_user(arg, label, strlen(label)); +mutex_unlock(root-fs_info-volume_mutex); + +return ret ? -EFAULT : 0; +} + long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -3797,6 +3809,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_qgroup_create(root, argp); case BTRFS_IOC_QGROUP_LIMIT: return btrfs_ioctl_qgroup_limit(root, argp); +case BTRFS_IOC_GET_FSLABEL: +return btrfs_ioctl_get_fslabel(root, argp); } return -ENOTTY; diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index 731e287..98f896f 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -453,4 +453,6 @@ struct btrfs_ioctl_send_args { struct btrfs_ioctl_qgroup_limit_args) #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) +#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 53, \ + char[BTRFS_LABEL_SIZE]) #endif According to the project idea web, https://btrfs.wiki.kernel.org/index.php/Project_ideas#Development_notes.2C_please_read number 53 has been used for the drive swapping commands, so I think we should choose another one. Maybe 49 is good since the above web says number 50 has been reserved for set/change label command. Ok, let's waiting for others comments, I'll follow up the final confirmation. Thanks, -Jeff -- 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 -- 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
Re: How to find (out if) files sharing content?
On 11/06/2012 06:45 AM, David Sterba wrote: On Wed, Oct 31, 2012 at 09:02:15PM +0800, Jeff Liu wrote: I propose this because OCFS2 report shared space in this way combine with du(1). An old patch set to teach du(1) aware of reflinked file: https://oss.oracle.com/pipermail/ocfs2-devel/2010-September/007293.html Patch looks ok, the shared size is requested by an option. Do you means that the costs is very expensive for userland extent status checkup per file? The most expensive part is IMO not in userspace, it does in-memory lookups. And without any possibility to turn this off,I'm afraid this will render FIEMAP unusable in practice. For OCFS2, the FIEMAP_EXTENT_SHARED flag will be set upon fiemap ioctl(2) if an extent is OCFS2_EXT_REFCOUNTED(i.e. reflinked or cloned), which means that FIEMAP_EXTENT_SHARED is not a persistent flag, but I have no idea how Btrfs would be in this point. :( After some research, I think this could work for btrfs without unwanted performance penalties. There's the fiemap::fm_flags field that can be extended to request the shared extent info from fiemap, so the information is not computed unconditionally (that was my concern before). The rest is only implementation details how to speed up the file extent - refcount info lookups. Thanks for your confirmation. -Jeff david -- 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 -- 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
Re: How to find (out if) files sharing content?
On 10/31/2012 07:31 PM, David Sterba wrote: On Wed, Oct 31, 2012 at 10:30:22AM +0800, Jeff Liu wrote: One idea is to mark those cloned extents as FIEMAP_EXTENT_SHARED so that we can go through a file to figure out how many extents are shared through fiemap(2), and calculate the real storage(fs/subvolume) footprint in the end. This will cost at least one more seek per extent to find out that the extent is shared, could be quite expensive. I propose this because OCFS2 report shared space in this way combine with du(1). An old patch set to teach du(1) aware of reflinked file: https://oss.oracle.com/pipermail/ocfs2-devel/2010-September/007293.html Do you means that the costs is very expensive for userland extent status checkup per file? If yes, I have once tested an 50Gb OCFS2 partition filled with reflinked files on an old laptop, it spent around 4 minutes to show the totally results if I recalled correct, but this definitely depending on the real world scenarios. And without any possibility to turn this off,I'm afraid this will render FIEMAP unusable in practice. For OCFS2, the FIEMAP_EXTENT_SHARED flag will be set upon fiemap ioctl(2) if an extent is OCFS2_EXT_REFCOUNTED(i.e. reflinked or cloned), which means that FIEMAP_EXTENT_SHARED is not a persistent flag, but I have no idea how Btrfs would be in this point. :( Thanks, -Jeff david -- 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 -- 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
Re: How to find (out if) files sharing content?
On 10/31/2012 08:40 AM, Liu Bo wrote: On 10/30/2012 11:20 PM, Gábor Nyers wrote: Hi, How could one find out if 2 files share any extents on a btrfs file system? A more generic variation of the above: How to list files on the same file system/subvolume sharing content? One idea is to mark those cloned extents as FIEMAP_EXTENT_SHARED so that we can go through a file to figure out how many extents are shared through fiemap(2), and calculate the real storage(fs/subvolume) footprint in the end. Thanks, -Jeff Indeed ocfs2 already has the feature where you can get shared parts via 'du', we're planning to support this in btrfs, too. thanks, liubo Thanks, Gábor -- 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 -- 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
[PATCH] Btrfs: Remove the invalid shrink size check up from btrfs_shrink_dev()
Remove an invalid size check up from btrfs_shrink_dev(). The new size should not larger than the device-total_bytes as it was already verified before coming to here(i.e. new_size old_size). Remove invalid check up for btrfs_shrink_dev(). Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/ioctl.c |2 +- fs/btrfs/volumes.c |3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 9df50fa..a747da9 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -1383,7 +1383,7 @@ static noinline int btrfs_ioctl_resize(struct btrfs_root *root, btrfs_commit_transaction(trans, root); } else if (new_size old_size) { ret = btrfs_shrink_device(device, new_size); - } + } /* equal, nothing need to do */ out_free: kfree(vol_args); diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 88b969a..8fda4fb 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -3038,9 +3038,6 @@ int btrfs_shrink_device(struct btrfs_device *device, u64 new_size) u64 old_size = device-total_bytes; u64 diff = device-total_bytes - new_size; - if (new_size = device-total_bytes) - return -EINVAL; - path = btrfs_alloc_path(); if (!path) return -ENOMEM; -- 1.7.9.5 -- 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
Re: [RFC PATCH 0/5] btrfs-progs: snapshot diff function
Hi Alex, Thanks for taking a look. On 08/24/2012 09:09 PM, Alex Lyakas wrote: Hi Jeff, how do you see this snapshot-diff functionality vs the send/receive functionality that was recently added? I think that the binary stream that the send code produces, can be interpreted by printing out text messages, which will essentially give the same information (although much more detailed) as your snapshot-diff tool. send/receive has not yet implemented when I working on this feature(back to the end of last year). looks there really has some duplicate efforts. Just as you said, the produced stream of send need to interpret to show readable info, if the binary stream is became huge enough, maybe that will make some silly user crying like me :). Also, it is mainly focus on backup purpose IMHO, please correct me if I missing something in this point. The diff utility is designed to list any changes between two snapshots in a straightforward way consider the command interface, it also can be improved to show the differences at a given time range. But sure, send/receive is just awesome, if we can introduce a interpreted script which do same thing to make end user's life easier, that would be fine. Thanks, -Jeff Apologies if I somehow misunderstood what your snapshot-diff code does. Thanks, Alex. On Tue, Aug 7, 2012 at 11:56 AM, Jeff Liu jeff@oracle.com wrote: Hello, I've done a prototype implementation of snapshot diff utility many months ago. It was originally meant to analyze the differences between two snapshots which are inherited from the same subvolume/snapshot. Moreover, the upstream LXC userland tools has been released with a dedicated template to create new containers combine with btrfs subvolume/snapshot create function, so this path set might be useful if someone is suffering from a broken container guest(it was cloned from a health one in previous but it does not work with some configurations now). In this case, this feature could works as an assistant to help investigating the root cause by listing those changed files from the snapshot that the container resides. This patch set works to three kinds of change for now. - new_file: new created files at the destination snapshot. - removed_file: those files are still resides on source subvolume/snapshot but they have been removed from the destination. - updated_file: files are resides on both subvolumes/snapshots, but they might be changed. Currently, the user could do diff business on any two subvolumes/snapshots, if the destination snapshot is not inherited from the same subvolume/snapshot upon the source one, he must be surprised by the results, so it's better to improve it with pre-check for that if possible. Another issue is, - if we created some new files or updated some existing files under the source snapshot, they will be marked as REMOVED/UPDATED out of the source from the destination snapshot's point of view, so the results might looks a bit strange. A quick demo: root@kdev:/btrfs# btrfs subvolume diff-snapshot one two [REMOVED REGFILE] one/regfile_in_one objectid 264 transid 50 [REMOVED DIR] one/dir_02_at_one objectid 262 transid 36 [REMOVED REGFILE] one/dir_02_at_one/file_at_dir02_one objectid 263 transid 37 [REMOVED DIR] one/dir_at_one objectid 258 transid 29 [REMOVED REGFILE] one/dir_at_one/file_02_at_one_dir objectid 260 transid 32 [REMOVED REGFILE] one/dir_at_one/file_03_at_one_dir objectid 261 transid 35 [REMOVED REGFILE] one/dir_at_one/file_at_one_dir objectid 259 transid 30 [REMOVED REGFILE] one/file_at_one objectid 257 transid 26 [NEW REGFILE] two/regfile_in_two objectid 265 transid 50 [NEW DIR] two/dir_at_two objectid 262 transid 40 [NEW REGFILE] two/dir_at_two/file01_at_dir_of_two objectid 263 transid 41 [NEW SYMLINK] two/dir_at_two/passwd objectid 264 transid 42 [NEW REGFILE] two/file_02 objectid 258 transid 23 [NEW REGFILE] two/file_03 objectid 270 transid 68 [NEW REGFILE] two/file_04 objectid 275 transid 68 Any comments are appreciated! Thanks, -Jeff Makefile |2 +- btrfs-list.c |3 +- cmds-subvolume.c | 90 + diff-snapshot.c | 1026 ++ diff-snapshot.h | 47 +++ 5 files changed, 1165 insertions(+), 3 deletions(-) -- 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 -- 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 -- 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
[RFC PATCH 0/5] btrfs-progs: snapshot diff function
Hello, I've done a prototype implementation of snapshot diff utility many months ago. It was originally meant to analyze the differences between two snapshots which are inherited from the same subvolume/snapshot. Moreover, the upstream LXC userland tools has been released with a dedicated template to create new containers combine with btrfs subvolume/snapshot create function, so this path set might be useful if someone is suffering from a broken container guest(it was cloned from a health one in previous but it does not work with some configurations now). In this case, this feature could works as an assistant to help investigating the root cause by listing those changed files from the snapshot that the container resides. This patch set works to three kinds of change for now. - new_file: new created files at the destination snapshot. - removed_file: those files are still resides on source subvolume/snapshot but they have been removed from the destination. - updated_file: files are resides on both subvolumes/snapshots, but they might be changed. Currently, the user could do diff business on any two subvolumes/snapshots, if the destination snapshot is not inherited from the same subvolume/snapshot upon the source one, he must be surprised by the results, so it's better to improve it with pre-check for that if possible. Another issue is, - if we created some new files or updated some existing files under the source snapshot, they will be marked as REMOVED/UPDATED out of the source from the destination snapshot's point of view, so the results might looks a bit strange. A quick demo: root@kdev:/btrfs# btrfs subvolume diff-snapshot one two [REMOVED REGFILE] one/regfile_in_one objectid 264 transid 50 [REMOVED DIR] one/dir_02_at_one objectid 262 transid 36 [REMOVED REGFILE] one/dir_02_at_one/file_at_dir02_one objectid 263 transid 37 [REMOVED DIR] one/dir_at_one objectid 258 transid 29 [REMOVED REGFILE] one/dir_at_one/file_02_at_one_dir objectid 260 transid 32 [REMOVED REGFILE] one/dir_at_one/file_03_at_one_dir objectid 261 transid 35 [REMOVED REGFILE] one/dir_at_one/file_at_one_dir objectid 259 transid 30 [REMOVED REGFILE] one/file_at_one objectid 257 transid 26 [NEW REGFILE] two/regfile_in_two objectid 265 transid 50 [NEW DIR] two/dir_at_two objectid 262 transid 40 [NEW REGFILE] two/dir_at_two/file01_at_dir_of_two objectid 263 transid 41 [NEW SYMLINK] two/dir_at_two/passwd objectid 264 transid 42 [NEW REGFILE] two/file_02 objectid 258 transid 23 [NEW REGFILE] two/file_03 objectid 270 transid 68 [NEW REGFILE] two/file_04 objectid 275 transid 68 Any comments are appreciated! Thanks, -Jeff Makefile |2 +- btrfs-list.c |3 +- cmds-subvolume.c | 90 + diff-snapshot.c | 1026 ++ diff-snapshot.h | 47 +++ 5 files changed, 1165 insertions(+), 3 deletions(-) -- 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
[RFC PATCH 1/5] btrfs-progs: make ino_resovle() shared
Make ino_resolve() shared so that we can call it at snapshot diff module. Signed-off-by: Jie Liu jeff@oracle.com --- btrfs-list.c |3 +-- btrfs-list.h | 22 ++ 2 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 btrfs-list.h diff --git a/btrfs-list.c b/btrfs-list.c index c53d016..75681e1 100644 --- a/btrfs-list.c +++ b/btrfs-list.c @@ -474,8 +474,7 @@ char *build_name(char *dirid, char *name) * cache the results so we can avoid tree searches if a later call goes * to the same directory or file name */ -static char *ino_resolve(int fd, u64 ino, u64 *cache_dirid, char **cache_name) - +char *ino_resolve(int fd, u64 ino, u64 *cache_dirid, char **cache_name) { u64 dirid; char *dirname; diff --git a/btrfs-list.h b/btrfs-list.h new file mode 100644 index 000..70b2078 --- /dev/null +++ b/btrfs-list.h @@ -0,0 +1,22 @@ +/* + * Copyright (C) 2007 Oracle. All rights reserved. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public + * License v2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public + * License along with this program; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 021110-1307, USA. + */ + +#ifndef __BTRFS_LIST_ +#define __BTRFS_LIST_ +char *ino_resolve(int fd, u64 ino, u64 *cache_dirid, char **cache_name); +#endif -- 1.7.4.1 -- 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
[RFC PATCH 2/5] btrfs-progs: header file of snapshot diff
Maybe it's better to put those #defines to the source file of snapshot diff as no other modules need them. Signed-off-by: Jie Liu jeff@oracle.com --- diff-snapshot.h | 47 +++ 1 files changed, 47 insertions(+), 0 deletions(-) create mode 100644 diff-snapshot.h diff --git a/diff-snapshot.h b/diff-snapshot.h new file mode 100644 index 000..0ba09da --- /dev/null +++ b/diff-snapshot.h @@ -0,0 +1,47 @@ +/* + * Copyright (C) 2012 Oracle. All rights reserved. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public + * License v2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public + * License along with this program; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 021110-1307, USA. + */ + +#ifndef __SNAPSHOT_DIFF_ +#define __SNAPSHOT_DIFF_ + +#define SNAPSHOT_DIFF_LIST_NEW_ITEM(1 0) +#define SNAPSHOT_DIFF_LIST_REMOVED_ITEM(1 1) +#define SNAPSHOT_DIFF_LIST_UPDATED_ITEM(1 2) +#define SNAPSHOT_DIFF_LIST_ALL (1 3) + +#define SNAPSHOT_DIFF_SHOW_NEW_ITEM(flags) \ + ((flags SNAPSHOT_DIFF_LIST_NEW_ITEM) || \ +(flags SNAPSHOT_DIFF_LIST_ALL)) + +#define SNAPSHOT_DIFF_SHOW_REMOVED_ITEM(flags) \ + ((flags SNAPSHOT_DIFF_LIST_REMOVED_ITEM) || \ +(flags SNAPSHOT_DIFF_LIST_ALL)) + +#define SNAPSHOT_DIFF_SHOW_UPDATED_ITEM(flags) \ + ((flags SNAPSHOT_DIFF_LIST_UPDATED_ITEM) || \ +(flags SNAPSHOT_DIFF_LIST_ALL)) + +enum { + SNAPSHOT_DIFF_NEW_ITEM = 0, + SNAPSHOT_DIFF_REMOVED_ITEM, + SNAPSHOT_DIFF_UPDATED_ITEM, +}; + +int snapshot_diff(int src_fd, int dst_fd, const char *src_snapshot, + const char *dest_snapshot, unsigned int diff_flags); +#endif -- 1.7.4.1 -- 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
[RFC PATCH 3/5] btrfs-progs: souce file of snapshot diff
Now the source file is coming. Signed-off-by: Jie Liu jeff@oracle.com --- diff-snapshot.c | 1026 +++ 1 files changed, 1026 insertions(+), 0 deletions(-) create mode 100644 diff-snapshot.c diff --git a/diff-snapshot.c b/diff-snapshot.c new file mode 100644 index 000..7b7f4c7 --- /dev/null +++ b/diff-snapshot.c @@ -0,0 +1,1026 @@ +/* + * Copyright (C) 2012 Oracle. All rights reserved. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public + * License v2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public + * License along with this program; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 021110-1307, USA. + */ + +#include stdio.h +#include stdlib.h +#include stdbool.h +#include sys/types.h +#include dirent.h +#include sys/stat.h +#include sys/ioctl.h +#include uuid/uuid.h +#include fcntl.h +#include unistd.h +#include assert.h +#include ctree.h +#include ioctl.h +#include utils.h +#include btrfs-list.h +#include diff-snapshot.h + +/* + * scan and cache the number of items from both snapshots at a time. + * FIXME: maybe it's better to let user to specify this value according + * to their memory status, cache more items every time can improve the + * overall performance as it could reduce the efforts to retrieve items + * through ioctl(2). or we can implement a routine to calculate this value + * based on the available memory, maybe sounds more reasonable. + */ +static unsigned int nr_item_scan = 4096; + +struct snapshot_diff_info { + /* source snapshot scan info */ + struct snapshot_scan_info *src_scan_info; + + /* destination snapshot scan info */ + struct snapshot_scan_info *dest_scan_info; +}; + +struct snapshot_scan_info { + /* name */ + char *snapshot; + + /* snapshot dir id */ + int fd; + + /* snapshot tree id */ + u64 tree_id; + + /* the latest object id in last round of scan */ + u64 last_objectid; + + /* cache the scanned items on */ + struct rb_root scanned_items; + + /* +* rb-tree to cache those items which have already been +* processed by lookup snapshot, so it will not be cached +* again. the memory allocated for it would be reclaimed +* back once we determined that the business for it was +* done. +*/ + struct rb_root processed_items; + + /* no more items can be fetched from a snapshot */ + int scan_done; +}; + +/* item info at cache */ +struct snapshot_scan_item { + struct rb_node si_node; + u64 transid; + u64 objectid; + char *path; + u8 type; +}; + +/* + * processed items are those who are not returned in the current + * round of scan, but they are resides on snapshot. + */ +struct processed_item { + struct rb_node pi_node; + char *path; +}; + +static const char *decode_item_type(u8 type) +{ + char *typestr = UNKNOWN; + + switch (type) { + case BTRFS_FT_DIR: + typestr = DIR; + break; + case BTRFS_FT_REG_FILE: + typestr = REGFILE; + break; + case BTRFS_FT_CHRDEV: + typestr = CHRDEV; + break; + case BTRFS_FT_BLKDEV: + typestr = BLKDEV; + break; + case BTRFS_FT_FIFO: + typestr = FIFO; + break; + case BTRFS_FT_SOCK: + typestr = SOCKET; + break; + case BTRFS_FT_SYMLINK: + typestr = SYMLINK; + break; + case BTRFS_FT_XATTR: + typestr = XATTR; + break; + default: + break; + } + + return typestr; +} + +/* + * we found an item on both snapshots, check if it was updated or not + * by comparing ctime mtime. + */ +static inline int item_is_updated(const char *src_snapshot, + const char *dest_snapshot, + const char *path, u8 item_type, + int *updated) +{ + char src_full_path[BTRFS_PATH_NAME_MAX + 1]; + char dest_full_path[BTRFS_PATH_NAME_MAX + 1]; + struct stat st1; + struct stat st2; + int ret = 0; + + ret = snprintf(src_full_path, sizeof(src_full_path), %s/%s, + src_snapshot, path); + if (ret 0) { + fprintf(stderr, failed to build full path [%s/%s]\n, + src_snapshot, path); + goto out; + }
[RFC PATCH 4/5] btrfs-progs: teach Makefile aware of the new comer
Signed-off-by: Jie Liu jeff@oracle.com --- Makefile |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/Makefile b/Makefile index 969..f8b517d 100644 --- a/Makefile +++ b/Makefile @@ -4,7 +4,7 @@ CFLAGS = -g -O0 objects = ctree.o disk-io.o radix-tree.o extent-tree.o print-tree.o \ root-tree.o dir-item.o file-item.o inode-item.o \ inode-map.o crc32c.o rbtree.o extent-cache.o extent_io.o \ - volumes.o utils.o btrfs-list.o btrfslabel.o repair.o + volumes.o utils.o btrfs-list.o btrfslabel.o repair.o diff-snapshot.o cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \ cmds-inspect.o cmds-balance.o -- 1.7.4.1 -- 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
[RFC PATCH 5/5] btrfs-progs: let this feature works at subvolume command group.
make this feature works as `btrfs subvolume diff-snapshot [options] src dest`. Signed-off-by: Jie Liu jeff@oracle.com --- cmds-subvolume.c | 90 ++ 1 files changed, 90 insertions(+), 0 deletions(-) diff --git a/cmds-subvolume.c b/cmds-subvolume.c index 3508ce6..e7cc3cc 100644 --- a/cmds-subvolume.c +++ b/cmds-subvolume.c @@ -28,6 +28,7 @@ #include ioctl.h #include commands.h +#include diff-snapshot.h /* btrfs-list.c */ int list_subvols(int fd, int print_parent, int get_default); @@ -515,6 +516,94 @@ static int cmd_find_new(int argc, char **argv) return 0; } +static const char * const cmd_snapshot_diff_usage[] = { + btrfs subvolume diff-snapshot [options] source dest, + List the differences between two snapshots, + By default, list all changes in destination snapshot towards source, + , + -n list the new items in destination snapshot\n, + -r list the removed items in destination snapshot\n, + -u list the updated items in destination snapshot\n, + NULL +}; + +static int cmd_snapshot_diff(int argc, char **argv) +{ + int ret; + int src_fd; + int dst_fd; + char *src_snapshot; + char *dest_snapshot; + unsigned int flags = 0; + + while (1) { + int c = getopt(argc, argv, rnu); + if (c 0) + break; + switch (c) { + case 'r': + flags |= SNAPSHOT_DIFF_LIST_REMOVED_ITEM; + break; + case 'n': + flags |= SNAPSHOT_DIFF_LIST_NEW_ITEM; + break; + case 'u': + flags |= SNAPSHOT_DIFF_LIST_UPDATED_ITEM; + break; + default: + fprintf(stderr, ERROR: snapshot diff args invalid.\n +-r list removed items\n +-n list new items\n +-u list updated items\n); + return 1; + } + } + + src_snapshot = argv[argc - 2]; + dest_snapshot = argv[argc - 1]; + + ret = test_issubvolume(src_snapshot); + if (ret 0) { + fprintf(stderr, ERROR: error accessing '%s'\n, + src_snapshot); + return 12; + } + if (!ret) { + fprintf(stderr, ERROR: '%s' is not a subvolume\n, + src_snapshot); + return 13; + } + + ret = test_issubvolume(dest_snapshot); + if (ret 0) { + fprintf(stderr, ERROR: error accessing '%s'\n, + dest_snapshot); + return 12; + } + if (!ret) { + fprintf(stderr, ERROR: '%s' is not a subvolume\n, + dest_snapshot); + return 13; + } + + src_fd = open_file_or_dir(src_snapshot); + if (src_fd 0) { + fprintf(stderr, ERROR: can't access '%s'\n, src_snapshot); + return 12; + } + + dst_fd = open_file_or_dir(dest_snapshot); + if (dst_fd 0) { + fprintf(stderr, ERROR: can't access '%s'\n, dest_snapshot); + return 12; + } + + ret = snapshot_diff(src_fd, dst_fd, src_snapshot, dest_snapshot, flags); + if (ret) + return 19; + return 0; +} + const struct cmd_group subvolume_cmd_group = { subvolume_cmd_group_usage, NULL, { { create, cmd_subvol_create, cmd_subvol_create_usage, NULL, 0 }, @@ -526,6 +615,7 @@ const struct cmd_group subvolume_cmd_group = { { set-default, cmd_subvol_set_default, cmd_subvol_set_default_usage, NULL, 0 }, { find-new, cmd_find_new, cmd_find_new_usage, NULL, 0 }, + { diff-snapshot, cmd_snapshot_diff, cmd_snapshot_diff_usage, NULL, 0 }, { 0, 0, 0, 0, 0 } } }; -- 1.7.4.1 -- 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
Re: btrfs_print_tree?
On 07/01/2012 05:49 PM, Zhi Yong Wu wrote: On Sun, Jul 1, 2012 at 5:41 PM, Mike Fleetwood mike.fleetw...@googlemail.com wrote: On 1 July 2012 05:53, Zhi Yong Wu zwu.ker...@gmail.com wrote: HI, Do anyone know where btrfs_print_tree is invoked? thanks. -- Regards, Zhi Yong Wu Is this the answer you are after? $ grep -r btrfs_print_tree fs/btrfs/ fs/btrfs/print-tree.c:void btrfs_print_tree(struct btrfs_root *root, struct extent_buffer *c) fs/btrfs/print-tree.c: btrfs_print_tree(root, next); fs/btrfs/print-tree.h:void btrfs_print_tree(struct btrfs_root *root, struct extent_buffer *t); No, i also did as this, but didn't find out who will invoke this function. From above output, we only saw that it invokes itself one time. Looks this is a helper routine exported to btrfs-progs previously, it is used by debug-tree, quick-test, etc... But this function has been implemented at btrfs-progs now, maybe it could be safely removed from kernel, not sure. :) Thanks, -Jeff Mike -- 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
[PATCH] Btrfs: return EUCLEAN rather than ENXIO once internal error has occurred for SEEK_DATA/SEEK_HOLE inquiry
By referring to http://linux.die.net/man/2/lseek, return ENXIO only when offset beyond EOF for either SEEK_DATA or SEEK_HOLE inquiry. But we return it in case of internal issue too if btrfs_get_extent_fiemap() failed due to other issues. This will confuse the user applications to be expecting ENXIO when trying to find a specific data or hole location once it has occurred. Thanks Dave for pointing that out in XFS thread. This patch fix it to return EUCLEAN, or maybe another particular errno is more reasonable in Btrfs to indicate this fatal error? Thanks, -Jeff Cc: da...@fromorbit.com Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/file.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index 97fbe93..6693040 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -1761,7 +1761,7 @@ static int find_desired_extent(struct inode *inode, loff_t *offset, int origin) start - root-sectorsize, root-sectorsize, 0); if (IS_ERR(em)) { - ret = -ENXIO; + ret = -EUCLEAN; goto out; } last_end = em-start + em-len; @@ -1773,7 +1773,7 @@ static int find_desired_extent(struct inode *inode, loff_t *offset, int origin) while (1) { em = btrfs_get_extent_fiemap(inode, NULL, 0, start, len, 0); if (IS_ERR(em)) { - ret = -ENXIO; + ret = -EUCLEAN; break; } -- 1.7.9 -- 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
Re: [PATCH] Btrfs: return EUCLEAN rather than ENXIO once internal error has occurred for SEEK_DATA/SEEK_HOLE inquiry
On 02/09/2012 11:46 AM, Jeff Liu wrote: By referring to http://linux.die.net/man/2/lseek, return ENXIO only when offset beyond EOF for either SEEK_DATA or SEEK_HOLE inquiry. But we return it in case of internal issue too if btrfs_get_extent_fiemap() failed due to other issues. This will confuse the user applications to be expecting ENXIO when trying to find a specific data or hole location once it has occurred. Thanks Dave for pointing that out in XFS thread. This patch fix it to return EUCLEAN, or maybe another particular errno is more reasonable in Btrfs to indicate this fatal error? Or maybe just return the error that was happened at internal routine, to give user more accurate error info, which is better? Thanks, -Jeff Thanks, -Jeff Cc: da...@fromorbit.com Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/file.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index 97fbe93..6693040 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -1761,7 +1761,7 @@ static int find_desired_extent(struct inode *inode, loff_t *offset, int origin) start - root-sectorsize, root-sectorsize, 0); if (IS_ERR(em)) { - ret = -ENXIO; + ret = -EUCLEAN; goto out; } last_end = em-start + em-len; @@ -1773,7 +1773,7 @@ static int find_desired_extent(struct inode *inode, loff_t *offset, int origin) while (1) { em = btrfs_get_extent_fiemap(inode, NULL, 0, start, len, 0); if (IS_ERR(em)) { - ret = -ENXIO; + ret = -EUCLEAN; break; } -- 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
Re: [PATCH] Btrfs: return EUCLEAN rather than ENXIO once internal error has occurred for SEEK_DATA/SEEK_HOLE inquiry
On 02/09/2012 12:51 PM, Dave Chinner wrote: On Thu, Feb 09, 2012 at 12:08:47PM +0800, Jeff Liu wrote: On 02/09/2012 11:46 AM, Jeff Liu wrote: By referring to http://linux.die.net/man/2/lseek, return ENXIO only when offset beyond EOF for either SEEK_DATA or SEEK_HOLE inquiry. But we return it in case of internal issue too if btrfs_get_extent_fiemap() failed due to other issues. This will confuse the user applications to be expecting ENXIO when trying to find a specific data or hole location once it has occurred. Thanks Dave for pointing that out in XFS thread. This patch fix it to return EUCLEAN, or maybe another particular errno is more reasonable in Btrfs to indicate this fatal error? Or maybe just return the error that was happened at internal routine, to give user more accurate error info, which is better? Return the internal error unchanged - a failure to read the extent list (EIO) is different to a corruption detected in the extent map read from disk (EUCLEAN). Having a user report the appropriate error makes our life much simpler when it comes to trying to understand their problem Definitely. I will repost this patch later. Thanks, -Jeff Cheers, Dave. -- 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
[PATCH v2] Btrfs: return the internal error unchanged if btrfs_get_extent_fiemap() call failed for SEEK_DATA/SEEK_HOLE inquiry
Given that ENXIO only means offset beyond EOF for either SEEK_DATA or SEEK_HOLE inquiry in a desired file range, so we should return the internal error unchanged if btrfs_get_extent_fiemap() call failed, rather than ENXIO. Cc: Dave Chinner da...@fromorbit.com Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/file.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index 97fbe93..6d9e796 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -1761,7 +1761,7 @@ static int find_desired_extent(struct inode *inode, loff_t *offset, int origin) start - root-sectorsize, root-sectorsize, 0); if (IS_ERR(em)) { - ret = -ENXIO; + ret = PTR_ERR(em); goto out; } last_end = em-start + em-len; @@ -1773,7 +1773,7 @@ static int find_desired_extent(struct inode *inode, loff_t *offset, int origin) while (1) { em = btrfs_get_extent_fiemap(inode, NULL, 0, start, len, 0); if (IS_ERR(em)) { - ret = -ENXIO; + ret = PTR_ERR(em); break; } -- 1.7.9 -- 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
Re: [PATCH 2/2] Btrfs: set the i_nlink to 2 for an initial dir inode
On 11/29/2011 11:48 PM, Chris Mason wrote: On Tue, Nov 29, 2011 at 02:04:37PM +0800, Jeff Liu wrote: Please ignore this patch for now, it can cause the file system corrupted and failed to mount again, sorry for the noise! Directories always have a link count of 1 in btrfs. This tells find not to use the link count as the count of subdirectories in the directory. Thank you for your clarification! -Jeff -chris -- 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 -- 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
Re: [PATCH 2/2] Btrfs: set the i_nlink to 2 for an initial dir inode
Please ignore this patch for now, it can cause the file system corrupted and failed to mount again, sorry for the noise! -Jeff On 11/28/2011 03:47 PM, Jeff Liu wrote: For an initial dir inode, stat(1) show it links as 1, IMHO it should be 2 by default. Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/inode.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 116ab67..92b3cb9 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -4734,6 +4734,7 @@ static int btrfs_mkdir(struct inode *dir, struct dentry *dentry, int mode) inode-i_op = btrfs_dir_inode_operations; inode-i_fop = btrfs_dir_file_operations; + set_nlink(inode, 2); btrfs_i_size_write(inode, 0); err = btrfs_update_inode(trans, root, inode); if (err) -- 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
[PATCH 1/2] Btrfs: adjust the variables indentation to TAB
Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/ioctl.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 4a34c47..795c415 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -1689,9 +1689,9 @@ err: static noinline int btrfs_ioctl_tree_search(struct file *file, void __user *argp) { -struct btrfs_ioctl_search_args *args; -struct inode *inode; -int ret; + struct btrfs_ioctl_search_args *args; + struct inode *inode; + int ret; if (!capable(CAP_SYS_ADMIN)) return -EPERM; -- 1.7.4.1 -- 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
[PATCH 2/2] Btrfs: set the i_nlink to 2 for an initial dir inode
For an initial dir inode, stat(1) show it links as 1, IMHO it should be 2 by default. Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/inode.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 116ab67..92b3cb9 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -4734,6 +4734,7 @@ static int btrfs_mkdir(struct inode *dir, struct dentry *dentry, int mode) inode-i_op = btrfs_dir_inode_operations; inode-i_fop = btrfs_dir_file_operations; + set_nlink(inode, 2); btrfs_i_size_write(inode, 0); err = btrfs_update_inode(trans, root, inode); if (err) -- 1.7.4.1 -- 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
Re: [PATCH 1/7] BTRFS: Fix lseek return value for error
Hi Andreas and Andi, Thanks for your comments. On 09/18/2011 09:46 AM, Andi Kleen wrote: with an additional improvement if the offset is larger or equal to the file size, return -ENXIO in directly: if (offset = inode-i_size) { mutex_unlock(inode-i_mutex); return -ENXIO; } Except that is wrong, because it would then be impossible to write sparse files. Per my tryout, except that, if the offset = source file size, call lseek(fd, offset, SEEK_DATA/SEEK_HOLE) against Btrfs will always return the total file size rather than -ENXIO. however, our desired result it -ENXIO in this case, Am I right? And also i_size must be always read with i_size_read() Thanks for pointing this out! Would you please kindly review the revised as below? Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/file.c |9 +++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index e7872e4..40c1ef3 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -1813,6 +1813,11 @@ static loff_t btrfs_file_llseek(struct file *file, loff_t offset, int origin) goto out; case SEEK_DATA: case SEEK_HOLE: + if (offset = i_size_read(inode)) { + mutex_unlock(inode-i_mutex); + return -ENXIO; + } + ret = find_desired_extent(inode, offset, origin); if (ret) { mutex_unlock(inode-i_mutex); @@ -1821,11 +1826,11 @@ static loff_t btrfs_file_llseek(struct file *file, loff_t offset, int origin) } if (offset 0 !(file-f_mode FMODE_UNSIGNED_OFFSET)) { - ret = -EINVAL; + offset = -EINVAL; goto out; } if (offset inode-i_sb-s_maxbytes) { - ret = -EINVAL; + offset = -EINVAL; goto out; } -- 1.7.4.1 -- 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
Re: [PATCH 1/7] BTRFS: Fix lseek return value for error
在 2011-9-18,下午4:42, Marco Stornelli 写道: Il 18/09/2011 09:29, Jeff Liu ha scritto: Hi Andreas and Andi, Thanks for your comments. On 09/18/2011 09:46 AM, Andi Kleen wrote: with an additional improvement if the offset is larger or equal to the file size, return -ENXIO in directly: if (offset= inode-i_size) { mutex_unlock(inode-i_mutex); return -ENXIO; } Except that is wrong, because it would then be impossible to write sparse files. Per my tryout, except that, if the offset= source file size, call lseek(fd, offset, SEEK_DATA/SEEK_HOLE) against Btrfs will always return the total file size rather than -ENXIO. however, our desired result it -ENXIO in this case, Am I right? Yes, ENXIO should be the operation result. Thanks for your kind confirmation. -Jeff -- 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
Re: [PATCH 1/7] BTRFS: Fix lseek return value for error
I once posted a similar patch for this issue which can be found at: http://www.spinics.net/lists/linux-btrfs/msg12169.html with an additional improvement if the offset is larger or equal to the file size, return -ENXIO in directly: if (offset = inode-i_size) { mutex_unlock(inode-i_mutex); return -ENXIO; } Thanks, -Jeff On 09/16/2011 11:48 PM, Christoph Hellwig wrote: On Thu, Sep 15, 2011 at 04:06:47PM -0700, Andi Kleen wrote: From: Andi Kleen a...@linux.intel.com Introduced by 9a4327ca1f45f82edad7dc0a4e52ce9316e0950c I think this should go to Chris/Linus ASAP. But a slightly better patch description wouldn't hurt either. Also any reason you captialize BTRFS? Signed-off-by: Andi Kleen a...@linux.intel.com --- fs/btrfs/file.c | 13 +++-- 1 files changed, 7 insertions(+), 6 deletions(-) diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index 3c3abff..7ec0a24 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -1818,19 +1818,17 @@ static loff_t btrfs_file_llseek(struct file *file, loff_t offset, int origin) case SEEK_DATA: case SEEK_HOLE: ret = find_desired_extent(inode, offset, origin); -if (ret) { -mutex_unlock(inode-i_mutex); -return ret; -} +if (ret) +goto error; } if (offset 0 !(file-f_mode FMODE_UNSIGNED_OFFSET)) { ret = -EINVAL; -goto out; +goto error; } if (offset inode-i_sb-s_maxbytes) { ret = -EINVAL; -goto out; +goto error; } /* Special lock needed here? */ @@ -1841,6 +1839,9 @@ static loff_t btrfs_file_llseek(struct file *file, loff_t offset, int origin) out: mutex_unlock(inode-i_mutex); return offset; +error: +mutex_unlock(inode-i_mutex); +return ret; } const struct file_operations btrfs_file_operations = { -- 1.7.4.4 -- To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ---end quoted text--- -- 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 -- 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
[PATCH V2] btrfs: trivial fix, a potential memory leak in btrfs_parse_early_options()
Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/super.c | 10 -- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 15634d4..16f31e1 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -406,7 +406,7 @@ static int btrfs_parse_early_options(const char *options, fmode_t flags, u64 *subvol_rootid, struct btrfs_fs_devices **fs_devices) { substring_t args[MAX_OPT_ARGS]; - char *opts, *orig, *p; + char *device_name, *opts, *orig, *p; int error = 0; int intarg; @@ -457,8 +457,14 @@ static int btrfs_parse_early_options(const char *options, fmode_t flags, } break; case Opt_device: - error = btrfs_scan_one_device(match_strdup(args[0]), + device_name = match_strdup(args[0]); + if (!device_name) { + error = -ENOMEM; + goto out_free_opts; + } + error = btrfs_scan_one_device(device_name, flags, holder, fs_devices); + kfree(device_name); if (error) goto out_free_opts; break; -- 1.7.4.1 -- 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
Re: [PATCH 2/2] Btrfs: don't change inode flag of the dest clone file
Add CC to Coreutils, cp --reflink performs btrfs clone operation. Thanks, -Jeff On 09/15/2011 07:43 PM, David Sterba wrote: On Wed, Sep 14, 2011 at 01:25:36PM +0800, Li Zefan wrote: The dst file will have the same inode flags with dst file after file clone, and I think it's unexpected. For example, the dst file will suddenly become immutable after getting some share of data with src file, if the src is immutable. Good catch! (I did not find any further direct assignment of two inode flags.) Signed-off-by: Li Zefan l...@cn.fujitsu.com Reviewed-by: David Sterba dste...@suse.cz IMNSHO should go to stable. david --- fs/btrfs/ioctl.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index dc82bbb..a401514 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -2434,7 +2434,6 @@ static noinline long btrfs_ioctl_clone(struct file *file, unsigned long srcfd, if (endoff inode-i_size) btrfs_i_size_write(inode, endoff); -BTRFS_I(inode)-flags = BTRFS_I(src)-flags; ret = btrfs_update_inode(trans, root, inode); BUG_ON(ret); btrfs_end_transaction(trans, root); -- 1.7.3.1 -- 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 -- 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 -- 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
Re: [PATCH] btrfs: trivial fix, a potential memory leak in btrfs_parse_early_options()
On 09/14/2011 01:40 PM, Li Zefan wrote: 14:06, Jeff Liu wrote: Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/super.c | 10 -- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 15634d4..16f31e1 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -406,7 +406,7 @@ static int btrfs_parse_early_options(const char *options, fmode_t flags, u64 *subvol_rootid, struct btrfs_fs_devices **fs_devices) { substring_t args[MAX_OPT_ARGS]; -char *opts, *orig, *p; +char *device_name, *opts, *orig, *p; Seems your email client replaced tabs with spaces. Fixed, thank you. Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/super.c | 10 -- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 15634d4..16f31e1 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -406,7 +406,7 @@ static int btrfs_parse_early_options(const char *options, fmode_t flags, u64 *subvol_rootid, struct btrfs_fs_devices **fs_devices) { substring_t args[MAX_OPT_ARGS]; - char *opts, *orig, *p; + char *device_name, *opts, *orig, *p; int error = 0; int intarg; @@ -457,8 +457,14 @@ static int btrfs_parse_early_options(const char *options, fmode_t flags, } break; case Opt_device: - error = btrfs_scan_one_device(match_strdup(args[0]), + device_name = match_strdup(args[0]); + if (!device_name) { + error = -ENOMEM; + goto out_free_opts; + } + error = btrfs_scan_one_device(device_name, flags, holder, fs_devices); + kfree(device_name); if (error) goto out_free_opts; break; -- 1.7.4.1 Please read Documentation/email-clients.txt int error = 0; int intarg; @@ -457,8 +457,14 @@ static int btrfs_parse_early_options(const char *options, fmode_t flags, } break; case Opt_device: -error = btrfs_scan_one_device(match_strdup(args[0]), +device_name = match_strdup(args[0]); +if (!device_name) { +error = -ENOMEM; +goto out_free_opts; +} +error = btrfs_scan_one_device(device_name, flags, holder, fs_devices); +kfree(device_name); if (error) goto out_free_opts; break; -- 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 -- 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
[PATCH] btrfs: trivial fix, a potential memory leak in btrfs_parse_early_options()
Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/super.c | 10 -- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 15634d4..16f31e1 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -406,7 +406,7 @@ static int btrfs_parse_early_options(const char *options, fmode_t flags, u64 *subvol_rootid, struct btrfs_fs_devices **fs_devices) { substring_t args[MAX_OPT_ARGS]; -char *opts, *orig, *p; +char *device_name, *opts, *orig, *p; int error = 0; int intarg; @@ -457,8 +457,14 @@ static int btrfs_parse_early_options(const char *options, fmode_t flags, } break; case Opt_device: -error = btrfs_scan_one_device(match_strdup(args[0]), +device_name = match_strdup(args[0]); +if (!device_name) { +error = -ENOMEM; +goto out_free_opts; +} +error = btrfs_scan_one_device(device_name, flags, holder, fs_devices); +kfree(device_name); if (error) goto out_free_opts; break; -- 1.7.4.1 -- 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
Re: mount a multi-device filesystem using a loopback device
On 09/08/2011 01:32 PM, C Anthony Risinger wrote: On Wed, Sep 7, 2011 at 1:44 AM, Jeff Liujeff@oracle.com wrote: On 09/07/2011 12:37 PM, cwillu wrote: 1. Create and format two images, the 1st in 400Mbytes, and 2nd in 286Mbytes. root@pibroch:/btrfs-progs# ls -lh /usr/src/linux-3.0/img* -rw-r--r-- 1 jeff jeff 400M 2011-09-07 12:00 /usr/src/linux-3.0/img0 -rw-r--r-- 1 jeff jeff 286M 2011-09-07 12:00 /usr/src/linux-3.0/img1 Very small btrfs filesystem require special handling, mixed block groups in particular, which I believe also requires an updated mkfs from the integration repo. Thanks for your response, however, even if I enlarged the image size to 2G, still got no luck. Per Zefan's comments, run 'btrfs device scan' fixed this issue. you should be able to achieve this via `device=` mount option(s?) as well: https://btrfs.wiki.kernel.org/index.php/Getting_started#Mount_Options ... for completeness :-) ... and posterity or whatever. Nice, thanks for your sharing. 8-) -Jeff C Anthony -- 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 -- 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
Re: mount a multi-device filesystem using a loopback device
On 09/07/2011 02:31 PM, Li Zefan wrote: Is it a bug? or Am I missing something? You need to run this before mounting the devices: # btrfs device scan Thank you, it works by executing device scan. -Jeff -- 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
Re: mount a multi-device filesystem using a loopback device
On 09/07/2011 12:37 PM, cwillu wrote: 1. Create and format two images, the 1st in 400Mbytes, and 2nd in 286Mbytes. root@pibroch:/btrfs-progs# ls -lh /usr/src/linux-3.0/img* -rw-r--r-- 1 jeff jeff 400M 2011-09-07 12:00 /usr/src/linux-3.0/img0 -rw-r--r-- 1 jeff jeff 286M 2011-09-07 12:00 /usr/src/linux-3.0/img1 Very small btrfs filesystem require special handling, mixed block groups in particular, which I believe also requires an updated mkfs from the integration repo. Thanks for your response, however, even if I enlarged the image size to 2G, still got no luck. Per Zefan's comments, run 'btrfs device scan' fixed this issue. -Jeff -- 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 -- 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
mount a multi-device filesystem using a loopback device
Hello, I was trying to create a multi-device Btrfs filesystem using two loopback devices, by referring to the following page: https://btrfs.wiki.kernel.org/index.php/Using_Btrfs_with_Multiple_Devices However, I met a strange thing while mounting the first loop device without any extra mount options, 1. Create and format two images, the 1st in 400Mbytes, and 2nd in 286Mbytes. root@pibroch:/btrfs-progs# ls -lh /usr/src/linux-3.0/img* -rw-r--r-- 1 jeff jeff 400M 2011-09-07 12:00 /usr/src/linux-3.0/img0 -rw-r--r-- 1 jeff jeff 286M 2011-09-07 12:00 /usr/src/linux-3.0/img1 root@pibroch:/btrfs-progs# mkfs.btrfs /usr/src/linux-3.0/img0 /usr/src/linux-3.0/img1 WARNING! - Btrfs v0.19-35-g1b444cd IS EXPERIMENTAL WARNING! - see http://btrfs.wiki.kernel.org before using failed to read /dev/sr0 failed to read /dev/sr0 adding device /usr/src/linux-3.0/img1 id 2 fs created label (null) on /usr/src/linux-3.0/img0 nodesize 4096 leafsize 4096 sectorsize 4096 size 685.50MB Btrfs v0.19-35-g1b444cd 2. Setup the loopback manually: root@pibroch:/btrfs-progs# losetup /dev/loop0 /usr/src/linux-3.0/img0 root@pibroch:/btrfs-progs# losetup /dev/loop1 /usr/src/linux-3.0/img1 root@pibroch:/btrfs-progs# losetup -a /dev/loop0: [0802]:1736614 (/usr/src/linux-3.0/img0) /dev/loop1: [0802]:1736726 (/usr/src/linux-3.0/img1) 3. Try to mount loop0, it will failed as below: root@pibroch:/btrfs-progs# mount /dev/loop0 /mnt mount: wrong fs type, bad option, bad superblock on /dev/loop0, missing codepage or helper program, or other error In some cases useful info is found in syslog - try dmesg | tail or so 4. Try to mount loop1, it works: root@pibroch:/btrfs-progs# mount /dev/loop1 /mnt root@pibroch:/btrfs-progs# df -h FilesystemSize Used Avail Use% Mounted on /dev/sda2 46G 23G 21G 52% / none 896M 700K 895M 1% /dev none 957M 80K 957M 1% /dev/shm none 957M 104K 957M 1% /var/run none 957M 0 957M 0% /var/lock /dev/loop1686M 56K 493M 1% /mnt 5. umount loop1, and try to mount loop0 again, it works at this time: root@pibroch:/btrfs-progs# umount /mnt root@pibroch:/btrfs-progs# mount /dev/loop0 /mnt root@pibroch:/btrfs-progs# df -h FilesystemSize Used Avail Use% Mounted on /dev/sda2 46G 23G 21G 52% / none 896M 700K 895M 1% /dev none 957M 80K 957M 1% /dev/shm none 957M 104K 957M 1% /var/run none 957M 0 957M 0% /var/lock /dev/loop0686M 56K 493M 1% /mnt According to my debugging result, for the 1st time, mount loop0 failed at around volumes.c:3468: map-stripes[i].dev = btrfs_find_device(root, devid, uuid, NULL); if (!map-stripes[i].dev !btrfs_test_opt(root, DEGRADED)) { kfree(map); free_extent_map(em); return -EIO; So I repeat the above steps again, and try to mount loop0 with -o degraded option, it works as expected: root@pibroch:/btrfs-progs# mount -o degraded /dev/loop0 /mnt root@pibroch:/btrfs-progs# df -h FilesystemSize Used Avail Use% Mounted on /dev/sda2 46G 23G 21G 52% / none 896M 700K 895M 1% /dev none 957M 140K 957M 1% /dev/shm none 957M 104K 957M 1% /var/run none 957M 0 957M 0% /var/lock /dev/loop0686M 56K 136M 1% /mnt kernel version 3.1.0-rc2+. Is it a bug? or Am I missing something? Thanks, -Jeff -- 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
[PATCH] Btrfs: added new ioctl to set fs label V3
Hello, According to Hugo and David's advise, the ioctl number of BTRFS_IOC_FS_SETLABEL ioctl was changed to 50 now. Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/ctree.h |4 fs/btrfs/ioctl.c | 36 fs/btrfs/ioctl.h |2 ++ 3 files changed, 42 insertions(+), 0 deletions(-) diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index 03912c5..a4669f0 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -1259,6 +1259,10 @@ struct btrfs_ioctl_defrag_range_args { }; +struct btrfs_ioctl_fs_label_args { +char label[BTRFS_LABEL_SIZE]; +}; + /* * inode items have the data typically returned from stat and store other * info about object characteristics. There is one for every file and dir in diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 970977a..c872e88 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -268,6 +268,40 @@ static int btrfs_ioctl_getversion(struct file *file, int __user *arg) return put_user(inode-i_generation, arg); } +static int btrfs_ioctl_fs_setlabel(struct btrfs_root *root, void __user *arg) +{ +struct btrfs_super_block *super_block = (root-fs_info-super_copy); +struct btrfs_ioctl_fs_label_args *label_args; +struct btrfs_trans_handle *trans; +int ret; + +if (!capable(CAP_SYS_ADMIN)) +return -EPERM; + +if (btrfs_root_readonly(root)) +return -EROFS; + +label_args = memdup_user(arg, sizeof(*label_args)); +if (IS_ERR(label_args)) +return PTR_ERR(label_args); + +label_args-label[BTRFS_LABEL_SIZE - 1] = '\0'; + +mutex_lock(root-fs_info-volume_mutex); +trans = btrfs_start_transaction(root, 0); +if (IS_ERR(trans)) { +ret = PTR_ERR(trans); +goto out_unlock; +} +strcpy(super_block-label, label_args-label); +btrfs_end_transaction(trans, root); + +out_unlock: +mutex_unlock(root-fs_info-volume_mutex); +kfree(label_args); +return 0; +} + static noinline int btrfs_ioctl_fitrim(struct file *file, void __user *arg) { struct btrfs_root *root = fdentry(file)-d_sb-s_fs_info; @@ -2876,6 +2910,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_fs_info(root, argp); case BTRFS_IOC_DEV_INFO: return btrfs_ioctl_dev_info(root, argp); +case BTRFS_IOC_FS_SETLABEL: +return btrfs_ioctl_fs_setlabel(root, argp); case BTRFS_IOC_BALANCE: return btrfs_balance(root-fs_info-dev_root); case BTRFS_IOC_CLONE: diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index ad1ea78..1117fe8 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -248,4 +248,6 @@ struct btrfs_ioctl_space_args { struct btrfs_ioctl_dev_info_args) #define BTRFS_IOC_FS_INFO _IOR(BTRFS_IOCTL_MAGIC, 31, \ struct btrfs_ioctl_fs_info_args) +#define BTRFS_IOC_FS_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \ + struct btrfs_ioctl_fs_label_args) #endif -- 1.7.4.1 -- 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
[PATCH] Btrfs-progs: added btrfs filesystem label [label] [path] support V3
Fix the ioctl number of BTRFS_IOC_FS_SETLABEL to 50. Signed-off-by: Jie Liu jeff@oracle.com --- btrfs.c |7 +++ btrfs_cmds.c | 34 ++ btrfs_cmds.h |1 + ctree.h |4 ioctl.h |2 ++ mkfs.c | 19 --- utils.c | 18 ++ utils.h |1 + 8 files changed, 63 insertions(+), 23 deletions(-) diff --git a/btrfs.c b/btrfs.c index 46314cf..6d414f1 100644 --- a/btrfs.c +++ b/btrfs.c @@ -108,11 +108,10 @@ static struct Command commands[] = { device delete, dev [dev..] path\n Remove a device from a filesystem. }, -/* coming soon -{ 2, filesystem label, label path\n +{ do_set_label, 2, + filesystem label, label path\n Set the label of a filesystem -} -*/ +}, { 0, 0 , 0 } }; diff --git a/btrfs_cmds.c b/btrfs_cmds.c index 8031c58..2a879c0 100644 --- a/btrfs_cmds.c +++ b/btrfs_cmds.c @@ -834,6 +834,40 @@ int do_set_default_subvol(int nargs, char **argv) return 0; } +int do_set_label(int nargs, char **argv) +{ +intfd, ret; +char*path = argv[2]; +char*label = parse_label(argv[1]); +size_tlen = strlen(label); +structbtrfs_ioctl_fs_label_args label_args; + +if (len == 0 || len = BTRFS_LABEL_SIZE) { +fprintf(stderr, ERROR: label length too long ('%s')\n, +label); +free(label); +return 14; +} + +fd = open_file_or_dir(path); +if (fd 0) { +free(label); +fprintf(stderr, ERROR: can't access to '%s'\n, path); +return 12; +} + +strcpy(label_args.label, label); +ret = ioctl(fd, BTRFS_IOC_FS_SETLABEL, label_args); +close(fd); +free(label); +if(ret 0) { +fprintf(stderr, ERROR: unable to set a new label\n); +return 30; +} + +return 0; +} + int do_df_filesystem(int nargs, char **argv) { struct btrfs_ioctl_space_args *sargs; diff --git a/btrfs_cmds.h b/btrfs_cmds.h index 7bde191..29ded22 100644 --- a/btrfs_cmds.h +++ b/btrfs_cmds.h @@ -32,3 +32,4 @@ int list_subvols(int fd); int do_df_filesystem(int nargs, char **argv); int find_updated_files(int fd, u64 root_id, u64 oldest_gen); int do_find_newer(int argc, char **argv); +int do_set_label(int argc, char **argv); diff --git a/ctree.h b/ctree.h index b79e238..745879b 100644 --- a/ctree.h +++ b/ctree.h @@ -345,6 +345,10 @@ struct btrfs_super_block { u8 sys_chunk_array[BTRFS_SYSTEM_CHUNK_ARRAY_SIZE]; } __attribute__ ((__packed__)); +struct btrfs_ioctl_fs_label_args { +char label[BTRFS_LABEL_SIZE]; +}; + /* * Compat flags that we support. If any incompat flags are set other than the * ones specified below then we will fail to mount diff --git a/ioctl.h b/ioctl.h index 776d7a9..9126ce8 100644 --- a/ioctl.h +++ b/ioctl.h @@ -169,4 +169,6 @@ struct btrfs_ioctl_space_args { #define BTRFS_IOC_DEFAULT_SUBVOL _IOW(BTRFS_IOCTL_MAGIC, 19, u64) #define BTRFS_IOC_SPACE_INFO _IOWR(BTRFS_IOCTL_MAGIC, 20, \ struct btrfs_ioctl_space_args) +#define BTRFS_IOC_FS_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \ + struct btrfs_ioctl_fs_label_args) #endif diff --git a/mkfs.c b/mkfs.c index 1598aae..93c1636 100644 --- a/mkfs.c +++ b/mkfs.c @@ -303,25 +303,6 @@ static u64 parse_profile(char *s) return 0; } -static char *parse_label(char *input) -{ -int i; -int len = strlen(input); - -if (len = BTRFS_LABEL_SIZE) { -fprintf(stderr, Label %s is too long (max %d)\n, input, -BTRFS_LABEL_SIZE - 1); -exit(1); -} -for (i = 0; i len; i++) { -if (input[i] == '/' || input[i] == '\\') { -fprintf(stderr, invalid label %s\n, input); -exit(1); -} -} -return strdup(input); -} - static struct option long_options[] = { { alloc-start, 1, NULL, 'A'}, { byte-count, 1, NULL, 'b' }, diff --git a/utils.c b/utils.c index fd894f3..5d77503 100644 --- a/utils.c +++ b/utils.c @@ -993,3 +993,21 @@ char *pretty_sizes(u64 size) return pretty; } +char *parse_label(const char *input) +{ +int i; +int len = strlen(input); + +if (len = BTRFS_LABEL_SIZE) { +fprintf(stderr, Label %s is too long (max %d)\n, input, +BTRFS_LABEL_SIZE - 1); +exit(1); +} +for (i = 0; i len; i++) { +if (input[i] == '/' || input[i] == '\\') { +fprintf(stderr, invalid label %s\n, input); +exit(1); +} +} +return strdup(input); +} diff --git a/utils.h b/utils.h index 9dce5b0..9212a75 100644 --- a/utils.h +++ b/utils.h @@ -40,4 +40,5 @@ int check_mounted(const char *devicename); int btrfs_device_already_in_root(struct btrfs_root *root, int fd, int super_offset); char *pretty_sizes(u64 size); +char *parse_label(const char *); #endif -- 1.7.4.1 On 09/05/2011 01:34 PM, Jeff Liu wrote: On 09/05/2011 01:03 AM, Hugo Mills
Re: [PATCH] Btrfs-progs: added btrfs filesystem label [label] [path] support V2
On 09/05/2011 01:03 AM, Hugo Mills wrote: On Sat, Sep 03, 2011 at 11:11:36AM +0800, Jeff liu wrote: 在 2011-9-2,下午11:48, David Sterba 写道: On Fri, Sep 02, 2011 at 09:13:34PM +0800, Jeff Liu wrote: --- a/ioctl.h +++ b/ioctl.h @@ -140,6 +140,8 @@ struct btrfs_ioctl_space_args { struct btrfs_ioctl_vol_args) #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \ struct btrfs_ioctl_vol_args) +#define BTRFS_IOC_FS_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 5, \ + struct btrfs_ioctl_fs_label_args) /* trans start and trans end are dangerous, and only for * use by applications that know how to avoid the * resulting deadlocks well, it is an unassigned number, but a newly added features should IMHO allocate greater than current max value, ie over 31 in coordination with https://btrfs.wiki.kernel.org/index.php/Project_ideas#Development_notes.2C_please_read table. It sounds reasonable to allocate a greater value, could anyone please confirm it? I'd just take number 50 for yours -- Li can update his patches later. Thank you, I'll post a patch for this change later. -Jeff Hugo. What's your ioctl range for online fsck? -- 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
Re: [PATCH] Btrfs-progs: added btrfs filesystem label [label] [path] support V2
Hi David, On 09/02/2011 08:52 PM, David Sterba wrote: Hi, are you aware that there is a label support already? Though only for unmounted system, but please have a look at these patches: https://patchwork.kernel.org/patch/381141/ https://patchwork.kernel.org/patch/842602/ and the patches are part of Hugo's integration for a long time, rather check latest versions so you do not duplicate work. Thanks for your info! I was not aware of that at that time. :( I am definitely a newbie to Btrfs and the kernel world, just started to explore it at last week. :-) On Thu, Sep 01, 2011 at 07:52:54PM +0800, Jeff Liu wrote: Revise the patch according to kernel side change. Leave original commit message. If you want to document changes between revised patch version put them ... ok, fixed as below: From e2990b69ecd3bac8da8023a64c866d16c81a1679 Mon Sep 17 00:00:00 2001 From: Jie Liu jeff@oracle.com Date: Fri, 2 Sep 2011 21:07:35 +0800 Subject: [PATCH 1/1] Add btrfs filesystem label [label] [path] support through ioctl. Signed-off-by: Jie Liu jeff@oracle.com --- btrfs.c |7 +++ btrfs_cmds.c | 34 ++ btrfs_cmds.h |1 + ctree.h |4 ioctl.h |2 ++ mkfs.c | 19 --- utils.c | 18 ++ utils.h |1 + 8 files changed, 63 insertions(+), 23 deletions(-) diff --git a/btrfs.c b/btrfs.c index 46314cf..6d414f1 100644 --- a/btrfs.c +++ b/btrfs.c @@ -108,11 +108,10 @@ static struct Command commands[] = { device delete, dev [dev..] path\n Remove a device from a filesystem. }, -/* coming soon -{ 2, filesystem label, label path\n +{ do_set_label, 2, + filesystem label, label path\n Set the label of a filesystem -} -*/ +}, { 0, 0 , 0 } }; diff --git a/btrfs_cmds.c b/btrfs_cmds.c index 8031c58..2a879c0 100644 --- a/btrfs_cmds.c +++ b/btrfs_cmds.c @@ -834,6 +834,40 @@ int do_set_default_subvol(int nargs, char **argv) return 0; } +int do_set_label(int nargs, char **argv) +{ +intfd, ret; +char*path = argv[2]; +char*label = parse_label(argv[1]); +size_tlen = strlen(label); +structbtrfs_ioctl_fs_label_args label_args; + +if (len == 0 || len = BTRFS_LABEL_SIZE) { +fprintf(stderr, ERROR: label length too long ('%s')\n, +label); +free(label); +return 14; +} + +fd = open_file_or_dir(path); +if (fd 0) { +free(label); +fprintf(stderr, ERROR: can't access to '%s'\n, path); +return 12; +} + +strcpy(label_args.label, label); +ret = ioctl(fd, BTRFS_IOC_FS_SETLABEL, label_args); +close(fd); +free(label); +if(ret 0) { +fprintf(stderr, ERROR: unable to set a new label\n); +return 30; +} + +return 0; +} + int do_df_filesystem(int nargs, char **argv) { struct btrfs_ioctl_space_args *sargs; diff --git a/btrfs_cmds.h b/btrfs_cmds.h index 7bde191..29ded22 100644 --- a/btrfs_cmds.h +++ b/btrfs_cmds.h @@ -32,3 +32,4 @@ int list_subvols(int fd); int do_df_filesystem(int nargs, char **argv); int find_updated_files(int fd, u64 root_id, u64 oldest_gen); int do_find_newer(int argc, char **argv); +int do_set_label(int argc, char **argv); diff --git a/ctree.h b/ctree.h index b79e238..745879b 100644 --- a/ctree.h +++ b/ctree.h @@ -345,6 +345,10 @@ struct btrfs_super_block { u8 sys_chunk_array[BTRFS_SYSTEM_CHUNK_ARRAY_SIZE]; } __attribute__ ((__packed__)); +struct btrfs_ioctl_fs_label_args { +char label[BTRFS_LABEL_SIZE]; +}; + /* * Compat flags that we support. If any incompat flags are set other than the * ones specified below then we will fail to mount diff --git a/ioctl.h b/ioctl.h index 776d7a9..98acd63 100644 --- a/ioctl.h +++ b/ioctl.h @@ -140,6 +140,8 @@ struct btrfs_ioctl_space_args { struct btrfs_ioctl_vol_args) #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \ struct btrfs_ioctl_vol_args) +#define BTRFS_IOC_FS_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 5, \ + struct btrfs_ioctl_fs_label_args) /* trans start and trans end are dangerous, and only for * use by applications that know how to avoid the * resulting deadlocks diff --git a/mkfs.c b/mkfs.c index 1598aae..93c1636 100644 --- a/mkfs.c +++ b/mkfs.c @@ -303,25 +303,6 @@ static u64 parse_profile(char *s) return 0; } -static char *parse_label(char *input) -{ -int i; -int len = strlen(input); - -if (len = BTRFS_LABEL_SIZE) { -fprintf(stderr, Label %s is too long (max %d)\n, input, -BTRFS_LABEL_SIZE - 1); -exit(1); -} -for (i = 0; i len; i++) { -if (input[i] == '/' || input[i] == '\\') { -fprintf(stderr, invalid label %s\n, input); -exit(1); -} -} -return strdup(input); -} - static struct option long_options[] = { { alloc-start, 1, NULL
Re: [PATCH] Btrfs-progs: added btrfs filesystem label [label] [path] support V2
在 2011-9-2,下午11:48, David Sterba 写道: On Fri, Sep 02, 2011 at 09:13:34PM +0800, Jeff Liu wrote: --- a/ioctl.h +++ b/ioctl.h @@ -140,6 +140,8 @@ struct btrfs_ioctl_space_args { struct btrfs_ioctl_vol_args) #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \ struct btrfs_ioctl_vol_args) +#define BTRFS_IOC_FS_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 5, \ + struct btrfs_ioctl_fs_label_args) /* trans start and trans end are dangerous, and only for * use by applications that know how to avoid the * resulting deadlocks well, it is an unassigned number, but a newly added features should IMHO allocate greater than current max value, ie over 31 in coordination with https://btrfs.wiki.kernel.org/index.php/Project_ideas#Development_notes.2C_please_read table. It sounds reasonable to allocate a greater value, could anyone please confirm it? Hi Zefan, What's your ioctl range for online fsck? Thanks, -Jeff david -- 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 -- 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
[PATCH] Btrfs: added new ioctl to set fs label
Hello, I'd like to introduce a new ioctl to set file system label. With this feature, we can execute `btrfs filesystem label [label] [path]` through btrfs tools to set or change the label. Signed-off-by: Jie Liu jeff@oracle.com --- fs/btrfs/ctree.h |6 ++ fs/btrfs/ioctl.c | 37 + fs/btrfs/ioctl.h |2 ++ 3 files changed, 45 insertions(+), 0 deletions(-) diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index 03912c5..2889b5e 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -1259,6 +1259,12 @@ struct btrfs_ioctl_defrag_range_args { }; +struct btrfs_ioctl_fs_label_args { +/* label length in bytes */ +__u32 len; +char label[BTRFS_LABEL_SIZE]; +}; + /* * inode items have the data typically returned from stat and store other * info about object characteristics. There is one for every file and dir in diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 970977a..9e01628 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -268,6 +268,41 @@ static int btrfs_ioctl_getversion(struct file *file, int __user *arg) return put_user(inode-i_generation, arg); } +static int btrfs_ioctl_fs_setlabel(struct btrfs_root *root, void __user *arg) +{ +struct btrfs_super_block *super_block = (root-fs_info-super_copy); +struct btrfs_ioctl_fs_label_args *label_args; +struct btrfs_trans_handle *trans; + +if (!capable(CAP_SYS_ADMIN)) +return -EPERM; + +if (btrfs_root_readonly(root)) +return -EROFS; + +label_args = memdup_user(arg, sizeof(*label_args)); +if (IS_ERR(label_args)) +return PTR_ERR(label_args); + +if (label_args-len = sizeof(label_args-label)) +return -EINVAL; + +mutex_lock(root-fs_info-volume_mutex); +trans = btrfs_start_transaction(root, 0); +if (IS_ERR(trans)) +return PTR_ERR(trans); + +if (snprintf(super_block-label, BTRFS_LABEL_SIZE, %s, + label_args-label) != label_args-len) +return -EFAULT; + +btrfs_end_transaction(trans, root); +mutex_unlock(root-fs_info-volume_mutex); + +kfree(label_args); +return 0; +} + static noinline int btrfs_ioctl_fitrim(struct file *file, void __user *arg) { struct btrfs_root *root = fdentry(file)-d_sb-s_fs_info; @@ -2876,6 +2911,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_fs_info(root, argp); case BTRFS_IOC_DEV_INFO: return btrfs_ioctl_dev_info(root, argp); +case BTRFS_IOC_FS_SETLABEL: +return btrfs_ioctl_fs_setlabel(root, argp); case BTRFS_IOC_BALANCE: return btrfs_balance(root-fs_info-dev_root); case BTRFS_IOC_CLONE: diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index ad1ea78..1e0ca2a 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -248,4 +248,6 @@ struct btrfs_ioctl_space_args { struct btrfs_ioctl_dev_info_args) #define BTRFS_IOC_FS_INFO _IOR(BTRFS_IOCTL_MAGIC, 31, \ struct btrfs_ioctl_fs_info_args) +#define BTRFS_IOC_FS_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 32, \ + struct btrfs_ioctl_fs_label_args) #endif -- 1.7.4.1 -- 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
[PATCH] Btrfs-progs: added btrfs filesystem label [label] [path] support
Hello, This patch make use of the new ioctl(2) to set Btrfs label via `btrfs filesystem label` command. Signed-off-by: Jie Liu jeff@oracle.com --- btrfs.c |7 +++ btrfs_cmds.c | 27 +++ btrfs_cmds.h |1 + ctree.h |6 ++ ioctl.h |2 ++ mkfs.c | 19 --- utils.c | 18 ++ utils.h |1 + 8 files changed, 58 insertions(+), 23 deletions(-) diff --git a/btrfs.c b/btrfs.c index 46314cf..6d414f1 100644 --- a/btrfs.c +++ b/btrfs.c @@ -108,11 +108,10 @@ static struct Command commands[] = { device delete, dev [dev..] path\n Remove a device from a filesystem. }, -/* coming soon -{ 2, filesystem label, label path\n +{ do_set_label, 2, + filesystem label, label path\n Set the label of a filesystem -} -*/ +}, { 0, 0 , 0 } }; diff --git a/btrfs_cmds.c b/btrfs_cmds.c index 8031c58..8d2b8e1 100644 --- a/btrfs_cmds.c +++ b/btrfs_cmds.c @@ -834,6 +834,33 @@ int do_set_default_subvol(int nargs, char **argv) return 0; } +int do_set_label(int nargs, char **argv) +{ +intfd, ret = 0; +char*path = argv[2]; +char*label = parse_label(argv[1]); +size_tlen = strlen(label); +structbtrfs_ioctl_fs_label_args label_args; + +fd = open_file_or_dir(path); +if (fd 0) { +fprintf(stderr, ERROR: can't access to '%s'\n, path); +return 12; +} + +label_args.len = len; +snprintf(label_args.label, BTRFS_LABEL_SIZE, %s, label); +ret = ioctl(fd, BTRFS_IOC_FS_SETLABEL, label_args); +close(fd); +free(label); +if(ret 0) { +fprintf(stderr, ERROR: unable to set a new label\n); +return 30; +} + +return 0; +} + int do_df_filesystem(int nargs, char **argv) { struct btrfs_ioctl_space_args *sargs; diff --git a/btrfs_cmds.h b/btrfs_cmds.h index 7bde191..29ded22 100644 --- a/btrfs_cmds.h +++ b/btrfs_cmds.h @@ -32,3 +32,4 @@ int list_subvols(int fd); int do_df_filesystem(int nargs, char **argv); int find_updated_files(int fd, u64 root_id, u64 oldest_gen); int do_find_newer(int argc, char **argv); +int do_set_label(int argc, char **argv); diff --git a/ctree.h b/ctree.h index b79e238..4924b88 100644 --- a/ctree.h +++ b/ctree.h @@ -345,6 +345,12 @@ struct btrfs_super_block { u8 sys_chunk_array[BTRFS_SYSTEM_CHUNK_ARRAY_SIZE]; } __attribute__ ((__packed__)); +struct btrfs_ioctl_fs_label_args { +/* label length in bytes */ +__u32 len; +char label[BTRFS_LABEL_SIZE]; +}; + /* * Compat flags that we support. If any incompat flags are set other than the * ones specified below then we will fail to mount diff --git a/ioctl.h b/ioctl.h index 776d7a9..5750f3a 100644 --- a/ioctl.h +++ b/ioctl.h @@ -169,4 +169,6 @@ struct btrfs_ioctl_space_args { #define BTRFS_IOC_DEFAULT_SUBVOL _IOW(BTRFS_IOCTL_MAGIC, 19, u64) #define BTRFS_IOC_SPACE_INFO _IOWR(BTRFS_IOCTL_MAGIC, 20, \ struct btrfs_ioctl_space_args) +#define BTRFS_IOC_FS_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 32, \ + struct btrfs_ioctl_fs_label_args) #endif diff --git a/mkfs.c b/mkfs.c index 1598aae..93c1636 100644 --- a/mkfs.c +++ b/mkfs.c @@ -303,25 +303,6 @@ static u64 parse_profile(char *s) return 0; } -static char *parse_label(char *input) -{ -int i; -int len = strlen(input); - -if (len = BTRFS_LABEL_SIZE) { -fprintf(stderr, Label %s is too long (max %d)\n, input, -BTRFS_LABEL_SIZE - 1); -exit(1); -} -for (i = 0; i len; i++) { -if (input[i] == '/' || input[i] == '\\') { -fprintf(stderr, invalid label %s\n, input); -exit(1); -} -} -return strdup(input); -} - static struct option long_options[] = { { alloc-start, 1, NULL, 'A'}, { byte-count, 1, NULL, 'b' }, diff --git a/utils.c b/utils.c index fd894f3..5d77503 100644 --- a/utils.c +++ b/utils.c @@ -993,3 +993,21 @@ char *pretty_sizes(u64 size) return pretty; } +char *parse_label(const char *input) +{ +int i; +int len = strlen(input); + +if (len = BTRFS_LABEL_SIZE) { +fprintf(stderr, Label %s is too long (max %d)\n, input, +BTRFS_LABEL_SIZE - 1); +exit(1); +} +for (i = 0; i len; i++) { +if (input[i] == '/' || input[i] == '\\') { +fprintf(stderr, invalid label %s\n, input); +exit(1); +} +} +return strdup(input); +} diff --git a/utils.h b/utils.h index 9dce5b0..9212a75 100644 --- a/utils.h +++ b/utils.h @@ -40,4 +40,5 @@ int check_mounted(const char *devicename); int btrfs_device_already_in_root(struct btrfs_root *root, int fd, int super_offset); char *pretty_sizes(u64 size); +char *parse_label(const char *); #endif -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More
Re: [PATCH] Btrfs: added new ioctl to set fs label
Hi Hugo, On 09/01/2011 05:00 PM, Hugo Mills wrote: On Thu, Sep 01, 2011 at 04:47:47PM +0800, Jeff Liu wrote: Hello, I'd like to introduce a new ioctl to set file system label. With this feature, we can execute `btrfs filesystem label [label] [path]` through btrfs tools to set or change the label. Signed-off-by: Jie Liujeff@oracle.com --- fs/btrfs/ctree.h |6 ++ fs/btrfs/ioctl.c | 37 + fs/btrfs/ioctl.h |2 ++ 3 files changed, 45 insertions(+), 0 deletions(-) diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index 03912c5..2889b5e 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -1259,6 +1259,12 @@ struct btrfs_ioctl_defrag_range_args { }; +struct btrfs_ioctl_fs_label_args { +/* label length in bytes */ +__u32 len; +char label[BTRFS_LABEL_SIZE]; +}; Why do we need to provide a length here? Simply force a zero byte at the end of the string when you copy it into kernel space, and then use strcpy(). Then you have no need to test for length at all. At first, I am afraid if an evil user may input an unexpected label string with huge bytes to consume memory. /* * inode items have the data typically returned from stat and store other * info about object characteristics. There is one for every file and dir in diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 970977a..9e01628 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -268,6 +268,41 @@ static int btrfs_ioctl_getversion(struct file *file, int __user *arg) return put_user(inode-i_generation, arg); } +static int btrfs_ioctl_fs_setlabel(struct btrfs_root *root, void __user *arg) +{ +struct btrfs_super_block *super_block =(root-fs_info-super_copy); +struct btrfs_ioctl_fs_label_args *label_args; +struct btrfs_trans_handle *trans; + +if (!capable(CAP_SYS_ADMIN)) +return -EPERM; + +if (btrfs_root_readonly(root)) +return -EROFS; + +label_args = memdup_user(arg, sizeof(*label_args)); +if (IS_ERR(label_args)) +return PTR_ERR(label_args); + +if (label_args-len= sizeof(label_args-label)) +return -EINVAL; Memory leak... you're not freeing label_args +mutex_lock(root-fs_info-volume_mutex); +trans = btrfs_start_transaction(root, 0); +if (IS_ERR(trans)) +return PTR_ERR(trans); and here -- and you're leaving the mutex locked +if (snprintf(super_block-label, BTRFS_LABEL_SIZE, %s, + label_args-label) != label_args-len) +return -EFAULT; and here -- plus the transaction is still running Sorry for my stupid mistake and thanks for pointing those out! +btrfs_end_transaction(trans, root); +mutex_unlock(root-fs_info-volume_mutex); + +kfree(label_args); +return 0; +} + static noinline int btrfs_ioctl_fitrim(struct file *file, void __user *arg) { struct btrfs_root *root = fdentry(file)-d_sb-s_fs_info; @@ -2876,6 +2911,8 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_fs_info(root, argp); case BTRFS_IOC_DEV_INFO: return btrfs_ioctl_dev_info(root, argp); +case BTRFS_IOC_FS_SETLABEL: +return btrfs_ioctl_fs_setlabel(root, argp); case BTRFS_IOC_BALANCE: return btrfs_balance(root-fs_info-dev_root); case BTRFS_IOC_CLONE: diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h index ad1ea78..1e0ca2a 100644 --- a/fs/btrfs/ioctl.h +++ b/fs/btrfs/ioctl.h @@ -248,4 +248,6 @@ struct btrfs_ioctl_space_args { struct btrfs_ioctl_dev_info_args) #define BTRFS_IOC_FS_INFO _IOR(BTRFS_IOCTL_MAGIC, 31, \ struct btrfs_ioctl_fs_info_args) +#define BTRFS_IOC_FS_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 32, \ + struct btrfs_ioctl_fs_label_args) Could you use an unassigned number from [1], and update the wiki page, please? (The page only went up yesterday, but it's been needed for a while). Ok, looks number 5 is free. :) I'll update the wiki later. Regards, -Jeff #endif Will there be a userspace patch for this along shortly? Hugo. [1] https://btrfs.wiki.kernel.org/index.php/Project_ideas#Development_notes.2C_please_read -- 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