Home » Mailing list archives » linux-kernel » 2008 » January » 25

Re: [patch 25/26] mount options: fix udf

Previous thread: [patch 23/26] mount options: fix spufs by Miklos Szeredi on Thursday, January 24, 2008 - 12:34 pm. (1 message)

Next thread: [patch 24/26] mount options: fix tmpfs by Miklos Szeredi on Thursday, January 24, 2008 - 12:34 pm. (4 messages)
[view original message]
From: Miklos Szeredi
Subject: [patch 25/26] mount options: fix udf
Date: Thursday, January 24, 2008 - 12:34 pm

From: Miklos Szeredi <mszeredi@suse.cz>

Add a .show_options super operation to udf.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/udf/super.c
===================================================================
--- linux.orig/fs/udf/super.c	2008-01-24 13:48:37.000000000 +0100
+++ linux/fs/udf/super.c	2008-01-24 15:58:21.000000000 +0100
@@ -53,6 +53,8 @@
 #include <linux/vfs.h>
 #include <linux/vmalloc.h>
 #include <linux/errno.h>
+#include <linux/mount.h>
+#include <linux/seq_file.h>
 #include <asm/byteorder.h>
 
 #include <linux/udf_fs.h>
@@ -71,6 +73,8 @@
 #define VDS_POS_TERMINATING_DESC	6
 #define VDS_POS_LENGTH			7
 
+#define UDF_DEFAULT_BLOCKSIZE 2048
+
 static char error_buf[1024];
 
 /* These are the "meat" - everything else is stuffing */
@@ -95,6 +99,7 @@ static void udf_open_lvid(struct super_b
 static void udf_close_lvid(struct super_block *);
 static unsigned int udf_count_free(struct super_block *);
 static int udf_statfs(struct dentry *, struct kstatfs *);
+static int udf_show_options(struct seq_file *, struct vfsmount *);
 
 struct logicalVolIntegrityDescImpUse *udf_sb_lvidiu(struct udf_sb_info *sbi)
 {
@@ -181,6 +186,7 @@ static const struct super_operations udf
 	.write_super	= udf_write_super,
 	.statfs		= udf_statfs,
 	.remount_fs	= udf_remount_fs,
+	.show_options	= udf_show_options,
 };
 
 struct udf_options {
@@ -247,6 +253,56 @@ static int udf_sb_alloc_partition_maps(s
 	return 0;
 }
 
+static int udf_show_options(struct seq_file *seq, struct vfsmount *mnt)
+{
+	struct super_block *sb = mnt->mnt_sb;
+	struct udf_sb_info *sbi = UDF_SB(sb);
+
+	if (!UDF_QUERY_FLAG(sb, UDF_FLAG_STRICT))
+		seq_puts(seq, ",nostrict");
+	if (sb->s_blocksize != UDF_DEFAULT_BLOCKSIZE)
+		seq_printf(seq, ",bs=%lu", sb->s_blocksize);
+	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UNHIDE))
+		seq_puts(seq, ",unhide");
+	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UNDELETE))
+		seq_puts(seq, ",undelete");
+	if (!UDF_QUERY_FLAG(sb, ...
[ message continues ]
[view original message]
From: Cyrill Gorcunov
Subject: Re: [patch 25/26] mount options: fix udf
Date: Thursday, January 24, 2008 - 12:51 pm

[Miklos Szeredi - Thu, Jan 24, 2008 at 08:34:06PM +0100]
| From: Miklos Szeredi <mszeredi@suse.cz>
| 
| Add a .show_options super operation to udf.
| 
| Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
| ---
| 
| Index: linux/fs/udf/super.c
| ===================================================================
| --- linux.orig/fs/udf/super.c	2008-01-24 13:48:37.000000000 +0100
| +++ linux/fs/udf/super.c	2008-01-24 15:58:21.000000000 +0100
| @@ -53,6 +53,8 @@
|  #include <linux/vfs.h>
|  #include <linux/vmalloc.h>
|  #include <linux/errno.h>
| +#include <linux/mount.h>
| +#include <linux/seq_file.h>
|  #include <asm/byteorder.h>
|  
|  #include <linux/udf_fs.h>
| @@ -71,6 +73,8 @@
|  #define VDS_POS_TERMINATING_DESC	6
|  #define VDS_POS_LENGTH			7
|  
| +#define UDF_DEFAULT_BLOCKSIZE 2048
| +
|  static char error_buf[1024];
|  
|  /* These are the "meat" - everything else is stuffing */
| @@ -95,6 +99,7 @@ static void udf_open_lvid(struct super_b
|  static void udf_close_lvid(struct super_block *);
|  static unsigned int udf_count_free(struct super_block *);
|  static int udf_statfs(struct dentry *, struct kstatfs *);
| +static int udf_show_options(struct seq_file *, struct vfsmount *);
|  
|  struct logicalVolIntegrityDescImpUse *udf_sb_lvidiu(struct udf_sb_info *sbi)
|  {
| @@ -181,6 +186,7 @@ static const struct super_operations udf
|  	.write_super	= udf_write_super,
|  	.statfs		= udf_statfs,
|  	.remount_fs	= udf_remount_fs,
| +	.show_options	= udf_show_options,
|  };
|  
|  struct udf_options {
| @@ -247,6 +253,56 @@ static int udf_sb_alloc_partition_maps(s
|  	return 0;
|  }
|  
| +static int udf_show_options(struct seq_file *seq, struct vfsmount *mnt)
| +{
| +	struct super_block *sb = mnt->mnt_sb;
| +	struct udf_sb_info *sbi = UDF_SB(sb);
| +
| +	if (!UDF_QUERY_FLAG(sb, UDF_FLAG_STRICT))
| +		seq_puts(seq, ",nostrict");
| +	if (sb->s_blocksize != UDF_DEFAULT_BLOCKSIZE)
| +		seq_printf(seq, ",bs=%lu", sb->s_blocksize);
| +	if (UDF_QUERY_FLAG(sb, ...
[ message continues ]
[view original message]
From: Cyrill Gorcunov
Subject: Re: [patch 25/26] mount options: fix udf
Date: Thursday, January 24, 2008 - 1:20 pm

[Miklos Szeredi - Thu, Jan 24, 2008 at 08:34:06PM +0100]
| From: Miklos Szeredi <mszeredi@suse.cz>
| 
| Add a .show_options super operation to udf.
| 
| Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
| ---
| 
| Index: linux/fs/udf/super.c
| ===================================================================
| --- linux.orig/fs/udf/super.c	2008-01-24 13:48:37.000000000 +0100
| +++ linux/fs/udf/super.c	2008-01-24 15:58:21.000000000 +0100
| @@ -53,6 +53,8 @@
|  #include <linux/vfs.h>
|  #include <linux/vmalloc.h>
|  #include <linux/errno.h>
| +#include <linux/mount.h>
| +#include <linux/seq_file.h>
|  #include <asm/byteorder.h>
|  
|  #include <linux/udf_fs.h>
| @@ -71,6 +73,8 @@
|  #define VDS_POS_TERMINATING_DESC	6
|  #define VDS_POS_LENGTH			7
|  
| +#define UDF_DEFAULT_BLOCKSIZE 2048
| +

thanks, that is good cleanup

|  static char error_buf[1024];
|  
|  /* These are the "meat" - everything else is stuffing */
| @@ -95,6 +99,7 @@ static void udf_open_lvid(struct super_b
|  static void udf_close_lvid(struct super_block *);
|  static unsigned int udf_count_free(struct super_block *);
|  static int udf_statfs(struct dentry *, struct kstatfs *);
| +static int udf_show_options(struct seq_file *, struct vfsmount *);
|  
|  struct logicalVolIntegrityDescImpUse *udf_sb_lvidiu(struct udf_sb_info *sbi)
|  {
| @@ -181,6 +186,7 @@ static const struct super_operations udf
|  	.write_super	= udf_write_super,
|  	.statfs		= udf_statfs,
|  	.remount_fs	= udf_remount_fs,
| +	.show_options	= udf_show_options,
|  };
|  
|  struct udf_options {
| @@ -247,6 +253,56 @@ static int udf_sb_alloc_partition_maps(s
|  	return 0;
|  }
|  
| +static int udf_show_options(struct seq_file *seq, struct vfsmount *mnt)
| +{
| +	struct super_block *sb = mnt->mnt_sb;
| +	struct udf_sb_info *sbi = UDF_SB(sb);
| +
| +	if (!UDF_QUERY_FLAG(sb, UDF_FLAG_STRICT))
| +		seq_puts(seq, ",nostrict");
| +	if (sb->s_blocksize != UDF_DEFAULT_BLOCKSIZE)
| +		seq_printf(seq, ",bs=%lu", ...
[ message continues ]
[view original message]
From: Miklos Szeredi
Subject: Re: [patch 25/26] mount options: fix udf
Date: Friday, January 25, 2008 - 2:29 am

I think UDF_SB_ANCHOR macro was removed by some patch in -mm.

I'm more interested if the second element of the s_anchor array really
does always have the value of the 'anchor=N' mount option.  I haven't
been able to verify that fully.  Do you have some insight into that?

Thanks,
Miklos
--


[ message continues ]
[view original message]
From: Cyrill Gorcunov
Subject: Re: [patch 25/26] mount options: fix udf
Date: Friday, January 25, 2008 - 3:57 am

Miklos,

I'll check this today evening (a bit busy now).

- Cyrill -
--


[ message continues ]
[view original message]
From: Cyrill Gorcunov
Subject: Re: [patch 25/26] mount options: fix udf
Date: Friday, January 25, 2008 - 6:41 am

[Miklos Szeredi - Fri, Jan 25, 2008 at 10:29:21AM +0100]
| > | +	/* is this correct? */
| > | +	if (sbi->s_anchor[2] != 0)
| > | +		seq_printf(seq, ",anchor=%u", sbi->s_anchor[2]);
| > 
| > you know, I would prefer to use form UDF_SB_ANCHOR(sb)[2]
| > in sake of style unification but we should wait for Jan's
| > decision (i'm not the expert in this area ;)
| 
| I think UDF_SB_ANCHOR macro was removed by some patch in -mm.
| 
| I'm more interested if the second element of the s_anchor array really
| does always have the value of the 'anchor=N' mount option.  I haven't
| been able to verify that fully.  Do you have some insight into that?
| 
| Thanks,
| Miklos
| 

Hello Miklos,

well, actually - no. anchor entities can be set to 0 if we have been failed
to read them in udf_find_anchor(). So it seems you've to use some
additional flag to store it.

Btw, Miklos the patch is over -mm tree?

		- Cyrill -
--


[ message continues ]
[view original message]
From: Jan Kara
Subject: Re: [patch 25/26] mount options: fix udf
Date: Friday, January 25, 2008 - 8:27 am

Yes, it's going to be removed so don't use it. Actually, basing this
patch on top of -mm is a good idea because there are quite some changes
  As Cyrill wrote, it could be zeroed out in case there is no anchor in
the specified block. So I guess you have to store the passed value
somewhere else..

									Honza
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs
--


[ message continues ]
[view original message]
From: Miklos Szeredi
Subject: Re: [patch 25/26] mount options: fix udf
Date: Friday, January 25, 2008 - 8:50 am

But in that case, would the value of the anchor= option matter?

This is actually a somewhat philosophical question about what the
mount options in /proc/mounts mean:

 1) Options _given_ by the user for the mount
 2) Options which are _effective_ for the mount

If we take interpretation 2) and there was no anchor (whatever that
means), then the anchor=N option wasn't effective, and not giving it
would have had the same effect.

This could be confusing to the user, though...

Thanks,
Miklos
--


[ message continues ]
[view original message]
From: Cyrill Gorcunov
Subject: Re: [patch 25/26] mount options: fix udf
Date: Friday, January 25, 2008 - 8:57 am

[Miklos Szeredi - Fri, Jan 25, 2008 at 04:50:15PM +0100]
| > > > | +	/* is this correct? */
| > > > | +	if (sbi->s_anchor[2] != 0)
| > > > | +		seq_printf(seq, ",anchor=%u", sbi->s_anchor[2]);
| > > > 
| > > > you know, I would prefer to use form UDF_SB_ANCHOR(sb)[2]
| > > > in sake of style unification but we should wait for Jan's
| > > > decision (i'm not the expert in this area ;)
| > > 
| > > I think UDF_SB_ANCHOR macro was removed by some patch in -mm.
| >   Yes, it's going to be removed so don't use it. Actually, basing this
| > patch on top of -mm is a good idea because there are quite some changes
| > in Andrew's queue.
| > 
| > > I'm more interested if the second element of the s_anchor array really
| > > does always have the value of the 'anchor=N' mount option.  I haven't
| > > been able to verify that fully.  Do you have some insight into that?
| >   As Cyrill wrote, it could be zeroed out in case there is no anchor in
| > the specified block. So I guess you have to store the passed value
| > somewhere else..
| 
| But in that case, would the value of the anchor= option matter?
| 
| This is actually a somewhat philosophical question about what the
| mount options in /proc/mounts mean:
| 
|  1) Options _given_ by the user for the mount
|  2) Options which are _effective_ for the mount
| 
| If we take interpretation 2) and there was no anchor (whatever that
| means), then the anchor=N option wasn't effective, and not giving it
| would have had the same effect.
| 
| This could be confusing to the user, though...
| 
| Thanks,
| Miklos
| 

I think _effective_ options is much more important - they could
show you that something bad happened (and if this zeroing of anchor
has been happened udf print debug message) Anyway, Miklos, I think
the options _given_ by a user does not mean anything in that case
because it just doesn't reveal what is being used in _real_.

		- Cyrill -
--


[ message continues ]
[view original message]
From: Jan Kara
Subject: Re: [patch 25/26] mount options: fix udf
Date: Friday, January 25, 2008 - 9:07 am

Hmm, given that options are modified by remount for some filesystems,
it's probably the best to display the effective state. So your code should
display the right thing as it is.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR
--


[ message continues ]
[view original message]
From: Miklos Szeredi
Subject: Re: [patch 25/26] mount options: fix udf
Date: Friday, January 25, 2008 - 9:10 am

OK.  Cyrill, Jan, thanks for the reviews.

Miklos
--


[ message continues ]
[view original message]
From: Cyrill Gorcunov
Subject: Re: [patch 25/26] mount options: fix udf
Date: Thursday, January 24, 2008 - 1:40 pm

[Miklos Szeredi - Thu, Jan 24, 2008 at 08:34:06PM +0100]
| From: Miklos Szeredi <mszeredi@suse.cz>
| 
| Add a .show_options super operation to udf.
| 
| Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
| ---
| 
| Index: linux/fs/udf/super.c
| ===================================================================
| --- linux.orig/fs/udf/super.c	2008-01-24 13:48:37.000000000 +0100
| +++ linux/fs/udf/super.c	2008-01-24 15:58:21.000000000 +0100
| @@ -53,6 +53,8 @@
|  #include <linux/vfs.h>
|  #include <linux/vmalloc.h>
|  #include <linux/errno.h>
| +#include <linux/mount.h>
| +#include <linux/seq_file.h>
|  #include <asm/byteorder.h>
|  
|  #include <linux/udf_fs.h>
| @@ -71,6 +73,8 @@
|  #define VDS_POS_TERMINATING_DESC	6
|  #define VDS_POS_LENGTH			7
|  
| +#define UDF_DEFAULT_BLOCKSIZE 2048
| +
|  static char error_buf[1024];
|  
|  /* These are the "meat" - everything else is stuffing */
| @@ -95,6 +99,7 @@ static void udf_open_lvid(struct super_b
|  static void udf_close_lvid(struct super_block *);
|  static unsigned int udf_count_free(struct super_block *);
|  static int udf_statfs(struct dentry *, struct kstatfs *);
| +static int udf_show_options(struct seq_file *, struct vfsmount *);
|  
|  struct logicalVolIntegrityDescImpUse *udf_sb_lvidiu(struct udf_sb_info *sbi)
|  {
| @@ -181,6 +186,7 @@ static const struct super_operations udf
|  	.write_super	= udf_write_super,
|  	.statfs		= udf_statfs,
|  	.remount_fs	= udf_remount_fs,
| +	.show_options	= udf_show_options,
|  };
|  
|  struct udf_options {
| @@ -247,6 +253,56 @@ static int udf_sb_alloc_partition_maps(s
|  	return 0;
|  }
|  
| +static int udf_show_options(struct seq_file *seq, struct vfsmount *mnt)
| +{
| +	struct super_block *sb = mnt->mnt_sb;
| +	struct udf_sb_info *sbi = UDF_SB(sb);
| +
| +	if (!UDF_QUERY_FLAG(sb, UDF_FLAG_STRICT))
| +		seq_puts(seq, ",nostrict");
| +	if (sb->s_blocksize != UDF_DEFAULT_BLOCKSIZE)
| +		seq_printf(seq, ",bs=%lu", sb->s_blocksize);
| +	if (UDF_QUERY_FLAG(sb, ...
[ message continues ]
[view original message]
From: Jan Kara
Subject: Re: [patch 25/26] mount options: fix udf
Date: Friday, January 25, 2008 - 8:30 am

Please use just 'int' for 'remount' option. We are slowly trying to
get rid of these strange things in UDF code so adding new ones isn't
  Otherwise (apart from comments I wrote in the other email) the patch
is fine.

								Honza
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs
--


[ message continues ]
[view original message]
From: Miklos Szeredi
Subject: Re: [patch 25/26] mount options: fix udf
Date: Friday, January 25, 2008 - 8:56 am

What's wrong with bool?

I'm not advocating mass replacements, but all new code _should_ use
it, because it's a very useful and good type.

We are just not very much used to it yet, but don't tell me it's
confusing to see a type like this ;)

Miklos
--


[ message continues ]
[view original message]
From: Jan Kara
Subject: Re: [patch 25/26] mount options: fix udf
Date: Friday, January 25, 2008 - 9:04 am

It's not so confusing but one really isn't used to it ;) But OK, I don't
mind that much...
								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR
--


[ message continues ]
Previous thread: [patch 23/26] mount options: fix spufs by Miklos Szeredi on Thursday, January 24, 2008 - 12:34 pm. (1 message)

Next thread: [patch 24/26] mount options: fix tmpfs by Miklos Szeredi on Thursday, January 24, 2008 - 12:34 pm. (4 messages)