| Message ID | 20211112082019.22078-1-lczerner@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] ext4: implement support for get/set fs label | expand |
On Nov 12, 2021, at 1:20 AM, Lukas Czerner <lczerner@redhat.com> wrote: > > Implement support for FS_IOC_GETFSLABEL and FS_IOC_SETFSLABEL ioctls for > online reading and setting of file system label. > > ext4_ioctl_getlabel() is simple, just get the label from the primary > superblock bh. This might not be the first sb on the file system if > 'sb=' mount option is used. > > In ext4_ioctl_setlabel() we update what ext4 currently views as a > primary superblock and then proceed to update backup superblocks. There > are two caveats: > - the primary superblock might not be the first superblock and so it > might not be the one used by userspace tools if read directly > off the disk. > - because the primary superblock might not be the first superblock we > potentialy have to update it as part of backup superblock update. > However the first sb location is a bit more complicated than the rest > so we have to account for that. > > Tested with generic/492 with various configurations. I also checked the > behavior with 'sb=' mount options, including very large file systems > with and without sparse_super/sparse_super2. > > Signed-off-by: Lukas Czerner <lczerner@redhat.com> > --- One minor issue/question inline. > +static int ext4_ioctl_setlabel(struct file *filp, const char __user *user_label) > +{ > + size_t len; > + handle_t *handle; > + ext4_group_t ngroups; > + ext4_fsblk_t sb_block; > + struct buffer_head *bh; > + int ret = 0, ret2, grp; > + unsigned long offset = 0; > + char new_label[EXT4_LABEL_MAX + 1]; > + struct super_block *sb = file_inode(filp)->i_sb; > + struct ext4_sb_info *sbi = EXT4_SB(sb); > + struct ext4_super_block *es = sbi->s_es; > + > + /* Sanity check, this should never happen */ > + BUILD_BUG_ON(sizeof(es->s_volume_name) < EXT4_LABEL_MAX); > + > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + /* > + * Copy the maximum length allowed for ext4 label with one more to > + * find the required terminating null byte in order to test the > + * label length. The on disk label doesn't need to be null terminated. > + */ > + if (copy_from_user(new_label, user_label, EXT4_LABEL_MAX + 1)) > + return -EFAULT; > + > + len = strnlen(new_label, EXT4_LABEL_MAX + 1); > + if (len > EXT4_LABEL_MAX) > + return -EINVAL; > + > + ret = mnt_want_write_file(filp); > + if (ret) > + return ret; > + > + handle = ext4_journal_start_sb(sb, EXT4_HT_MISC, EXT4_MAX_TRANS_DATA); > + if (IS_ERR(handle)) { > + ret = PTR_ERR(handle); > + goto err_out; > + } > + /* Update the primary superblock first */ > + ret = ext4_journal_get_write_access(handle, sb, > + sbi->s_sbh, > + EXT4_JTR_NONE); > + if (ret) > + goto err_journal; > + > + lock_buffer(sbi->s_sbh); > + memset(es->s_volume_name, 0, sizeof(es->s_volume_name)); > + memcpy(es->s_volume_name, new_label, len); (minor) this introduces a very small window where s_volume_name is unset. Since "new_label" is already a temporary buffer of the correct size, it would be better IMHO to zero it out, copy the new label from userspace into it, and then copy EXT4_LABEL_MAX bytes of new_label to s_volume_name. It still isn't perfect, but reduces the window significantly. > + /* Update backup superblocks */ > + ngroups = ext4_get_groups_count(sb); > + for (grp = 0; grp < ngroups; grp++) { : : > + ext4_debug("update backup superblock %llu\n", sb_block); > + BUFFER_TRACE(bh, "get_write_access"); > + ret = ext4_journal_get_write_access(handle, sb, > + bh, > + EXT4_JTR_NONE); > + if (ret) { > + brelse(bh); > + break; > + } > + > + es = (struct ext4_super_block *) (bh->b_data + offset); > + lock_buffer(bh); > + if (ext4_has_metadata_csum(sb) && > + es->s_checksum != ext4_superblock_csum(sb, es)) { > + ext4_msg(sb, KERN_ERR, "Invalid checksum for backup " > + "superblock %llu\n", sb_block); > + unlock_buffer(bh); > + brelse(bh); > + ret = -EFSBADCRC; > + break; > + } > + memset(es->s_volume_name, 0, sizeof(es->s_volume_name)); > + memcpy(es->s_volume_name, new_label, len); Same here. The rest looks fine. Cheers, Andreas
On Mon, Nov 29, 2021 at 01:28:09PM -0700, Andreas Dilger wrote: > On Nov 12, 2021, at 1:20 AM, Lukas Czerner <lczerner@redhat.com> wrote: > > > > Implement support for FS_IOC_GETFSLABEL and FS_IOC_SETFSLABEL ioctls for > > online reading and setting of file system label. > > > > ext4_ioctl_getlabel() is simple, just get the label from the primary > > superblock bh. This might not be the first sb on the file system if > > 'sb=' mount option is used. > > > > In ext4_ioctl_setlabel() we update what ext4 currently views as a > > primary superblock and then proceed to update backup superblocks. There > > are two caveats: > > - the primary superblock might not be the first superblock and so it > > might not be the one used by userspace tools if read directly > > off the disk. > > - because the primary superblock might not be the first superblock we > > potentialy have to update it as part of backup superblock update. > > However the first sb location is a bit more complicated than the rest > > so we have to account for that. > > > > Tested with generic/492 with various configurations. I also checked the > > behavior with 'sb=' mount options, including very large file systems > > with and without sparse_super/sparse_super2. > > > > Signed-off-by: Lukas Czerner <lczerner@redhat.com> > > --- > > One minor issue/question inline. > > > +static int ext4_ioctl_setlabel(struct file *filp, const char __user *user_label) > > +{ > > + size_t len; > > + handle_t *handle; > > + ext4_group_t ngroups; > > + ext4_fsblk_t sb_block; > > + struct buffer_head *bh; > > + int ret = 0, ret2, grp; > > + unsigned long offset = 0; > > + char new_label[EXT4_LABEL_MAX + 1]; > > + struct super_block *sb = file_inode(filp)->i_sb; > > + struct ext4_sb_info *sbi = EXT4_SB(sb); > > + struct ext4_super_block *es = sbi->s_es; > > + > > + /* Sanity check, this should never happen */ > > + BUILD_BUG_ON(sizeof(es->s_volume_name) < EXT4_LABEL_MAX); > > + > > + if (!capable(CAP_SYS_ADMIN)) > > + return -EPERM; > > + /* > > + * Copy the maximum length allowed for ext4 label with one more to > > + * find the required terminating null byte in order to test the > > + * label length. The on disk label doesn't need to be null terminated. > > + */ > > + if (copy_from_user(new_label, user_label, EXT4_LABEL_MAX + 1)) > > + return -EFAULT; > > + > > + len = strnlen(new_label, EXT4_LABEL_MAX + 1); > > + if (len > EXT4_LABEL_MAX) > > + return -EINVAL; > > + > > + ret = mnt_want_write_file(filp); > > + if (ret) > > + return ret; > > + > > + handle = ext4_journal_start_sb(sb, EXT4_HT_MISC, EXT4_MAX_TRANS_DATA); > > + if (IS_ERR(handle)) { > > + ret = PTR_ERR(handle); > > + goto err_out; > > + } > > + /* Update the primary superblock first */ > > + ret = ext4_journal_get_write_access(handle, sb, > > + sbi->s_sbh, > > + EXT4_JTR_NONE); > > + if (ret) > > + goto err_journal; > > + > > + lock_buffer(sbi->s_sbh); > > + memset(es->s_volume_name, 0, sizeof(es->s_volume_name)); > > + memcpy(es->s_volume_name, new_label, len); > > (minor) this introduces a very small window where s_volume_name is unset. > Since "new_label" is already a temporary buffer of the correct size, it > would be better IMHO to zero it out, copy the new label from userspace > into it, and then copy EXT4_LABEL_MAX bytes of new_label to s_volume_name. > > It still isn't perfect, but reduces the window significantly. Very good point, I'll fix that in the next version. Thanks! -Lukas > > > + /* Update backup superblocks */ > > + ngroups = ext4_get_groups_count(sb); > > + for (grp = 0; grp < ngroups; grp++) { > > : > : > > > + ext4_debug("update backup superblock %llu\n", sb_block); > > + BUFFER_TRACE(bh, "get_write_access"); > > + ret = ext4_journal_get_write_access(handle, sb, > > + bh, > > + EXT4_JTR_NONE); > > + if (ret) { > > + brelse(bh); > > + break; > > + } > > + > > + es = (struct ext4_super_block *) (bh->b_data + offset); > > + lock_buffer(bh); > > + if (ext4_has_metadata_csum(sb) && > > + es->s_checksum != ext4_superblock_csum(sb, es)) { > > + ext4_msg(sb, KERN_ERR, "Invalid checksum for backup " > > + "superblock %llu\n", sb_block); > > + unlock_buffer(bh); > > + brelse(bh); > > + ret = -EFSBADCRC; > > + break; > > + } > > + memset(es->s_volume_name, 0, sizeof(es->s_volume_name)); > > + memcpy(es->s_volume_name, new_label, len); > > Same here. > > The rest looks fine. > > Cheers, Andreas > > > > >
On Fri, Nov 12, 2021 at 09:20:19AM +0100, Lukas Czerner wrote: > + /* Update backup superblocks */ > + ngroups = ext4_get_groups_count(sb); > + for (grp = 0; grp < ngroups; grp++) { > + ... > + ret = ext4_journal_ensure_credits_fn(handle, 1, > + EXT4_MAX_TRANS_DATA, > + 0, 0); > + if (ret < 0) > + break; This doesn't look right. This will try to make sure there is at least one credit left on the handle, and if there isn't it will attempt to add EXT4_MAX_TRANS_DATA to the handle --- and if there isn't enough room remaining in the journal to add that number of credits, no credits will be added, and ext4_journal_ensure_credits_fn() will return a positive integer (in our current implementation it will always return 1). So once run out of credits, and there is no more room in the journal, we we will proceed, and when we try to modify the backup superblock, a WARN_ON will be triggered and ext4_handle_dirty_metadata() will trigger an ext4_error(), which would be unfortunate. I'd also point out that for very large file systems, I'm not convinced that we need to atomically update all of the backup superblocks at the same time. Sure, probably makes sense to update the primary, and superblocks for block groups 0 and 1 atomically (or s_backup_bgs[0,1] a sparse_super2 file system) using the journal. But after that? I'd suggest not running the updates for the rest through the journal at all, and just write them out directly. Nothing else will try to read or write the backup superblock blocks, so there's no reason why we have to be super careful writing out the rest. If we crash after we've only updated the first 20 backup superblocks --- that's probably 18 more than a user will actually use in the first place. That allows us to simply reserve 3 credits, and we won't need to try to extend the handle, which means we don't have to implement some kind of fallback logic in case the handle extension fails. One other comment. Eventually (and not so in the distant future) we're going to want to use the same superblock updating logic to handle changing the UUID, and possibly, for other tune2fs operations. The reason for this is that there are some people who are trying to update the UUID and resize the file system to fit the size of the cloud block device (e.g., either an Amazon EBS or GCE's PD) in separate systemd unit scripts. This results in race conditions that can cause either the tune2fs or resize2fs to fail --- rarely, but if you are starting up thousands and thousands of VM's per day, even the rare becomes common place. This is the reason of e2fsprogs commit 6338a8467564 ("libext2fs: retry reading superblock on open when checksum is bad") but that turns out not to be enough; although it does reduce the incidence rate by another order of magnitude or two. So.... we should probably have a mutex which prevents two ioctls which is modifying the superblock from running at the same time. It's *probably* going to be OK for now, since the second ioctl racing to update the superblock will update the checksum, and so long as we have journalling enabled, we shouldn't have a bad checksum end up on disk. But we're going to want to add an ioctl to fetch the superblock, and at that point we'll definitely need the mutex to protect the superblock getter from getting an inconsistent view of the superblock. The other thing that might be nice would be if the superblock update function was abstracted out, and the FS_IOC_SETLABEL ioctl provided a callback function which updates the label. Neither of these two suggestions are strictly necessary for your patch series (although the mutex will prevent problems with racing FS_IOC_SETLABEL and FS_IOC_GETLABEL ioctls), so if you don't want to make these changes now, I'm not going to insist on them; we can always make these improvements when we implement FS_IOC_SETUUID, FS_IOC_GETUUID, and EXT4_IOC_GET_SB. (BTW, I believe Darrick has patches to implement FS_IOC_[SG]ETUUID for xfs and possibly some other file systems, IIRC, but those have never been landed in Linus's tree.) And finally, thanks for working on FS_IOC_SETLABEL! It has been on my todo list for a long time, but it's never managed to make the top of the priority queue... Cheers, = Ted
On Mon, Nov 29, 2021 at 10:00:08PM -0500, Theodore Y. Ts'o wrote: > On Fri, Nov 12, 2021 at 09:20:19AM +0100, Lukas Czerner wrote: > > + /* Update backup superblocks */ > > + ngroups = ext4_get_groups_count(sb); > > + for (grp = 0; grp < ngroups; grp++) { > > + > ... > > + ret = ext4_journal_ensure_credits_fn(handle, 1, > > + EXT4_MAX_TRANS_DATA, > > + 0, 0); > > + if (ret < 0) > > + break; > > This doesn't look right. This will try to make sure there is at least > one credit left on the handle, and if there isn't it will attempt to > add EXT4_MAX_TRANS_DATA to the handle --- and if there isn't enough > room remaining in the journal to add that number of credits, no > credits will be added, and ext4_journal_ensure_credits_fn() will > return a positive integer (in our current implementation it will > always return 1). Oops, I was sure I've seen this somewhere in the code, but I guess I was wrong. Should have checked what it actually returns. Thanks for pointing this out. > > So once run out of credits, and there is no more room in the journal, > we we will proceed, and when we try to modify the backup superblock, a > WARN_ON will be triggered and ext4_handle_dirty_metadata() will > trigger an ext4_error(), which would be unfortunate. > > I'd also point out that for very large file systems, I'm not convinced > that we need to atomically update all of the backup superblocks at the > same time. Sure, probably makes sense to update the primary, and > superblocks for block groups 0 and 1 atomically (or s_backup_bgs[0,1] > a sparse_super2 file system) using the journal. > > But after that? I'd suggest not running the updates for the rest > through the journal at all, and just write them out directly. Nothing > else will try to read or write the backup superblock blocks, so > there's no reason why we have to be super careful writing out the > rest. If we crash after we've only updated the first 20 backup > superblocks --- that's probably 18 more than a user will actually use > in the first place. > > That allows us to simply reserve 3 credits, and we won't need to try > to extend the handle, which means we don't have to implement some kind > of fallback logic in case the handle extension fails. I think I agree. But in this case should we at least attempt to check and update the backup superblocks in fsck? Not sure if we do that already. > > > One other comment. Eventually (and not so in the distant future) > we're going to want to use the same superblock updating logic to > handle changing the UUID, and possibly, for other tune2fs operations. > The reason for this is that there are some people who are trying to > update the UUID and resize the file system to fit the size of the > cloud block device (e.g., either an Amazon EBS or GCE's PD) in > separate systemd unit scripts. This results in race conditions that > can cause either the tune2fs or resize2fs to fail --- rarely, but if > you are starting up thousands and thousands of VM's per day, even the > rare becomes common place. This is the reason of e2fsprogs commit > 6338a8467564 ("libext2fs: retry reading superblock on open when > checksum is bad") but that turns out not to be enough; although it > does reduce the incidence rate by another order of magnitude or two. > > So.... we should probably have a mutex which prevents two ioctls > which is modifying the superblock from running at the same time. It's > *probably* going to be OK for now, since the second ioctl racing to > update the superblock will update the checksum, and so long as we have > journalling enabled, we shouldn't have a bad checksum end up on disk. > But we're going to want to add an ioctl to fetch the superblock, and > at that point we'll definitely need the mutex to protect the > superblock getter from getting an inconsistent view of the superblock. > > The other thing that might be nice would be if the superblock update > function was abstracted out, and the FS_IOC_SETLABEL ioctl provided a > callback function which updates the label. > > Neither of these two suggestions are strictly necessary for your patch > series (although the mutex will prevent problems with racing > FS_IOC_SETLABEL and FS_IOC_GETLABEL ioctls), so if you don't want to > make these changes now, I'm not going to insist on them; we can > always make these improvements when we implement FS_IOC_SETUUID, > FS_IOC_GETUUID, and EXT4_IOC_GET_SB. (BTW, I believe Darrick has > patches to implement FS_IOC_[SG]ETUUID for xfs and possibly some other > file systems, IIRC, but those have never been landed in Linus's tree.) It's not a critical functionality so it can wait. I'll think about implementing the superblock modification system. Thanks for the useful pointers. > > And finally, thanks for working on FS_IOC_SETLABEL! It has been on my > todo list for a long time, but it's never managed to make the top of > the priority queue... No problem, I am happy to help. -Lukas > > Cheers, > > = Ted >
On Tue, Nov 30, 2021 at 10:49:50AM +0100, Lukas Czerner wrote: > > But after that? I'd suggest not running the updates for the rest > > through the journal at all, and just write them out directly. Nothing > > else will try to read or write the backup superblock blocks, so > > there's no reason why we have to be super careful writing out the > > rest. If we crash after we've only updated the first 20 backup > > superblocks --- that's probably 18 more than a user will actually use > > in the first place. > > > > That allows us to simply reserve 3 credits, and we won't need to try > > to extend the handle, which means we don't have to implement some kind > > of fallback logic in case the handle extension fails. > > I think I agree. But in this case should we at least attempt to check > and update the backup superblocks in fsck? Not sure if we do that > already. Well, after a successful file system check by fsck, we'll update all of the backup superblocks. If we've done a full file system check we know that the primary superblock is consistent with the rest of the file system, so at that point it's safe to write it to all of the backup superblocks in the file system. But if we haven't done the full file system check, we won't know whether it is the primary or the backup superblock which is incorrect. I guess we could do the basic superblock checks, and if there are at least two additional superblocks, we see if we have do a 2 out of 3 voting check. Or if there are differences between the primary and the backup we could force a full check. I think in practice though, so long as the primary and two backup superblocks are part of the jbd2 transaction, that should be good enough in terms of recovery since usually most users only use the first backup superblock to recover if the primary is damaged. Whether we update the rest of the backup superblocks improves things, but it's not really going to make a difference 99.99% of the time. - Ted
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 3825195539d7..0856afb629e3 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -1297,6 +1297,8 @@ extern void ext4_set_bits(void *bm, int cur, int len); /* Metadata checksum algorithm codes */ #define EXT4_CRC32C_CHKSUM 1 +#define EXT4_LABEL_MAX 16 + /* * Structure of the super block */ @@ -1346,7 +1348,7 @@ struct ext4_super_block { /*60*/ __le32 s_feature_incompat; /* incompatible feature set */ __le32 s_feature_ro_compat; /* readonly-compatible feature set */ /*68*/ __u8 s_uuid[16]; /* 128-bit uuid for volume */ -/*78*/ char s_volume_name[16]; /* volume name */ +/*78*/ char s_volume_name[EXT4_LABEL_MAX]; /* volume name */ /*88*/ char s_last_mounted[64] __nonstring; /* directory where last mounted */ /*C8*/ __le32 s_algorithm_usage_bitmap; /* For compression */ /* @@ -3109,6 +3111,8 @@ extern int ext4_read_bh_lock(struct buffer_head *bh, int op_flags, bool wait); extern void ext4_sb_breadahead_unmovable(struct super_block *sb, sector_t block); extern int ext4_seq_options_show(struct seq_file *seq, void *offset); extern int ext4_calculate_overhead(struct super_block *sb); +extern __le32 ext4_superblock_csum(struct super_block *sb, + struct ext4_super_block *es); extern void ext4_superblock_csum_set(struct super_block *sb); extern int ext4_alloc_flex_bg_array(struct super_block *sb, ext4_group_t ngroup); diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c index 606dee9e08a3..1199090e0dbb 100644 --- a/fs/ext4/ioctl.c +++ b/fs/ext4/ioctl.c @@ -850,6 +850,166 @@ static int ext4_ioctl_checkpoint(struct file *filp, unsigned long arg) return err; } +static int ext4_ioctl_getlabel(struct ext4_sb_info *sbi, char __user *user_label) +{ + char label[EXT4_LABEL_MAX + 1]; + + /* + * EXT4_LABEL_MAX must always be smaller than FSLABEL_MAX because + * FSLABEL_MAX must include terminating null byte, while s_volume_name + * does not have to. + */ + BUILD_BUG_ON(EXT4_LABEL_MAX >= FSLABEL_MAX); + + memset(label, 0, sizeof(label)); + lock_buffer(sbi->s_sbh); + strncpy(label, sbi->s_es->s_volume_name, EXT4_LABEL_MAX); + unlock_buffer(sbi->s_sbh); + + if (copy_to_user(user_label, label, sizeof(label))) + return -EFAULT; + return 0; +} + +static int ext4_ioctl_setlabel(struct file *filp, const char __user *user_label) +{ + size_t len; + handle_t *handle; + ext4_group_t ngroups; + ext4_fsblk_t sb_block; + struct buffer_head *bh; + int ret = 0, ret2, grp; + unsigned long offset = 0; + char new_label[EXT4_LABEL_MAX + 1]; + struct super_block *sb = file_inode(filp)->i_sb; + struct ext4_sb_info *sbi = EXT4_SB(sb); + struct ext4_super_block *es = sbi->s_es; + + /* Sanity check, this should never happen */ + BUILD_BUG_ON(sizeof(es->s_volume_name) < EXT4_LABEL_MAX); + + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + /* + * Copy the maximum length allowed for ext4 label with one more to + * find the required terminating null byte in order to test the + * label length. The on disk label doesn't need to be null terminated. + */ + if (copy_from_user(new_label, user_label, EXT4_LABEL_MAX + 1)) + return -EFAULT; + + len = strnlen(new_label, EXT4_LABEL_MAX + 1); + if (len > EXT4_LABEL_MAX) + return -EINVAL; + + ret = mnt_want_write_file(filp); + if (ret) + return ret; + + handle = ext4_journal_start_sb(sb, EXT4_HT_MISC, EXT4_MAX_TRANS_DATA); + if (IS_ERR(handle)) { + ret = PTR_ERR(handle); + goto err_out; + } + /* Update the primary superblock first */ + ret = ext4_journal_get_write_access(handle, sb, + sbi->s_sbh, + EXT4_JTR_NONE); + if (ret) + goto err_journal; + + lock_buffer(sbi->s_sbh); + memset(es->s_volume_name, 0, sizeof(es->s_volume_name)); + memcpy(es->s_volume_name, new_label, len); + ext4_superblock_csum_set(sb); + unlock_buffer(sbi->s_sbh); + + ret = ext4_handle_dirty_metadata(handle, NULL, sbi->s_sbh); + if (ret) + goto err_journal; + sync_dirty_buffer(sbi->s_sbh); + + /* Update backup superblocks */ + ngroups = ext4_get_groups_count(sb); + for (grp = 0; grp < ngroups; grp++) { + + if (!ext4_bg_has_super(sb, grp)) + continue; + + /* + * For the group 0 there is always 1k padding, so we have + * either adjust offset, or sb_block depending on blocksize + */ + if (grp == 0) { + sb_block = 1 * EXT4_MIN_BLOCK_SIZE; + offset = do_div(sb_block, sb->s_blocksize); + } else { + sb_block = ext4_group_first_block_no(sb, grp); + offset = 0; + } + + /* + * Skip primary superblock, it's already done. Note that the + * primary superblock is not always at group 0 + */ + if (sbi->s_sbh->b_blocknr == sb_block) + continue; + + ret = ext4_journal_ensure_credits_fn(handle, 1, + EXT4_MAX_TRANS_DATA, + 0, 0); + if (ret < 0) + break; + + bh = ext4_sb_bread(sb, sb_block, 0); + if (IS_ERR(bh)) { + ret = PTR_ERR(bh); + break; + } + + ext4_debug("update backup superblock %llu\n", sb_block); + BUFFER_TRACE(bh, "get_write_access"); + ret = ext4_journal_get_write_access(handle, sb, + bh, + EXT4_JTR_NONE); + if (ret) { + brelse(bh); + break; + } + + es = (struct ext4_super_block *) (bh->b_data + offset); + lock_buffer(bh); + if (ext4_has_metadata_csum(sb) && + es->s_checksum != ext4_superblock_csum(sb, es)) { + ext4_msg(sb, KERN_ERR, "Invalid checksum for backup " + "superblock %llu\n", sb_block); + unlock_buffer(bh); + brelse(bh); + ret = -EFSBADCRC; + break; + } + memset(es->s_volume_name, 0, sizeof(es->s_volume_name)); + memcpy(es->s_volume_name, new_label, len); + if (ext4_has_metadata_csum(sb)) + es->s_checksum = ext4_superblock_csum(sb, es); + unlock_buffer(bh); + + ret = ext4_handle_dirty_metadata(handle, NULL, bh); + if (unlikely(ret)) + ext4_std_error(sb, ret); + brelse(bh); + } + +err_journal: + ret2 = ext4_journal_stop(handle); + if (ret2 && !ret) + ret = ret2; +err_out: + mnt_drop_write_file(filp); + ext4_std_error(sb, ret); + return ret; +} + static long __ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { struct inode *inode = file_inode(filp); @@ -1266,6 +1426,13 @@ static long __ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) case EXT4_IOC_CHECKPOINT: return ext4_ioctl_checkpoint(filp, arg); + case FS_IOC_GETFSLABEL: + return ext4_ioctl_getlabel(EXT4_SB(sb), (void __user *)arg); + + case FS_IOC_SETFSLABEL: + return ext4_ioctl_setlabel(filp, + (const void __user *)arg); + default: return -ENOTTY; } @@ -1347,6 +1514,8 @@ long ext4_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg) case EXT4_IOC_GETSTATE: case EXT4_IOC_GET_ES_CACHE: case EXT4_IOC_CHECKPOINT: + case FS_IOC_GETFSLABEL: + case FS_IOC_SETFSLABEL: break; default: return -ENOIOCTLCMD; diff --git a/fs/ext4/super.c b/fs/ext4/super.c index a320c54202d9..f6c2f44ab221 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -260,8 +260,8 @@ static int ext4_verify_csum_type(struct super_block *sb, return es->s_checksum_type == EXT4_CRC32C_CHKSUM; } -static __le32 ext4_superblock_csum(struct super_block *sb, - struct ext4_super_block *es) +__le32 ext4_superblock_csum(struct super_block *sb, + struct ext4_super_block *es) { struct ext4_sb_info *sbi = EXT4_SB(sb); int offset = offsetof(struct ext4_super_block, s_checksum);
Implement support for FS_IOC_GETFSLABEL and FS_IOC_SETFSLABEL ioctls for online reading and setting of file system label. ext4_ioctl_getlabel() is simple, just get the label from the primary superblock bh. This might not be the first sb on the file system if 'sb=' mount option is used. In ext4_ioctl_setlabel() we update what ext4 currently views as a primary superblock and then proceed to update backup superblocks. There are two caveats: - the primary superblock might not be the first superblock and so it might not be the one used by userspace tools if read directly off the disk. - because the primary superblock might not be the first superblock we potentialy have to update it as part of backup superblock update. However the first sb location is a bit more complicated than the rest so we have to account for that. Tested with generic/492 with various configurations. I also checked the behavior with 'sb=' mount options, including very large file systems with and without sparse_super/sparse_super2. Signed-off-by: Lukas Czerner <lczerner@redhat.com> --- V2: Fix typo. Place constant in BUILD_BUG_ON comparison on the right side fs/ext4/ext4.h | 6 +- fs/ext4/ioctl.c | 169 ++++++++++++++++++++++++++++++++++++++++++++++++ fs/ext4/super.c | 4 +- 3 files changed, 176 insertions(+), 3 deletions(-)