On Fri, Oct 03, 2014 at 05:05:46PM -0500, Ben Myers wrote:
> From: Ben Myers <bpm@xxxxxxx>
>
> This adds a utf8version field to the xfs_fs_geom structure. An
> important characteristic of this version of the ioctl is that
> fsgeo.version needs to be set by the caller to specify which version of
> the structure to return.
>
> Signed-off-by: Ben Myers <bpm@xxxxxxx>
> ---
> fs/xfs/xfs_fs.h | 31 +++++++++++++++++++++++++++++++
> fs/xfs/xfs_fsops.c | 13 ++++++++++++-
> fs/xfs/xfs_fsops.h | 2 +-
> fs/xfs/xfs_ioctl.c | 31 +++++++++++++++++++++++++++++++
> 4 files changed, 75 insertions(+), 2 deletions(-)
>
> diff --git a/fs/xfs/xfs_fs.h b/fs/xfs/xfs_fs.h
> index fd45cbe..2f4d430 100644
> --- a/fs/xfs/xfs_fs.h
> +++ b/fs/xfs/xfs_fs.h
> @@ -206,6 +206,34 @@ typedef struct xfs_fsop_geom_v2 {
> __u32 logsunit; /* log stripe unit, bytes */
> } xfs_fsop_geom_v2_t;
>
> +/*
> + * Output for XFS_IOC_FSGEOMETRY
> + */
> +typedef struct xfs_fsop_geom {
> + __u32 blocksize; /* filesystem (data) block size */
> + __u32 rtextsize; /* realtime extent size */
> + __u32 agblocks; /* fsblocks in an AG */
> + __u32 agcount; /* number of allocation groups */
> + __u32 logblocks; /* fsblocks in the log */
> + __u32 sectsize; /* (data) sector size, bytes */
> + __u32 inodesize; /* inode size in bytes */
> + __u32 imaxpct; /* max allowed inode space(%) */
> + __u64 datablocks; /* fsblocks in data subvolume */
> + __u64 rtblocks; /* fsblocks in realtime subvol */
> + __u64 rtextents; /* rt extents in realtime subvol*/
> + __u64 logstart; /* starting fsblock of the log */
> + unsigned char uuid[16]; /* unique id of the filesystem */
> + __u32 sunit; /* stripe unit, fsblocks */
> + __u32 swidth; /* stripe width, fsblocks */
> + __s32 version; /* structure version */
> + __u32 flags; /* superblock version flags */
> + __u32 logsectsize; /* log sector size, bytes */
> + __u32 rtsectsize; /* realtime sector size, bytes */
> + __u32 dirblocksize; /* directory block size, bytes */
> + __u32 logsunit; /* log stripe unit, bytes */
> + __u32 utf8version; /* Unicode version */
> +} xfs_fsop_geom_t;
New structure defintion, not a redefinition of the old name, please.
Drop the typedef, and the structure needs to be 64 bit size
aligned so we don't get problems with 32 bit userspace on 64 bit
kernels (e.g. we have a v1 compat ioctl handler because of this
issue).
Further, lets avoid needing to rev the ioctl again in future by
adding a bunch of "must be zero" padding to the new structure so we
can extend the information we push to userspace easily. i.e. padding
only becomes meaningful when the superblock flag that exposes
meaning is set. i.e. userspace can do this to conditionally access
the ut8version value if it is meaningful:
utf8_ver = 0;
if (geo.flags & XFS_FSOP_GEOM_FLAGS_UTF8)
utf8_ver = geo->utf8version;
i.e. let's make the new structure forwards compatible with new
features...
> @@ -115,6 +117,15 @@ xfs_fs_geometry(
> XFS_FSOP_GEOM_FLAGS_LOGV2 : 0);
> geo->logsunit = mp->m_sb.sb_logsunit;
> }
> + if (new_version >= XFS_FSOP_GEOM_VERSION5) {
> + geo->version = XFS_FSOP_GEOM_VERSION5;
> + geo->flags |= (xfs_sb_version_hasutf8(&mp->m_sb) ?
> + XFS_FSOP_GEOM_FLAGS_UTF8 : 0);
> + geo->utf8version = mp->m_sb.sb_utf8version;
> + if (bytes)
> + *bytes = sizeof(xfs_fsop_geom_v2_t) +
> + sizeof(geo->utf8version);
Further indication that the *bytes variable should die.
> + }
> return 0;
> }
>
> diff --git a/fs/xfs/xfs_fsops.h b/fs/xfs/xfs_fsops.h
> index 74e1fee..b723f36 100644
> --- a/fs/xfs/xfs_fsops.h
> +++ b/fs/xfs/xfs_fsops.h
> @@ -18,7 +18,7 @@
> #ifndef __XFS_FSOPS_H__
> #define __XFS_FSOPS_H__
>
> -extern int xfs_fs_geometry(xfs_mount_t *mp, xfs_fsop_geom_v2_t *geo,
> +extern int xfs_fs_geometry(xfs_mount_t *mp, void *buffer,
> int new_version, size_t *bytes);
> extern int xfs_growfs_data(xfs_mount_t *mp, xfs_growfs_data_t *in);
> extern int xfs_growfs_log(xfs_mount_t *mp, xfs_growfs_log_t *in);
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 1657ce5..6308680 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -859,6 +859,34 @@ xfs_ioc_fsgeometry_v2(
> return 0;
> }
>
> +STATIC int
> +xfs_ioc_fsgeometry(
> + struct xfs_mount *mp,
> + void __user *arg)
> +{
> + xfs_fsop_geom_t fsgeo;
> + int version, error;
> + size_t bytes;
> +
> + /* offsetof(version)? XXX just get 32 bits? */
> + if (copy_from_user(&fsgeo, arg, sizeof(xfs_fsop_geom_v1_t)))
> + return -EFAULT;
It's best to copy in the entire structure rather than play offset
games.
> + version = fsgeo.version;
> +
> + if (version < XFS_FSOP_GEOM_VERSION5)
> + return -EINVAL;
Here it rejects anything that is not a v3 structure aware of the
unicode extensions, which means it breaks any old recompiled
application that hasn't been updated to support
XFS_FSOP_GEOM_VERSION5 despite the fact that they will compile
against headers with the new definition without warnings or errors.
> +
> + memset(&fsgeo, 0, sizeof(fsgeo));
> + error = xfs_fs_geometry(mp, &fsgeo, version, &bytes);
> + if (error)
> + return error;
> +
> + if (copy_to_user(arg, &fsgeo, bytes))
> + return -EFAULT;
and you can use sizeof(struct xfs_fs_geom_v3) here instead of bytes.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
