diff mbox

[4/5,V2] xfs: implement online get/set fs label

Message ID 2a638f02-ea72-0d5e-30a4-1d95cfe6ff69@sandeen.net (mailing list archive)
State Superseded
Headers show

Commit Message

Eric Sandeen May 1, 2018, 11:04 p.m. UTC
The GET ioctl is trivial, just return the current label.

The SET ioctl is more involved:
It transactionally modifies the superblock to write a new filesystem
label to the primary super.

A new variant of xfs_sync_sb then writes the superblock buffer
immediately to disk so that the change is visible from userspace.

It then invalidates any page cache that userspace might have previously
read on the block device so that i.e. blkid can see the change
immediately, and updates all secondary superblocks as userspace relable
does.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

V2: rework the force-sb-to-disk approach, invalidate the whole block
device, and block waiting for the growfs lock.  Also remove too-long-label
printk.

Thanks to dchinner for the xfs_sync_sb_buf suggestion & framework.

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

Comments

Brian Foster May 2, 2018, 10:48 a.m. UTC | #1
On Tue, May 01, 2018 at 06:04:09PM -0500, Eric Sandeen wrote:
> The GET ioctl is trivial, just return the current label.
> 
> The SET ioctl is more involved:
> It transactionally modifies the superblock to write a new filesystem
> label to the primary super.
> 
> A new variant of xfs_sync_sb then writes the superblock buffer
> immediately to disk so that the change is visible from userspace.
> 
> It then invalidates any page cache that userspace might have previously
> read on the block device so that i.e. blkid can see the change
> immediately, and updates all secondary superblocks as userspace relable
> does.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> V2: rework the force-sb-to-disk approach, invalidate the whole block
> device, and block waiting for the growfs lock.  Also remove too-long-label
> printk.
> 
> Thanks to dchinner for the xfs_sync_sb_buf suggestion & framework.
> 
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index d9b94bd..54992e8 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -888,6 +888,37 @@ struct xfs_perag *
>  	return xfs_trans_commit(tp);
>  }
>  
> +/*
> + * Same behavior as xfs_sync_sb, except that it is always synchronous and it
> + * also writes the superblock buffer to disk sector 0 immediately.
> + */
> +int
> +xfs_sync_sb_buf(
> +	struct xfs_mount	*mp)
> +{
> +	struct xfs_trans	*tp;
> +	int			error;
> +
> +	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_sb, 0, 0,
> +			XFS_TRANS_NO_WRITECOUNT, &tp);
> +	if (error)
> +		return error;
> +
> +	xfs_log_sb(tp);
> +	xfs_trans_bhold(tp, mp->m_sb_bp);
> +	xfs_trans_set_sync(tp);
> +	error = xfs_trans_commit(tp);
> +	if (error)
> +		goto out;
> +	/*
> +	 * write out the sb buffer to get the changes to disk
> +	 */
> +	error = xfs_bwrite(mp->m_sb_bp);

Ah, much more simple to just write out the buffer directly. Looks pretty
good to me once those comment updates are incorporated.. ;)

Brian

> +out:
> +	xfs_buf_relse(mp->m_sb_bp);
> +	return error;
> +}
> +
>  int
>  xfs_fs_geometry(
>  	struct xfs_sb		*sbp,
> diff --git a/fs/xfs/libxfs/xfs_sb.h b/fs/xfs/libxfs/xfs_sb.h
> index 63dcd2a..2268272 100644
> --- a/fs/xfs/libxfs/xfs_sb.h
> +++ b/fs/xfs/libxfs/xfs_sb.h
> @@ -29,6 +29,7 @@ extern struct xfs_perag *xfs_perag_get_tag(struct xfs_mount *, xfs_agnumber_t,
>  
>  extern void	xfs_log_sb(struct xfs_trans *tp);
>  extern int	xfs_sync_sb(struct xfs_mount *mp, bool wait);
> +extern int	xfs_sync_sb_buf(struct xfs_mount *mp);
>  extern void	xfs_sb_mount_common(struct xfs_mount *mp, struct xfs_sb *sbp);
>  extern void	xfs_sb_from_disk(struct xfs_sb *to, struct xfs_dsb *from);
>  extern void	xfs_sb_to_disk(struct xfs_dsb *to, struct xfs_sb *from);
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 89fb1eb..effb23a 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1811,6 +1811,72 @@ struct getfsmap_info {
>  	return error;
>  }
>  
> +static int
> +xfs_ioc_getlabel(
> +	struct xfs_mount	*mp,
> +	char			__user *label)
> +{
> +	struct xfs_sb		*sbp = &mp->m_sb;
> +
> +	/* Paranoia */
> +	BUILD_BUG_ON(sizeof(sbp->sb_fname) > FSLABEL_MAX);
> +
> +	if (copy_to_user(label, sbp->sb_fname, sizeof(sbp->sb_fname)))
> +		return -EFAULT;
> +	return 0;
> +}
> +
> +static int
> +xfs_ioc_setlabel(
> +	struct file		*filp,
> +	struct xfs_mount	*mp,
> +	char			__user *newlabel)
> +{
> +	struct xfs_sb		*sbp = &mp->m_sb;
> +	char			label[FSLABEL_MAX];
> +	size_t			len;
> +	int			error;
> +
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EPERM;
> +
> +	if (copy_from_user(label, newlabel, FSLABEL_MAX))
> +		return -EFAULT;
> +	/*
> +	 * The generic ioctl allows up to FSLABEL_MAX chars, but XFS is much
> +	 * smaller, at 12 bytes.
> +	 * NB: The on disk label doesn't need to be null terminated.
> +	 */
> +	len = strnlen(label, FSLABEL_MAX);
> +	if (len > sizeof(sbp->sb_fname))
> +		return -EINVAL;
> +
> +	error = mnt_want_write_file(filp);
> +	if (error)
> +		return error;
> +
> +	spin_lock(&mp->m_sb_lock);
> +	memset(sbp->sb_fname, 0, sizeof(sbp->sb_fname));
> +	strncpy(sbp->sb_fname, label, sizeof(sbp->sb_fname));
> +	spin_unlock(&mp->m_sb_lock);
> +
> +	/* Log primary superblock label changes & force to disk. */
> +	error = xfs_sync_sb_buf(mp);
> +	if (error)
> +		goto out;
> +
> +	/* Invalidate any cached bdev page for userspace visibility. */
> +	invalidate_bdev(mp->m_ddev_targp->bt_bdev);
> +
> +	/* Update the backup superblocks like userspace does. */
> +	mutex_lock(&mp->m_growlock);
> +	error = xfs_update_secondary_supers(mp, mp->m_sb.sb_agcount, 0);
> +	mutex_unlock(&mp->m_growlock);
> +out:
> +	mnt_drop_write_file(filp);
> +	return error;
> +}
> +
>  /*
>   * Note: some of the ioctl's return positive numbers as a
>   * byte count indicating success, such as readlink_by_handle.
> @@ -1834,6 +1900,10 @@ struct getfsmap_info {
>  	switch (cmd) {
>  	case FITRIM:
>  		return xfs_ioc_trim(mp, arg);
> +	case FS_IOC_GET_FSLABEL:
> +		return xfs_ioc_getlabel(mp, arg);
> +	case FS_IOC_SET_FSLABEL:
> +		return xfs_ioc_setlabel(filp, mp, arg);
>  	case XFS_IOC_ALLOCSP:
>  	case XFS_IOC_FREESP:
>  	case XFS_IOC_RESVSP:
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Darrick J. Wong May 2, 2018, 2:24 p.m. UTC | #2
On Tue, May 01, 2018 at 06:04:09PM -0500, Eric Sandeen wrote:
> The GET ioctl is trivial, just return the current label.
> 
> The SET ioctl is more involved:
> It transactionally modifies the superblock to write a new filesystem
> label to the primary super.
> 
> A new variant of xfs_sync_sb then writes the superblock buffer
> immediately to disk so that the change is visible from userspace.
> 
> It then invalidates any page cache that userspace might have previously
> read on the block device so that i.e. blkid can see the change
> immediately, and updates all secondary superblocks as userspace relable
> does.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> V2: rework the force-sb-to-disk approach, invalidate the whole block
> device, and block waiting for the growfs lock.  Also remove too-long-label
> printk.
> 
> Thanks to dchinner for the xfs_sync_sb_buf suggestion & framework.
> 
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index d9b94bd..54992e8 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -888,6 +888,37 @@ struct xfs_perag *
>  	return xfs_trans_commit(tp);
>  }
>  
> +/*
> + * Same behavior as xfs_sync_sb, except that it is always synchronous and it
> + * also writes the superblock buffer to disk sector 0 immediately.
> + */
> +int
> +xfs_sync_sb_buf(
> +	struct xfs_mount	*mp)
> +{
> +	struct xfs_trans	*tp;
> +	int			error;
> +
> +	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_sb, 0, 0,
> +			XFS_TRANS_NO_WRITECOUNT, &tp);

I suppose this is a straight clone of xfs_sync_sb, but do we need
NO_WRITECOUNT here?  Will this get called while the fs is frozen?

> +	if (error)
> +		return error;
> +
> +	xfs_log_sb(tp);
> +	xfs_trans_bhold(tp, mp->m_sb_bp);
> +	xfs_trans_set_sync(tp);
> +	error = xfs_trans_commit(tp);
> +	if (error)
> +		goto out;
> +	/*
> +	 * write out the sb buffer to get the changes to disk
> +	 */
> +	error = xfs_bwrite(mp->m_sb_bp);
> +out:
> +	xfs_buf_relse(mp->m_sb_bp);
> +	return error;
> +}
> +
>  int
>  xfs_fs_geometry(
>  	struct xfs_sb		*sbp,
> diff --git a/fs/xfs/libxfs/xfs_sb.h b/fs/xfs/libxfs/xfs_sb.h
> index 63dcd2a..2268272 100644
> --- a/fs/xfs/libxfs/xfs_sb.h
> +++ b/fs/xfs/libxfs/xfs_sb.h
> @@ -29,6 +29,7 @@ extern struct xfs_perag *xfs_perag_get_tag(struct xfs_mount *, xfs_agnumber_t,
>  
>  extern void	xfs_log_sb(struct xfs_trans *tp);
>  extern int	xfs_sync_sb(struct xfs_mount *mp, bool wait);
> +extern int	xfs_sync_sb_buf(struct xfs_mount *mp);
>  extern void	xfs_sb_mount_common(struct xfs_mount *mp, struct xfs_sb *sbp);
>  extern void	xfs_sb_from_disk(struct xfs_sb *to, struct xfs_dsb *from);
>  extern void	xfs_sb_to_disk(struct xfs_dsb *to, struct xfs_sb *from);
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 89fb1eb..effb23a 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1811,6 +1811,72 @@ struct getfsmap_info {
>  	return error;
>  }
>  
> +static int
> +xfs_ioc_getlabel(
> +	struct xfs_mount	*mp,
> +	char			__user *label)
> +{
> +	struct xfs_sb		*sbp = &mp->m_sb;
> +
> +	/* Paranoia */
> +	BUILD_BUG_ON(sizeof(sbp->sb_fname) > FSLABEL_MAX);
> +
> +	if (copy_to_user(label, sbp->sb_fname, sizeof(sbp->sb_fname)))

Needs to ensure that a null is set at the end of the (userspace) buffer
just in case the label is "123456789012".

There's nothing in the documentation for this ioctl <cough> that says
the passed in buffer must already be zeroed.

> +		return -EFAULT;
> +	return 0;
> +}
> +
> +static int
> +xfs_ioc_setlabel(
> +	struct file		*filp,
> +	struct xfs_mount	*mp,
> +	char			__user *newlabel)
> +{
> +	struct xfs_sb		*sbp = &mp->m_sb;
> +	char			label[FSLABEL_MAX];
> +	size_t			len;
> +	int			error;
> +
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EPERM;
> +
> +	if (copy_from_user(label, newlabel, FSLABEL_MAX))
> +		return -EFAULT;
> +	/*
> +	 * The generic ioctl allows up to FSLABEL_MAX chars, but XFS is much
> +	 * smaller, at 12 bytes.
> +	 * NB: The on disk label doesn't need to be null terminated.
> +	 */
> +	len = strnlen(label, FSLABEL_MAX);
> +	if (len > sizeof(sbp->sb_fname))
> +		return -EINVAL;
> +
> +	error = mnt_want_write_file(filp);
> +	if (error)
> +		return error;
> +
> +	spin_lock(&mp->m_sb_lock);
> +	memset(sbp->sb_fname, 0, sizeof(sbp->sb_fname));
> +	strncpy(sbp->sb_fname, label, sizeof(sbp->sb_fname));
> +	spin_unlock(&mp->m_sb_lock);
> +
> +	/* Log primary superblock label changes & force to disk. */
> +	error = xfs_sync_sb_buf(mp);
> +	if (error)
> +		goto out;
> +
> +	/* Invalidate any cached bdev page for userspace visibility. */
> +	invalidate_bdev(mp->m_ddev_targp->bt_bdev);
> +
> +	/* Update the backup superblocks like userspace does. */
> +	mutex_lock(&mp->m_growlock);
> +	error = xfs_update_secondary_supers(mp, mp->m_sb.sb_agcount, 0);
> +	mutex_unlock(&mp->m_growlock);
> +out:
> +	mnt_drop_write_file(filp);
> +	return error;

Looks ok enough to start testing.

--D

> +}
> +
>  /*
>   * Note: some of the ioctl's return positive numbers as a
>   * byte count indicating success, such as readlink_by_handle.
> @@ -1834,6 +1900,10 @@ struct getfsmap_info {
>  	switch (cmd) {
>  	case FITRIM:
>  		return xfs_ioc_trim(mp, arg);
> +	case FS_IOC_GET_FSLABEL:
> +		return xfs_ioc_getlabel(mp, arg);
> +	case FS_IOC_SET_FSLABEL:
> +		return xfs_ioc_setlabel(filp, mp, arg);
>  	case XFS_IOC_ALLOCSP:
>  	case XFS_IOC_FREESP:
>  	case XFS_IOC_RESVSP:
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Sandeen May 2, 2018, 2:28 p.m. UTC | #3
On 5/2/18 9:24 AM, Darrick J. Wong wrote:
> On Tue, May 01, 2018 at 06:04:09PM -0500, Eric Sandeen wrote:
>> The GET ioctl is trivial, just return the current label.
>>
>> The SET ioctl is more involved:
>> It transactionally modifies the superblock to write a new filesystem
>> label to the primary super.
>>
>> A new variant of xfs_sync_sb then writes the superblock buffer
>> immediately to disk so that the change is visible from userspace.
>>
>> It then invalidates any page cache that userspace might have previously
>> read on the block device so that i.e. blkid can see the change
>> immediately, and updates all secondary superblocks as userspace relable
>> does.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> V2: rework the force-sb-to-disk approach, invalidate the whole block
>> device, and block waiting for the growfs lock.  Also remove too-long-label
>> printk.
>>
>> Thanks to dchinner for the xfs_sync_sb_buf suggestion & framework.
>>
>> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
>> index d9b94bd..54992e8 100644
>> --- a/fs/xfs/libxfs/xfs_sb.c
>> +++ b/fs/xfs/libxfs/xfs_sb.c
>> @@ -888,6 +888,37 @@ struct xfs_perag *
>>  	return xfs_trans_commit(tp);
>>  }
>>  
>> +/*
>> + * Same behavior as xfs_sync_sb, except that it is always synchronous and it
>> + * also writes the superblock buffer to disk sector 0 immediately.
>> + */
>> +int
>> +xfs_sync_sb_buf(
>> +	struct xfs_mount	*mp)
>> +{
>> +	struct xfs_trans	*tp;
>> +	int			error;
>> +
>> +	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_sb, 0, 0,
>> +			XFS_TRANS_NO_WRITECOUNT, &tp);
> 
> I suppose this is a straight clone of xfs_sync_sb, but do we need
> NO_WRITECOUNT here?  Will this get called while the fs is frozen?

No, I should remove that, thanks.  I might see how ugly it'd be to just
convert xfs_sync_sb() to take flags for wait, no_writecount, and flush_sb
or something. 
...
 
>> +static int
>> +xfs_ioc_getlabel(
>> +	struct xfs_mount	*mp,
>> +	char			__user *label)
>> +{
>> +	struct xfs_sb		*sbp = &mp->m_sb;
>> +
>> +	/* Paranoia */
>> +	BUILD_BUG_ON(sizeof(sbp->sb_fname) > FSLABEL_MAX);
>> +
>> +	if (copy_to_user(label, sbp->sb_fname, sizeof(sbp->sb_fname)))
> 
> Needs to ensure that a null is set at the end of the (userspace) buffer
> just in case the label is "123456789012".

Oh, yup!

> There's nothing in the documentation for this ioctl <cough> that says
> the passed in buffer must already be zeroed.

where /do/ we document ioctls like this, anyway?

Documentation/ioctl/* I guess, though of course we don't.

Thanks,
-Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Darrick J. Wong May 2, 2018, 2:38 p.m. UTC | #4
On Wed, May 02, 2018 at 09:28:35AM -0500, Eric Sandeen wrote:
> On 5/2/18 9:24 AM, Darrick J. Wong wrote:
> > On Tue, May 01, 2018 at 06:04:09PM -0500, Eric Sandeen wrote:
> >> The GET ioctl is trivial, just return the current label.
> >>
> >> The SET ioctl is more involved:
> >> It transactionally modifies the superblock to write a new filesystem
> >> label to the primary super.
> >>
> >> A new variant of xfs_sync_sb then writes the superblock buffer
> >> immediately to disk so that the change is visible from userspace.
> >>
> >> It then invalidates any page cache that userspace might have previously
> >> read on the block device so that i.e. blkid can see the change
> >> immediately, and updates all secondary superblocks as userspace relable
> >> does.
> >>
> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> >> ---
> >>
> >> V2: rework the force-sb-to-disk approach, invalidate the whole block
> >> device, and block waiting for the growfs lock.  Also remove too-long-label
> >> printk.
> >>
> >> Thanks to dchinner for the xfs_sync_sb_buf suggestion & framework.
> >>
> >> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> >> index d9b94bd..54992e8 100644
> >> --- a/fs/xfs/libxfs/xfs_sb.c
> >> +++ b/fs/xfs/libxfs/xfs_sb.c
> >> @@ -888,6 +888,37 @@ struct xfs_perag *
> >>  	return xfs_trans_commit(tp);
> >>  }
> >>  
> >> +/*
> >> + * Same behavior as xfs_sync_sb, except that it is always synchronous and it
> >> + * also writes the superblock buffer to disk sector 0 immediately.
> >> + */
> >> +int
> >> +xfs_sync_sb_buf(
> >> +	struct xfs_mount	*mp)
> >> +{
> >> +	struct xfs_trans	*tp;
> >> +	int			error;
> >> +
> >> +	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_sb, 0, 0,
> >> +			XFS_TRANS_NO_WRITECOUNT, &tp);
> > 
> > I suppose this is a straight clone of xfs_sync_sb, but do we need
> > NO_WRITECOUNT here?  Will this get called while the fs is frozen?
> 
> No, I should remove that, thanks.  I might see how ugly it'd be to just
> convert xfs_sync_sb() to take flags for wait, no_writecount, and flush_sb
> or something. 
> ...
>  
> >> +static int
> >> +xfs_ioc_getlabel(
> >> +	struct xfs_mount	*mp,
> >> +	char			__user *label)
> >> +{
> >> +	struct xfs_sb		*sbp = &mp->m_sb;
> >> +
> >> +	/* Paranoia */
> >> +	BUILD_BUG_ON(sizeof(sbp->sb_fname) > FSLABEL_MAX);
> >> +
> >> +	if (copy_to_user(label, sbp->sb_fname, sizeof(sbp->sb_fname)))
> > 
> > Needs to ensure that a null is set at the end of the (userspace) buffer
> > just in case the label is "123456789012".
> 
> Oh, yup!
> 
> > There's nothing in the documentation for this ioctl <cough> that says
> > the passed in buffer must already be zeroed.
> 
> where /do/ we document ioctls like this, anyway?
> 
> Documentation/ioctl/* I guess, though of course we don't.

In the man-pages project, of course:
https://www.kernel.org/doc/man-pages/contributing.html

See example of previous efforts:
http://man7.org/linux/man-pages/man2/ioctl_getfsmap.2.html

--D

> 
> Thanks,
> -Eric
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dave Chinner May 2, 2018, 9:57 p.m. UTC | #5
On Wed, May 02, 2018 at 09:28:35AM -0500, Eric Sandeen wrote:
> On 5/2/18 9:24 AM, Darrick J. Wong wrote:
> > On Tue, May 01, 2018 at 06:04:09PM -0500, Eric Sandeen wrote:
> >> The GET ioctl is trivial, just return the current label.
> >> +static int
> >> +xfs_ioc_getlabel(
> >> +	struct xfs_mount	*mp,
> >> +	char			__user *label)
> >> +{
> >> +	struct xfs_sb		*sbp = &mp->m_sb;
> >> +
> >> +	/* Paranoia */
> >> +	BUILD_BUG_ON(sizeof(sbp->sb_fname) > FSLABEL_MAX);
> >> +
> >> +	if (copy_to_user(label, sbp->sb_fname, sizeof(sbp->sb_fname)))
> > 
> > Needs to ensure that a null is set at the end of the (userspace) buffer
> > just in case the label is "123456789012".
> 
> Oh, yup!
> 
> > There's nothing in the documentation for this ioctl <cough> that says
> > the passed in buffer must already be zeroed.
> 
> where /do/ we document ioctls like this, anyway?

man pages. You should really cc linux-api@vger.kernel.org on any
patch that introduces/hoists a new VFS ioctl, as well as the man
page patch to document it.

Cheers,

Dave.
diff mbox

Patch

diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index d9b94bd..54992e8 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -888,6 +888,37 @@  struct xfs_perag *
 	return xfs_trans_commit(tp);
 }
 
+/*
+ * Same behavior as xfs_sync_sb, except that it is always synchronous and it
+ * also writes the superblock buffer to disk sector 0 immediately.
+ */
+int
+xfs_sync_sb_buf(
+	struct xfs_mount	*mp)
+{
+	struct xfs_trans	*tp;
+	int			error;
+
+	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_sb, 0, 0,
+			XFS_TRANS_NO_WRITECOUNT, &tp);
+	if (error)
+		return error;
+
+	xfs_log_sb(tp);
+	xfs_trans_bhold(tp, mp->m_sb_bp);
+	xfs_trans_set_sync(tp);
+	error = xfs_trans_commit(tp);
+	if (error)
+		goto out;
+	/*
+	 * write out the sb buffer to get the changes to disk
+	 */
+	error = xfs_bwrite(mp->m_sb_bp);
+out:
+	xfs_buf_relse(mp->m_sb_bp);
+	return error;
+}
+
 int
 xfs_fs_geometry(
 	struct xfs_sb		*sbp,
diff --git a/fs/xfs/libxfs/xfs_sb.h b/fs/xfs/libxfs/xfs_sb.h
index 63dcd2a..2268272 100644
--- a/fs/xfs/libxfs/xfs_sb.h
+++ b/fs/xfs/libxfs/xfs_sb.h
@@ -29,6 +29,7 @@  extern struct xfs_perag *xfs_perag_get_tag(struct xfs_mount *, xfs_agnumber_t,
 
 extern void	xfs_log_sb(struct xfs_trans *tp);
 extern int	xfs_sync_sb(struct xfs_mount *mp, bool wait);
+extern int	xfs_sync_sb_buf(struct xfs_mount *mp);
 extern void	xfs_sb_mount_common(struct xfs_mount *mp, struct xfs_sb *sbp);
 extern void	xfs_sb_from_disk(struct xfs_sb *to, struct xfs_dsb *from);
 extern void	xfs_sb_to_disk(struct xfs_dsb *to, struct xfs_sb *from);
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 89fb1eb..effb23a 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1811,6 +1811,72 @@  struct getfsmap_info {
 	return error;
 }
 
+static int
+xfs_ioc_getlabel(
+	struct xfs_mount	*mp,
+	char			__user *label)
+{
+	struct xfs_sb		*sbp = &mp->m_sb;
+
+	/* Paranoia */
+	BUILD_BUG_ON(sizeof(sbp->sb_fname) > FSLABEL_MAX);
+
+	if (copy_to_user(label, sbp->sb_fname, sizeof(sbp->sb_fname)))
+		return -EFAULT;
+	return 0;
+}
+
+static int
+xfs_ioc_setlabel(
+	struct file		*filp,
+	struct xfs_mount	*mp,
+	char			__user *newlabel)
+{
+	struct xfs_sb		*sbp = &mp->m_sb;
+	char			label[FSLABEL_MAX];
+	size_t			len;
+	int			error;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (copy_from_user(label, newlabel, FSLABEL_MAX))
+		return -EFAULT;
+	/*
+	 * The generic ioctl allows up to FSLABEL_MAX chars, but XFS is much
+	 * smaller, at 12 bytes.
+	 * NB: The on disk label doesn't need to be null terminated.
+	 */
+	len = strnlen(label, FSLABEL_MAX);
+	if (len > sizeof(sbp->sb_fname))
+		return -EINVAL;
+
+	error = mnt_want_write_file(filp);
+	if (error)
+		return error;
+
+	spin_lock(&mp->m_sb_lock);
+	memset(sbp->sb_fname, 0, sizeof(sbp->sb_fname));
+	strncpy(sbp->sb_fname, label, sizeof(sbp->sb_fname));
+	spin_unlock(&mp->m_sb_lock);
+
+	/* Log primary superblock label changes & force to disk. */
+	error = xfs_sync_sb_buf(mp);
+	if (error)
+		goto out;
+
+	/* Invalidate any cached bdev page for userspace visibility. */
+	invalidate_bdev(mp->m_ddev_targp->bt_bdev);
+
+	/* Update the backup superblocks like userspace does. */
+	mutex_lock(&mp->m_growlock);
+	error = xfs_update_secondary_supers(mp, mp->m_sb.sb_agcount, 0);
+	mutex_unlock(&mp->m_growlock);
+out:
+	mnt_drop_write_file(filp);
+	return error;
+}
+
 /*
  * Note: some of the ioctl's return positive numbers as a
  * byte count indicating success, such as readlink_by_handle.
@@ -1834,6 +1900,10 @@  struct getfsmap_info {
 	switch (cmd) {
 	case FITRIM:
 		return xfs_ioc_trim(mp, arg);
+	case FS_IOC_GET_FSLABEL:
+		return xfs_ioc_getlabel(mp, arg);
+	case FS_IOC_SET_FSLABEL:
+		return xfs_ioc_setlabel(filp, mp, arg);
 	case XFS_IOC_ALLOCSP:
 	case XFS_IOC_FREESP:
 	case XFS_IOC_RESVSP: