diff mbox

[1/2] Btrfs: make space to keep default mount options

Message ID 5057CE50.1050706@jp.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hidetoshi Seto Sept. 18, 2012, 1:28 a.m. UTC
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(-)

Comments

David Sterba Sept. 18, 2012, 12:10 p.m. UTC | #1
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
--
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
Hidetoshi Seto Sept. 19, 2012, 8:31 a.m. UTC | #2
(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

--
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
David Sterba Sept. 20, 2012, 11:05 a.m. UTC | #3
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
--
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
diff mbox

Patch

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);