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@xxxxxxxxxx>
> >> ---
> >>
> >> 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@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
