[PATCH v4] Btrfs: ability to add label to snapshot and subvol
Generally snapshots are machine generated, so at any point in time
if a sysadmin looks at a list of snapshots there should be some
info about the snapshots to indicate purpose of it being created.
With this patch and along with the corresponding btrfs-progs patch
a 32byte info can be added to the snapshots/subvol.
Further, ioctl is preferred over the attribute to write the label
since btrfs-progs is not only the application which might be
interacting with the subvol to write the label, for example
btrfs-gui might as well write the label, so that needs an
additional efforts to maintain a consistent keyword across the
applications is difficult in the long run.
Signed-off-by: Anand Jain
---
fs/btrfs/ctree.h | 12 +-
fs/btrfs/ioctl.c | 58 ++
fs/btrfs/transaction.c | 1 +
include/uapi/linux/btrfs.h | 2 ++
4 files changed, 72 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 1679051..6b527bc 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -392,6 +392,7 @@ struct btrfs_header {
*/
#define BTRFS_SYSTEM_CHUNK_ARRAY_SIZE 2048
#define BTRFS_LABEL_SIZE 256
+#define BTRFS_SUBVOL_LABEL_SIZE 32
/*
* just in case we somehow lose the roots and are not able to mount,
@@ -769,7 +770,8 @@ struct btrfs_root_item {
struct btrfs_timespec otime;
struct btrfs_timespec stime;
struct btrfs_timespec rtime;
- __le64 reserved[8]; /* for future */
+ char label[BTRFS_SUBVOL_LABEL_SIZE];
+ __le64 reserved[4]; /* for future */
} __attribute__ ((__packed__));
/*
@@ -2546,6 +2548,14 @@ static inline bool btrfs_root_readonly(struct btrfs_root
*root)
{
return (root->root_item.flags & cpu_to_le64(BTRFS_ROOT_SUBVOL_RDONLY))
!= 0;
}
+static inline char * btrfs_root_label(struct btrfs_root_item *root_item)
+{
+ return (root_item->label);
+}
+static inline void btrfs_root_set_label(struct btrfs_root_item *root_item,
char *val)
+{
+ memcpy(root_item->label, val, BTRFS_SUBVOL_LABEL_SIZE);
+}
/* struct btrfs_root_backup */
BTRFS_SETGET_STACK_FUNCS(backup_tree_root, struct btrfs_root_backup,
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 2bbbed5..211b319 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3982,6 +3982,60 @@ out_unlock:
return ret;
}
+static int btrfs_ioctl_subvol_getlabel(struct btrfs_root *root,
+ void __user *arg)
+{
+ char *label;
+
+ spin_lock(&root->root_item_lock);
+ label = btrfs_root_label(&root->root_item);
+ if (copy_to_user(arg, label, BTRFS_SUBVOL_LABEL_SIZE)) {
+ spin_unlock(&root->root_item_lock);
+ return -EFAULT;
+ }
+ spin_unlock(&root->root_item_lock);
+ return 0;
+}
+
+static int btrfs_ioctl_subvol_setlabel(struct file *file,
+ void __user *arg)
+{
+ char label[BTRFS_SUBVOL_LABEL_SIZE+1];
+ struct btrfs_trans_handle *trans;
+ struct btrfs_root *root = BTRFS_I(fdentry(file)->d_inode)->root;
+ struct inode *inode = file->f_path.dentry->d_inode;
+ int ret;
+
+ if (btrfs_root_readonly(root))
+ return -EROFS;
+
+ if (!inode_owner_or_capable(inode))
+ return -EACCES;
+
+ ret = mnt_want_write_file(file);
+ if (ret)
+ return ret;
+
+ if (copy_from_user(label, arg, BTRFS_SUBVOL_LABEL_SIZE)) {
+ ret = -EFAULT;
+ goto out;
+ }
+
+ trans = btrfs_join_transaction(root);
+ if (IS_ERR(trans)) {
+ ret = PTR_ERR(trans);
+ goto out;
+ }
+ spin_lock(&root->root_item_lock);
+ btrfs_root_set_label(&root->root_item, label);
+ spin_unlock(&root->root_item_lock);
+ btrfs_end_transaction(trans, root);
+
+out:
+ mnt_drop_write_file(file);
+ return ret;
+}
+
long btrfs_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
{
@@ -4086,6 +4140,10 @@ long btrfs_ioctl(struct file *file, unsigned int
return btrfs_ioctl_get_fslabel(file, argp);
case BTRFS_IOC_SET_FSLABEL:
return btrfs_ioctl_set_fslabel(file, argp);
+ case BTRFS_IOC_SUBVOL_GETLABEL:
+ return btrfs_ioctl_subvol_getlabel(root, argp);
+ case BTRFS_IOC_SUBVOL_SETLABEL:
+ return btrfs_ioctl_subvol_setlabel(file, argp);
}
return -ENOTTY;
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 955204c..955097e 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -1181,6 +1181,7 @@ static noinline int create_pending_snapshot(struct
btrfs_trans_handle *trans,
memset(&new_root_item->rtime, 0, sizeof(new_root_item->rtime));
btrfs_set_root_stransid(new_root_item, 0);
btrfs_set_root_rtransid(new_root_item, 0);
+ mems
[PATCH v4] Btrfs: Add support for subvol label
This is the btrfs kernel changes to add supprt for the subvol label. The related (v4) Btrfs-progs patches are Btrfs-progs: add feature to label subvol and snapshot Btrfs-progs: cmd option to show or set the subvol label v4: rebased to Josef Bacik btrfs-next Anand Jain (1): Btrfs: ability to add label to snapshot and subvol fs/btrfs/ctree.h | 12 +- fs/btrfs/ioctl.c | 58 ++ fs/btrfs/transaction.c | 1 + include/uapi/linux/btrfs.h | 2 ++ 4 files changed, 72 insertions(+), 1 deletion(-) -- 1.8.1.227.g44fe835 -- 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 v4] Btrfs-progs: add feature to label subvol and snapshot
btrfs-progs framework to add support to label the subvol and
snapshots. This patch is for the ioctl-way to write label
to the subvol and snapshot. (the other way is to use attribute
if then only this patch has to be backed out in btrfs-progs
and replace with the patch titled
Btrfs-progs: add attribute label for subvol and snapshot)
Signed-off-by: Anand Jain
---
btrfslabel.c | 45 +
btrfslabel.h | 4 +++-
ctree.h | 4 +++-
ioctl.h | 2 ++
print-tree.c | 2 ++
5 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/btrfslabel.c b/btrfslabel.c
index a421a8b..b3ad540 100644
--- a/btrfslabel.c
+++ b/btrfslabel.c
@@ -126,3 +126,48 @@ int set_label(char *btrfs_dev, char *nLabel)
}
return change_label_unmounted(btrfs_dev, nLabel);
}
+
+int get_subvol_label(char *subvol, char *labelp)
+{
+ int fd, e=0;
+
+ fd = open_file_or_dir(subvol);
+
+ if(ioctl(fd, BTRFS_IOC_SUBVOL_GETLABEL, labelp) < 0) {
+ e = errno;
+ fprintf(stderr, "ERROR: get subvol label failed, %s\n",
+ strerror(e));
+ close(fd);
+ return -e;
+ }
+ labelp[BTRFS_LABEL_SIZE] = '\0';
+ if (!strlen(labelp))
+ return -1;
+ close(fd);
+ return 0;
+}
+
+int set_subvol_label(char *subvol, char *labelp)
+{
+ int fd, e=0;
+ char label[BTRFS_SUBVOL_LABEL_SIZE+1];
+
+ if (strlen(labelp) > BTRFS_SUBVOL_LABEL_SIZE) {
+ fprintf(stderr, "ERROR: Subvol label is more than max length
%d\n",
+ BTRFS_SUBVOL_LABEL_SIZE);
+ return -1;
+ }
+ memset(label, 0, BTRFS_SUBVOL_LABEL_SIZE+1);
+ strcpy(label, labelp);
+ fd = open_file_or_dir(subvol);
+
+ if(ioctl(fd, BTRFS_IOC_SUBVOL_SETLABEL, label) < 0) {
+ e = errno;
+ fprintf(stderr, "ERROR: set subvol label failed, %s\n",
+ strerror(e));
+ close(fd);
+ return -e;
+ }
+ close(fd);
+ return 0;
+}
diff --git a/btrfslabel.h b/btrfslabel.h
index abf43ad..1abc483 100644
--- a/btrfslabel.h
+++ b/btrfslabel.h
@@ -2,4 +2,6 @@
int get_label(char *btrfs_dev);
-int set_label(char *btrfs_dev, char *nLabel);
\ No newline at end of file
+int set_label(char *btrfs_dev, char *nLabel);
+int get_subvol_label(char *subvol, char *labelp);
+int set_subvol_label(char *subvol, char *labelp);
diff --git a/ctree.h b/ctree.h
index 12f8fe3..bacb7d1 100644
--- a/ctree.h
+++ b/ctree.h
@@ -332,6 +332,7 @@ struct btrfs_header {
*/
#define BTRFS_SYSTEM_CHUNK_ARRAY_SIZE 2048
#define BTRFS_LABEL_SIZE 256
+#define BTRFS_SUBVOL_LABEL_SIZE 32
/*
* just in case we somehow lose the roots and are not able to mount,
@@ -721,7 +722,8 @@ struct btrfs_root_item {
struct btrfs_timespec otime;
struct btrfs_timespec stime;
struct btrfs_timespec rtime;
-__le64 reserved[8]; /* for future */
+ char label[BTRFS_SUBVOL_LABEL_SIZE];
+__le64 reserved[4]; /* for future */
} __attribute__ ((__packed__));
/*
diff --git a/ioctl.h b/ioctl.h
index 3e7e451..4cbae3a 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -524,6 +524,8 @@ struct btrfs_ioctl_clone_range_args {
struct btrfs_ioctl_get_dev_stats)
#define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \
struct btrfs_ioctl_dev_replace_args)
+#define BTRFS_IOC_SUBVOL_GETLABEL _IOWR(BTRFS_IOCTL_MAGIC, 55, __u64)
+#define BTRFS_IOC_SUBVOL_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 56, __u64)
#ifdef __cplusplus
}
diff --git a/print-tree.c b/print-tree.c
index c9e891b..3d097dd 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -366,6 +366,8 @@ static void print_root(struct extent_buffer *leaf, int slot)
btrfs_root_stransid(&root_item),
btrfs_root_rtransid(&root_item));
}
+ if (strlen(root_item.label))
+ printf("\t\tlabel %s\n", root_item.label);
}
if (btrfs_root_refs(&root_item) == 0) {
struct btrfs_key drop_key;
--
1.8.1.227.g44fe835
--
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 v4] Btrfs-progs: cmd option to show or set the subvol label
This adds the command option label to the subvol sub-command,
this is a generic patch which will stay irrespective of which
approach we take, that is the ioctl-way or the attributes.
Signed-off-by: Anand Jain
---
cmds-subvolume.c | 37 +
man/btrfs.8.in | 6 ++
2 files changed, 43 insertions(+)
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index ea128fc..1951c51 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -35,12 +35,48 @@
#include "utils.h"
#include "btrfs-list.h"
#include "utils.h"
+#include "btrfslabel.h"
static const char * const subvolume_cmd_group_usage[] = {
"btrfs subvolume ",
NULL
};
+static const char * const cmd_subvol_label_usage[] = {
+ "btrfs subvolume label [label]",
+ "Show or set label for the subvol or snapshot",
+ NULL
+};
+
+static int cmd_subvol_label(int argc, char **argv)
+{
+ struct stat st;
+ char label[BTRFS_SUBVOL_LABEL_SIZE+1];
+ int ret;
+
+ if (check_argc_min(argc, 2) || check_argc_max(argc, 3))
+ usage(cmd_subvol_label_usage);
+
+ if (stat(argv[1], &st) < 0) {
+ fprintf(stderr, "Error: %s\n",strerror(errno));
+ return -errno;
+ }
+ if (!S_ISDIR(st.st_mode)) {
+ fprintf(stderr, "Error: Not a dir\n");
+ return -1;
+ }
+ if (argc > 2)
+ return set_subvol_label(argv[1], argv[2]);
+ else {
+ ret = get_subvol_label(argv[1], label);
+ if (ret)
+ return ret;
+ label[BTRFS_SUBVOL_LABEL_SIZE]=0;
+ printf("%s\n",label);
+ }
+ return 0;
+}
+
/*
* test if path is a directory
* this function return
@@ -933,6 +969,7 @@ const struct cmd_group subvolume_cmd_group = {
cmd_subvol_set_default_usage, NULL, 0 },
{ "find-new", cmd_find_new, cmd_find_new_usage, NULL, 0 },
{ "show", cmd_subvol_show, cmd_subvol_show_usage, NULL, 0 },
+ { "label", cmd_subvol_label, cmd_subvol_label_usage, NULL, 0 },
{ 0, 0, 0, 0, 0 }
}
};
diff --git a/man/btrfs.8.in b/man/btrfs.8.in
index 94f4ffe..cd2e4e5 100644
--- a/man/btrfs.8.in
+++ b/man/btrfs.8.in
@@ -17,6 +17,8 @@ btrfs \- control a btrfs filesystem
.PP
\fBbtrfs\fP \fBsubvolume get-default\fP\fI \fP
.PP
+\fBbtrfs\fP \fBsubvolume label\fP\fI [label]\fP
+.PP
\fBbtrfs\fP \fBsubvolume find-new\fP\fI \fP
.PP
\fBbtrfs\fP \fBsubvolume show\fP\fI \fP
@@ -177,6 +179,10 @@ Get the default subvolume of the filesystem \fI\fR.
The output format
is similar to \fBsubvolume list\fR command.
.TP
+\fBsubvolume label\fR\fI [label]\fR
+Show or set \fI[label]\fR for the subvolume or the snapshot \fI\fR.
+.TP
+
\fBsubvolume find-new\fR\fI \fR
List the recently modified files in a subvolume, after \fI\fR ID.
.TP
--
1.8.1.227.g44fe835
--
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 v4] Btrfs-progs: Add support for subvol label
This is the Btrfs-progs changes to add support for the subvol label. As discussed in the mailing-list there are two ways we could get the label attached to the subvol, using kernel-ioctl method and the other method is by using attributes. I preferred kernel-ioctl method over the attribute since there might be more than just the btrfs-progs which is reading and writing the label, if used attributes, the keyword is defined and used at the application level, which will be difficult to publicize and maintain the consistent keyword across any application in the long run. In the kernel-ioctl method we don't have this problem. For kernel-ioctl method you need the below two Btrfs-progs patches and the following kernel patch which is posted to mailing-list Btrfs: ability to add label to snapshot and subvol fs/btrfs/ctree.h | 12 +- fs/btrfs/ioctl.c | 58 ++ fs/btrfs/transaction.c | 1 + include/uapi/linux/btrfs.h | 2 ++ 4 files changed, 72 insertions(+), 1 deletion(-) Still in any case if we decide to use the attributes method over the kernel-ioctl method, then following btrfs-progs patch Btrfs-progs: add feature to label subvol and snapshot should be replaced with Btrfs-progs: add attribute label for subvol and snapshot (also posted in the mailing-list) and the above kernel patch should be removed. v4: rebased to David's integration-20130219 branch Anand Jain (2): Btrfs-progs: add feature to label subvol and snapshot Btrfs-progs: cmd option to show or set the subvol label btrfslabel.c | 45 + btrfslabel.h | 4 +++- cmds-subvolume.c | 37 + ctree.h | 4 +++- ioctl.h | 2 ++ man/btrfs.8.in | 6 ++ print-tree.c | 2 ++ 7 files changed, 98 insertions(+), 2 deletions(-) -- 1.8.1.227.g44fe835 -- 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: update inode flags when renaming
On Mon, Feb 25, 2013 at 12:23:03PM +0800, Miao Xie wrote: > Onmon, 25 Feb 2013 11:50:01 +0800, Liu Bo wrote: > > On Fri, Feb 22, 2013 at 11:04:40PM +0100, David Sterba wrote: > >> On Fri, Feb 22, 2013 at 05:34:47PM +0800, Miao Xie wrote: > >>> Onfri, 22 Feb 2013 16:40:35 +0800, Liu Bo wrote: > On Fri, Feb 22, 2013 at 03:32:50AM -0500, Marios Titas wrote: > > Sorry, but the bug persists even with the above patch. > > > > touch test > > chattr +C test > > lsattr test > > mv test test2 > > lsattr test2 > > > > In the above scenario test2 will not have the C flag. > > What do you expect? IMO it's right that test2 does not have the C flag. > >>> > >>> No, it's not right. > >>> For the users, they expect the C flag is not lost because they just do > >>> a rename operation. but fixup_inode_flags() re-sets the flags by the > >>> parent directory's flag. > >>> > >>> I think we should inherit the flags from the parent just when we create > >>> a new file/directory, in the other cases, just give a option to the users. > >>> How do you think about? > >> > >> I agree with that. The COW status of a file should not be changed at all > >> when renamed. The typical users are database files and vm images, losing > >> the NOCOW flag just from moving here and back is quite unexpected. > >> > >> david > > > > Yeah, I agree to remove this bad 'change in rename', will send a patch to > > address it. > > I think we can add a mount option, if the option is set, when we move a file > to a new directory, or create a new file, we will inherit the flags of the > parent. > If not set, we inherit the flags only when create a new file. > How do you think about it? > I'm ok with the option, but... from some reports on the list, end users are more likely to control, use chattr files by themselves, inheriting flags via moving a file to a new directory is indeed not very welcomed. So for practical use, I assume that it's fairly enough to inherit flags only on creation? thanks, liubo -- 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: Needed change in Wiki
You may need to update the minimum version of libblkid-dev that is required. Since. latest btrfs-progs needs blkid_probe_get_wholedisk_devno() from the libblkid-dev. found libblkid-dev version 2.17.2 does not work, and 2.22 works. Not sure which version introduced the function required here. HTH Anand On 02/25/2013 01:24 AM, Goffredo Baroncelli wrote: On 02/24/2013 03:13 PM, Hendrik Friedel wrote: Hello, I don't see how to change the wiki, but it needs an update: apt-get build-dep btrfs-tools -or- apt-get install uuid-dev libattr1-dev zlib1g-dev libacl1-dev e2fslibs-dev here libblkid-dev is missing -at least for the latest git version of the btrfs-progs. Updated, thanks Greetings, Hendrik -- 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: update inode flags when renaming
On mon, 25 Feb 2013 11:50:01 +0800, Liu Bo wrote: > On Fri, Feb 22, 2013 at 11:04:40PM +0100, David Sterba wrote: >> On Fri, Feb 22, 2013 at 05:34:47PM +0800, Miao Xie wrote: >>> On fri, 22 Feb 2013 16:40:35 +0800, Liu Bo wrote: On Fri, Feb 22, 2013 at 03:32:50AM -0500, Marios Titas wrote: > Sorry, but the bug persists even with the above patch. > > touch test > chattr +C test > lsattr test > mv test test2 > lsattr test2 > > In the above scenario test2 will not have the C flag. What do you expect? IMO it's right that test2 does not have the C flag. >>> >>> No, it's not right. >>> For the users, they expect the C flag is not lost because they just do >>> a rename operation. but fixup_inode_flags() re-sets the flags by the >>> parent directory's flag. >>> >>> I think we should inherit the flags from the parent just when we create >>> a new file/directory, in the other cases, just give a option to the users. >>> How do you think about? >> >> I agree with that. The COW status of a file should not be changed at all >> when renamed. The typical users are database files and vm images, losing >> the NOCOW flag just from moving here and back is quite unexpected. >> >> david > > Yeah, I agree to remove this bad 'change in rename', will send a patch to > address it. I think we can add a mount option, if the option is set, when we move a file to a new directory, or create a new file, we will inherit the flags of the parent. If not set, we inherit the flags only when create a new file. How do you think about it? Thanks Miao > > thanks, > liubo > -- > 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: do not change inode flags in rename
Before we forced to change a file's NOCOW and COMPRESS flag due to
the parent directory's, but this ends up a bad idea, because it
confuses end users a lot about file's NOCOW status, eg. if someone
change a file to NOCOW via 'chattr' and then rename it in the current
directory which is without NOCOW attribute, the file will lose the
NOCOW flag silently.
This diables 'change flags in rename', so from now on we'll only
inherit flags from the parent directory on creation stage while in
other places we can use 'chattr' to set NOCOW or COMPRESS flags.
Reported-by: Marios Titas
Signed-off-by: Liu Bo
---
fs/btrfs/inode.c | 25 -
1 files changed, 0 insertions(+), 25 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index d9984fa..383a7d8 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -7325,29 +7325,6 @@ static int btrfs_getattr(struct vfsmount *mnt,
return 0;
}
-/*
- * If a file is moved, it will inherit the cow and compression flags of the new
- * directory.
- */
-static void fixup_inode_flags(struct inode *dir, struct inode *inode)
-{
- struct btrfs_inode *b_dir = BTRFS_I(dir);
- struct btrfs_inode *b_inode = BTRFS_I(inode);
-
- if (b_dir->flags & BTRFS_INODE_NODATACOW)
- b_inode->flags |= BTRFS_INODE_NODATACOW;
- else
- b_inode->flags &= ~BTRFS_INODE_NODATACOW;
-
- if (b_dir->flags & BTRFS_INODE_COMPRESS) {
- b_inode->flags |= BTRFS_INODE_COMPRESS;
- b_inode->flags &= ~BTRFS_INODE_NOCOMPRESS;
- } else {
- b_inode->flags &= ~(BTRFS_INODE_COMPRESS |
- BTRFS_INODE_NOCOMPRESS);
- }
-}
-
static int btrfs_rename(struct inode *old_dir, struct dentry *old_dentry,
struct inode *new_dir, struct dentry *new_dentry)
{
@@ -7513,8 +7490,6 @@ static int btrfs_rename(struct inode *old_dir, struct
dentry *old_dentry,
}
}
- fixup_inode_flags(new_dir, old_inode);
-
ret = btrfs_add_link(trans, new_dir, old_inode,
new_dentry->d_name.name,
new_dentry->d_name.len, 0, index);
--
1.7.7.6
--
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: update inode flags when renaming
On Fri, Feb 22, 2013 at 11:04:40PM +0100, David Sterba wrote: > On Fri, Feb 22, 2013 at 05:34:47PM +0800, Miao Xie wrote: > > On fri, 22 Feb 2013 16:40:35 +0800, Liu Bo wrote: > > > On Fri, Feb 22, 2013 at 03:32:50AM -0500, Marios Titas wrote: > > >> Sorry, but the bug persists even with the above patch. > > >> > > >> touch test > > >> chattr +C test > > >> lsattr test > > >> mv test test2 > > >> lsattr test2 > > >> > > >> In the above scenario test2 will not have the C flag. > > > > > > What do you expect? IMO it's right that test2 does not have the C flag. > > > > No, it's not right. > > For the users, they expect the C flag is not lost because they just do > > a rename operation. but fixup_inode_flags() re-sets the flags by the > > parent directory's flag. > > > > I think we should inherit the flags from the parent just when we create > > a new file/directory, in the other cases, just give a option to the users. > > How do you think about? > > I agree with that. The COW status of a file should not be changed at all > when renamed. The typical users are database files and vm images, losing > the NOCOW flag just from moving here and back is quite unexpected. > > david Yeah, I agree to remove this bad 'change in rename', will send a patch to address it. thanks, liubo -- 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
WARNING: at fs/btrfs/inode.c:2165 btrfs_orphan_commit_root+0xcb/0xdf()
Is this useful to anyone?
Got this after a crash/reboot:
if (block_rsv) {
WARN_ON(block_rsv->size > 0); <<
btrfs_free_block_rsv(root, block_rsv);
}
[ cut here ]
WARNING: at fs/btrfs/inode.c:2165 btrfs_orphan_commit_root+0xcb/0xdf()
Hardware name: 2429A78
Modules linked in: tun ppdev cpufreq_userspace cpufreq_stats
cpufreq_conservative cpufreq_powersave rfcomm bnep autofs4 pci_stub vboxpci(O)
vboxnetadp(O) vboxnetflt(O) vboxdrv(O) binfmt_misc uinput nfsd auth_rpcgss
nfs_acl nfs lockd fscache sunrpc fuse configs parport_pc lp parport
input_polldev loop firewire_sbp2 firewire_core crc_itu_t ecryptfs
snd_hda_codec_realtek uvcvideo videobuf2_vmalloc videobuf2_memops
videobuf2_core videodev media btusb bluetooth hid_generic usbhid hid arc4
coretemp kvm_intel kvm iwldvm ghash_clmulni_intel mac80211 snd_hda_intel
snd_hda_codec snd_hwdep snd_pcm_oss snd_mixer_oss iTCO_wdt snd_pcm
iTCO_vendor_support snd_page_alloc snd_seq_midi snd_seq_midi_event snd_rawmidi
snd_seq battery thinkpad_acpi tpm_tis nvram ac tpm snd_seq_device tpm_bios
snd_timer snd evdev iwlwifi wmi cfg80211 sdhci_pci i915 rfkill sdhci microcode
mmc_core video pcspkr psmouse lpc_ich e1000e soundcore serio_raw button mei
drm_kms_helper drm xhci_hcd ehci_hcd i2c_i801 i2c_algo_bit acpi_cpufreq mperf
i2c_core usbcore usb_common processor raid456 multipath dm_snapshot dm_mirror
dm_region_hash dm_log dm_crypt dm_mod async_raid6_recov async_pq async_xor
raid6_pq async_memcpy async_tx xor blowfish_x86_64 blowfish_common ecb
crc32c_intel aesni_intel xts aes_x86_64 lrw gf128mul ablk_helper cryptd thermal
thermal_sys
Pid: 500, comm: btrfs-transacti Tainted: G O
3.7.8-amd64-preempt-20130222 #1
Call Trace:
[] warn_slowpath_common+0x7e/0x96
[] ? alloc_extent_state+0x59/0xa4
[] warn_slowpath_null+0x15/0x17
[] btrfs_orphan_commit_root+0xcb/0xdf
[] commit_fs_roots.isra.24+0x99/0x153
[] ? _raw_spin_lock+0x1b/0x1f
[] ? _raw_spin_unlock+0x27/0x32
[] btrfs_commit_transaction+0x45a/0x954
[] ? add_wait_queue+0x44/0x44
[] transaction_kthread+0xe7/0x18a
[] ? try_to_freeze+0x33/0x33
[] kthread+0x88/0x90
[] ? kthread_freezable_should_stop+0x39/0x39
[] ret_from_fork+0x7c/0xb0
[] ? kthread_freezable_should_stop+0x39/0x39
---[ end trace ee93ff454aca9d81 ]---
--
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems
what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/
--
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/8] Add some helpers to manage the strings allocation/deallocation.
On 2/23/13 7:46 AM, Goffredo Baroncelli wrote:
> From: Goffredo Baroncelli
>
> This patch adds some helpers to manage the strings allocation and
> deallocation.
> The function string_list_add(char *) adds the passed string to a list;
> the function string_list_free() frees all the strings together.
I have to say, I agree with Zach that there are more straightforward
and less error-prone ways to accomplish this same result.
You said:
> your suggestion is not so easy applicable
to Zach's suggestion. Why do you consider it to be not applicable?
Maybe I am missing some subtlety, but it'd help if you could explain
the problem that you see.
I think that the asymmetry in i.e. _cmd_disk_free where you do:
+ printf("Disk size:\t\t%*s\n", width,
+ df_pretty_sizes(total_disk, mode));
and then:
+ string_list_free();
... because obviously (?) "df_pretty_sizes" needs this cleanup ...
is unexpected, magic, and error-prone.
-Eric
thanks,
-Eric
> Signed-off-by: Goffredo Baroncelli
> ---
> Makefile |3 ++-
> string_list.c | 63
> +
> string_list.h | 23 +
> 3 files changed, 88 insertions(+), 1 deletion(-)
> create mode 100644 string_list.c
> create mode 100644 string_list.h
>
> diff --git a/Makefile b/Makefile
> index 596bf93..0d6c43a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -5,7 +5,8 @@ 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 raid6.o
> + send-stream.o send-utils.o qgroup.o raid6.o \
> + string_list.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 cmds-replace.o
> diff --git a/string_list.c b/string_list.c
> new file mode 100644
> index 000..8d8cc1f
> --- /dev/null
> +++ b/string_list.c
> @@ -0,0 +1,63 @@
> +/*
> + * 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
> +#include
> +#include
> +#include
> +
> +/* To store the strings */
> +static void **strings_to_free;
> +static int count_string_to_free;
> +
> +/*
> + * Add a string to the dynamic allocated string list
> + */
> +char *string_list_add(char *s)
> +{
> + int size;
> +
> + size = sizeof(void *) * ++count_string_to_free;
> + strings_to_free = realloc(strings_to_free, size);
> +
> + /* if we don't have enough memory, we have more serius
> +problem than that a wrong handling of not enough memory */
> + if (!strings_to_free) {
> + fprintf(stderr, "add_string_to_free(): Not enough memory\n");
> + count_string_to_free = 0;
> + return NULL;
> + }
> +
> + strings_to_free[count_string_to_free-1] = s;
> + return s;
> +}
> +
> +/*
> + * Free the dynamic allocated strings list
> + */
> +void string_list_free()
> +{
> + int i;
> + for (i = 0 ; i < count_string_to_free ; i++)
> + free(strings_to_free[i]);
> +
> + free(strings_to_free);
> +
> + strings_to_free = 0;
> + count_string_to_free = 0;
> +}
> +
> +
> diff --git a/string_list.h b/string_list.h
> new file mode 100644
> index 000..f974fbc
> --- /dev/null
> +++ b/string_list.h
> @@ -0,0 +1,23 @@
> +/*
> + * 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 STRING_LIST_H
> +#define STRING_LIST_H
> +
> +void string_list_add(char *s);
> +void string_l
Re: [PATCH 2/3] Btrfs: fix the deadlock between the transaction start/attach and commit
Hi Miao,
can you please explain your solution a bit more.
On Wed, Feb 20, 2013 at 11:16 AM, Miao Xie wrote:
> Now btrfs_commit_transaction() does this
>
> ret = btrfs_run_ordered_operations(root, 0)
>
> which async flushes all inodes on the ordered operations list, it introduced
> a deadlock that transaction-start task, transaction-commit task and the flush
> workers waited for each other.
> (See the following URL to get the detail
> http://marc.info/?l=linux-btrfs&m=136070705732646&w=2)
>
> As we know, if ->in_commit is set, it means someone is committing the
> current transaction, we should not try to join it if we are not JOIN
> or JOIN_NOLOCK, wait is the best choice for it. In this way, we can avoid
> the above problem. In this way, there is another benefit: there is no new
> transaction handle to block the transaction which is on the way of commit,
> once we set ->in_commit.
>
> Signed-off-by: Miao Xie
> ---
> fs/btrfs/transaction.c | 17 -
> 1 files changed, 16 insertions(+), 1 deletions(-)
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index bc2f2d1..71b7e2e 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -51,6 +51,14 @@ static noinline void switch_commit_root(struct btrfs_root
> *root)
> root->commit_root = btrfs_root_node(root);
> }
>
> +static inline int can_join_transaction(struct btrfs_transaction *trans,
> + int type)
> +{
> + return !(trans->in_commit &&
> +type != TRANS_JOIN &&
> +type != TRANS_JOIN_NOLOCK);
> +}
> +
> /*
> * either allocate a new transaction or hop into the existing one
> */
> @@ -86,6 +94,10 @@ loop:
> spin_unlock(&fs_info->trans_lock);
> return cur_trans->aborted;
> }
> + if (!can_join_transaction(cur_trans, type)) {
> + spin_unlock(&fs_info->trans_lock);
> + return -EBUSY;
> + }
> atomic_inc(&cur_trans->use_count);
> atomic_inc(&cur_trans->num_writers);
> cur_trans->num_joined++;
> @@ -360,8 +372,11 @@ again:
>
> do {
> ret = join_transaction(root, type);
> - if (ret == -EBUSY)
> + if (ret == -EBUSY) {
> wait_current_trans(root);
> + if (unlikely(type == TRANS_ATTACH))
> + ret = -ENOENT;
> + }
So I understand that instead of incrementing num_writes and joining
the current transaction, you do not join and wait for the current
transaction to unblock.
Which task in Josef's example
http://marc.info/?l=linux-btrfs&m=136070705732646&w=2
task 1, task 2 or task 3 is the one that will not join the
transaction, but instead wait?
Also, I think I don't fully understand Josef's example. What is
preventing from async flushing to complete?
Is task 3 waiting because trans_no_join is set?
Is task 3 the one that actually does the delalloc flush?
Thanks,
Alex.
> } while (ret == -EBUSY);
>
> if (ret < 0) {
> --
> 1.6.5.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
--
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: Needed change in Wiki
On 02/24/2013 03:13 PM, Hendrik Friedel wrote: > Hello, > > I don't see how to change the wiki, but it needs an update: > > apt-get build-dep btrfs-tools > -or- > apt-get install uuid-dev libattr1-dev zlib1g-dev libacl1-dev e2fslibs-dev > > here libblkid-dev is missing -at least for the latest git version of the > btrfs-progs. Updated, thanks > Greetings, > Hendrik > -- > 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 > -- gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it> Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5 -- 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
Needed change in Wiki
Hello, I don't see how to change the wiki, but it needs an update: apt-get build-dep btrfs-tools -or- apt-get install uuid-dev libattr1-dev zlib1g-dev libacl1-dev e2fslibs-dev here libblkid-dev is missing -at least for the latest git version of the btrfs-progs. Greetings, Hendrik -- 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
