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.
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
