linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Btrfs: set mount options permanently
@ 2012-09-18  1:25 Hidetoshi Seto
  2012-09-18  1:28 ` [PATCH 1/2] Btrfs: make space to keep default mount options Hidetoshi Seto
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Hidetoshi Seto @ 2012-09-18  1:25 UTC (permalink / raw)
  To: linux-btrfs

Following patches are going to implement one of unclaimed features
listed in the btrfs wiki:

https://btrfs.wiki.kernel.org/index.php/Project_ideas#Set_mount_options_permanently

Special thanks to Kazuhiro Yamashita for his time and efforts.
Your comments/reviews are welcomed.

Thanks,
H.Seto


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/2] Btrfs: make space to keep default mount options
  2012-09-18  1:25 [PATCH 0/2] Btrfs: set mount options permanently Hidetoshi Seto
@ 2012-09-18  1:28 ` Hidetoshi Seto
  2012-09-18 12:10   ` David Sterba
  2012-09-18  1:30 ` [PATCH 2/2] Btrfs-progs: add mount-option command Hidetoshi Seto
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Hidetoshi Seto @ 2012-09-18  1:28 UTC (permalink / raw)
  To: linux-btrfs

This patch create space to hold default mount option,
and to use saved default mount option change super.c
to read default mount option first when mount devices.

Signed-off-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
---
 fs/btrfs/ctree.h |    5 ++++-
 fs/btrfs/super.c |    2 ++
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index fa5c45b..3eb0551 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -458,8 +458,11 @@ struct btrfs_super_block {
 
 	__le64 cache_generation;
 
+	/* default mount options */
+	unsigned long default_mount_opt;
+
 	/* future expansion */
-	__le64 reserved[31];
+	__le64 reserved[30];
 	u8 sys_chunk_array[BTRFS_SYSTEM_CHUNK_ARRAY_SIZE];
 	struct btrfs_root_backup super_roots[BTRFS_NUM_BACKUP_ROOTS];
 } __attribute__ ((__packed__));
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index e239915..7ef4a2e 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -340,6 +340,8 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
 	char *compress_type;
 	bool compress_force = false;
 
+	info->mount_opt = info->super_copy->default_mount_opt;
+
 	cache_gen = btrfs_super_cache_generation(root->fs_info->super_copy);
 	if (cache_gen)
 		btrfs_set_opt(info->mount_opt, SPACE_CACHE);
-- 
1.7.7.6



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/2] Btrfs-progs: add mount-option command
  2012-09-18  1:25 [PATCH 0/2] Btrfs: set mount options permanently Hidetoshi Seto
  2012-09-18  1:28 ` [PATCH 1/2] Btrfs: make space to keep default mount options Hidetoshi Seto
@ 2012-09-18  1:30 ` Hidetoshi Seto
  2012-09-18  2:31   ` Miao Xie
  2012-09-18 12:30   ` David Sterba
  2012-09-18 10:03 ` R: " Goffredo Baroncelli <kreijack@libero.it>
  2012-09-18 11:37 ` R: " Goffredo Baroncelli <kreijack@libero.it>
  3 siblings, 2 replies; 18+ messages in thread
From: Hidetoshi Seto @ 2012-09-18  1:30 UTC (permalink / raw)
  To: linux-btrfs

This patch adds mount-option command.
The command can set/get default mount options.
Now, the command can set/get 24 options.
These options are equal to mount options which store
in fs_info/mount-opt.

Signed-off-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
---
 Makefile             |    5 +-
 btrfs-parse-mntopt.c |  111 +++++++++++++++++++++++++++++++++++++
 btrfs-parse-mntopt.h |   65 ++++++++++++++++++++++
 btrfs.c              |    1 +
 cmds-mount.c         |  150 ++++++++++++++++++++++++++++++++++++++++++++++++++
 commands.h           |    2 +
 ctree.h              |   41 +++++++++++++-
 7 files changed, 372 insertions(+), 3 deletions(-)
 create mode 100644 btrfs-parse-mntopt.c
 create mode 100644 btrfs-parse-mntopt.h
 create mode 100644 cmds-mount.c

diff --git a/Makefile b/Makefile
index c0aaa3d..6f67f4c 100644
--- a/Makefile
+++ b/Makefile
@@ -5,9 +5,10 @@ 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
+	  send-stream.o send-utils.o btrfs-parse-mntopt.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-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \
+	       cmds-mount.o
 
 CHECKFLAGS= -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ -Wbitwise \
 	    -Wuninitialized -Wshadow -Wundef
diff --git a/btrfs-parse-mntopt.c b/btrfs-parse-mntopt.c
new file mode 100644
index 0000000..87b341c
--- /dev/null
+++ b/btrfs-parse-mntopt.c
@@ -0,0 +1,111 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include "ctree.h"
+#include "btrfs-parse-mntopt.h"
+
+void btrfs_parse_string2mntopt(struct btrfs_root *root, char **options)
+{
+	struct btrfs_super_block *sb = &root->fs_info->super_copy;
+	char *p = NULL;
+	int i = 0;
+
+	memset(&sb->default_mount_opt, 0, sizeof(unsigned long));
+	while ((p = strsep(options, ",")) != NULL) {
+		int token = DEF_MNTOPT_NUM + 1;
+
+		if (!*p)
+			continue;
+		for (i = 0; i < DEF_MNTOPT_NUM; i++) {
+			if (!strcmp(p, toke[i].pattern)) {
+				token = toke[i].token;
+				break;
+			}
+		}
+		if (token > DEF_MNTOPT_NUM) {
+			printf("error: %s\n", p);
+			return;
+		}
+
+		switch (token) {
+		case Opt_degraded:
+			btrfs_set_opt(sb->default_mount_opt, DEGRADED);
+			break;
+
+		case Opt_nodatasum:
+			btrfs_set_opt(sb->default_mount_opt, NODATASUM);
+			break;
+		case Opt_nodatacow:
+			btrfs_set_opt(sb->default_mount_opt, NODATACOW);
+			btrfs_set_opt(sb->default_mount_opt, NODATASUM);
+			break;
+		case Opt_ssd:
+			btrfs_set_opt(sb->default_mount_opt, SSD);
+			break;
+		case Opt_ssd_spread:
+			btrfs_set_opt(sb->default_mount_opt, SSD);
+			btrfs_set_opt(sb->default_mount_opt, SSD_SPREAD);
+			break;
+		case Opt_nossd:
+			btrfs_set_opt(sb->default_mount_opt, NOSSD);
+			btrfs_clear_opt(sb->default_mount_opt, SSD);
+			btrfs_clear_opt(sb->default_mount_opt, SSD_SPREAD);
+			break;
+		case Opt_nobarrier:
+			btrfs_set_opt(sb->default_mount_opt, NOBARRIER);
+			break;
+		case Opt_notreelog:
+			btrfs_set_opt(sb->default_mount_opt, NOTREELOG);
+			break;
+		case Opt_flushoncommit:
+			btrfs_set_opt(sb->default_mount_opt, FLUSHONCOMMIT);
+			break;
+		case Opt_discard:
+			btrfs_set_opt(sb->default_mount_opt, DISCARD);
+			break;
+		case Opt_space_cache:
+			btrfs_set_opt(sb->default_mount_opt, SPACE_CACHE);
+			break;
+		case Opt_no_space_cache:
+			btrfs_clear_opt(sb->default_mount_opt, SPACE_CACHE);
+			break;
+		case Opt_inode_cache:
+			btrfs_set_opt(sb->default_mount_opt, INODE_MAP_CACHE);
+			break;
+		case Opt_clear_cache:
+			btrfs_set_opt(sb->default_mount_opt, CLEAR_CACHE);
+			break;
+		case Opt_user_subvol_rm_allowed:
+			btrfs_set_opt(sb->default_mount_opt,
+					USER_SUBVOL_RM_ALLOWED);
+			break;
+		case Opt_enospc_debug:
+			btrfs_set_opt(sb->default_mount_opt, ENOSPC_DEBUG);
+			break;
+		case Opt_defrag:
+			btrfs_set_opt(sb->default_mount_opt, AUTO_DEFRAG);
+			break;
+		case Opt_recovery:
+			btrfs_set_opt(sb->default_mount_opt, RECOVERY);
+			break;
+		case Opt_skip_balance:
+			btrfs_set_opt(sb->default_mount_opt, SKIP_BALANCE);
+			break;
+		default:
+			break;
+		}
+	}
+}
+
+void btrfs_parse_mntopt2string(unsigned long def_opt)
+{
+	if (!def_opt)
+		printf("no default options\n");
+	else {
+		int i = 0;
+		for (i = 0; i < DEF_MNTOPT_NUM; i++) {
+			if (def_opt & (1 << toke[i].token))
+				printf("%s\n", toke[i].pattern);
+		}
+	}
+}
diff --git a/btrfs-parse-mntopt.h b/btrfs-parse-mntopt.h
new file mode 100644
index 0000000..a2745ee
--- /dev/null
+++ b/btrfs-parse-mntopt.h
@@ -0,0 +1,65 @@
+/*
+ * 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.
+ */
+
+struct match_token {
+	int token;
+	const char *pattern;
+};
+
+typedef struct match_token match_table_t[];
+
+enum {
+	Opt_nodatasum, Opt_nodatacow, Opt_nobarrier, Opt_ssd,
+	Opt_degraded, Opt_compress, Opt_notreelog, Opt_flushoncommit,
+	Opt_ssd_spread, Opt_nossd, Opt_discard, Opt_compress_force,
+	Opt_space_cache, Opt_clear_cache, Opt_user_subvol_rm_allowed,
+	Opt_enospc_debug, Opt_defrag, Opt_inode_cache, Opt_recovery,
+	Opt_skip_balance, Opt_check_integrity,
+	Opt_check_integrity_including_extent_data, Opt_fatal_errors,
+	Opt_no_space_cache, DEF_MNTOPT_NUM,
+};
+
+static match_table_t toke = {
+	{Opt_degraded, "degraded"},
+	{Opt_nodatasum, "nodatasum"},
+	{Opt_nodatacow, "nodatacow"},
+	{Opt_nobarrier, "nobarrier"},
+	{Opt_compress, "compress"},
+	{Opt_compress_force, "compress-force"},
+	{Opt_ssd, "ssd"},
+	{Opt_ssd_spread, "ssd_spread"},
+	{Opt_nossd, "nossd"},
+	{Opt_notreelog, "notreelog"},
+	{Opt_flushoncommit, "flushoncommit"},
+	{Opt_discard, "discard"},
+	{Opt_space_cache, "space_cache"},
+	{Opt_no_space_cache, "no_space_cache"},
+	{Opt_clear_cache, "clear_cache"},
+	{Opt_user_subvol_rm_allowed, "user_subvol_rm_allowed"},
+	{Opt_enospc_debug, "enospc_debug"},
+	{Opt_defrag, "auto_defrag"},
+	{Opt_inode_cache, "inode_map_cache"},
+	{Opt_recovery, "recovery"},
+	{Opt_skip_balance, "skip_balance"},
+	{Opt_check_integrity, "check_int"},
+	{Opt_check_integrity_including_extent_data, "check_int_data"},
+	{Opt_fatal_errors, "fatal_errors"},
+};
+
+void btrfs_parse_string2mntopt(struct btrfs_root *root, char **options);
+void btrfs_parse_mntopt2string(unsigned long def_opt);
diff --git a/btrfs.c b/btrfs.c
index e9d54f8..0d6c9a7 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -246,6 +246,7 @@ const struct cmd_group btrfs_cmd_group = {
 		{ "device", cmd_device, NULL, &device_cmd_group, 0 },
 		{ "scrub", cmd_scrub, NULL, &scrub_cmd_group, 0 },
 		{ "inspect-internal", cmd_inspect, NULL, &inspect_cmd_group, 0 },
+		{ "mount-option", cmd_mount, NULL, &mount_cmd_group, 0 },
 		{ "send", cmd_send, NULL, &send_cmd_group, 0 },
 		{ "receive", cmd_receive, NULL, &receive_cmd_group, 0 },
 		{ "help", cmd_help, cmd_help_usage, NULL, 0 },
diff --git a/cmds-mount.c b/cmds-mount.c
new file mode 100644
index 0000000..cbdb006
--- /dev/null
+++ b/cmds-mount.c
@@ -0,0 +1,150 @@
+/*
+ * 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 <sys/ioctl.h>
+#include "ioctl.h"
+#include "ctree.h"
+#include "transaction.h"
+#include "commands.h"
+#include "disk-io.h"
+#include "utils.h"
+#include "btrfs-parse-mntopt.h"
+
+static const char * const mount_cmd_group_usage[] = {
+	"btrfs mount-option <command> [<args>]",
+	NULL
+};
+
+static const char * const cmd_set_mntopt_usage[] = {
+	"btrfs mount-option set [<option>,...] <device>",
+	"Set default mount options",
+	NULL
+};
+
+static int cmd_set_mntopt(int argc, char **argv)
+{
+	struct btrfs_root *root = NULL;
+	struct btrfs_trans_handle *trans = NULL;
+	int ret = 0;
+
+	if (argc != 3)
+		usage(cmd_set_mntopt_usage);
+
+	if (check_mounted(argv[argc - 1])) {
+		fprintf(stderr, "%s is mounted\n", argv[argc - 1]);
+		return 1;
+	}
+
+	root = open_ctree(argv[argc - 1], 0, 1);
+
+	if (!root)
+		printf("error root\n");
+
+	trans = btrfs_start_transaction(root, 1);
+	btrfs_parse_string2mntopt(root, &argv[1]);
+	ret = btrfs_commit_transaction(trans, root);
+
+	close_ctree(root);
+	return ret;
+};
+
+static const char * const cmd_get_mntopt_usage[] = {
+	"btrfs mount-option get <device>",
+	"Get default mount options",
+	NULL
+};
+
+static int cmd_get_mntopt(int argc, char **argv)
+{
+	struct btrfs_root *root = NULL;
+	unsigned long def_opt;
+
+	memset(&def_opt, 0, sizeof(def_opt));
+	if (argc != 2) {
+		usage(cmd_get_mntopt_usage);
+		return 1;
+	}
+
+	if (check_mounted(argv[argc - 1])) {
+		fprintf(stderr, "%s is mounted\n", argv[argc - 1]);
+		return 1;
+	}
+
+	root = open_ctree(argv[argc - 1], 0, 0);
+
+	if (!root) {
+		printf("error root\n");
+		return 1;
+	}
+
+	def_opt = btrfs_super_default_mount_opt(&root->fs_info->super_copy);
+
+	btrfs_parse_mntopt2string(def_opt);
+
+	close_ctree(root);
+	return 0;
+};
+
+static const char * const cmd_clear_mntopt_usage[] = {
+	"btrfs mount-option clear <device>",
+	"Clear default mount options",
+	NULL
+};
+
+static int cmd_clear_mntopt(int argc, char **argv)
+{
+	struct btrfs_root *root = NULL;
+	struct btrfs_trans_handle *trans = NULL;
+	int ret = 0;
+
+	if (argc != 2) {
+		usage(cmd_clear_mntopt_usage);
+		return 1;
+	}
+
+	if (check_mounted(argv[argc - 1])) {
+		fprintf(stderr, "%s is mounted\n", argv[argc - 1]);
+		return 1;
+	}
+
+	root = open_ctree(argv[argc - 1], 0, 1);
+
+	if (!root)
+		printf("error root\n");
+
+	trans = btrfs_start_transaction(root, 1);
+	btrfs_set_super_default_mount_opt(&root->fs_info->super_copy, 0);
+	ret = btrfs_commit_transaction(trans, root);
+
+	close_ctree(root);
+	return ret;
+};
+
+const struct cmd_group mount_cmd_group = {
+	mount_cmd_group_usage, NULL, {
+		{ "set", cmd_set_mntopt, cmd_set_mntopt_usage, NULL, 0 },
+		{ "get", cmd_get_mntopt, cmd_get_mntopt_usage, NULL, 0 },
+		{ "clear", cmd_clear_mntopt, cmd_clear_mntopt_usage, NULL, 0},
+		{0, 0, 0, 0, 0 }
+	}
+};
+
+int cmd_mount(int argc, char **argv)
+{
+	return handle_command_group(&mount_cmd_group, argc, argv);
+}
diff --git a/commands.h b/commands.h
index 1ece87a..6cf91fc 100644
--- a/commands.h
+++ b/commands.h
@@ -88,6 +88,7 @@ extern const struct cmd_group balance_cmd_group;
 extern const struct cmd_group device_cmd_group;
 extern const struct cmd_group scrub_cmd_group;
 extern const struct cmd_group inspect_cmd_group;
+extern const struct cmd_group mount_cmd_group;
 extern const struct cmd_group send_cmd_group;
 extern const struct cmd_group receive_cmd_group;
 
@@ -97,5 +98,6 @@ int cmd_balance(int argc, char **argv);
 int cmd_device(int argc, char **argv);
 int cmd_scrub(int argc, char **argv);
 int cmd_inspect(int argc, char **argv);
+int cmd_mount(int argc, char **argv);
 int cmd_send(int argc, char **argv);
 int cmd_receive(int argc, char **argv);
diff --git a/ctree.h b/ctree.h
index d218b88..4a1bb5b 100644
--- a/ctree.h
+++ b/ctree.h
@@ -401,8 +401,11 @@ struct btrfs_super_block {
 
 	__le64 cache_generation;
 
+	/* default mount options */
+	unsigned long default_mount_opt;
+
 	/* future expansion */
-	__le64 reserved[31];
+	__le64 reserved[30];
 	u8 sys_chunk_array[BTRFS_SYSTEM_CHUNK_ARRAY_SIZE];
 	struct btrfs_root_backup super_roots[BTRFS_NUM_BACKUP_ROOTS];
 } __attribute__ ((__packed__));
@@ -1774,6 +1777,8 @@ BTRFS_SETGET_STACK_FUNCS(super_csum_type, struct btrfs_super_block,
 			 csum_type, 16);
 BTRFS_SETGET_STACK_FUNCS(super_cache_generation, struct btrfs_super_block,
 			 cache_generation, 64);
+BTRFS_SETGET_STACK_FUNCS(super_default_mount_opt, struct btrfs_super_block,
+			 default_mount_opt, 64);
 
 static inline int btrfs_super_csum_size(struct btrfs_super_block *s)
 {
@@ -2128,3 +2133,37 @@ int btrfs_csum_truncate(struct btrfs_trans_handle *trans,
 			struct btrfs_root *root, struct btrfs_path *path,
 			u64 isize);
 #endif
+
+/*
+ * Flags for mount options.
+ *
+ * Note: don't forget to add new options to btrfs_show_options()
+ */
+#define BTRFS_MOUNT_NODATASUM		(1 << 0)
+#define BTRFS_MOUNT_NODATACOW		(1 << 1)
+#define BTRFS_MOUNT_NOBARRIER		(1 << 2)
+#define BTRFS_MOUNT_SSD			(1 << 3)
+#define BTRFS_MOUNT_DEGRADED		(1 << 4)
+#define BTRFS_MOUNT_COMPRESS		(1 << 5)
+#define BTRFS_MOUNT_NOTREELOG           (1 << 6)
+#define BTRFS_MOUNT_FLUSHONCOMMIT       (1 << 7)
+#define BTRFS_MOUNT_SSD_SPREAD		(1 << 8)
+#define BTRFS_MOUNT_NOSSD		(1 << 9)
+#define BTRFS_MOUNT_DISCARD		(1 << 10)
+#define BTRFS_MOUNT_FORCE_COMPRESS      (1 << 11)
+#define BTRFS_MOUNT_SPACE_CACHE		(1 << 12)
+#define BTRFS_MOUNT_CLEAR_CACHE		(1 << 13)
+#define BTRFS_MOUNT_USER_SUBVOL_RM_ALLOWED (1 << 14)
+#define BTRFS_MOUNT_ENOSPC_DEBUG	 (1 << 15)
+#define BTRFS_MOUNT_AUTO_DEFRAG		(1 << 16)
+#define BTRFS_MOUNT_INODE_MAP_CACHE	(1 << 17)
+#define BTRFS_MOUNT_RECOVERY		(1 << 18)
+#define BTRFS_MOUNT_SKIP_BALANCE	(1 << 19)
+#define BTRFS_MOUNT_CHECK_INTEGRITY	(1 << 20)
+#define BTRFS_MOUNT_CHECK_INTEGRITY_INCLUDING_EXTENT_DATA (1 << 21)
+#define BTRFS_MOUNT_PANIC_ON_FATAL_ERROR	(1 << 22)
+
+#define btrfs_clear_opt(o, opt)		((o) &= ~BTRFS_MOUNT_##opt)
+#define btrfs_set_opt(o, opt)		((o) |= BTRFS_MOUNT_##opt)
+#define btrfs_test_opt(root, opt)	((root)->fs_info->mount_opt& \
+					 BTRFS_MOUNT_##opt)
-- 
1.7.7.6



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] Btrfs-progs: add mount-option command
  2012-09-18  1:30 ` [PATCH 2/2] Btrfs-progs: add mount-option command Hidetoshi Seto
@ 2012-09-18  2:31   ` Miao Xie
  2012-09-18  4:19     ` Roman Mamedov
  2012-09-19  8:32     ` Hidetoshi Seto
  2012-09-18 12:30   ` David Sterba
  1 sibling, 2 replies; 18+ messages in thread
From: Miao Xie @ 2012-09-18  2:31 UTC (permalink / raw)
  To: Hidetoshi Seto; +Cc: linux-btrfs

On tue, 18 Sep 2012 10:30:17 +0900, Hidetoshi Seto wrote:
> This patch adds mount-option command.
> The command can set/get default mount options.
> Now, the command can set/get 24 options.
> These options are equal to mount options which store
> in fs_info/mount-opt.

I don't think we need implement a separate command to do this,
we can add it into btrfstune just like ext3/4. If so, the users
who used ext3/4 before can be familiar with btrfs command as soon
as possible.

Beside that, why not add a option into mkfs.btrfs?

Thanks
Miao

> 
> Signed-off-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
> ---
>  Makefile             |    5 +-
>  btrfs-parse-mntopt.c |  111 +++++++++++++++++++++++++++++++++++++
>  btrfs-parse-mntopt.h |   65 ++++++++++++++++++++++
>  btrfs.c              |    1 +
>  cmds-mount.c         |  150 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  commands.h           |    2 +
>  ctree.h              |   41 +++++++++++++-
>  7 files changed, 372 insertions(+), 3 deletions(-)
>  create mode 100644 btrfs-parse-mntopt.c
>  create mode 100644 btrfs-parse-mntopt.h
>  create mode 100644 cmds-mount.c
> 
> diff --git a/Makefile b/Makefile
> index c0aaa3d..6f67f4c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -5,9 +5,10 @@ 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
> +	  send-stream.o send-utils.o btrfs-parse-mntopt.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-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \
> +	       cmds-mount.o
>  
>  CHECKFLAGS= -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ -Wbitwise \
>  	    -Wuninitialized -Wshadow -Wundef
> diff --git a/btrfs-parse-mntopt.c b/btrfs-parse-mntopt.c
> new file mode 100644
> index 0000000..87b341c
> --- /dev/null
> +++ b/btrfs-parse-mntopt.c
> @@ -0,0 +1,111 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include "ctree.h"
> +#include "btrfs-parse-mntopt.h"
> +
> +void btrfs_parse_string2mntopt(struct btrfs_root *root, char **options)
> +{
> +	struct btrfs_super_block *sb = &root->fs_info->super_copy;
> +	char *p = NULL;
> +	int i = 0;
> +
> +	memset(&sb->default_mount_opt, 0, sizeof(unsigned long));
> +	while ((p = strsep(options, ",")) != NULL) {
> +		int token = DEF_MNTOPT_NUM + 1;
> +
> +		if (!*p)
> +			continue;
> +		for (i = 0; i < DEF_MNTOPT_NUM; i++) {
> +			if (!strcmp(p, toke[i].pattern)) {
> +				token = toke[i].token;
> +				break;
> +			}
> +		}
> +		if (token > DEF_MNTOPT_NUM) {
> +			printf("error: %s\n", p);
> +			return;
> +		}
> +
> +		switch (token) {
> +		case Opt_degraded:
> +			btrfs_set_opt(sb->default_mount_opt, DEGRADED);
> +			break;
> +
> +		case Opt_nodatasum:
> +			btrfs_set_opt(sb->default_mount_opt, NODATASUM);
> +			break;
> +		case Opt_nodatacow:
> +			btrfs_set_opt(sb->default_mount_opt, NODATACOW);
> +			btrfs_set_opt(sb->default_mount_opt, NODATASUM);
> +			break;
> +		case Opt_ssd:
> +			btrfs_set_opt(sb->default_mount_opt, SSD);
> +			break;
> +		case Opt_ssd_spread:
> +			btrfs_set_opt(sb->default_mount_opt, SSD);
> +			btrfs_set_opt(sb->default_mount_opt, SSD_SPREAD);
> +			break;
> +		case Opt_nossd:
> +			btrfs_set_opt(sb->default_mount_opt, NOSSD);
> +			btrfs_clear_opt(sb->default_mount_opt, SSD);
> +			btrfs_clear_opt(sb->default_mount_opt, SSD_SPREAD);
> +			break;
> +		case Opt_nobarrier:
> +			btrfs_set_opt(sb->default_mount_opt, NOBARRIER);
> +			break;
> +		case Opt_notreelog:
> +			btrfs_set_opt(sb->default_mount_opt, NOTREELOG);
> +			break;
> +		case Opt_flushoncommit:
> +			btrfs_set_opt(sb->default_mount_opt, FLUSHONCOMMIT);
> +			break;
> +		case Opt_discard:
> +			btrfs_set_opt(sb->default_mount_opt, DISCARD);
> +			break;
> +		case Opt_space_cache:
> +			btrfs_set_opt(sb->default_mount_opt, SPACE_CACHE);
> +			break;
> +		case Opt_no_space_cache:
> +			btrfs_clear_opt(sb->default_mount_opt, SPACE_CACHE);
> +			break;
> +		case Opt_inode_cache:
> +			btrfs_set_opt(sb->default_mount_opt, INODE_MAP_CACHE);
> +			break;
> +		case Opt_clear_cache:
> +			btrfs_set_opt(sb->default_mount_opt, CLEAR_CACHE);
> +			break;
> +		case Opt_user_subvol_rm_allowed:
> +			btrfs_set_opt(sb->default_mount_opt,
> +					USER_SUBVOL_RM_ALLOWED);
> +			break;
> +		case Opt_enospc_debug:
> +			btrfs_set_opt(sb->default_mount_opt, ENOSPC_DEBUG);
> +			break;
> +		case Opt_defrag:
> +			btrfs_set_opt(sb->default_mount_opt, AUTO_DEFRAG);
> +			break;
> +		case Opt_recovery:
> +			btrfs_set_opt(sb->default_mount_opt, RECOVERY);
> +			break;
> +		case Opt_skip_balance:
> +			btrfs_set_opt(sb->default_mount_opt, SKIP_BALANCE);
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +}
> +
> +void btrfs_parse_mntopt2string(unsigned long def_opt)
> +{
> +	if (!def_opt)
> +		printf("no default options\n");
> +	else {
> +		int i = 0;
> +		for (i = 0; i < DEF_MNTOPT_NUM; i++) {
> +			if (def_opt & (1 << toke[i].token))
> +				printf("%s\n", toke[i].pattern);
> +		}
> +	}
> +}
> diff --git a/btrfs-parse-mntopt.h b/btrfs-parse-mntopt.h
> new file mode 100644
> index 0000000..a2745ee
> --- /dev/null
> +++ b/btrfs-parse-mntopt.h
> @@ -0,0 +1,65 @@
> +/*
> + * 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.
> + */
> +
> +struct match_token {
> +	int token;
> +	const char *pattern;
> +};
> +
> +typedef struct match_token match_table_t[];
> +
> +enum {
> +	Opt_nodatasum, Opt_nodatacow, Opt_nobarrier, Opt_ssd,
> +	Opt_degraded, Opt_compress, Opt_notreelog, Opt_flushoncommit,
> +	Opt_ssd_spread, Opt_nossd, Opt_discard, Opt_compress_force,
> +	Opt_space_cache, Opt_clear_cache, Opt_user_subvol_rm_allowed,
> +	Opt_enospc_debug, Opt_defrag, Opt_inode_cache, Opt_recovery,
> +	Opt_skip_balance, Opt_check_integrity,
> +	Opt_check_integrity_including_extent_data, Opt_fatal_errors,
> +	Opt_no_space_cache, DEF_MNTOPT_NUM,
> +};
> +
> +static match_table_t toke = {
> +	{Opt_degraded, "degraded"},
> +	{Opt_nodatasum, "nodatasum"},
> +	{Opt_nodatacow, "nodatacow"},
> +	{Opt_nobarrier, "nobarrier"},
> +	{Opt_compress, "compress"},
> +	{Opt_compress_force, "compress-force"},
> +	{Opt_ssd, "ssd"},
> +	{Opt_ssd_spread, "ssd_spread"},
> +	{Opt_nossd, "nossd"},
> +	{Opt_notreelog, "notreelog"},
> +	{Opt_flushoncommit, "flushoncommit"},
> +	{Opt_discard, "discard"},
> +	{Opt_space_cache, "space_cache"},
> +	{Opt_no_space_cache, "no_space_cache"},
> +	{Opt_clear_cache, "clear_cache"},
> +	{Opt_user_subvol_rm_allowed, "user_subvol_rm_allowed"},
> +	{Opt_enospc_debug, "enospc_debug"},
> +	{Opt_defrag, "auto_defrag"},
> +	{Opt_inode_cache, "inode_map_cache"},
> +	{Opt_recovery, "recovery"},
> +	{Opt_skip_balance, "skip_balance"},
> +	{Opt_check_integrity, "check_int"},
> +	{Opt_check_integrity_including_extent_data, "check_int_data"},
> +	{Opt_fatal_errors, "fatal_errors"},
> +};
> +
> +void btrfs_parse_string2mntopt(struct btrfs_root *root, char **options);
> +void btrfs_parse_mntopt2string(unsigned long def_opt);
> diff --git a/btrfs.c b/btrfs.c
> index e9d54f8..0d6c9a7 100644
> --- a/btrfs.c
> +++ b/btrfs.c
> @@ -246,6 +246,7 @@ const struct cmd_group btrfs_cmd_group = {
>  		{ "device", cmd_device, NULL, &device_cmd_group, 0 },
>  		{ "scrub", cmd_scrub, NULL, &scrub_cmd_group, 0 },
>  		{ "inspect-internal", cmd_inspect, NULL, &inspect_cmd_group, 0 },
> +		{ "mount-option", cmd_mount, NULL, &mount_cmd_group, 0 },
>  		{ "send", cmd_send, NULL, &send_cmd_group, 0 },
>  		{ "receive", cmd_receive, NULL, &receive_cmd_group, 0 },
>  		{ "help", cmd_help, cmd_help_usage, NULL, 0 },
> diff --git a/cmds-mount.c b/cmds-mount.c
> new file mode 100644
> index 0000000..cbdb006
> --- /dev/null
> +++ b/cmds-mount.c
> @@ -0,0 +1,150 @@
> +/*
> + * 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 <sys/ioctl.h>
> +#include "ioctl.h"
> +#include "ctree.h"
> +#include "transaction.h"
> +#include "commands.h"
> +#include "disk-io.h"
> +#include "utils.h"
> +#include "btrfs-parse-mntopt.h"
> +
> +static const char * const mount_cmd_group_usage[] = {
> +	"btrfs mount-option <command> [<args>]",
> +	NULL
> +};
> +
> +static const char * const cmd_set_mntopt_usage[] = {
> +	"btrfs mount-option set [<option>,...] <device>",
> +	"Set default mount options",
> +	NULL
> +};
> +
> +static int cmd_set_mntopt(int argc, char **argv)
> +{
> +	struct btrfs_root *root = NULL;
> +	struct btrfs_trans_handle *trans = NULL;
> +	int ret = 0;
> +
> +	if (argc != 3)
> +		usage(cmd_set_mntopt_usage);
> +
> +	if (check_mounted(argv[argc - 1])) {
> +		fprintf(stderr, "%s is mounted\n", argv[argc - 1]);
> +		return 1;
> +	}
> +
> +	root = open_ctree(argv[argc - 1], 0, 1);
> +
> +	if (!root)
> +		printf("error root\n");
> +
> +	trans = btrfs_start_transaction(root, 1);
> +	btrfs_parse_string2mntopt(root, &argv[1]);
> +	ret = btrfs_commit_transaction(trans, root);
> +
> +	close_ctree(root);
> +	return ret;
> +};
> +
> +static const char * const cmd_get_mntopt_usage[] = {
> +	"btrfs mount-option get <device>",
> +	"Get default mount options",
> +	NULL
> +};
> +
> +static int cmd_get_mntopt(int argc, char **argv)
> +{
> +	struct btrfs_root *root = NULL;
> +	unsigned long def_opt;
> +
> +	memset(&def_opt, 0, sizeof(def_opt));
> +	if (argc != 2) {
> +		usage(cmd_get_mntopt_usage);
> +		return 1;
> +	}
> +
> +	if (check_mounted(argv[argc - 1])) {
> +		fprintf(stderr, "%s is mounted\n", argv[argc - 1]);
> +		return 1;
> +	}
> +
> +	root = open_ctree(argv[argc - 1], 0, 0);
> +
> +	if (!root) {
> +		printf("error root\n");
> +		return 1;
> +	}
> +
> +	def_opt = btrfs_super_default_mount_opt(&root->fs_info->super_copy);
> +
> +	btrfs_parse_mntopt2string(def_opt);
> +
> +	close_ctree(root);
> +	return 0;
> +};
> +
> +static const char * const cmd_clear_mntopt_usage[] = {
> +	"btrfs mount-option clear <device>",
> +	"Clear default mount options",
> +	NULL
> +};
> +
> +static int cmd_clear_mntopt(int argc, char **argv)
> +{
> +	struct btrfs_root *root = NULL;
> +	struct btrfs_trans_handle *trans = NULL;
> +	int ret = 0;
> +
> +	if (argc != 2) {
> +		usage(cmd_clear_mntopt_usage);
> +		return 1;
> +	}
> +
> +	if (check_mounted(argv[argc - 1])) {
> +		fprintf(stderr, "%s is mounted\n", argv[argc - 1]);
> +		return 1;
> +	}
> +
> +	root = open_ctree(argv[argc - 1], 0, 1);
> +
> +	if (!root)
> +		printf("error root\n");
> +
> +	trans = btrfs_start_transaction(root, 1);
> +	btrfs_set_super_default_mount_opt(&root->fs_info->super_copy, 0);
> +	ret = btrfs_commit_transaction(trans, root);
> +
> +	close_ctree(root);
> +	return ret;
> +};
> +
> +const struct cmd_group mount_cmd_group = {
> +	mount_cmd_group_usage, NULL, {
> +		{ "set", cmd_set_mntopt, cmd_set_mntopt_usage, NULL, 0 },
> +		{ "get", cmd_get_mntopt, cmd_get_mntopt_usage, NULL, 0 },
> +		{ "clear", cmd_clear_mntopt, cmd_clear_mntopt_usage, NULL, 0},
> +		{0, 0, 0, 0, 0 }
> +	}
> +};
> +
> +int cmd_mount(int argc, char **argv)
> +{
> +	return handle_command_group(&mount_cmd_group, argc, argv);
> +}
> diff --git a/commands.h b/commands.h
> index 1ece87a..6cf91fc 100644
> --- a/commands.h
> +++ b/commands.h
> @@ -88,6 +88,7 @@ extern const struct cmd_group balance_cmd_group;
>  extern const struct cmd_group device_cmd_group;
>  extern const struct cmd_group scrub_cmd_group;
>  extern const struct cmd_group inspect_cmd_group;
> +extern const struct cmd_group mount_cmd_group;
>  extern const struct cmd_group send_cmd_group;
>  extern const struct cmd_group receive_cmd_group;
>  
> @@ -97,5 +98,6 @@ int cmd_balance(int argc, char **argv);
>  int cmd_device(int argc, char **argv);
>  int cmd_scrub(int argc, char **argv);
>  int cmd_inspect(int argc, char **argv);
> +int cmd_mount(int argc, char **argv);
>  int cmd_send(int argc, char **argv);
>  int cmd_receive(int argc, char **argv);
> diff --git a/ctree.h b/ctree.h
> index d218b88..4a1bb5b 100644
> --- a/ctree.h
> +++ b/ctree.h
> @@ -401,8 +401,11 @@ struct btrfs_super_block {
>  
>  	__le64 cache_generation;
>  
> +	/* default mount options */
> +	unsigned long default_mount_opt;
> +
>  	/* future expansion */
> -	__le64 reserved[31];
> +	__le64 reserved[30];
>  	u8 sys_chunk_array[BTRFS_SYSTEM_CHUNK_ARRAY_SIZE];
>  	struct btrfs_root_backup super_roots[BTRFS_NUM_BACKUP_ROOTS];
>  } __attribute__ ((__packed__));
> @@ -1774,6 +1777,8 @@ BTRFS_SETGET_STACK_FUNCS(super_csum_type, struct btrfs_super_block,
>  			 csum_type, 16);
>  BTRFS_SETGET_STACK_FUNCS(super_cache_generation, struct btrfs_super_block,
>  			 cache_generation, 64);
> +BTRFS_SETGET_STACK_FUNCS(super_default_mount_opt, struct btrfs_super_block,
> +			 default_mount_opt, 64);
>  
>  static inline int btrfs_super_csum_size(struct btrfs_super_block *s)
>  {
> @@ -2128,3 +2133,37 @@ int btrfs_csum_truncate(struct btrfs_trans_handle *trans,
>  			struct btrfs_root *root, struct btrfs_path *path,
>  			u64 isize);
>  #endif
> +
> +/*
> + * Flags for mount options.
> + *
> + * Note: don't forget to add new options to btrfs_show_options()
> + */
> +#define BTRFS_MOUNT_NODATASUM		(1 << 0)
> +#define BTRFS_MOUNT_NODATACOW		(1 << 1)
> +#define BTRFS_MOUNT_NOBARRIER		(1 << 2)
> +#define BTRFS_MOUNT_SSD			(1 << 3)
> +#define BTRFS_MOUNT_DEGRADED		(1 << 4)
> +#define BTRFS_MOUNT_COMPRESS		(1 << 5)
> +#define BTRFS_MOUNT_NOTREELOG           (1 << 6)
> +#define BTRFS_MOUNT_FLUSHONCOMMIT       (1 << 7)
> +#define BTRFS_MOUNT_SSD_SPREAD		(1 << 8)
> +#define BTRFS_MOUNT_NOSSD		(1 << 9)
> +#define BTRFS_MOUNT_DISCARD		(1 << 10)
> +#define BTRFS_MOUNT_FORCE_COMPRESS      (1 << 11)
> +#define BTRFS_MOUNT_SPACE_CACHE		(1 << 12)
> +#define BTRFS_MOUNT_CLEAR_CACHE		(1 << 13)
> +#define BTRFS_MOUNT_USER_SUBVOL_RM_ALLOWED (1 << 14)
> +#define BTRFS_MOUNT_ENOSPC_DEBUG	 (1 << 15)
> +#define BTRFS_MOUNT_AUTO_DEFRAG		(1 << 16)
> +#define BTRFS_MOUNT_INODE_MAP_CACHE	(1 << 17)
> +#define BTRFS_MOUNT_RECOVERY		(1 << 18)
> +#define BTRFS_MOUNT_SKIP_BALANCE	(1 << 19)
> +#define BTRFS_MOUNT_CHECK_INTEGRITY	(1 << 20)
> +#define BTRFS_MOUNT_CHECK_INTEGRITY_INCLUDING_EXTENT_DATA (1 << 21)
> +#define BTRFS_MOUNT_PANIC_ON_FATAL_ERROR	(1 << 22)
> +
> +#define btrfs_clear_opt(o, opt)		((o) &= ~BTRFS_MOUNT_##opt)
> +#define btrfs_set_opt(o, opt)		((o) |= BTRFS_MOUNT_##opt)
> +#define btrfs_test_opt(root, opt)	((root)->fs_info->mount_opt& \
> +					 BTRFS_MOUNT_##opt)
> 



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] Btrfs-progs: add mount-option command
  2012-09-18  2:31   ` Miao Xie
@ 2012-09-18  4:19     ` Roman Mamedov
  2012-09-18  8:19       ` Hugo Mills
  2012-09-19  8:32     ` Hidetoshi Seto
  1 sibling, 1 reply; 18+ messages in thread
From: Roman Mamedov @ 2012-09-18  4:19 UTC (permalink / raw)
  To: miaox; +Cc: Hidetoshi Seto, linux-btrfs

[-- Attachment #1: Type: text/plain, Size: 1047 bytes --]

On Tue, 18 Sep 2012 10:31:41 +0800
Miao Xie <miaox@cn.fujitsu.com> wrote:

> On tue, 18 Sep 2012 10:30:17 +0900, Hidetoshi Seto wrote:
> > This patch adds mount-option command.
> > The command can set/get default mount options.
> > Now, the command can set/get 24 options.
> > These options are equal to mount options which store
> > in fs_info/mount-opt.
> 
> I don't think we need implement a separate command to do this,
> we can add it into btrfstune just like ext3/4. If so, the users
> who used ext3/4 before can be familiar with btrfs command as soon
> as possible.

btrfstune currently only does one thing:

$ sudo btrfstune
usage: btrfstune [options] device
	-S value	enable/disable seeding

To me it'd seem more logical the other way, why not move this operation to the
base "btrfs" utility under some command, and remove "btrfstune" completely.

-- 
With respect,
Roman

~~~~~~~~~~~~~~~~~~~~~~~~~~~
"Stallman had a printer,
with code he could not see.
So he began to tinker,
and set the software free."

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] Btrfs-progs: add mount-option command
  2012-09-18  4:19     ` Roman Mamedov
@ 2012-09-18  8:19       ` Hugo Mills
  2012-09-18 12:06         ` David Sterba
  0 siblings, 1 reply; 18+ messages in thread
From: Hugo Mills @ 2012-09-18  8:19 UTC (permalink / raw)
  To: Roman Mamedov; +Cc: miaox, Hidetoshi Seto, linux-btrfs

[-- Attachment #1: Type: text/plain, Size: 1298 bytes --]

On Tue, Sep 18, 2012 at 10:19:30AM +0600, Roman Mamedov wrote:
> On Tue, 18 Sep 2012 10:31:41 +0800
> Miao Xie <miaox@cn.fujitsu.com> wrote:
> 
> > On tue, 18 Sep 2012 10:30:17 +0900, Hidetoshi Seto wrote:
> > > This patch adds mount-option command.
> > > The command can set/get default mount options.
> > > Now, the command can set/get 24 options.
> > > These options are equal to mount options which store
> > > in fs_info/mount-opt.
> > 
> > I don't think we need implement a separate command to do this,
> > we can add it into btrfstune just like ext3/4. If so, the users
> > who used ext3/4 before can be familiar with btrfs command as soon
> > as possible.
> 
> btrfstune currently only does one thing:
> 
> $ sudo btrfstune
> usage: btrfstune [options] device
> 	-S value	enable/disable seeding
> 
> To me it'd seem more logical the other way, why not move this operation to the
> base "btrfs" utility under some command, and remove "btrfstune" completely.

   I would suggest picking up and extending Alex Block's "btrfs
property set/get" idea for this.

   Hugo.

-- 
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
  PGP key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
              --- Welcome to Rivendell,  Mr Anderson... ---              

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* R: [PATCH 2/2] Btrfs-progs: add mount-option command
  2012-09-18  1:25 [PATCH 0/2] Btrfs: set mount options permanently Hidetoshi Seto
  2012-09-18  1:28 ` [PATCH 1/2] Btrfs: make space to keep default mount options Hidetoshi Seto
  2012-09-18  1:30 ` [PATCH 2/2] Btrfs-progs: add mount-option command Hidetoshi Seto
@ 2012-09-18 10:03 ` Goffredo Baroncelli <kreijack@libero.it>
  2012-09-19  8:31   ` Hidetoshi Seto
  2012-09-20 11:28   ` David Sterba
  2012-09-18 11:37 ` R: " Goffredo Baroncelli <kreijack@libero.it>
  3 siblings, 2 replies; 18+ messages in thread
From: Goffredo Baroncelli <kreijack@libero.it> @ 2012-09-18 10:03 UTC (permalink / raw)
  To: seto.hidetoshi, linux-btrfs

Hi Seto,

please could you update also the man page too ? 
Why it was not provided a way to clear a *single* flag ? To me it seems a bit 
too long to clear all the flag (btrfs mount-option clear) and then set the 
right one.

As user interface I suggest something like chmod:

   btrfs mount-option set +ssd,skip_balance -nodatacow /dev/sdX

or

   btrfs mount-option set =ssd,skip_balance,nodatacow /dev/sdX


Finally I have some small concern about two macro (see below)

>----Messaggio originale----
>Da: seto.hidetoshi@jp.fujitsu.com
>Data: 18/09/2012 3.30
>A: <linux-btrfs@vger.kernel.org>
>Ogg: [PATCH 2/2] Btrfs-progs: add mount-option command
>
[...]
>+/*
>+ * Flags for mount options.
>+ *
>+ * Note: don't forget to add new options to btrfs_show_options()
>+ */
>+#define BTRFS_MOUNT_NODATASUM		(1 << 0)
>+#define BTRFS_MOUNT_NODATACOW		(1 << 1)
>+#define BTRFS_MOUNT_NOBARRIER		(1 << 2)
>+#define BTRFS_MOUNT_SSD			(1 << 3)
>+#define BTRFS_MOUNT_DEGRADED		(1 << 4)
>+#define BTRFS_MOUNT_COMPRESS		(1 << 5)
>+#define BTRFS_MOUNT_NOTREELOG           (1 << 6)
>+#define BTRFS_MOUNT_FLUSHONCOMMIT       (1 << 7)
>+#define BTRFS_MOUNT_SSD_SPREAD		(1 << 8)
>+#define BTRFS_MOUNT_NOSSD		(1 << 9)
>+#define BTRFS_MOUNT_DISCARD		(1 << 10)
>+#define BTRFS_MOUNT_FORCE_COMPRESS      (1 << 11)
>+#define BTRFS_MOUNT_SPACE_CACHE		(1 << 12)
>+#define BTRFS_MOUNT_CLEAR_CACHE		(1 << 13)
>+#define BTRFS_MOUNT_USER_SUBVOL_RM_ALLOWED (1 << 14)
>+#define BTRFS_MOUNT_ENOSPC_DEBUG	 (1 << 15)
>+#define BTRFS_MOUNT_AUTO_DEFRAG		(1 << 16)
>+#define BTRFS_MOUNT_INODE_MAP_CACHE	(1 << 17)
>+#define BTRFS_MOUNT_RECOVERY		(1 << 18)
>+#define BTRFS_MOUNT_SKIP_BALANCE	(1 << 19)
>+#define BTRFS_MOUNT_CHECK_INTEGRITY	(1 << 20)
>+#define BTRFS_MOUNT_CHECK_INTEGRITY_INCLUDING_EXTENT_DATA (1 << 21)
>+#define BTRFS_MOUNT_PANIC_ON_FATAL_ERROR	(1 << 22)
>+
>+#define btrfs_clear_opt(o, opt)		((o) &= ~BTRFS_MOUNT_##opt)
>+#define btrfs_set_opt(o, opt)		((o) |= BTRFS_MOUNT_##opt)
>+#define btrfs_test_opt(root, opt)	((root)->fs_info->mount_opt& \
>+					 BTRFS_MOUNT_##opt)

I think that the macro names are too generic, I suggest to rename the three 
macros above as

#define btrfs_mount_XXXX_opt

Also, the last one should be 

#define btrfs_test_opt(o, opt)	( o & BTRFS_MOUNT_##opt)

Or better the other two

#define btrfs_mount_clear_opt(root, opt)		(((root)->fs_info->mount_opt) &= 
~BTRFS_MOUNT_##opt)
#define btrfs_mount_set_opt(root, opt)		(((root)->fs_info->mount_opt) |= 
BTRFS_MOUNT_##opt)




>-- 
>1.7.7.6
>
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



^ permalink raw reply	[flat|nested] 18+ messages in thread

* R: Re: [PATCH 2/2] Btrfs-progs: add mount-option command
  2012-09-18  1:25 [PATCH 0/2] Btrfs: set mount options permanently Hidetoshi Seto
                   ` (2 preceding siblings ...)
  2012-09-18 10:03 ` R: " Goffredo Baroncelli <kreijack@libero.it>
@ 2012-09-18 11:37 ` Goffredo Baroncelli <kreijack@libero.it>
  3 siblings, 0 replies; 18+ messages in thread
From: Goffredo Baroncelli <kreijack@libero.it> @ 2012-09-18 11:37 UTC (permalink / raw)
  To: rm, miaox; +Cc: Hidetoshi Seto, linux-btrfs, hugo-lkml, Alexander Block



>Da: rm@romanrm.ru
>Data: 18/09/2012 6.19
>A: <miaox@cn.fujitsu.com>
>Cc: "Hidetoshi Seto"<seto.hidetoshi@jp.fujitsu.com>, <linux-btrfs@vger.kernel.
org>
>Ogg: Re: [PATCH 2/2] Btrfs-progs: add mount-option command
>
>On Tue, 18 Sep 2012 10:31:41 +0800
>Miao Xie <miaox@cn.fujitsu.com> wrote:
>
>> On tue, 18 Sep 2012 10:30:17 +0900, Hidetoshi Seto wrote:
>> > This patch adds mount-option command.
>> > The command can set/get default mount options.
>> > Now, the command can set/get 24 options.
>> > These options are equal to mount options which store
>> > in fs_info/mount-opt.
>> 
>> I don't think we need implement a separate command to do this,
>> we can add it into btrfstune just like ext3/4. If so, the users
>> who used ext3/4 before can be familiar with btrfs command as soon
>> as possible.
>
>btrfstune currently only does one thing:
>
>$ sudo btrfstune
>usage: btrfstune [options] device
>	-S value	enable/disable seeding
>
>To me it'd seem more logical the other way, why not move this operation to 
the
>base "btrfs" utility under some command, and remove "btrfstune" completely.

I fully agree. It doesn't make sense to have btrfstune as separate command. 
Its functionality should be integrate in Hidetoshi's patch: at the end both 
clear/set some flags.

I am not happy about the "btrfs property *" syntax, however the Hugo 
suggestion is right: these flags (with the seed one) are filesystem properties, 
and should be integrated in the Alexander work... However I don't know the 
status if its patches...


>
>-- 
>With respect,
>Roman
>
>~~~~~~~~~~~~~~~~~~~~~~~~~~~
>"Stallman had a printer,
>with code he could not see.
>So he began to tinker,
>and set the software free."
>



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] Btrfs-progs: add mount-option command
  2012-09-18  8:19       ` Hugo Mills
@ 2012-09-18 12:06         ` David Sterba
  0 siblings, 0 replies; 18+ messages in thread
From: David Sterba @ 2012-09-18 12:06 UTC (permalink / raw)
  To: Hugo Mills, Roman Mamedov, miaox, Hidetoshi Seto, linux-btrfs

On Tue, Sep 18, 2012 at 09:19:40AM +0100, Hugo Mills wrote:
> On Tue, Sep 18, 2012 at 10:19:30AM +0600, Roman Mamedov wrote:
> > On Tue, 18 Sep 2012 10:31:41 +0800
> > Miao Xie <miaox@cn.fujitsu.com> wrote:
> > 
> > > On tue, 18 Sep 2012 10:30:17 +0900, Hidetoshi Seto wrote:
> > > > This patch adds mount-option command.
> > > > The command can set/get default mount options.
> > > > Now, the command can set/get 24 options.
> > > > These options are equal to mount options which store
> > > > in fs_info/mount-opt.
> > > 
> > > I don't think we need implement a separate command to do this,
> > > we can add it into btrfstune just like ext3/4. If so, the users
> > > who used ext3/4 before can be familiar with btrfs command as soon
> > > as possible.
> > 
> > btrfstune currently only does one thing:
> > 
> > $ sudo btrfstune
> > usage: btrfstune [options] device
> > 	-S value	enable/disable seeding
> > 
> > To me it'd seem more logical the other way, why not move this operation to the
> > base "btrfs" utility under some command, and remove "btrfstune" completely.
> 
>    I would suggest picking up and extending Alex Block's "btrfs
> property set/get" idea for this.

And I'm with you here. I'm not sure if btrfstune has widespread use, the
functionality should go into a single management tool that everybody
uses.

david

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] Btrfs: make space to keep default mount options
  2012-09-18  1:28 ` [PATCH 1/2] Btrfs: make space to keep default mount options Hidetoshi Seto
@ 2012-09-18 12:10   ` David Sterba
  2012-09-19  8:31     ` Hidetoshi Seto
  0 siblings, 1 reply; 18+ messages in thread
From: David Sterba @ 2012-09-18 12:10 UTC (permalink / raw)
  To: Hidetoshi Seto; +Cc: linux-btrfs

On Tue, Sep 18, 2012 at 10:28:48AM +0900, Hidetoshi Seto wrote:
> This patch create space to hold default mount option,
> and to use saved default mount option change super.c
> to read default mount option first when mount devices.
> 
> Signed-off-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
> ---
>  fs/btrfs/ctree.h |    5 ++++-
>  fs/btrfs/super.c |    2 ++
>  2 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index fa5c45b..3eb0551 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -458,8 +458,11 @@ struct btrfs_super_block {
>  
>  	__le64 cache_generation;
>  
> +	/* default mount options */
> +	unsigned long default_mount_opt;

you need to use __le64 here, unsigned long has not fixed size

> +
>  	/* future expansion */
> -	__le64 reserved[31];
> +	__le64 reserved[30];
>  	u8 sys_chunk_array[BTRFS_SYSTEM_CHUNK_ARRAY_SIZE];
>  	struct btrfs_root_backup super_roots[BTRFS_NUM_BACKUP_ROOTS];
>  } __attribute__ ((__packed__));
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index e239915..7ef4a2e 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -340,6 +340,8 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
>  	char *compress_type;
>  	bool compress_force = false;
>  
> +	info->mount_opt = info->super_copy->default_mount_opt;

the options have to respect some priority, eg. when I set default
options to a filesystem, but mount with a different set, I expect that
the explicit flags apply and override the defaults.

I don't remember if this was discussed in the mailinglist or on IRC
only, should be easy to dig up if needed.

> +
>  	cache_gen = btrfs_super_cache_generation(root->fs_info->super_copy);
>  	if (cache_gen)
>  		btrfs_set_opt(info->mount_opt, SPACE_CACHE);


david

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] Btrfs-progs: add mount-option command
  2012-09-18  1:30 ` [PATCH 2/2] Btrfs-progs: add mount-option command Hidetoshi Seto
  2012-09-18  2:31   ` Miao Xie
@ 2012-09-18 12:30   ` David Sterba
  2012-09-19  8:32     ` Hidetoshi Seto
  1 sibling, 1 reply; 18+ messages in thread
From: David Sterba @ 2012-09-18 12:30 UTC (permalink / raw)
  To: Hidetoshi Seto; +Cc: linux-btrfs

On Tue, Sep 18, 2012 at 10:30:17AM +0900, Hidetoshi Seto wrote:
> Now, the command can set/get 24 options.
> These options are equal to mount options which store
> in fs_info/mount-opt.

Some of the options do not IMO fit for being default, like DEGRADED,
maybe RECOVERY, maybe SKIP_BALANCE.

> +	mount_cmd_group_usage, NULL, {
> +		{ "set", cmd_set_mntopt, cmd_set_mntopt_usage, NULL, 0 },
> +		{ "get", cmd_get_mntopt, cmd_get_mntopt_usage, NULL, 0 },
> +		{ "clear", cmd_clear_mntopt, cmd_clear_mntopt_usage, NULL, 0},
> +		{0, 0, 0, 0, 0 }

I find this very long to type,

  btrfs mount-option clear something

and I'm missing a command to list all the possible options to set.

> +	}
> +};
> +
> +int cmd_mount(int argc, char **argv)
> +{
> +	return handle_command_group(&mount_cmd_group, argc, argv);
> +}
> diff --git a/commands.h b/commands.h
> index 1ece87a..6cf91fc 100644
> --- a/commands.h
> +++ b/commands.h
> @@ -88,6 +88,7 @@ extern const struct cmd_group balance_cmd_group;
>  extern const struct cmd_group device_cmd_group;
>  extern const struct cmd_group scrub_cmd_group;
>  extern const struct cmd_group inspect_cmd_group;
> +extern const struct cmd_group mount_cmd_group;
>  extern const struct cmd_group send_cmd_group;
>  extern const struct cmd_group receive_cmd_group;
>  
> @@ -97,5 +98,6 @@ int cmd_balance(int argc, char **argv);
>  int cmd_device(int argc, char **argv);
>  int cmd_scrub(int argc, char **argv);
>  int cmd_inspect(int argc, char **argv);
> +int cmd_mount(int argc, char **argv);
>  int cmd_send(int argc, char **argv);
>  int cmd_receive(int argc, char **argv);
> diff --git a/ctree.h b/ctree.h
> index d218b88..4a1bb5b 100644
> --- a/ctree.h
> +++ b/ctree.h
> @@ -401,8 +401,11 @@ struct btrfs_super_block {
>  
>  	__le64 cache_generation;
>  
> +	/* default mount options */
> +	unsigned long default_mount_opt;

__le64 here as well

> +
>  	/* future expansion */
> -	__le64 reserved[31];
> +	__le64 reserved[30];
>  	u8 sys_chunk_array[BTRFS_SYSTEM_CHUNK_ARRAY_SIZE];
>  	struct btrfs_root_backup super_roots[BTRFS_NUM_BACKUP_ROOTS];
>  } __attribute__ ((__packed__));
> @@ -1774,6 +1777,8 @@ BTRFS_SETGET_STACK_FUNCS(super_csum_type, struct btrfs_super_block,
>  			 csum_type, 16);
>  BTRFS_SETGET_STACK_FUNCS(super_cache_generation, struct btrfs_super_block,
>  			 cache_generation, 64);
> +BTRFS_SETGET_STACK_FUNCS(super_default_mount_opt, struct btrfs_super_block,
> +			 default_mount_opt, 64);
>  
>  static inline int btrfs_super_csum_size(struct btrfs_super_block *s)
>  {
> @@ -2128,3 +2133,37 @@ int btrfs_csum_truncate(struct btrfs_trans_handle *trans,
>  			struct btrfs_root *root, struct btrfs_path *path,
>  			u64 isize);
>  #endif
> +
> +/*
> + * Flags for mount options.
> + *
> + * Note: don't forget to add new options to btrfs_show_options()

(the note does not apply here, makes sense only for kernel)


So, you're basically implementing subset of the whole-filesystem
options. As has been mentioned, alternate way is to use the 'properties'
interface as a global entry point from the management tool. It would be
great if you base your patches on top of it.

(The props haven't been acked, but we've reached a wider consensus on
irc, patches are in the mailinglist, so we can bring it up again and
discuss outstanding issues or objections. I'm mainly concerned about
priority of setting the defaults and not-bloating the 'btrfs' interface
too much.)


thanks,
david

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] Btrfs: make space to keep default mount options
  2012-09-18 12:10   ` David Sterba
@ 2012-09-19  8:31     ` Hidetoshi Seto
  2012-09-20 11:05       ` David Sterba
  0 siblings, 1 reply; 18+ messages in thread
From: Hidetoshi Seto @ 2012-09-19  8:31 UTC (permalink / raw)
  To: linux-btrfs

(2012/09/18 21:10), David Sterba wrote:
> On Tue, Sep 18, 2012 at 10:28:48AM +0900, Hidetoshi Seto wrote:
>> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
>> index fa5c45b..3eb0551 100644
>> --- a/fs/btrfs/ctree.h
>> +++ b/fs/btrfs/ctree.h
>> @@ -458,8 +458,11 @@ struct btrfs_super_block {
>>  
>>  	__le64 cache_generation;
>>  
>> +	/* default mount options */
>> +	unsigned long default_mount_opt;
> 
> you need to use __le64 here, unsigned long has not fixed size

Indeed. I'll use __le64 next time.

>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>> index e239915..7ef4a2e 100644
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -340,6 +340,8 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
>>  	char *compress_type;
>>  	bool compress_force = false;
>>  
>> +	info->mount_opt = info->super_copy->default_mount_opt;
> 
> the options have to respect some priority, eg. when I set default
> options to a filesystem, but mount with a different set, I expect that
> the explicit flags apply and override the defaults.
> 
> I don't remember if this was discussed in the mailinglist or on IRC
> only, should be easy to dig up if needed.

At least I don't know whether this was already disscussed or not.
Now my code gives priority to the default options, and it would not
be so difficult in case if we have opposing options like "ssd" vs
"nossd", and "space_cache" vs "no_space_cache"...
Or we could use the default options only if there is no options
specified when it is being mount, but it will make the default
options useless. Humm, I'll try to pull together my thoughts
prior to my next post.

Thanks,
H.Seto


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: R: [PATCH 2/2] Btrfs-progs: add mount-option command
  2012-09-18 10:03 ` R: " Goffredo Baroncelli <kreijack@libero.it>
@ 2012-09-19  8:31   ` Hidetoshi Seto
  2012-09-20 11:28   ` David Sterba
  1 sibling, 0 replies; 18+ messages in thread
From: Hidetoshi Seto @ 2012-09-19  8:31 UTC (permalink / raw)
  To: Goffredo Baroncelli <kreijack@libero.it>; +Cc: linux-btrfs

(2012/09/18 19:03), Goffredo Baroncelli <kreijack@libero.it> wrote:
> Hi Seto,
> 
> please could you update also the man page too ? 

Sure. I'll update it next time.

> Why it was not provided a way to clear a *single* flag ? To me it seems a bit 
> too long to clear all the flag (btrfs mount-option clear) and then set the 
> right one.

Good point. We should have a way to clear defaults, by "clear" or
"set none" or so on. 

> 
> As user interface I suggest something like chmod:
> 
>    btrfs mount-option set +ssd,skip_balance -nodatacow /dev/sdX
> 
> or
> 
>    btrfs mount-option set =ssd,skip_balance,nodatacow /dev/sdX

Interesting. I guess you mean that "=<options>" will reset defaults
by specified options, while "+<options>" and "-<options>" will be
used to add/delete option to/from current defaults.

> 
> Finally I have some small concern about two macro (see below)
> 
>> ----Messaggio originale----
>> Da: seto.hidetoshi@jp.fujitsu.com
>> Data: 18/09/2012 3.30
>> A: <linux-btrfs@vger.kernel.org>
>> Ogg: [PATCH 2/2] Btrfs-progs: add mount-option command
>>
> [...]
>> +/*
>> + * Flags for mount options.
>> + *
>> + * Note: don't forget to add new options to btrfs_show_options()
>> + */
>> +#define BTRFS_MOUNT_NODATASUM		(1 << 0)
>> +#define BTRFS_MOUNT_NODATACOW		(1 << 1)
>> +#define BTRFS_MOUNT_NOBARRIER		(1 << 2)
>> +#define BTRFS_MOUNT_SSD			(1 << 3)
>> +#define BTRFS_MOUNT_DEGRADED		(1 << 4)
>> +#define BTRFS_MOUNT_COMPRESS		(1 << 5)
>> +#define BTRFS_MOUNT_NOTREELOG           (1 << 6)
>> +#define BTRFS_MOUNT_FLUSHONCOMMIT       (1 << 7)
>> +#define BTRFS_MOUNT_SSD_SPREAD		(1 << 8)
>> +#define BTRFS_MOUNT_NOSSD		(1 << 9)
>> +#define BTRFS_MOUNT_DISCARD		(1 << 10)
>> +#define BTRFS_MOUNT_FORCE_COMPRESS      (1 << 11)
>> +#define BTRFS_MOUNT_SPACE_CACHE		(1 << 12)
>> +#define BTRFS_MOUNT_CLEAR_CACHE		(1 << 13)
>> +#define BTRFS_MOUNT_USER_SUBVOL_RM_ALLOWED (1 << 14)
>> +#define BTRFS_MOUNT_ENOSPC_DEBUG	 (1 << 15)
>> +#define BTRFS_MOUNT_AUTO_DEFRAG		(1 << 16)
>> +#define BTRFS_MOUNT_INODE_MAP_CACHE	(1 << 17)
>> +#define BTRFS_MOUNT_RECOVERY		(1 << 18)
>> +#define BTRFS_MOUNT_SKIP_BALANCE	(1 << 19)
>> +#define BTRFS_MOUNT_CHECK_INTEGRITY	(1 << 20)
>> +#define BTRFS_MOUNT_CHECK_INTEGRITY_INCLUDING_EXTENT_DATA (1 << 21)
>> +#define BTRFS_MOUNT_PANIC_ON_FATAL_ERROR	(1 << 22)
>> +
>> +#define btrfs_clear_opt(o, opt)		((o) &= ~BTRFS_MOUNT_##opt)
>> +#define btrfs_set_opt(o, opt)		((o) |= BTRFS_MOUNT_##opt)
>> +#define btrfs_test_opt(root, opt)	((root)->fs_info->mount_opt& \
>> +					 BTRFS_MOUNT_##opt)
> 
> I think that the macro names are too generic, I suggest to rename the three 
> macros above as
> 
> #define btrfs_mount_XXXX_opt
> 
> Also, the last one should be 
> 
> #define btrfs_test_opt(o, opt)	( o & BTRFS_MOUNT_##opt)
> 
> Or better the other two
> 
> #define btrfs_mount_clear_opt(root, opt)		(((root)->fs_info->mount_opt) &= 
> ~BTRFS_MOUNT_##opt)
> #define btrfs_mount_set_opt(root, opt)		(((root)->fs_info->mount_opt) |= 
> BTRFS_MOUNT_##opt)

Right. I'll fix it.


Thanks,
H.Seto


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] Btrfs-progs: add mount-option command
  2012-09-18  2:31   ` Miao Xie
  2012-09-18  4:19     ` Roman Mamedov
@ 2012-09-19  8:32     ` Hidetoshi Seto
  2012-09-20 11:14       ` David Sterba
  1 sibling, 1 reply; 18+ messages in thread
From: Hidetoshi Seto @ 2012-09-19  8:32 UTC (permalink / raw)
  To: miaox; +Cc: linux-btrfs

(2012/09/18 11:31), Miao Xie wrote:
> On tue, 18 Sep 2012 10:30:17 +0900, Hidetoshi Seto wrote:
>> This patch adds mount-option command.
>> The command can set/get default mount options.
>> Now, the command can set/get 24 options.
>> These options are equal to mount options which store
>> in fs_info/mount-opt.
> 
> I don't think we need implement a separate command to do this,
> we can add it into btrfstune just like ext3/4. If so, the users
> who used ext3/4 before can be familiar with btrfs command as soon
> as possible.

As some people already pointed out, I also think that the btrfstune
command is a kind of obsolete, and therefore it would be better to
improve the btrfs command as centralized utility.

> 
> Beside that, why not add a option into mkfs.btrfs?

It is planned. Once we can fix a way how to save default options,
modifying mkfs.btfs would be easy.


Thanks,
H.Seto


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] Btrfs-progs: add mount-option command
  2012-09-18 12:30   ` David Sterba
@ 2012-09-19  8:32     ` Hidetoshi Seto
  0 siblings, 0 replies; 18+ messages in thread
From: Hidetoshi Seto @ 2012-09-19  8:32 UTC (permalink / raw)
  To: linux-btrfs

(2012/09/18 21:30), David Sterba wrote:
> On Tue, Sep 18, 2012 at 10:30:17AM +0900, Hidetoshi Seto wrote:
...
> 
> So, you're basically implementing subset of the whole-filesystem
> options. As has been mentioned, alternate way is to use the 'properties'
> interface as a global entry point from the management tool. It would be
> great if you base your patches on top of it.
> 
> (The props haven't been acked, but we've reached a wider consensus on
> irc, patches are in the mailinglist, so we can bring it up again and
> discuss outstanding issues or objections. I'm mainly concerned about
> priority of setting the defaults and not-bloating the 'btrfs' interface
> too much.)

Thank you very much for your review!
I'll rework my patch to reflect all comments.
Please let me know if there are still outstanding issues or objections
about/against this feature.


Thanks,
H.Seto


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] Btrfs: make space to keep default mount options
  2012-09-19  8:31     ` Hidetoshi Seto
@ 2012-09-20 11:05       ` David Sterba
  0 siblings, 0 replies; 18+ messages in thread
From: David Sterba @ 2012-09-20 11:05 UTC (permalink / raw)
  To: Hidetoshi Seto; +Cc: linux-btrfs

On Wed, Sep 19, 2012 at 05:31:25PM +0900, Hidetoshi Seto wrote:
> >>  
> >> +	info->mount_opt = info->super_copy->default_mount_opt;
> > 
> > the options have to respect some priority, eg. when I set default
> > options to a filesystem, but mount with a different set, I expect that
> > the explicit flags apply and override the defaults.
> > 
> > I don't remember if this was discussed in the mailinglist or on IRC
> > only, should be easy to dig up if needed.
> 
> At least I don't know whether this was already disscussed or not.

For the subset you've selected, whole-fs default options, I now think we
don't need it, the discussions we had were about option precedence for
per -file, -subvolume, -fs, -mount. At the time of mount only per-mount
and per-fs need to sort their precedence. Any per-file option has to be
evaluated at a specific time eg. when new data are written in case of
compression.

> Now my code gives priority to the default options, and it would not
> be so difficult in case if we have opposing options like "ssd" vs
> "nossd", and "space_cache" vs "no_space_cache"...

Yep, that's what I expect to temporarily disable a specific option, eg.
the space_cache. For that purpose the full set of options with their
no- counterparts would make sense from the usability POV (although there
may be exceptions).

> Or we could use the default options only if there is no options
> specified when it is being mount, but it will make the default
> options useless.

You mean in case of a simple

  mount /dev/ice /mnt

?

This seems like a limited use of the defaults and may be confusing.


david

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] Btrfs-progs: add mount-option command
  2012-09-19  8:32     ` Hidetoshi Seto
@ 2012-09-20 11:14       ` David Sterba
  0 siblings, 0 replies; 18+ messages in thread
From: David Sterba @ 2012-09-20 11:14 UTC (permalink / raw)
  To: Hidetoshi Seto; +Cc: miaox, linux-btrfs

On Wed, Sep 19, 2012 at 05:32:16PM +0900, Hidetoshi Seto wrote:
> (2012/09/18 11:31), Miao Xie wrote:
> > On tue, 18 Sep 2012 10:30:17 +0900, Hidetoshi Seto wrote:
> >> This patch adds mount-option command.
> >> The command can set/get default mount options.
> >> Now, the command can set/get 24 options.
> >> These options are equal to mount options which store
> >> in fs_info/mount-opt.
> > 
> > I don't think we need implement a separate command to do this,
> > we can add it into btrfstune just like ext3/4. If so, the users
> > who used ext3/4 before can be familiar with btrfs command as soon
> > as possible.
> 
> As some people already pointed out, I also think that the btrfstune
> command is a kind of obsolete, and therefore it would be better to
> improve the btrfs command as centralized utility.
> 
> > 
> > Beside that, why not add a option into mkfs.btrfs?
> 
> It is planned. Once we can fix a way how to save default options,
> modifying mkfs.btfs would be easy.

It's desirable to extend mkfs with explict feature options like other
filesystems do (eg. mke2fs -O features...) or be able to set
incompatibility flags directly or the mount defaults prior to mount and
actual fs use.

david

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: R: [PATCH 2/2] Btrfs-progs: add mount-option command
  2012-09-18 10:03 ` R: " Goffredo Baroncelli <kreijack@libero.it>
  2012-09-19  8:31   ` Hidetoshi Seto
@ 2012-09-20 11:28   ` David Sterba
  1 sibling, 0 replies; 18+ messages in thread
From: David Sterba @ 2012-09-20 11:28 UTC (permalink / raw)
  To: Goffredo Baroncelli <kreijack@libero.it>
  Cc: seto.hidetoshi, linux-btrfs

On Tue, Sep 18, 2012 at 12:03:47PM +0200, Goffredo Baroncelli <kreijack@libero.it> wrote:
> Why it was not provided a way to clear a *single* flag ? To me it seems a bit 
> too long to clear all the flag (btrfs mount-option clear) and then set the 
> right one.
> 
> As user interface I suggest something like chmod:
> 
>    btrfs mount-option set +ssd,skip_balance -nodatacow /dev/sdX
> 
> or
> 
>    btrfs mount-option set =ssd,skip_balance,nodatacow /dev/sdX

I agree with the +/-/= syntax, it's quite intuitive and already seen
(chattr).

I'd drop the 'set' command here, with some options specified it's
implied.

Without any it's 'get'

  btrfs mount-option /dev/...

And the 'clear' subcommand could be also dropped with this syntax

  btrfs mount-option = /dev/...

:)


BTW, this modifies the defaults only for the unmounted filesystem, but
the same should be possible for a mounted one in the future.

david

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2012-09-20 11:29 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-18  1:25 [PATCH 0/2] Btrfs: set mount options permanently Hidetoshi Seto
2012-09-18  1:28 ` [PATCH 1/2] Btrfs: make space to keep default mount options Hidetoshi Seto
2012-09-18 12:10   ` David Sterba
2012-09-19  8:31     ` Hidetoshi Seto
2012-09-20 11:05       ` David Sterba
2012-09-18  1:30 ` [PATCH 2/2] Btrfs-progs: add mount-option command Hidetoshi Seto
2012-09-18  2:31   ` Miao Xie
2012-09-18  4:19     ` Roman Mamedov
2012-09-18  8:19       ` Hugo Mills
2012-09-18 12:06         ` David Sterba
2012-09-19  8:32     ` Hidetoshi Seto
2012-09-20 11:14       ` David Sterba
2012-09-18 12:30   ` David Sterba
2012-09-19  8:32     ` Hidetoshi Seto
2012-09-18 10:03 ` R: " Goffredo Baroncelli <kreijack@libero.it>
2012-09-19  8:31   ` Hidetoshi Seto
2012-09-20 11:28   ` David Sterba
2012-09-18 11:37 ` R: " Goffredo Baroncelli <kreijack@libero.it>

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).