On Fri, Oct 03, 2014 at 05:00:31PM -0500, Ben Myers wrote:
> From: Ben Myers <bpm@xxxxxxx>
>
> The utf8 version a filesystem was created with needs to be stored in
> order that normalizations will remain stable over the lifetime of the
> filesystem. Convert sb_pad to sb_utf8version in the super block. This
> also adds checks at mount time to see whether the unicode normalization
> module has support for the version of unicode that the filesystem
> requires. If not we fail the mount.
>
> Signed-off-by: Ben Myers <bpm@xxxxxxx>
> ---
> fs/xfs/libxfs/xfs_dir2.c | 28 ++++++++++++++++---
> fs/xfs/libxfs/xfs_sb.c | 4 +--
> fs/xfs/libxfs/xfs_sb.h | 10 ++++---
> fs/xfs/libxfs/xfs_utf8.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++
> fs/xfs/libxfs/xfs_utf8.h | 24 +++++++++++++++++
> 5 files changed, 126 insertions(+), 10 deletions(-)
> create mode 100644 fs/xfs/libxfs/xfs_utf8.c
> create mode 100644 fs/xfs/libxfs/xfs_utf8.h
>
> diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
> index 4eb0973..2c89211 100644
> --- a/fs/xfs/libxfs/xfs_dir2.c
> +++ b/fs/xfs/libxfs/xfs_dir2.c
> @@ -157,10 +157,30 @@ xfs_da_mount(
> (uint)sizeof(xfs_da_node_entry_t);
> dageo->magicpct = (dageo->blksize * 37) / 100;
>
> - if (xfs_sb_version_hasasciici(&mp->m_sb))
> - mp->m_dirnameops = &xfs_ascii_ci_nameops;
> - else
> - mp->m_dirnameops = &xfs_default_nameops;
> + if (xfs_sb_version_hasutf8(&mp->m_sb)) {
> +#ifdef CONFIG_XFS_UTF8
> + if (!xfs_utf8_version_ok(mp))
> + return -ENOSYS;
> +
> + /* XXX these are replaced in the next patch need
> + to do some kind of reordering here */
> + if (xfs_sb_version_hasasciici(&mp->m_sb))
> + mp->m_dirnameops = &xfs_ascii_ci_nameops;
> + else
> + mp->m_dirnameops = &xfs_default_nameops;
> +#else
> + xfs_warn(mp,
> +"Recompile XFS with CONFIG_XFS_UTF8 to mount this filesystem");
> + kmem_free(mp->m_dir_geo);
> + kmem_free(mp->m_attr_geo);
> + return -ENOSYS;
> +#endif
This config check doesn't belong here. Validation of superblock
fields vs the current config goes in the superblock verifier. I also
think that indication of UTF8 support being compiled in needs to go
in the XFS_VERSION_STRING, not have ifdef hackery added into the
code.
i.e. the mount should fail very early on with a superblock
verification failure from xfs_mount_validate_sb().
> + } else {
> + if (xfs_sb_version_hasasciici(&mp->m_sb))
> + mp->m_dirnameops = &xfs_ascii_ci_nameops;
> + else
> + mp->m_dirnameops = &xfs_default_nameops;
> + }
>
> return 0;
> }
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index ad525a5..1ee7d33 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -99,7 +99,7 @@ static const struct {
> { offsetof(xfs_sb_t, sb_features_incompat), 0 },
> { offsetof(xfs_sb_t, sb_features_log_incompat), 0 },
> { offsetof(xfs_sb_t, sb_crc), 0 },
> - { offsetof(xfs_sb_t, sb_pad), 0 },
> + { offsetof(xfs_sb_t, sb_utf8version), 0 },
> { offsetof(xfs_sb_t, sb_pquotino), 0 },
> { offsetof(xfs_sb_t, sb_lsn), 0 },
> { sizeof(xfs_sb_t), 0 }
> @@ -443,7 +443,7 @@ __xfs_sb_from_disk(
> to->sb_features_incompat = be32_to_cpu(from->sb_features_incompat);
> to->sb_features_log_incompat =
> be32_to_cpu(from->sb_features_log_incompat);
> - to->sb_pad = 0;
> + to->sb_utf8version = be32_to_cpu(from->sb_utf8version);
> to->sb_pquotino = be64_to_cpu(from->sb_pquotino);
> to->sb_lsn = be64_to_cpu(from->sb_lsn);
> /* Convert on-disk flags to in-memory flags? */
> diff --git a/fs/xfs/libxfs/xfs_sb.h b/fs/xfs/libxfs/xfs_sb.h
> index 525eacb..dc7b6c6 100644
> --- a/fs/xfs/libxfs/xfs_sb.h
> +++ b/fs/xfs/libxfs/xfs_sb.h
> @@ -159,7 +159,7 @@ typedef struct xfs_sb {
> __uint32_t sb_features_log_incompat;
>
> __uint32_t sb_crc; /* superblock crc */
> - __uint32_t sb_pad;
> + __uint32_t sb_utf8version; /* unicode version */
>
> xfs_ino_t sb_pquotino; /* project quota inode */
> xfs_lsn_t sb_lsn; /* last write sequence */
> @@ -245,7 +245,7 @@ typedef struct xfs_dsb {
> __be32 sb_features_log_incompat;
>
> __le32 sb_crc; /* superblock crc */
> - __be32 sb_pad;
> + __be32 sb_utf8version; /* version of unicode */
>
> __be64 sb_pquotino; /* project quota inode */
> __be64 sb_lsn; /* last write sequence */
> @@ -271,7 +271,7 @@ typedef enum {
> XFS_SBS_LOGSECTLOG, XFS_SBS_LOGSECTSIZE, XFS_SBS_LOGSUNIT,
> XFS_SBS_FEATURES2, XFS_SBS_BAD_FEATURES2, XFS_SBS_FEATURES_COMPAT,
> XFS_SBS_FEATURES_RO_COMPAT, XFS_SBS_FEATURES_INCOMPAT,
> - XFS_SBS_FEATURES_LOG_INCOMPAT, XFS_SBS_CRC, XFS_SBS_PAD,
> + XFS_SBS_FEATURES_LOG_INCOMPAT, XFS_SBS_CRC, XFS_SBS_UTF8VERSION,
> XFS_SBS_PQUOTINO, XFS_SBS_LSN,
> XFS_SBS_FIELDCOUNT
> } xfs_sb_field_t;
> @@ -303,6 +303,7 @@ typedef enum {
> #define XFS_SB_FEATURES_INCOMPAT XFS_SB_MVAL(FEATURES_INCOMPAT)
> #define XFS_SB_FEATURES_LOG_INCOMPAT XFS_SB_MVAL(FEATURES_LOG_INCOMPAT)
> #define XFS_SB_CRC XFS_SB_MVAL(CRC)
> +#define XFS_SB_UTF8VERSION XFS_SB_MVAL(UTF8VERSION)
> #define XFS_SB_PQUOTINO XFS_SB_MVAL(PQUOTINO)
> #define XFS_SB_NUM_BITS ((int)XFS_SBS_FIELDCOUNT)
> #define XFS_SB_ALL_BITS ((1LL << XFS_SB_NUM_BITS) - 1)
> @@ -313,7 +314,8 @@ typedef enum {
> XFS_SB_ICOUNT | XFS_SB_IFREE | XFS_SB_FDBLOCKS | XFS_SB_FEATURES2 | \
> XFS_SB_BAD_FEATURES2 | XFS_SB_FEATURES_COMPAT | \
> XFS_SB_FEATURES_RO_COMPAT | XFS_SB_FEATURES_INCOMPAT | \
> - XFS_SB_FEATURES_LOG_INCOMPAT | XFS_SB_PQUOTINO)
> + XFS_SB_FEATURES_LOG_INCOMPAT | XFS_SB_UTF8VERSION | \
> + XFS_SB_PQUOTINO)
We should never be modifying the utf8 version number from the
kernel, so this shouldn't be set in the XFS_SB_MOD_BITS mask.
> diff --git a/fs/xfs/libxfs/xfs_utf8.c b/fs/xfs/libxfs/xfs_utf8.c
> new file mode 100644
> index 0000000..7e63111
> --- /dev/null
> +++ b/fs/xfs/libxfs/xfs_utf8.c
> @@ -0,0 +1,70 @@
> +/*
> + * Copyright (c) 2014 SGI.
> + * 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 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it would 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 the Free Software Foundation,
> + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include "xfs.h"
> +#include "xfs_fs.h"
> +#include "xfs_types.h"
> +#include "xfs_bit.h"
> +#include "xfs_log_format.h"
> +#include "xfs_inum.h"
> +#include "xfs_trans.h"
> +#include "xfs_trans_resv.h"
> +#include "xfs_sb.h"
> +#include "xfs_ag.h"
> +#include "xfs_da_format.h"
> +#include "xfs_da_btree.h"
> +#include "xfs_dir2.h"
> +#include "xfs_mount.h"
> +#include "xfs_da_btree.h"
> +#include "xfs_format.h"
> +#include "xfs_bmap_btree.h"
> +#include "xfs_alloc_btree.h"
> +#include "xfs_dinode.h"
> +#include "xfs_inode.h"
> +#include "xfs_inode_item.h"
> +#include "xfs_bmap.h"
> +#include "xfs_error.h"
> +#include "xfs_trace.h"
> +#include "xfs_utf8.h"
This may sound pedantic, but in all the libxfs rework I've managed
to standadise the initial include file order to be roughly:
#include "xfs.h"
#include "xfs_fs.h"
#include "xfs_shared.h"
#include "xfs_format.h"
#include "xfs_log_format.h"
#include "xfs_trans_resv.h"
#include "xfs_bit.h"
#include "xfs_inum.h"
#include "xfs_sb.h"
#include "xfs_ag.h"
#include "xfs_mount.h"
#include "xfs_da_format.h"
i.e. include all the type, shared and on-disk format information
first. Can you re-order these to follow the same convention?
> +#include <linux/utf8norm.h>
And that should end up being included from fs/xfs/xfs_linux.h,
because we can't include things directly from the linux kernel
headers in fs/xfs/libxfs/ files.
> +
> +int
Bool.
> +xfs_utf8_version_ok(
> + struct xfs_mount *mp)
> +{
> + int major, minor, revision;
> +
> + if (utf8version_is_supported(mp->m_sb.sb_utf8version))
> + return 1;
return true;
> +
> + major = mp->m_sb.sb_utf8version >> UNICODE_MAJ_SHIFT;
> + minor = (mp->m_sb.sb_utf8version & 0xff00) >> UNICODE_MIN_SHIFT;
> + revision = mp->m_sb.sb_utf8version & 0xff;
> +
> + if (revision) {
> + xfs_warn(mp,
> + "Unicode version %d.%d.%d not supported by utf8norm.ko",
> + major, minor, revision);
> + } else {
> + xfs_warn(mp,
> + "Unicode version %d.%d not supported by utf8norm.ko",
> + major, minor);
> + }
why do you need two different print statements? Version X.Y.0 is
pretty much recognisable as being the same as X.Y....
> +
> + return 0;
return false;
> +}
> diff --git a/fs/xfs/libxfs/xfs_utf8.h b/fs/xfs/libxfs/xfs_utf8.h
> new file mode 100644
> index 0000000..8a700de
> --- /dev/null
> +++ b/fs/xfs/libxfs/xfs_utf8.h
> @@ -0,0 +1,24 @@
> +/*
> + * Copyright (c) 2014 SGI.
> + * 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 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it would 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 the Free Software Foundation,
> + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef XFS_UTF8_H
> +#define XFS_UTF8_H
> +
> +extern int xfs_utf8_version_ok(struct xfs_mount *);
> +
> +#endif /* XFS_UTF8_H */
Do we really need a separate header file for this?
fs/xfs/libxfs/xfs_shared.h was created for such one-off or
limited definitions that need to be shared with userspace...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
