* [PATCH] e2fsprogs: journal code small cleanup and fixes
@ 2014-07-08 20:41 Azat Khuzhin
2014-07-08 20:41 ` [PATCH 1/3] journal: use consts instead of 1024 and add helper for journal with 1k blocksize Azat Khuzhin
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Azat Khuzhin @ 2014-07-08 20:41 UTC (permalink / raw)
To: linux-ext4; +Cc: tytso
Hi there, here is a patch set with various cleanups in e2fsprogs code for
working journal device and fix tune2fs to update UUID for journal dev properly.
(we need to copy UUID into jsb for this)
First first two are cleanups, and the last one is UUID fix:
[PATCH 1/3] journal: use consts instead of 1024 and add helper for
[PATCH 2/3] tune2fs: remove_journal_device(): use the correct block
[PATCH 3/3] tune2fs: update journal super block when changing UUID
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 1/3] journal: use consts instead of 1024 and add helper for journal with 1k blocksize 2014-07-08 20:41 [PATCH] e2fsprogs: journal code small cleanup and fixes Azat Khuzhin @ 2014-07-08 20:41 ` Azat Khuzhin 2014-07-08 23:30 ` Andreas Dilger 2014-07-08 20:41 ` [PATCH 2/3] tune2fs: remove_journal_device(): use the correct block to find jsb Azat Khuzhin 2014-07-08 20:41 ` [PATCH 3/3] tune2fs: update journal super block when changing UUID for fs Azat Khuzhin 2 siblings, 1 reply; 16+ messages in thread From: Azat Khuzhin @ 2014-07-08 20:41 UTC (permalink / raw) To: linux-ext4; +Cc: tytso, Azat Khuzhin Use EXT2_MIN_BLOCK_SIZE/JFS_MIN_JOURNAL_BLOCKS instead of hardcoded 1024 when it is okay, and also add a helper ext2fs_journal_sb_start() that will return start of journal sb with special case for fs with 1k block size. Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com> --- e2fsck/journal.c | 5 ++--- lib/ext2fs/ext2fs.h | 1 + lib/ext2fs/mkjournal.c | 28 ++++++++++++++++------------ misc/tune2fs.c | 6 +++--- 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/e2fsck/journal.c b/e2fsck/journal.c index a7b1150..5780be0 100644 --- a/e2fsck/journal.c +++ b/e2fsck/journal.c @@ -443,8 +443,7 @@ static errcode_t e2fsck_get_journal(e2fsck_t ctx, journal_t **ret_journal) if (ext_journal) { blk64_t maxlen; - if (ctx->fs->blocksize == 1024) - start = 1; + start = ext2fs_journal_sb_start(ctx->fs->blocksize) - 1; bh = getblk(dev_journal, start, ctx->fs->blocksize); if (!bh) { retval = EXT2_ET_NO_MEMORY; @@ -455,7 +454,7 @@ static errcode_t e2fsck_get_journal(e2fsck_t ctx, journal_t **ret_journal) brelse(bh); goto errout; } - memcpy(&jsuper, start ? bh->b_data : bh->b_data + 1024, + memcpy(&jsuper, start ? bh->b_data : bh->b_data + EXT2_MIN_BLOCK_SIZE, sizeof(jsuper)); brelse(bh); #ifdef WORDS_BIGENDIAN diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h index 599c972..a759741 100644 --- a/lib/ext2fs/ext2fs.h +++ b/lib/ext2fs/ext2fs.h @@ -1498,6 +1498,7 @@ extern errcode_t ext2fs_add_journal_inode(ext2_filsys fs, blk_t num_blocks, extern errcode_t ext2fs_add_journal_inode2(ext2_filsys fs, blk_t num_blocks, blk64_t goal, int flags); extern int ext2fs_default_journal_size(__u64 num_blocks); +extern int ext2fs_journal_sb_start(int blocksize); /* openfs.c */ extern errcode_t ext2fs_open(const char *name, int flags, int superblock, diff --git a/lib/ext2fs/mkjournal.c b/lib/ext2fs/mkjournal.c index 884d9c0..068eed7 100644 --- a/lib/ext2fs/mkjournal.c +++ b/lib/ext2fs/mkjournal.c @@ -49,7 +49,7 @@ errcode_t ext2fs_create_journal_superblock(ext2_filsys fs, errcode_t retval; journal_superblock_t *jsb; - if (num_blocks < 1024) + if (num_blocks < JFS_MIN_JOURNAL_BLOCKS) return EXT2_ET_JOURNAL_TOO_SMALL; if ((retval = ext2fs_get_mem(fs->blocksize, &jsb))) @@ -75,10 +75,7 @@ errcode_t ext2fs_create_journal_superblock(ext2_filsys fs, if (fs->super->s_feature_incompat & EXT3_FEATURE_INCOMPAT_JOURNAL_DEV) { jsb->s_nr_users = 0; - if (fs->blocksize == 1024) - jsb->s_first = htonl(3); - else - jsb->s_first = htonl(2); + jsb->s_first = ext2fs_journal_sb_start(fs->blocksize) + 1; } *ret_jsb = (char *) jsb; @@ -430,6 +427,13 @@ int ext2fs_default_journal_size(__u64 num_blocks) return 32768; } +int ext2fs_journal_sb_start(int blocksize) +{ + if (blocksize == EXT2_MIN_BLOCK_SIZE) + return 2; + return 1; +} + /* * This function adds a journal device to a filesystem */ @@ -437,7 +441,7 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) { struct stat st; errcode_t retval; - char buf[1024]; + char buf[SUPERBLOCK_SIZE]; journal_superblock_t *jsb; int start; __u32 i, nr_users; @@ -450,10 +454,9 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) return EXT2_ET_JOURNAL_NOT_BLOCK; /* Must be a block device */ /* Get the journal superblock */ - start = 1; - if (journal_dev->blocksize == 1024) - start++; - if ((retval = io_channel_read_blk64(journal_dev->io, start, -1024, + start = ext2fs_journal_sb_start(journal_dev->blocksize); + if ((retval = io_channel_read_blk64(journal_dev->io, start, + -SUPERBLOCK_SIZE, buf))) return retval; @@ -479,7 +482,8 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) } /* Writeback the journal superblock */ - if ((retval = io_channel_write_blk64(journal_dev->io, start, -1024, buf))) + if ((retval = io_channel_write_blk64(journal_dev->io, start, + -SUPERBLOCK_SIZE, buf))) return retval; fs->super->s_journal_inum = 0; @@ -632,7 +636,7 @@ main(int argc, char **argv) exit(1); } - retval = ext2fs_add_journal_inode(fs, 1024, 0); + retval = ext2fs_add_journal_inode(fs, JFS_MIN_JOURNAL_BLOCKS, 0); if (retval) { com_err(argv[0], retval, "while adding journal to %s", device_name); diff --git a/misc/tune2fs.c b/misc/tune2fs.c index d95cc3d..0e7caf2 100644 --- a/misc/tune2fs.c +++ b/misc/tune2fs.c @@ -187,7 +187,7 @@ static int remove_journal_device(ext2_filsys fs) { char *journal_path; ext2_filsys jfs; - char buf[1024]; + char buf[SUPERBLOCK_SIZE]; journal_superblock_t *jsb; int i, nr_users; errcode_t retval; @@ -230,7 +230,7 @@ static int remove_journal_device(ext2_filsys fs) } /* Get the journal superblock */ - if ((retval = io_channel_read_blk64(jfs->io, 1, -1024, buf))) { + if ((retval = io_channel_read_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { com_err(program_name, retval, "%s", _("while reading journal superblock")); goto no_valid_journal; @@ -261,7 +261,7 @@ static int remove_journal_device(ext2_filsys fs) jsb->s_nr_users = htonl(nr_users); /* Write back the journal superblock */ - if ((retval = io_channel_write_blk64(jfs->io, 1, -1024, buf))) { + if ((retval = io_channel_write_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { com_err(program_name, retval, "while writing journal superblock."); goto no_valid_journal; -- 2.0.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] journal: use consts instead of 1024 and add helper for journal with 1k blocksize 2014-07-08 20:41 ` [PATCH 1/3] journal: use consts instead of 1024 and add helper for journal with 1k blocksize Azat Khuzhin @ 2014-07-08 23:30 ` Andreas Dilger 2014-07-09 3:51 ` Azat Khuzhin 2014-07-09 3:56 ` [PATCH 1/3 v2] " Azat Khuzhin 0 siblings, 2 replies; 16+ messages in thread From: Andreas Dilger @ 2014-07-08 23:30 UTC (permalink / raw) To: Azat Khuzhin; +Cc: linux-ext4, tytso [-- Attachment #1: Type: text/plain, Size: 1188 bytes --] On Jul 8, 2014, at 2:41 PM, Azat Khuzhin <a3at.mail@gmail.com> wrote: > Use EXT2_MIN_BLOCK_SIZE/JFS_MIN_JOURNAL_BLOCKS instead of hardcoded 1024 > when it is okay, and also add a helper ext2fs_journal_sb_start() that > will return start of journal sb with special case for fs with 1k block > size. Seems like a good idea, but an issue below. > diff --git a/lib/ext2fs/mkjournal.c b/lib/ext2fs/mkjournal.c > index 884d9c0..068eed7 100644 > --- a/lib/ext2fs/mkjournal.c > +++ b/lib/ext2fs/mkjournal.c > @@ -75,10 +75,7 @@ errcode_t ext2fs_create_journal_superblock(ext2_filsys fs, > if (fs->super->s_feature_incompat & > EXT3_FEATURE_INCOMPAT_JOURNAL_DEV) { > jsb->s_nr_users = 0; > - if (fs->blocksize == 1024) > - jsb->s_first = htonl(3); > - else > - jsb->s_first = htonl(2); > + jsb->s_first = ext2fs_journal_sb_start(fs->blocksize) + 1; This looks like it is missing the htonl() conversion, and will break the on-disk format? The JBD code stores all data on-disk in big-endian to ensure that it is converted properly on the most common little-endian systems, and will therefore also work on big-endian systems. Cheers, Andreas [-- Attachment #2: Message signed with OpenPGP using GPGMail --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] journal: use consts instead of 1024 and add helper for journal with 1k blocksize 2014-07-08 23:30 ` Andreas Dilger @ 2014-07-09 3:51 ` Azat Khuzhin 2014-07-09 3:56 ` [PATCH 1/3 v2] " Azat Khuzhin 1 sibling, 0 replies; 16+ messages in thread From: Azat Khuzhin @ 2014-07-09 3:51 UTC (permalink / raw) To: Andreas Dilger; +Cc: linux-ext4, tytso On Tue, Jul 08, 2014 at 05:30:10PM -0600, Andreas Dilger wrote: > On Jul 8, 2014, at 2:41 PM, Azat Khuzhin <a3at.mail@gmail.com> wrote: > > Use EXT2_MIN_BLOCK_SIZE/JFS_MIN_JOURNAL_BLOCKS instead of hardcoded 1024 > > when it is okay, and also add a helper ext2fs_journal_sb_start() that > > will return start of journal sb with special case for fs with 1k block > > size. > > Seems like a good idea, but an issue below. > > > diff --git a/lib/ext2fs/mkjournal.c b/lib/ext2fs/mkjournal.c > > index 884d9c0..068eed7 100644 > > --- a/lib/ext2fs/mkjournal.c > > +++ b/lib/ext2fs/mkjournal.c > > @@ -75,10 +75,7 @@ errcode_t ext2fs_create_journal_superblock(ext2_filsys fs, > > if (fs->super->s_feature_incompat & > > EXT3_FEATURE_INCOMPAT_JOURNAL_DEV) { > > jsb->s_nr_users = 0; > > - if (fs->blocksize == 1024) > > - jsb->s_first = htonl(3); > > - else > > - jsb->s_first = htonl(2); > > + jsb->s_first = ext2fs_journal_sb_start(fs->blocksize) + 1; > > This looks like it is missing the htonl() conversion, and will break the on-disk format? Yeah, good catch, thanks! I will resend this patch with htonl(). Cheers, Azat. > > The JBD code stores all data on-disk in big-endian to ensure that it is converted properly > on the most common little-endian systems, and will therefore also work on big-endian systems. > > Cheers, Andreas > > > > > -- Respectfully Azat Khuzhin ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3 v2] journal: use consts instead of 1024 and add helper for journal with 1k blocksize 2014-07-08 23:30 ` Andreas Dilger 2014-07-09 3:51 ` Azat Khuzhin @ 2014-07-09 3:56 ` Azat Khuzhin 2014-07-09 4:46 ` Andreas Dilger 1 sibling, 1 reply; 16+ messages in thread From: Azat Khuzhin @ 2014-07-09 3:56 UTC (permalink / raw) To: adilger; +Cc: linux-ext4, tytso, Azat Khuzhin Use EXT2_MIN_BLOCK_SIZE/JFS_MIN_JOURNAL_BLOCKS instead of hardcoded 1024 when it is okay, and also add a helper ext2fs_journal_sb_start() that will return start of journal sb with special case for fs with 1k block size. Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com> --- v2: mixxed htonl() (thanks to Andreas Dilger) e2fsck/journal.c | 5 ++--- lib/ext2fs/ext2fs.h | 1 + lib/ext2fs/mkjournal.c | 28 ++++++++++++++++------------ misc/tune2fs.c | 6 +++--- 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/e2fsck/journal.c b/e2fsck/journal.c index a7b1150..5780be0 100644 --- a/e2fsck/journal.c +++ b/e2fsck/journal.c @@ -443,8 +443,7 @@ static errcode_t e2fsck_get_journal(e2fsck_t ctx, journal_t **ret_journal) if (ext_journal) { blk64_t maxlen; - if (ctx->fs->blocksize == 1024) - start = 1; + start = ext2fs_journal_sb_start(ctx->fs->blocksize) - 1; bh = getblk(dev_journal, start, ctx->fs->blocksize); if (!bh) { retval = EXT2_ET_NO_MEMORY; @@ -455,7 +454,7 @@ static errcode_t e2fsck_get_journal(e2fsck_t ctx, journal_t **ret_journal) brelse(bh); goto errout; } - memcpy(&jsuper, start ? bh->b_data : bh->b_data + 1024, + memcpy(&jsuper, start ? bh->b_data : bh->b_data + EXT2_MIN_BLOCK_SIZE, sizeof(jsuper)); brelse(bh); #ifdef WORDS_BIGENDIAN diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h index 599c972..a759741 100644 --- a/lib/ext2fs/ext2fs.h +++ b/lib/ext2fs/ext2fs.h @@ -1498,6 +1498,7 @@ extern errcode_t ext2fs_add_journal_inode(ext2_filsys fs, blk_t num_blocks, extern errcode_t ext2fs_add_journal_inode2(ext2_filsys fs, blk_t num_blocks, blk64_t goal, int flags); extern int ext2fs_default_journal_size(__u64 num_blocks); +extern int ext2fs_journal_sb_start(int blocksize); /* openfs.c */ extern errcode_t ext2fs_open(const char *name, int flags, int superblock, diff --git a/lib/ext2fs/mkjournal.c b/lib/ext2fs/mkjournal.c index 884d9c0..0ad674d 100644 --- a/lib/ext2fs/mkjournal.c +++ b/lib/ext2fs/mkjournal.c @@ -49,7 +49,7 @@ errcode_t ext2fs_create_journal_superblock(ext2_filsys fs, errcode_t retval; journal_superblock_t *jsb; - if (num_blocks < 1024) + if (num_blocks < JFS_MIN_JOURNAL_BLOCKS) return EXT2_ET_JOURNAL_TOO_SMALL; if ((retval = ext2fs_get_mem(fs->blocksize, &jsb))) @@ -75,10 +75,7 @@ errcode_t ext2fs_create_journal_superblock(ext2_filsys fs, if (fs->super->s_feature_incompat & EXT3_FEATURE_INCOMPAT_JOURNAL_DEV) { jsb->s_nr_users = 0; - if (fs->blocksize == 1024) - jsb->s_first = htonl(3); - else - jsb->s_first = htonl(2); + jsb->s_first = htonl(ext2fs_journal_sb_start(fs->blocksize) + 1); } *ret_jsb = (char *) jsb; @@ -430,6 +427,13 @@ int ext2fs_default_journal_size(__u64 num_blocks) return 32768; } +int ext2fs_journal_sb_start(int blocksize) +{ + if (blocksize == EXT2_MIN_BLOCK_SIZE) + return 2; + return 1; +} + /* * This function adds a journal device to a filesystem */ @@ -437,7 +441,7 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) { struct stat st; errcode_t retval; - char buf[1024]; + char buf[SUPERBLOCK_SIZE]; journal_superblock_t *jsb; int start; __u32 i, nr_users; @@ -450,10 +454,9 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) return EXT2_ET_JOURNAL_NOT_BLOCK; /* Must be a block device */ /* Get the journal superblock */ - start = 1; - if (journal_dev->blocksize == 1024) - start++; - if ((retval = io_channel_read_blk64(journal_dev->io, start, -1024, + start = ext2fs_journal_sb_start(journal_dev->blocksize); + if ((retval = io_channel_read_blk64(journal_dev->io, start, + -SUPERBLOCK_SIZE, buf))) return retval; @@ -479,7 +482,8 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) } /* Writeback the journal superblock */ - if ((retval = io_channel_write_blk64(journal_dev->io, start, -1024, buf))) + if ((retval = io_channel_write_blk64(journal_dev->io, start, + -SUPERBLOCK_SIZE, buf))) return retval; fs->super->s_journal_inum = 0; @@ -632,7 +636,7 @@ main(int argc, char **argv) exit(1); } - retval = ext2fs_add_journal_inode(fs, 1024, 0); + retval = ext2fs_add_journal_inode(fs, JFS_MIN_JOURNAL_BLOCKS, 0); if (retval) { com_err(argv[0], retval, "while adding journal to %s", device_name); diff --git a/misc/tune2fs.c b/misc/tune2fs.c index d95cc3d..0e7caf2 100644 --- a/misc/tune2fs.c +++ b/misc/tune2fs.c @@ -187,7 +187,7 @@ static int remove_journal_device(ext2_filsys fs) { char *journal_path; ext2_filsys jfs; - char buf[1024]; + char buf[SUPERBLOCK_SIZE]; journal_superblock_t *jsb; int i, nr_users; errcode_t retval; @@ -230,7 +230,7 @@ static int remove_journal_device(ext2_filsys fs) } /* Get the journal superblock */ - if ((retval = io_channel_read_blk64(jfs->io, 1, -1024, buf))) { + if ((retval = io_channel_read_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { com_err(program_name, retval, "%s", _("while reading journal superblock")); goto no_valid_journal; @@ -261,7 +261,7 @@ static int remove_journal_device(ext2_filsys fs) jsb->s_nr_users = htonl(nr_users); /* Write back the journal superblock */ - if ((retval = io_channel_write_blk64(jfs->io, 1, -1024, buf))) { + if ((retval = io_channel_write_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { com_err(program_name, retval, "while writing journal superblock."); goto no_valid_journal; -- 2.0.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3 v2] journal: use consts instead of 1024 and add helper for journal with 1k blocksize 2014-07-09 3:56 ` [PATCH 1/3 v2] " Azat Khuzhin @ 2014-07-09 4:46 ` Andreas Dilger 2014-07-09 4:54 ` Azat Khuzhin 0 siblings, 1 reply; 16+ messages in thread From: Andreas Dilger @ 2014-07-09 4:46 UTC (permalink / raw) To: Azat Khuzhin; +Cc: linux-ext4, tytso The patch looks good, and you can add: Reviewed-by: Andreas Dilger <adilger@dilger.ca> One thing that could probably be fixed if this patch is refreshed, or in a precursor patch is to rename SUPERBLOCK_SIZE to be EXT2_SUPERBLOCK_SIZE since this avoids potential name clashes. Cheers, Andreas > On Jul 8, 2014, at 21:56, Azat Khuzhin <a3at.mail@gmail.com> wrote: > > Use EXT2_MIN_BLOCK_SIZE/JFS_MIN_JOURNAL_BLOCKS instead of hardcoded 1024 > when it is okay, and also add a helper ext2fs_journal_sb_start() that > will return start of journal sb with special case for fs with 1k block > size. > > Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com> > --- > v2: mixxed htonl() (thanks to Andreas Dilger) > > e2fsck/journal.c | 5 ++--- > lib/ext2fs/ext2fs.h | 1 + > lib/ext2fs/mkjournal.c | 28 ++++++++++++++++------------ > misc/tune2fs.c | 6 +++--- > 4 files changed, 22 insertions(+), 18 deletions(-) > > diff --git a/e2fsck/journal.c b/e2fsck/journal.c > index a7b1150..5780be0 100644 > --- a/e2fsck/journal.c > +++ b/e2fsck/journal.c > @@ -443,8 +443,7 @@ static errcode_t e2fsck_get_journal(e2fsck_t ctx, journal_t **ret_journal) > if (ext_journal) { > blk64_t maxlen; > > - if (ctx->fs->blocksize == 1024) > - start = 1; > + start = ext2fs_journal_sb_start(ctx->fs->blocksize) - 1; > bh = getblk(dev_journal, start, ctx->fs->blocksize); > if (!bh) { > retval = EXT2_ET_NO_MEMORY; > @@ -455,7 +454,7 @@ static errcode_t e2fsck_get_journal(e2fsck_t ctx, journal_t **ret_journal) > brelse(bh); > goto errout; > } > - memcpy(&jsuper, start ? bh->b_data : bh->b_data + 1024, > + memcpy(&jsuper, start ? bh->b_data : bh->b_data + EXT2_MIN_BLOCK_SIZE, > sizeof(jsuper)); > brelse(bh); > #ifdef WORDS_BIGENDIAN > diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h > index 599c972..a759741 100644 > --- a/lib/ext2fs/ext2fs.h > +++ b/lib/ext2fs/ext2fs.h > @@ -1498,6 +1498,7 @@ extern errcode_t ext2fs_add_journal_inode(ext2_filsys fs, blk_t num_blocks, > extern errcode_t ext2fs_add_journal_inode2(ext2_filsys fs, blk_t num_blocks, > blk64_t goal, int flags); > extern int ext2fs_default_journal_size(__u64 num_blocks); > +extern int ext2fs_journal_sb_start(int blocksize); > > /* openfs.c */ > extern errcode_t ext2fs_open(const char *name, int flags, int superblock, > diff --git a/lib/ext2fs/mkjournal.c b/lib/ext2fs/mkjournal.c > index 884d9c0..0ad674d 100644 > --- a/lib/ext2fs/mkjournal.c > +++ b/lib/ext2fs/mkjournal.c > @@ -49,7 +49,7 @@ errcode_t ext2fs_create_journal_superblock(ext2_filsys fs, > errcode_t retval; > journal_superblock_t *jsb; > > - if (num_blocks < 1024) > + if (num_blocks < JFS_MIN_JOURNAL_BLOCKS) > return EXT2_ET_JOURNAL_TOO_SMALL; > > if ((retval = ext2fs_get_mem(fs->blocksize, &jsb))) > @@ -75,10 +75,7 @@ errcode_t ext2fs_create_journal_superblock(ext2_filsys fs, > if (fs->super->s_feature_incompat & > EXT3_FEATURE_INCOMPAT_JOURNAL_DEV) { > jsb->s_nr_users = 0; > - if (fs->blocksize == 1024) > - jsb->s_first = htonl(3); > - else > - jsb->s_first = htonl(2); > + jsb->s_first = htonl(ext2fs_journal_sb_start(fs->blocksize) + 1); > } > > *ret_jsb = (char *) jsb; > @@ -430,6 +427,13 @@ int ext2fs_default_journal_size(__u64 num_blocks) > return 32768; > } > > +int ext2fs_journal_sb_start(int blocksize) > +{ > + if (blocksize == EXT2_MIN_BLOCK_SIZE) > + return 2; > + return 1; > +} > + > /* > * This function adds a journal device to a filesystem > */ > @@ -437,7 +441,7 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) > { > struct stat st; > errcode_t retval; > - char buf[1024]; > + char buf[SUPERBLOCK_SIZE]; > journal_superblock_t *jsb; > int start; > __u32 i, nr_users; > @@ -450,10 +454,9 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) > return EXT2_ET_JOURNAL_NOT_BLOCK; /* Must be a block device */ > > /* Get the journal superblock */ > - start = 1; > - if (journal_dev->blocksize == 1024) > - start++; > - if ((retval = io_channel_read_blk64(journal_dev->io, start, -1024, > + start = ext2fs_journal_sb_start(journal_dev->blocksize); > + if ((retval = io_channel_read_blk64(journal_dev->io, start, > + -SUPERBLOCK_SIZE, > buf))) > return retval; > > @@ -479,7 +482,8 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) > } > > /* Writeback the journal superblock */ > - if ((retval = io_channel_write_blk64(journal_dev->io, start, -1024, buf))) > + if ((retval = io_channel_write_blk64(journal_dev->io, start, > + -SUPERBLOCK_SIZE, buf))) > return retval; > > fs->super->s_journal_inum = 0; > @@ -632,7 +636,7 @@ main(int argc, char **argv) > exit(1); > } > > - retval = ext2fs_add_journal_inode(fs, 1024, 0); > + retval = ext2fs_add_journal_inode(fs, JFS_MIN_JOURNAL_BLOCKS, 0); > if (retval) { > com_err(argv[0], retval, "while adding journal to %s", > device_name); > diff --git a/misc/tune2fs.c b/misc/tune2fs.c > index d95cc3d..0e7caf2 100644 > --- a/misc/tune2fs.c > +++ b/misc/tune2fs.c > @@ -187,7 +187,7 @@ static int remove_journal_device(ext2_filsys fs) > { > char *journal_path; > ext2_filsys jfs; > - char buf[1024]; > + char buf[SUPERBLOCK_SIZE]; > journal_superblock_t *jsb; > int i, nr_users; > errcode_t retval; > @@ -230,7 +230,7 @@ static int remove_journal_device(ext2_filsys fs) > } > > /* Get the journal superblock */ > - if ((retval = io_channel_read_blk64(jfs->io, 1, -1024, buf))) { > + if ((retval = io_channel_read_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { > com_err(program_name, retval, "%s", > _("while reading journal superblock")); > goto no_valid_journal; > @@ -261,7 +261,7 @@ static int remove_journal_device(ext2_filsys fs) > jsb->s_nr_users = htonl(nr_users); > > /* Write back the journal superblock */ > - if ((retval = io_channel_write_blk64(jfs->io, 1, -1024, buf))) { > + if ((retval = io_channel_write_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { > com_err(program_name, retval, > "while writing journal superblock."); > goto no_valid_journal; > -- > 2.0.0 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3 v2] journal: use consts instead of 1024 and add helper for journal with 1k blocksize 2014-07-09 4:46 ` Andreas Dilger @ 2014-07-09 4:54 ` Azat Khuzhin 2014-07-09 18:57 ` Darrick J. Wong 0 siblings, 1 reply; 16+ messages in thread From: Azat Khuzhin @ 2014-07-09 4:54 UTC (permalink / raw) To: Andreas Dilger; +Cc: linux-ext4, tytso On Tue, Jul 08, 2014 at 10:46:56PM -0600, Andreas Dilger wrote: > The patch looks good, and you can add: > > Reviewed-by: Andreas Dilger <adilger@dilger.ca> > > One thing that could probably be fixed if this patch is refreshed, > or in a precursor patch is to rename SUPERBLOCK_SIZE to be > EXT2_SUPERBLOCK_SIZE since this avoids potential name clashes. I don't have any objections, but this change could break the header-compatibily, no? (Since lib/ext2fs/ext2fs.h is public available). Cheers, Azat. > > Cheers, Andreas > > > On Jul 8, 2014, at 21:56, Azat Khuzhin <a3at.mail@gmail.com> wrote: > > > > Use EXT2_MIN_BLOCK_SIZE/JFS_MIN_JOURNAL_BLOCKS instead of hardcoded 1024 > > when it is okay, and also add a helper ext2fs_journal_sb_start() that > > will return start of journal sb with special case for fs with 1k block > > size. > > > > Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com> > > --- > > v2: mixxed htonl() (thanks to Andreas Dilger) > > > > e2fsck/journal.c | 5 ++--- > > lib/ext2fs/ext2fs.h | 1 + > > lib/ext2fs/mkjournal.c | 28 ++++++++++++++++------------ > > misc/tune2fs.c | 6 +++--- > > 4 files changed, 22 insertions(+), 18 deletions(-) > > > > diff --git a/e2fsck/journal.c b/e2fsck/journal.c > > index a7b1150..5780be0 100644 > > --- a/e2fsck/journal.c > > +++ b/e2fsck/journal.c > > @@ -443,8 +443,7 @@ static errcode_t e2fsck_get_journal(e2fsck_t ctx, journal_t **ret_journal) > > if (ext_journal) { > > blk64_t maxlen; > > > > - if (ctx->fs->blocksize == 1024) > > - start = 1; > > + start = ext2fs_journal_sb_start(ctx->fs->blocksize) - 1; > > bh = getblk(dev_journal, start, ctx->fs->blocksize); > > if (!bh) { > > retval = EXT2_ET_NO_MEMORY; > > @@ -455,7 +454,7 @@ static errcode_t e2fsck_get_journal(e2fsck_t ctx, journal_t **ret_journal) > > brelse(bh); > > goto errout; > > } > > - memcpy(&jsuper, start ? bh->b_data : bh->b_data + 1024, > > + memcpy(&jsuper, start ? bh->b_data : bh->b_data + EXT2_MIN_BLOCK_SIZE, > > sizeof(jsuper)); > > brelse(bh); > > #ifdef WORDS_BIGENDIAN > > diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h > > index 599c972..a759741 100644 > > --- a/lib/ext2fs/ext2fs.h > > +++ b/lib/ext2fs/ext2fs.h > > @@ -1498,6 +1498,7 @@ extern errcode_t ext2fs_add_journal_inode(ext2_filsys fs, blk_t num_blocks, > > extern errcode_t ext2fs_add_journal_inode2(ext2_filsys fs, blk_t num_blocks, > > blk64_t goal, int flags); > > extern int ext2fs_default_journal_size(__u64 num_blocks); > > +extern int ext2fs_journal_sb_start(int blocksize); > > > > /* openfs.c */ > > extern errcode_t ext2fs_open(const char *name, int flags, int superblock, > > diff --git a/lib/ext2fs/mkjournal.c b/lib/ext2fs/mkjournal.c > > index 884d9c0..0ad674d 100644 > > --- a/lib/ext2fs/mkjournal.c > > +++ b/lib/ext2fs/mkjournal.c > > @@ -49,7 +49,7 @@ errcode_t ext2fs_create_journal_superblock(ext2_filsys fs, > > errcode_t retval; > > journal_superblock_t *jsb; > > > > - if (num_blocks < 1024) > > + if (num_blocks < JFS_MIN_JOURNAL_BLOCKS) > > return EXT2_ET_JOURNAL_TOO_SMALL; > > > > if ((retval = ext2fs_get_mem(fs->blocksize, &jsb))) > > @@ -75,10 +75,7 @@ errcode_t ext2fs_create_journal_superblock(ext2_filsys fs, > > if (fs->super->s_feature_incompat & > > EXT3_FEATURE_INCOMPAT_JOURNAL_DEV) { > > jsb->s_nr_users = 0; > > - if (fs->blocksize == 1024) > > - jsb->s_first = htonl(3); > > - else > > - jsb->s_first = htonl(2); > > + jsb->s_first = htonl(ext2fs_journal_sb_start(fs->blocksize) + 1); > > } > > > > *ret_jsb = (char *) jsb; > > @@ -430,6 +427,13 @@ int ext2fs_default_journal_size(__u64 num_blocks) > > return 32768; > > } > > > > +int ext2fs_journal_sb_start(int blocksize) > > +{ > > + if (blocksize == EXT2_MIN_BLOCK_SIZE) > > + return 2; > > + return 1; > > +} > > + > > /* > > * This function adds a journal device to a filesystem > > */ > > @@ -437,7 +441,7 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) > > { > > struct stat st; > > errcode_t retval; > > - char buf[1024]; > > + char buf[SUPERBLOCK_SIZE]; > > journal_superblock_t *jsb; > > int start; > > __u32 i, nr_users; > > @@ -450,10 +454,9 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) > > return EXT2_ET_JOURNAL_NOT_BLOCK; /* Must be a block device */ > > > > /* Get the journal superblock */ > > - start = 1; > > - if (journal_dev->blocksize == 1024) > > - start++; > > - if ((retval = io_channel_read_blk64(journal_dev->io, start, -1024, > > + start = ext2fs_journal_sb_start(journal_dev->blocksize); > > + if ((retval = io_channel_read_blk64(journal_dev->io, start, > > + -SUPERBLOCK_SIZE, > > buf))) > > return retval; > > > > @@ -479,7 +482,8 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) > > } > > > > /* Writeback the journal superblock */ > > - if ((retval = io_channel_write_blk64(journal_dev->io, start, -1024, buf))) > > + if ((retval = io_channel_write_blk64(journal_dev->io, start, > > + -SUPERBLOCK_SIZE, buf))) > > return retval; > > > > fs->super->s_journal_inum = 0; > > @@ -632,7 +636,7 @@ main(int argc, char **argv) > > exit(1); > > } > > > > - retval = ext2fs_add_journal_inode(fs, 1024, 0); > > + retval = ext2fs_add_journal_inode(fs, JFS_MIN_JOURNAL_BLOCKS, 0); > > if (retval) { > > com_err(argv[0], retval, "while adding journal to %s", > > device_name); > > diff --git a/misc/tune2fs.c b/misc/tune2fs.c > > index d95cc3d..0e7caf2 100644 > > --- a/misc/tune2fs.c > > +++ b/misc/tune2fs.c > > @@ -187,7 +187,7 @@ static int remove_journal_device(ext2_filsys fs) > > { > > char *journal_path; > > ext2_filsys jfs; > > - char buf[1024]; > > + char buf[SUPERBLOCK_SIZE]; > > journal_superblock_t *jsb; > > int i, nr_users; > > errcode_t retval; > > @@ -230,7 +230,7 @@ static int remove_journal_device(ext2_filsys fs) > > } > > > > /* Get the journal superblock */ > > - if ((retval = io_channel_read_blk64(jfs->io, 1, -1024, buf))) { > > + if ((retval = io_channel_read_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { > > com_err(program_name, retval, "%s", > > _("while reading journal superblock")); > > goto no_valid_journal; > > @@ -261,7 +261,7 @@ static int remove_journal_device(ext2_filsys fs) > > jsb->s_nr_users = htonl(nr_users); > > > > /* Write back the journal superblock */ > > - if ((retval = io_channel_write_blk64(jfs->io, 1, -1024, buf))) { > > + if ((retval = io_channel_write_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { > > com_err(program_name, retval, > > "while writing journal superblock."); > > goto no_valid_journal; > > -- > > 2.0.0 > > -- Respectfully Azat Khuzhin ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3 v2] journal: use consts instead of 1024 and add helper for journal with 1k blocksize 2014-07-09 4:54 ` Azat Khuzhin @ 2014-07-09 18:57 ` Darrick J. Wong 2014-07-09 19:36 ` Azat Khuzhin 2014-07-09 19:39 ` [PATCH 1/3 v3] " Azat Khuzhin 0 siblings, 2 replies; 16+ messages in thread From: Darrick J. Wong @ 2014-07-09 18:57 UTC (permalink / raw) To: Azat Khuzhin; +Cc: Andreas Dilger, linux-ext4, tytso On Wed, Jul 09, 2014 at 08:54:35AM +0400, Azat Khuzhin wrote: > On Tue, Jul 08, 2014 at 10:46:56PM -0600, Andreas Dilger wrote: > > The patch looks good, and you can add: > > > > Reviewed-by: Andreas Dilger <adilger@dilger.ca> > > > > One thing that could probably be fixed if this patch is refreshed, > > or in a precursor patch is to rename SUPERBLOCK_SIZE to be > > EXT2_SUPERBLOCK_SIZE since this avoids potential name clashes. > > I don't have any objections, but this change could break the > header-compatibily, no? (Since lib/ext2fs/ext2fs.h is public available). Perhaps something like this? #define EXT2_SUPERBLOCK_SIZE 1024 #define SUPERBLOCK_SIZE EXT2_SUPERBLOCK_SIZE ...and we can deprecate the old cpp symbol the next major release? If anybody prepends EXT2_ to that symbol, please be sure to rename SUPERBLOCK_OFFSET too. > Cheers, Azat. > > > > > Cheers, Andreas > > > > > On Jul 8, 2014, at 21:56, Azat Khuzhin <a3at.mail@gmail.com> wrote: > > > > > > Use EXT2_MIN_BLOCK_SIZE/JFS_MIN_JOURNAL_BLOCKS instead of hardcoded 1024 > > > when it is okay, and also add a helper ext2fs_journal_sb_start() that > > > will return start of journal sb with special case for fs with 1k block > > > size. > > > > > > Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com> > > > --- > > > v2: mixxed htonl() (thanks to Andreas Dilger) > > > > > > e2fsck/journal.c | 5 ++--- > > > lib/ext2fs/ext2fs.h | 1 + > > > lib/ext2fs/mkjournal.c | 28 ++++++++++++++++------------ > > > misc/tune2fs.c | 6 +++--- > > > 4 files changed, 22 insertions(+), 18 deletions(-) > > > > > > diff --git a/e2fsck/journal.c b/e2fsck/journal.c > > > index a7b1150..5780be0 100644 > > > --- a/e2fsck/journal.c > > > +++ b/e2fsck/journal.c > > > @@ -443,8 +443,7 @@ static errcode_t e2fsck_get_journal(e2fsck_t ctx, journal_t **ret_journal) > > > if (ext_journal) { > > > blk64_t maxlen; > > > > > > - if (ctx->fs->blocksize == 1024) > > > - start = 1; > > > + start = ext2fs_journal_sb_start(ctx->fs->blocksize) - 1; > > > bh = getblk(dev_journal, start, ctx->fs->blocksize); > > > if (!bh) { > > > retval = EXT2_ET_NO_MEMORY; > > > @@ -455,7 +454,7 @@ static errcode_t e2fsck_get_journal(e2fsck_t ctx, journal_t **ret_journal) > > > brelse(bh); > > > goto errout; > > > } > > > - memcpy(&jsuper, start ? bh->b_data : bh->b_data + 1024, > > > + memcpy(&jsuper, start ? bh->b_data : bh->b_data + EXT2_MIN_BLOCK_SIZE, I ... think this 1024 looks more like an offset; did you mean SUPERBLOCK_OFFSET here? --D > > > sizeof(jsuper)); > > > brelse(bh); > > > #ifdef WORDS_BIGENDIAN > > > diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h > > > index 599c972..a759741 100644 > > > --- a/lib/ext2fs/ext2fs.h > > > +++ b/lib/ext2fs/ext2fs.h > > > @@ -1498,6 +1498,7 @@ extern errcode_t ext2fs_add_journal_inode(ext2_filsys fs, blk_t num_blocks, > > > extern errcode_t ext2fs_add_journal_inode2(ext2_filsys fs, blk_t num_blocks, > > > blk64_t goal, int flags); > > > extern int ext2fs_default_journal_size(__u64 num_blocks); > > > +extern int ext2fs_journal_sb_start(int blocksize); > > > > > > /* openfs.c */ > > > extern errcode_t ext2fs_open(const char *name, int flags, int superblock, > > > diff --git a/lib/ext2fs/mkjournal.c b/lib/ext2fs/mkjournal.c > > > index 884d9c0..0ad674d 100644 > > > --- a/lib/ext2fs/mkjournal.c > > > +++ b/lib/ext2fs/mkjournal.c > > > @@ -49,7 +49,7 @@ errcode_t ext2fs_create_journal_superblock(ext2_filsys fs, > > > errcode_t retval; > > > journal_superblock_t *jsb; > > > > > > - if (num_blocks < 1024) > > > + if (num_blocks < JFS_MIN_JOURNAL_BLOCKS) > > > return EXT2_ET_JOURNAL_TOO_SMALL; > > > > > > if ((retval = ext2fs_get_mem(fs->blocksize, &jsb))) > > > @@ -75,10 +75,7 @@ errcode_t ext2fs_create_journal_superblock(ext2_filsys fs, > > > if (fs->super->s_feature_incompat & > > > EXT3_FEATURE_INCOMPAT_JOURNAL_DEV) { > > > jsb->s_nr_users = 0; > > > - if (fs->blocksize == 1024) > > > - jsb->s_first = htonl(3); > > > - else > > > - jsb->s_first = htonl(2); > > > + jsb->s_first = htonl(ext2fs_journal_sb_start(fs->blocksize) + 1); > > > } > > > > > > *ret_jsb = (char *) jsb; > > > @@ -430,6 +427,13 @@ int ext2fs_default_journal_size(__u64 num_blocks) > > > return 32768; > > > } > > > > > > +int ext2fs_journal_sb_start(int blocksize) > > > +{ > > > + if (blocksize == EXT2_MIN_BLOCK_SIZE) > > > + return 2; > > > + return 1; > > > +} > > > + > > > /* > > > * This function adds a journal device to a filesystem > > > */ > > > @@ -437,7 +441,7 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) > > > { > > > struct stat st; > > > errcode_t retval; > > > - char buf[1024]; > > > + char buf[SUPERBLOCK_SIZE]; > > > journal_superblock_t *jsb; > > > int start; > > > __u32 i, nr_users; > > > @@ -450,10 +454,9 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) > > > return EXT2_ET_JOURNAL_NOT_BLOCK; /* Must be a block device */ > > > > > > /* Get the journal superblock */ > > > - start = 1; > > > - if (journal_dev->blocksize == 1024) > > > - start++; > > > - if ((retval = io_channel_read_blk64(journal_dev->io, start, -1024, > > > + start = ext2fs_journal_sb_start(journal_dev->blocksize); > > > + if ((retval = io_channel_read_blk64(journal_dev->io, start, > > > + -SUPERBLOCK_SIZE, > > > buf))) > > > return retval; > > > > > > @@ -479,7 +482,8 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) > > > } > > > > > > /* Writeback the journal superblock */ > > > - if ((retval = io_channel_write_blk64(journal_dev->io, start, -1024, buf))) > > > + if ((retval = io_channel_write_blk64(journal_dev->io, start, > > > + -SUPERBLOCK_SIZE, buf))) > > > return retval; > > > > > > fs->super->s_journal_inum = 0; > > > @@ -632,7 +636,7 @@ main(int argc, char **argv) > > > exit(1); > > > } > > > > > > - retval = ext2fs_add_journal_inode(fs, 1024, 0); > > > + retval = ext2fs_add_journal_inode(fs, JFS_MIN_JOURNAL_BLOCKS, 0); > > > if (retval) { > > > com_err(argv[0], retval, "while adding journal to %s", > > > device_name); > > > diff --git a/misc/tune2fs.c b/misc/tune2fs.c > > > index d95cc3d..0e7caf2 100644 > > > --- a/misc/tune2fs.c > > > +++ b/misc/tune2fs.c > > > @@ -187,7 +187,7 @@ static int remove_journal_device(ext2_filsys fs) > > > { > > > char *journal_path; > > > ext2_filsys jfs; > > > - char buf[1024]; > > > + char buf[SUPERBLOCK_SIZE]; > > > journal_superblock_t *jsb; > > > int i, nr_users; > > > errcode_t retval; > > > @@ -230,7 +230,7 @@ static int remove_journal_device(ext2_filsys fs) > > > } > > > > > > /* Get the journal superblock */ > > > - if ((retval = io_channel_read_blk64(jfs->io, 1, -1024, buf))) { > > > + if ((retval = io_channel_read_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { > > > com_err(program_name, retval, "%s", > > > _("while reading journal superblock")); > > > goto no_valid_journal; > > > @@ -261,7 +261,7 @@ static int remove_journal_device(ext2_filsys fs) > > > jsb->s_nr_users = htonl(nr_users); > > > > > > /* Write back the journal superblock */ > > > - if ((retval = io_channel_write_blk64(jfs->io, 1, -1024, buf))) { > > > + if ((retval = io_channel_write_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { > > > com_err(program_name, retval, > > > "while writing journal superblock."); > > > goto no_valid_journal; > > > -- > > > 2.0.0 > > > > > -- > Respectfully > Azat Khuzhin > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3 v2] journal: use consts instead of 1024 and add helper for journal with 1k blocksize 2014-07-09 18:57 ` Darrick J. Wong @ 2014-07-09 19:36 ` Azat Khuzhin 2014-07-09 19:39 ` [PATCH 1/3 v3] " Azat Khuzhin 1 sibling, 0 replies; 16+ messages in thread From: Azat Khuzhin @ 2014-07-09 19:36 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Andreas Dilger, linux-ext4, tytso On Wed, Jul 09, 2014 at 11:57:42AM -0700, Darrick J. Wong wrote: > On Wed, Jul 09, 2014 at 08:54:35AM +0400, Azat Khuzhin wrote: > > On Tue, Jul 08, 2014 at 10:46:56PM -0600, Andreas Dilger wrote: > > > The patch looks good, and you can add: > > > > > > Reviewed-by: Andreas Dilger <adilger@dilger.ca> > > > > > > One thing that could probably be fixed if this patch is refreshed, > > > or in a precursor patch is to rename SUPERBLOCK_SIZE to be > > > EXT2_SUPERBLOCK_SIZE since this avoids potential name clashes. > > > > I don't have any objections, but this change could break the > > header-compatibily, no? (Since lib/ext2fs/ext2fs.h is public available). > > Perhaps something like this? > > #define EXT2_SUPERBLOCK_SIZE 1024 > #define SUPERBLOCK_SIZE EXT2_SUPERBLOCK_SIZE Hi Darrick, this should works, thanks. I think that this will be the separate patch. > > ...and we can deprecate the old cpp symbol the next major release? > > If anybody prepends EXT2_ to that symbol, please be sure to rename > SUPERBLOCK_OFFSET too. > > > Cheers, Azat. > > > > > > > > Cheers, Andreas > > > > > > > On Jul 8, 2014, at 21:56, Azat Khuzhin <a3at.mail@gmail.com> wrote: > > > > > > > > Use EXT2_MIN_BLOCK_SIZE/JFS_MIN_JOURNAL_BLOCKS instead of hardcoded 1024 > > > > when it is okay, and also add a helper ext2fs_journal_sb_start() that > > > > will return start of journal sb with special case for fs with 1k block > > > > size. > > > > > > > > Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com> > > > > --- > > > > v2: mixxed htonl() (thanks to Andreas Dilger) > > > > > > > > e2fsck/journal.c | 5 ++--- > > > > lib/ext2fs/ext2fs.h | 1 + > > > > lib/ext2fs/mkjournal.c | 28 ++++++++++++++++------------ > > > > misc/tune2fs.c | 6 +++--- > > > > 4 files changed, 22 insertions(+), 18 deletions(-) > > > > > > > > diff --git a/e2fsck/journal.c b/e2fsck/journal.c > > > > index a7b1150..5780be0 100644 > > > > --- a/e2fsck/journal.c > > > > +++ b/e2fsck/journal.c > > > > @@ -443,8 +443,7 @@ static errcode_t e2fsck_get_journal(e2fsck_t ctx, journal_t **ret_journal) > > > > if (ext_journal) { > > > > blk64_t maxlen; > > > > > > > > - if (ctx->fs->blocksize == 1024) > > > > - start = 1; > > > > + start = ext2fs_journal_sb_start(ctx->fs->blocksize) - 1; > > > > bh = getblk(dev_journal, start, ctx->fs->blocksize); > > > > if (!bh) { > > > > retval = EXT2_ET_NO_MEMORY; > > > > @@ -455,7 +454,7 @@ static errcode_t e2fsck_get_journal(e2fsck_t ctx, journal_t **ret_journal) > > > > brelse(bh); > > > > goto errout; > > > > } > > > > - memcpy(&jsuper, start ? bh->b_data : bh->b_data + 1024, > > > > + memcpy(&jsuper, start ? bh->b_data : bh->b_data + EXT2_MIN_BLOCK_SIZE, > > I ... think this 1024 looks more like an offset; did you mean SUPERBLOCK_OFFSET > here? Yeah, good catch, thanks. I will resend updated patch shortly. > > --D > > > > > sizeof(jsuper)); > > > > brelse(bh); > > > > #ifdef WORDS_BIGENDIAN > > > > diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h > > > > index 599c972..a759741 100644 > > > > --- a/lib/ext2fs/ext2fs.h > > > > +++ b/lib/ext2fs/ext2fs.h > > > > @@ -1498,6 +1498,7 @@ extern errcode_t ext2fs_add_journal_inode(ext2_filsys fs, blk_t num_blocks, > > > > extern errcode_t ext2fs_add_journal_inode2(ext2_filsys fs, blk_t num_blocks, > > > > blk64_t goal, int flags); > > > > extern int ext2fs_default_journal_size(__u64 num_blocks); > > > > +extern int ext2fs_journal_sb_start(int blocksize); > > > > > > > > /* openfs.c */ > > > > extern errcode_t ext2fs_open(const char *name, int flags, int superblock, > > > > diff --git a/lib/ext2fs/mkjournal.c b/lib/ext2fs/mkjournal.c > > > > index 884d9c0..0ad674d 100644 > > > > --- a/lib/ext2fs/mkjournal.c > > > > +++ b/lib/ext2fs/mkjournal.c > > > > @@ -49,7 +49,7 @@ errcode_t ext2fs_create_journal_superblock(ext2_filsys fs, > > > > errcode_t retval; > > > > journal_superblock_t *jsb; > > > > > > > > - if (num_blocks < 1024) > > > > + if (num_blocks < JFS_MIN_JOURNAL_BLOCKS) > > > > return EXT2_ET_JOURNAL_TOO_SMALL; > > > > > > > > if ((retval = ext2fs_get_mem(fs->blocksize, &jsb))) > > > > @@ -75,10 +75,7 @@ errcode_t ext2fs_create_journal_superblock(ext2_filsys fs, > > > > if (fs->super->s_feature_incompat & > > > > EXT3_FEATURE_INCOMPAT_JOURNAL_DEV) { > > > > jsb->s_nr_users = 0; > > > > - if (fs->blocksize == 1024) > > > > - jsb->s_first = htonl(3); > > > > - else > > > > - jsb->s_first = htonl(2); > > > > + jsb->s_first = htonl(ext2fs_journal_sb_start(fs->blocksize) + 1); > > > > } > > > > > > > > *ret_jsb = (char *) jsb; > > > > @@ -430,6 +427,13 @@ int ext2fs_default_journal_size(__u64 num_blocks) > > > > return 32768; > > > > } > > > > > > > > +int ext2fs_journal_sb_start(int blocksize) > > > > +{ > > > > + if (blocksize == EXT2_MIN_BLOCK_SIZE) > > > > + return 2; > > > > + return 1; > > > > +} > > > > + > > > > /* > > > > * This function adds a journal device to a filesystem > > > > */ > > > > @@ -437,7 +441,7 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) > > > > { > > > > struct stat st; > > > > errcode_t retval; > > > > - char buf[1024]; > > > > + char buf[SUPERBLOCK_SIZE]; > > > > journal_superblock_t *jsb; > > > > int start; > > > > __u32 i, nr_users; > > > > @@ -450,10 +454,9 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) > > > > return EXT2_ET_JOURNAL_NOT_BLOCK; /* Must be a block device */ > > > > > > > > /* Get the journal superblock */ > > > > - start = 1; > > > > - if (journal_dev->blocksize == 1024) > > > > - start++; > > > > - if ((retval = io_channel_read_blk64(journal_dev->io, start, -1024, > > > > + start = ext2fs_journal_sb_start(journal_dev->blocksize); > > > > + if ((retval = io_channel_read_blk64(journal_dev->io, start, > > > > + -SUPERBLOCK_SIZE, > > > > buf))) > > > > return retval; > > > > > > > > @@ -479,7 +482,8 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) > > > > } > > > > > > > > /* Writeback the journal superblock */ > > > > - if ((retval = io_channel_write_blk64(journal_dev->io, start, -1024, buf))) > > > > + if ((retval = io_channel_write_blk64(journal_dev->io, start, > > > > + -SUPERBLOCK_SIZE, buf))) > > > > return retval; > > > > > > > > fs->super->s_journal_inum = 0; > > > > @@ -632,7 +636,7 @@ main(int argc, char **argv) > > > > exit(1); > > > > } > > > > > > > > - retval = ext2fs_add_journal_inode(fs, 1024, 0); > > > > + retval = ext2fs_add_journal_inode(fs, JFS_MIN_JOURNAL_BLOCKS, 0); > > > > if (retval) { > > > > com_err(argv[0], retval, "while adding journal to %s", > > > > device_name); > > > > diff --git a/misc/tune2fs.c b/misc/tune2fs.c > > > > index d95cc3d..0e7caf2 100644 > > > > --- a/misc/tune2fs.c > > > > +++ b/misc/tune2fs.c > > > > @@ -187,7 +187,7 @@ static int remove_journal_device(ext2_filsys fs) > > > > { > > > > char *journal_path; > > > > ext2_filsys jfs; > > > > - char buf[1024]; > > > > + char buf[SUPERBLOCK_SIZE]; > > > > journal_superblock_t *jsb; > > > > int i, nr_users; > > > > errcode_t retval; > > > > @@ -230,7 +230,7 @@ static int remove_journal_device(ext2_filsys fs) > > > > } > > > > > > > > /* Get the journal superblock */ > > > > - if ((retval = io_channel_read_blk64(jfs->io, 1, -1024, buf))) { > > > > + if ((retval = io_channel_read_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { > > > > com_err(program_name, retval, "%s", > > > > _("while reading journal superblock")); > > > > goto no_valid_journal; > > > > @@ -261,7 +261,7 @@ static int remove_journal_device(ext2_filsys fs) > > > > jsb->s_nr_users = htonl(nr_users); > > > > > > > > /* Write back the journal superblock */ > > > > - if ((retval = io_channel_write_blk64(jfs->io, 1, -1024, buf))) { > > > > + if ((retval = io_channel_write_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { > > > > com_err(program_name, retval, > > > > "while writing journal superblock."); > > > > goto no_valid_journal; > > > > -- > > > > 2.0.0 > > > > > > > > -- > > Respectfully > > Azat Khuzhin > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Respectfully Azat Khuzhin ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3 v3] journal: use consts instead of 1024 and add helper for journal with 1k blocksize 2014-07-09 18:57 ` Darrick J. Wong 2014-07-09 19:36 ` Azat Khuzhin @ 2014-07-09 19:39 ` Azat Khuzhin 2014-07-09 20:33 ` Andreas Dilger 1 sibling, 1 reply; 16+ messages in thread From: Azat Khuzhin @ 2014-07-09 19:39 UTC (permalink / raw) To: darrick.wong; +Cc: adilger, linux-ext4, tytso, Azat Khuzhin Use EXT2_MIN_BLOCK_SIZE/JFS_MIN_JOURNAL_BLOCKS instead of hardcoded 1024 when it is okay, and also add a helper ext2fs_journal_sb_start() that will return start of journal sb with special case for fs with 1k block size. Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com> --- v2: missed htonl() (thanks to Andreas Dilger) v3: use SUPERBLOCK_OFFSET instead of EXT2_MIN_BLOCK_SIZE (thanks to Darrick J. Wong) e2fsck/journal.c | 5 ++--- lib/ext2fs/ext2fs.h | 1 + lib/ext2fs/mkjournal.c | 28 ++++++++++++++++------------ misc/tune2fs.c | 6 +++--- 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/e2fsck/journal.c b/e2fsck/journal.c index a7b1150..6df0165 100644 --- a/e2fsck/journal.c +++ b/e2fsck/journal.c @@ -443,8 +443,7 @@ static errcode_t e2fsck_get_journal(e2fsck_t ctx, journal_t **ret_journal) if (ext_journal) { blk64_t maxlen; - if (ctx->fs->blocksize == 1024) - start = 1; + start = ext2fs_journal_sb_start(ctx->fs->blocksize) - 1; bh = getblk(dev_journal, start, ctx->fs->blocksize); if (!bh) { retval = EXT2_ET_NO_MEMORY; @@ -455,7 +454,7 @@ static errcode_t e2fsck_get_journal(e2fsck_t ctx, journal_t **ret_journal) brelse(bh); goto errout; } - memcpy(&jsuper, start ? bh->b_data : bh->b_data + 1024, + memcpy(&jsuper, start ? bh->b_data : bh->b_data + SUPERBLOCK_OFFSET, sizeof(jsuper)); brelse(bh); #ifdef WORDS_BIGENDIAN diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h index 599c972..a759741 100644 --- a/lib/ext2fs/ext2fs.h +++ b/lib/ext2fs/ext2fs.h @@ -1498,6 +1498,7 @@ extern errcode_t ext2fs_add_journal_inode(ext2_filsys fs, blk_t num_blocks, extern errcode_t ext2fs_add_journal_inode2(ext2_filsys fs, blk_t num_blocks, blk64_t goal, int flags); extern int ext2fs_default_journal_size(__u64 num_blocks); +extern int ext2fs_journal_sb_start(int blocksize); /* openfs.c */ extern errcode_t ext2fs_open(const char *name, int flags, int superblock, diff --git a/lib/ext2fs/mkjournal.c b/lib/ext2fs/mkjournal.c index 884d9c0..0ad674d 100644 --- a/lib/ext2fs/mkjournal.c +++ b/lib/ext2fs/mkjournal.c @@ -49,7 +49,7 @@ errcode_t ext2fs_create_journal_superblock(ext2_filsys fs, errcode_t retval; journal_superblock_t *jsb; - if (num_blocks < 1024) + if (num_blocks < JFS_MIN_JOURNAL_BLOCKS) return EXT2_ET_JOURNAL_TOO_SMALL; if ((retval = ext2fs_get_mem(fs->blocksize, &jsb))) @@ -75,10 +75,7 @@ errcode_t ext2fs_create_journal_superblock(ext2_filsys fs, if (fs->super->s_feature_incompat & EXT3_FEATURE_INCOMPAT_JOURNAL_DEV) { jsb->s_nr_users = 0; - if (fs->blocksize == 1024) - jsb->s_first = htonl(3); - else - jsb->s_first = htonl(2); + jsb->s_first = htonl(ext2fs_journal_sb_start(fs->blocksize) + 1); } *ret_jsb = (char *) jsb; @@ -430,6 +427,13 @@ int ext2fs_default_journal_size(__u64 num_blocks) return 32768; } +int ext2fs_journal_sb_start(int blocksize) +{ + if (blocksize == EXT2_MIN_BLOCK_SIZE) + return 2; + return 1; +} + /* * This function adds a journal device to a filesystem */ @@ -437,7 +441,7 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) { struct stat st; errcode_t retval; - char buf[1024]; + char buf[SUPERBLOCK_SIZE]; journal_superblock_t *jsb; int start; __u32 i, nr_users; @@ -450,10 +454,9 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) return EXT2_ET_JOURNAL_NOT_BLOCK; /* Must be a block device */ /* Get the journal superblock */ - start = 1; - if (journal_dev->blocksize == 1024) - start++; - if ((retval = io_channel_read_blk64(journal_dev->io, start, -1024, + start = ext2fs_journal_sb_start(journal_dev->blocksize); + if ((retval = io_channel_read_blk64(journal_dev->io, start, + -SUPERBLOCK_SIZE, buf))) return retval; @@ -479,7 +482,8 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) } /* Writeback the journal superblock */ - if ((retval = io_channel_write_blk64(journal_dev->io, start, -1024, buf))) + if ((retval = io_channel_write_blk64(journal_dev->io, start, + -SUPERBLOCK_SIZE, buf))) return retval; fs->super->s_journal_inum = 0; @@ -632,7 +636,7 @@ main(int argc, char **argv) exit(1); } - retval = ext2fs_add_journal_inode(fs, 1024, 0); + retval = ext2fs_add_journal_inode(fs, JFS_MIN_JOURNAL_BLOCKS, 0); if (retval) { com_err(argv[0], retval, "while adding journal to %s", device_name); diff --git a/misc/tune2fs.c b/misc/tune2fs.c index d95cc3d..0e7caf2 100644 --- a/misc/tune2fs.c +++ b/misc/tune2fs.c @@ -187,7 +187,7 @@ static int remove_journal_device(ext2_filsys fs) { char *journal_path; ext2_filsys jfs; - char buf[1024]; + char buf[SUPERBLOCK_SIZE]; journal_superblock_t *jsb; int i, nr_users; errcode_t retval; @@ -230,7 +230,7 @@ static int remove_journal_device(ext2_filsys fs) } /* Get the journal superblock */ - if ((retval = io_channel_read_blk64(jfs->io, 1, -1024, buf))) { + if ((retval = io_channel_read_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { com_err(program_name, retval, "%s", _("while reading journal superblock")); goto no_valid_journal; @@ -261,7 +261,7 @@ static int remove_journal_device(ext2_filsys fs) jsb->s_nr_users = htonl(nr_users); /* Write back the journal superblock */ - if ((retval = io_channel_write_blk64(jfs->io, 1, -1024, buf))) { + if ((retval = io_channel_write_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { com_err(program_name, retval, "while writing journal superblock."); goto no_valid_journal; -- 2.0.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3 v3] journal: use consts instead of 1024 and add helper for journal with 1k blocksize 2014-07-09 19:39 ` [PATCH 1/3 v3] " Azat Khuzhin @ 2014-07-09 20:33 ` Andreas Dilger 0 siblings, 0 replies; 16+ messages in thread From: Andreas Dilger @ 2014-07-09 20:33 UTC (permalink / raw) To: Azat Khuzhin; +Cc: darrick.wong, linux-ext4, tytso [-- Attachment #1: Type: text/plain, Size: 6186 bytes --] On Jul 9, 2014, at 1:39 PM, Azat Khuzhin <a3at.mail@gmail.com> wrote: > Use EXT2_MIN_BLOCK_SIZE/JFS_MIN_JOURNAL_BLOCKS instead of hardcoded 1024 > when it is okay, and also add a helper ext2fs_journal_sb_start() that > will return start of journal sb with special case for fs with 1k block > size. > > Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com> Reviewed-by: Andreas Dilger <adilger@dilger.ca> > --- > v2: missed htonl() (thanks to Andreas Dilger) > v3: use SUPERBLOCK_OFFSET instead of EXT2_MIN_BLOCK_SIZE (thanks to Darrick J. > Wong) > > e2fsck/journal.c | 5 ++--- > lib/ext2fs/ext2fs.h | 1 + > lib/ext2fs/mkjournal.c | 28 ++++++++++++++++------------ > misc/tune2fs.c | 6 +++--- > 4 files changed, 22 insertions(+), 18 deletions(-) > > diff --git a/e2fsck/journal.c b/e2fsck/journal.c > index a7b1150..6df0165 100644 > --- a/e2fsck/journal.c > +++ b/e2fsck/journal.c > @@ -443,8 +443,7 @@ static errcode_t e2fsck_get_journal(e2fsck_t ctx, journal_t **ret_journal) > if (ext_journal) { > blk64_t maxlen; > > - if (ctx->fs->blocksize == 1024) > - start = 1; > + start = ext2fs_journal_sb_start(ctx->fs->blocksize) - 1; > bh = getblk(dev_journal, start, ctx->fs->blocksize); > if (!bh) { > retval = EXT2_ET_NO_MEMORY; > @@ -455,7 +454,7 @@ static errcode_t e2fsck_get_journal(e2fsck_t ctx, journal_t **ret_journal) > brelse(bh); > goto errout; > } > - memcpy(&jsuper, start ? bh->b_data : bh->b_data + 1024, > + memcpy(&jsuper, start ? bh->b_data : bh->b_data + SUPERBLOCK_OFFSET, > sizeof(jsuper)); > brelse(bh); > #ifdef WORDS_BIGENDIAN > diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h > index 599c972..a759741 100644 > --- a/lib/ext2fs/ext2fs.h > +++ b/lib/ext2fs/ext2fs.h > @@ -1498,6 +1498,7 @@ extern errcode_t ext2fs_add_journal_inode(ext2_filsys fs, blk_t num_blocks, > extern errcode_t ext2fs_add_journal_inode2(ext2_filsys fs, blk_t num_blocks, > blk64_t goal, int flags); > extern int ext2fs_default_journal_size(__u64 num_blocks); > +extern int ext2fs_journal_sb_start(int blocksize); > > /* openfs.c */ > extern errcode_t ext2fs_open(const char *name, int flags, int superblock, > diff --git a/lib/ext2fs/mkjournal.c b/lib/ext2fs/mkjournal.c > index 884d9c0..0ad674d 100644 > --- a/lib/ext2fs/mkjournal.c > +++ b/lib/ext2fs/mkjournal.c > @@ -49,7 +49,7 @@ errcode_t ext2fs_create_journal_superblock(ext2_filsys fs, > errcode_t retval; > journal_superblock_t *jsb; > > - if (num_blocks < 1024) > + if (num_blocks < JFS_MIN_JOURNAL_BLOCKS) > return EXT2_ET_JOURNAL_TOO_SMALL; > > if ((retval = ext2fs_get_mem(fs->blocksize, &jsb))) > @@ -75,10 +75,7 @@ errcode_t ext2fs_create_journal_superblock(ext2_filsys fs, > if (fs->super->s_feature_incompat & > EXT3_FEATURE_INCOMPAT_JOURNAL_DEV) { > jsb->s_nr_users = 0; > - if (fs->blocksize == 1024) > - jsb->s_first = htonl(3); > - else > - jsb->s_first = htonl(2); > + jsb->s_first = htonl(ext2fs_journal_sb_start(fs->blocksize) + 1); > } > > *ret_jsb = (char *) jsb; > @@ -430,6 +427,13 @@ int ext2fs_default_journal_size(__u64 num_blocks) > return 32768; > } > > +int ext2fs_journal_sb_start(int blocksize) > +{ > + if (blocksize == EXT2_MIN_BLOCK_SIZE) > + return 2; > + return 1; > +} > + > /* > * This function adds a journal device to a filesystem > */ > @@ -437,7 +441,7 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) > { > struct stat st; > errcode_t retval; > - char buf[1024]; > + char buf[SUPERBLOCK_SIZE]; > journal_superblock_t *jsb; > int start; > __u32 i, nr_users; > @@ -450,10 +454,9 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) > return EXT2_ET_JOURNAL_NOT_BLOCK; /* Must be a block device */ > > /* Get the journal superblock */ > - start = 1; > - if (journal_dev->blocksize == 1024) > - start++; > - if ((retval = io_channel_read_blk64(journal_dev->io, start, -1024, > + start = ext2fs_journal_sb_start(journal_dev->blocksize); > + if ((retval = io_channel_read_blk64(journal_dev->io, start, > + -SUPERBLOCK_SIZE, > buf))) > return retval; > > @@ -479,7 +482,8 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev) > } > > /* Writeback the journal superblock */ > - if ((retval = io_channel_write_blk64(journal_dev->io, start, -1024, buf))) > + if ((retval = io_channel_write_blk64(journal_dev->io, start, > + -SUPERBLOCK_SIZE, buf))) > return retval; > > fs->super->s_journal_inum = 0; > @@ -632,7 +636,7 @@ main(int argc, char **argv) > exit(1); > } > > - retval = ext2fs_add_journal_inode(fs, 1024, 0); > + retval = ext2fs_add_journal_inode(fs, JFS_MIN_JOURNAL_BLOCKS, 0); > if (retval) { > com_err(argv[0], retval, "while adding journal to %s", > device_name); > diff --git a/misc/tune2fs.c b/misc/tune2fs.c > index d95cc3d..0e7caf2 100644 > --- a/misc/tune2fs.c > +++ b/misc/tune2fs.c > @@ -187,7 +187,7 @@ static int remove_journal_device(ext2_filsys fs) > { > char *journal_path; > ext2_filsys jfs; > - char buf[1024]; > + char buf[SUPERBLOCK_SIZE]; > journal_superblock_t *jsb; > int i, nr_users; > errcode_t retval; > @@ -230,7 +230,7 @@ static int remove_journal_device(ext2_filsys fs) > } > > /* Get the journal superblock */ > - if ((retval = io_channel_read_blk64(jfs->io, 1, -1024, buf))) { > + if ((retval = io_channel_read_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { > com_err(program_name, retval, "%s", > _("while reading journal superblock")); > goto no_valid_journal; > @@ -261,7 +261,7 @@ static int remove_journal_device(ext2_filsys fs) > jsb->s_nr_users = htonl(nr_users); > > /* Write back the journal superblock */ > - if ((retval = io_channel_write_blk64(jfs->io, 1, -1024, buf))) { > + if ((retval = io_channel_write_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { > com_err(program_name, retval, > "while writing journal superblock."); > goto no_valid_journal; > -- > 2.0.0 > Cheers, Andreas [-- Attachment #2: Message signed with OpenPGP using GPGMail --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] tune2fs: remove_journal_device(): use the correct block to find jsb 2014-07-08 20:41 [PATCH] e2fsprogs: journal code small cleanup and fixes Azat Khuzhin 2014-07-08 20:41 ` [PATCH 1/3] journal: use consts instead of 1024 and add helper for journal with 1k blocksize Azat Khuzhin @ 2014-07-08 20:41 ` Azat Khuzhin 2014-07-09 4:48 ` Andreas Dilger 2014-07-08 20:41 ` [PATCH 3/3] tune2fs: update journal super block when changing UUID for fs Azat Khuzhin 2 siblings, 1 reply; 16+ messages in thread From: Azat Khuzhin @ 2014-07-08 20:41 UTC (permalink / raw) To: linux-ext4; +Cc: tytso, Azat Khuzhin Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com> --- misc/tune2fs.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/misc/tune2fs.c b/misc/tune2fs.c index 0e7caf2..811cb4d 100644 --- a/misc/tune2fs.c +++ b/misc/tune2fs.c @@ -193,6 +193,7 @@ static int remove_journal_device(ext2_filsys fs) errcode_t retval; int commit_remove_journal = 0; io_manager io_ptr; + int start; if (f_flag) commit_remove_journal = 1; /* force removal even if error */ @@ -229,8 +230,10 @@ static int remove_journal_device(ext2_filsys fs) goto no_valid_journal; } + start = ext2fs_journal_sb_start(fs->blocksize); /* Get the journal superblock */ - if ((retval = io_channel_read_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { + if ((retval = io_channel_read_blk64(jfs->io, start, + -SUPERBLOCK_SIZE, buf))) { com_err(program_name, retval, "%s", _("while reading journal superblock")); goto no_valid_journal; @@ -261,7 +264,8 @@ static int remove_journal_device(ext2_filsys fs) jsb->s_nr_users = htonl(nr_users); /* Write back the journal superblock */ - if ((retval = io_channel_write_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { + if ((retval = io_channel_write_blk64(jfs->io, start, + -SUPERBLOCK_SIZE, buf))) { com_err(program_name, retval, "while writing journal superblock."); goto no_valid_journal; -- 2.0.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] tune2fs: remove_journal_device(): use the correct block to find jsb 2014-07-08 20:41 ` [PATCH 2/3] tune2fs: remove_journal_device(): use the correct block to find jsb Azat Khuzhin @ 2014-07-09 4:48 ` Andreas Dilger 0 siblings, 0 replies; 16+ messages in thread From: Andreas Dilger @ 2014-07-09 4:48 UTC (permalink / raw) To: Azat Khuzhin; +Cc: linux-ext4, tytso Looks good. You can add: Reviewed-by: Andreas Dilger <adilger@dilger.ca> Cheers, Andreas > On Jul 8, 2014, at 14:41, Azat Khuzhin <a3at.mail@gmail.com> wrote: > > Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com> > --- > misc/tune2fs.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/misc/tune2fs.c b/misc/tune2fs.c > index 0e7caf2..811cb4d 100644 > --- a/misc/tune2fs.c > +++ b/misc/tune2fs.c > @@ -193,6 +193,7 @@ static int remove_journal_device(ext2_filsys fs) > errcode_t retval; > int commit_remove_journal = 0; > io_manager io_ptr; > + int start; > > if (f_flag) > commit_remove_journal = 1; /* force removal even if error */ > @@ -229,8 +230,10 @@ static int remove_journal_device(ext2_filsys fs) > goto no_valid_journal; > } > > + start = ext2fs_journal_sb_start(fs->blocksize); > /* Get the journal superblock */ > - if ((retval = io_channel_read_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { > + if ((retval = io_channel_read_blk64(jfs->io, start, > + -SUPERBLOCK_SIZE, buf))) { > com_err(program_name, retval, "%s", > _("while reading journal superblock")); > goto no_valid_journal; > @@ -261,7 +264,8 @@ static int remove_journal_device(ext2_filsys fs) > jsb->s_nr_users = htonl(nr_users); > > /* Write back the journal superblock */ > - if ((retval = io_channel_write_blk64(jfs->io, 1, -SUPERBLOCK_SIZE, buf))) { > + if ((retval = io_channel_write_blk64(jfs->io, start, > + -SUPERBLOCK_SIZE, buf))) { > com_err(program_name, retval, > "while writing journal superblock."); > goto no_valid_journal; > -- > 2.0.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] tune2fs: update journal super block when changing UUID for fs. 2014-07-08 20:41 [PATCH] e2fsprogs: journal code small cleanup and fixes Azat Khuzhin 2014-07-08 20:41 ` [PATCH 1/3] journal: use consts instead of 1024 and add helper for journal with 1k blocksize Azat Khuzhin 2014-07-08 20:41 ` [PATCH 2/3] tune2fs: remove_journal_device(): use the correct block to find jsb Azat Khuzhin @ 2014-07-08 20:41 ` Azat Khuzhin 2014-07-09 20:38 ` Andreas Dilger 2 siblings, 1 reply; 16+ messages in thread From: Azat Khuzhin @ 2014-07-08 20:41 UTC (permalink / raw) To: linux-ext4; +Cc: tytso, Azat Khuzhin Using -U option you can change the UUID for fs, however it will not work for journal device, since it have a copy of this UUID inside jsb (i.e. journal super block). So copy UUID on change into that block. Here is the initial thread: http://comments.gmane.org/gmane.comp.file-systems.ext4/44532 You can reproduce this by executing following commands: $ fallocate -l100M /tmp/dev $ fallocate -l100M /tmp/journal $ sudo /sbin/losetup /dev/loop1 /tmp/dev $ sudo /sbin/losetup /dev/loop0 /tmp/journal $ mke2fs -O journal_dev /tmp/journal $ tune2fs -U da1f2ed0-60f6-aaaa-92fd-738701418523 /tmp/journal $ sudo mke2fs -t ext4 -J device=/dev/loop0 /dev/loop1 $ dumpe2fs -h /tmp/dev | fgrep UUID dumpe2fs 1.43-WIP (18-May-2014) Filesystem UUID: 8a776be9-12eb-411f-8e88-b873575ecfb6 Journal UUID: e3d02151-e776-4865-af25-aecb7291e8e5 $ sudo e2fsck /dev/vdc e2fsck 1.43-WIP (18-May-2014) External journal does not support this filesystem /dev/loop1: ********** WARNING: Filesystem still has errors ********** Reported-by: Chin Tzung Cheng <chintzung@gmail.com> Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com> --- misc/tune2fs.c | 74 ++++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 56 insertions(+), 18 deletions(-) diff --git a/misc/tune2fs.c b/misc/tune2fs.c index 811cb4d..898a1b3 100644 --- a/misc/tune2fs.c +++ b/misc/tune2fs.c @@ -180,6 +180,38 @@ static __u32 clear_ok_features[3] = { EXT4_FEATURE_RO_COMPAT_METADATA_CSUM }; +/** + * Try to get journal super block if any + */ +static int get_journal_sb(ext2_filsys jfs, char buf[SUPERBLOCK_SIZE]) +{ + int retval; + int start; + journal_superblock_t *jsb; + + if (!(jfs->super->s_feature_incompat & + EXT3_FEATURE_INCOMPAT_JOURNAL_DEV)) { + return EXT2_ET_UNSUPP_FEATURE; + } + + /* Get the journal superblock */ + if ((retval = io_channel_read_blk64(jfs->io, + ext2fs_journal_sb_start(jfs->blocksize), -SUPERBLOCK_SIZE, buf))) { + com_err(program_name, retval, "%s", + _("while reading journal superblock")); + return retval; + } + + jsb = (journal_superblock_t *) buf; + if ((jsb->s_header.h_magic != (unsigned)ntohl(JFS_MAGIC_NUMBER)) || + (jsb->s_header.h_blocktype != (unsigned)ntohl(JFS_SUPERBLOCK_V2))) { + fputs(_("Journal superblock not found!\n"), stderr); + return EXT2_ET_BAD_MAGIC; + } + + return 0; +} + /* * Remove an external journal from the filesystem */ @@ -223,29 +255,15 @@ static int remove_journal_device(ext2_filsys fs) _("while trying to open external journal")); goto no_valid_journal; } - if (!(jfs->super->s_feature_incompat & - EXT3_FEATURE_INCOMPAT_JOURNAL_DEV)) { - fprintf(stderr, _("%s is not a journal device.\n"), - journal_path); - goto no_valid_journal; - } - start = ext2fs_journal_sb_start(fs->blocksize); - /* Get the journal superblock */ - if ((retval = io_channel_read_blk64(jfs->io, start, - -SUPERBLOCK_SIZE, buf))) { - com_err(program_name, retval, "%s", - _("while reading journal superblock")); + if ((retval = get_journal_sb(jfs, buf))) { + if (retval == EXT2_ET_UNSUPP_FEATURE) + fprintf(stderr, _("%s is not a journal device.\n"), + journal_path); goto no_valid_journal; } jsb = (journal_superblock_t *) buf; - if ((jsb->s_header.h_magic != (unsigned)ntohl(JFS_MAGIC_NUMBER)) || - (jsb->s_header.h_blocktype != (unsigned)ntohl(JFS_SUPERBLOCK_V2))) { - fputs(_("Journal superblock not found!\n"), stderr); - goto no_valid_journal; - } - /* Find the filesystem UUID */ nr_users = ntohl(jsb->s_nr_users); for (i = 0; i < nr_users; i++) { @@ -2695,6 +2713,7 @@ retry_open: if (U_flag) { int set_csum = 0; dgrp_t i; + char buf[SUPERBLOCK_SIZE]; if (ext2fs_has_group_desc_csum(fs)) { /* @@ -2740,6 +2759,25 @@ retry_open: ext2fs_group_desc_csum_set(fs, i); fs->flags &= ~EXT2_FLAG_SUPER_ONLY; } + + /* If this is a journal dev, we need to copy UUID into jsb */ + if (!(rc = get_journal_sb(fs, buf))) { + journal_superblock_t *jsb; + + jsb = (journal_superblock_t *) buf; + fputs(_("Need to update journal superblock.\n"), stdout); + memcpy(jsb->s_uuid, sb->s_uuid, sizeof(sb->s_uuid)); + + /* Writeback the journal superblock */ + if ((rc = io_channel_write_blk64(fs->io, + ext2fs_journal_sb_start(fs->blocksize), + -SUPERBLOCK_SIZE, buf))) + goto closefs; + } else if (rc != EXT2_ET_UNSUPP_FEATURE) + goto closefs; + else + rc = 0; /** Reset rc to avoid ext2fs_mmp_stop() */ + ext2fs_mark_super_dirty(fs); if (EXT2_HAS_RO_COMPAT_FEATURE(fs->super, EXT4_FEATURE_RO_COMPAT_METADATA_CSUM)) -- 2.0.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] tune2fs: update journal super block when changing UUID for fs. 2014-07-08 20:41 ` [PATCH 3/3] tune2fs: update journal super block when changing UUID for fs Azat Khuzhin @ 2014-07-09 20:38 ` Andreas Dilger 2014-07-10 6:03 ` Azat Khuzhin 0 siblings, 1 reply; 16+ messages in thread From: Andreas Dilger @ 2014-07-09 20:38 UTC (permalink / raw) To: Azat Khuzhin; +Cc: linux-ext4, tytso [-- Attachment #1: Type: text/plain, Size: 5579 bytes --] On Jul 8, 2014, at 2:41 PM, Azat Khuzhin <a3at.mail@gmail.com> wrote: > Using -U option you can change the UUID for fs, however it will not work > for journal device, since it have a copy of this UUID inside jsb (i.e. > journal super block). So copy UUID on change into that block. > > Here is the initial thread: > http://comments.gmane.org/gmane.comp.file-systems.ext4/44532 > > You can reproduce this by executing following commands: > $ fallocate -l100M /tmp/dev > $ fallocate -l100M /tmp/journal > $ sudo /sbin/losetup /dev/loop1 /tmp/dev > $ sudo /sbin/losetup /dev/loop0 /tmp/journal > $ mke2fs -O journal_dev /tmp/journal > $ tune2fs -U da1f2ed0-60f6-aaaa-92fd-738701418523 /tmp/journal > $ sudo mke2fs -t ext4 -J device=/dev/loop0 /dev/loop1 > $ dumpe2fs -h /tmp/dev | fgrep UUID > dumpe2fs 1.43-WIP (18-May-2014) > Filesystem UUID: 8a776be9-12eb-411f-8e88-b873575ecfb6 > Journal UUID: e3d02151-e776-4865-af25-aecb7291e8e5 > $ sudo e2fsck /dev/vdc > e2fsck 1.43-WIP (18-May-2014) > External journal does not support this filesystem > > /dev/loop1: ********** WARNING: Filesystem still has errors ********** > > Reported-by: Chin Tzung Cheng <chintzung@gmail.com> > Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com> It may be that the reverse problem also exists - if the UUID is changed on the filesystem, it probably is not updated in the journal "users" list? I'm not sure either way, but it is worthwhile to check. Reviewed-by: Andreas Dilger <adilger@dilger.ca> > --- > misc/tune2fs.c | 74 ++++++++++++++++++++++++++++++++++++++++++++-------------- > 1 file changed, 56 insertions(+), 18 deletions(-) > > diff --git a/misc/tune2fs.c b/misc/tune2fs.c > index 811cb4d..898a1b3 100644 > --- a/misc/tune2fs.c > +++ b/misc/tune2fs.c > @@ -180,6 +180,38 @@ static __u32 clear_ok_features[3] = { > EXT4_FEATURE_RO_COMPAT_METADATA_CSUM > }; > > +/** > + * Try to get journal super block if any > + */ > +static int get_journal_sb(ext2_filsys jfs, char buf[SUPERBLOCK_SIZE]) > +{ > + int retval; > + int start; > + journal_superblock_t *jsb; > + > + if (!(jfs->super->s_feature_incompat & > + EXT3_FEATURE_INCOMPAT_JOURNAL_DEV)) { > + return EXT2_ET_UNSUPP_FEATURE; > + } > + > + /* Get the journal superblock */ > + if ((retval = io_channel_read_blk64(jfs->io, > + ext2fs_journal_sb_start(jfs->blocksize), -SUPERBLOCK_SIZE, buf))) { > + com_err(program_name, retval, "%s", > + _("while reading journal superblock")); > + return retval; > + } > + > + jsb = (journal_superblock_t *) buf; > + if ((jsb->s_header.h_magic != (unsigned)ntohl(JFS_MAGIC_NUMBER)) || > + (jsb->s_header.h_blocktype != (unsigned)ntohl(JFS_SUPERBLOCK_V2))) { > + fputs(_("Journal superblock not found!\n"), stderr); > + return EXT2_ET_BAD_MAGIC; > + } > + > + return 0; > +} > + > /* > * Remove an external journal from the filesystem > */ > @@ -223,29 +255,15 @@ static int remove_journal_device(ext2_filsys fs) > _("while trying to open external journal")); > goto no_valid_journal; > } > - if (!(jfs->super->s_feature_incompat & > - EXT3_FEATURE_INCOMPAT_JOURNAL_DEV)) { > - fprintf(stderr, _("%s is not a journal device.\n"), > - journal_path); > - goto no_valid_journal; > - } > > - start = ext2fs_journal_sb_start(fs->blocksize); > - /* Get the journal superblock */ > - if ((retval = io_channel_read_blk64(jfs->io, start, > - -SUPERBLOCK_SIZE, buf))) { > - com_err(program_name, retval, "%s", > - _("while reading journal superblock")); > + if ((retval = get_journal_sb(jfs, buf))) { > + if (retval == EXT2_ET_UNSUPP_FEATURE) > + fprintf(stderr, _("%s is not a journal device.\n"), > + journal_path); > goto no_valid_journal; > } > > jsb = (journal_superblock_t *) buf; > - if ((jsb->s_header.h_magic != (unsigned)ntohl(JFS_MAGIC_NUMBER)) || > - (jsb->s_header.h_blocktype != (unsigned)ntohl(JFS_SUPERBLOCK_V2))) { > - fputs(_("Journal superblock not found!\n"), stderr); > - goto no_valid_journal; > - } > - > /* Find the filesystem UUID */ > nr_users = ntohl(jsb->s_nr_users); > for (i = 0; i < nr_users; i++) { > @@ -2695,6 +2713,7 @@ retry_open: > if (U_flag) { > int set_csum = 0; > dgrp_t i; > + char buf[SUPERBLOCK_SIZE]; > > if (ext2fs_has_group_desc_csum(fs)) { > /* > @@ -2740,6 +2759,25 @@ retry_open: > ext2fs_group_desc_csum_set(fs, i); > fs->flags &= ~EXT2_FLAG_SUPER_ONLY; > } > + > + /* If this is a journal dev, we need to copy UUID into jsb */ > + if (!(rc = get_journal_sb(fs, buf))) { > + journal_superblock_t *jsb; > + > + jsb = (journal_superblock_t *) buf; > + fputs(_("Need to update journal superblock.\n"), stdout); > + memcpy(jsb->s_uuid, sb->s_uuid, sizeof(sb->s_uuid)); > + > + /* Writeback the journal superblock */ > + if ((rc = io_channel_write_blk64(fs->io, > + ext2fs_journal_sb_start(fs->blocksize), > + -SUPERBLOCK_SIZE, buf))) > + goto closefs; > + } else if (rc != EXT2_ET_UNSUPP_FEATURE) > + goto closefs; > + else > + rc = 0; /** Reset rc to avoid ext2fs_mmp_stop() */ > + > ext2fs_mark_super_dirty(fs); > if (EXT2_HAS_RO_COMPAT_FEATURE(fs->super, > EXT4_FEATURE_RO_COMPAT_METADATA_CSUM)) > -- > 2.0.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Cheers, Andreas [-- Attachment #2: Message signed with OpenPGP using GPGMail --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] tune2fs: update journal super block when changing UUID for fs. 2014-07-09 20:38 ` Andreas Dilger @ 2014-07-10 6:03 ` Azat Khuzhin 0 siblings, 0 replies; 16+ messages in thread From: Azat Khuzhin @ 2014-07-10 6:03 UTC (permalink / raw) To: Andreas Dilger; +Cc: linux-ext4, tytso On Wed, Jul 09, 2014 at 02:38:19PM -0600, Andreas Dilger wrote: > > On Jul 8, 2014, at 2:41 PM, Azat Khuzhin <a3at.mail@gmail.com> wrote: > > > Using -U option you can change the UUID for fs, however it will not work > > for journal device, since it have a copy of this UUID inside jsb (i.e. > > journal super block). So copy UUID on change into that block. > > > > Here is the initial thread: > > http://comments.gmane.org/gmane.comp.file-systems.ext4/44532 > > > > You can reproduce this by executing following commands: > > $ fallocate -l100M /tmp/dev > > $ fallocate -l100M /tmp/journal > > $ sudo /sbin/losetup /dev/loop1 /tmp/dev > > $ sudo /sbin/losetup /dev/loop0 /tmp/journal > > $ mke2fs -O journal_dev /tmp/journal > > $ tune2fs -U da1f2ed0-60f6-aaaa-92fd-738701418523 /tmp/journal > > $ sudo mke2fs -t ext4 -J device=/dev/loop0 /dev/loop1 > > $ dumpe2fs -h /tmp/dev | fgrep UUID > > dumpe2fs 1.43-WIP (18-May-2014) > > Filesystem UUID: 8a776be9-12eb-411f-8e88-b873575ecfb6 > > Journal UUID: e3d02151-e776-4865-af25-aecb7291e8e5 > > $ sudo e2fsck /dev/vdc > > e2fsck 1.43-WIP (18-May-2014) > > External journal does not support this filesystem > > > > /dev/loop1: ********** WARNING: Filesystem still has errors ********** > > > > Reported-by: Chin Tzung Cheng <chintzung@gmail.com> > > Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com> > > It may be that the reverse problem also exists - if the UUID is changed > on the filesystem, it probably is not updated in the journal "users" list? > I'm not sure either way, but it is worthwhile to check. You are right, it will not update the 'Journal users' field, there is no code for U_flag in tune2fs for this (I also test it). But to make patch clean I need more time, I will send it later. > > Reviewed-by: Andreas Dilger <adilger@dilger.ca> Thanks! -- Respectfully Azat Khuzhin ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2014-07-10 6:03 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-07-08 20:41 [PATCH] e2fsprogs: journal code small cleanup and fixes Azat Khuzhin 2014-07-08 20:41 ` [PATCH 1/3] journal: use consts instead of 1024 and add helper for journal with 1k blocksize Azat Khuzhin 2014-07-08 23:30 ` Andreas Dilger 2014-07-09 3:51 ` Azat Khuzhin 2014-07-09 3:56 ` [PATCH 1/3 v2] " Azat Khuzhin 2014-07-09 4:46 ` Andreas Dilger 2014-07-09 4:54 ` Azat Khuzhin 2014-07-09 18:57 ` Darrick J. Wong 2014-07-09 19:36 ` Azat Khuzhin 2014-07-09 19:39 ` [PATCH 1/3 v3] " Azat Khuzhin 2014-07-09 20:33 ` Andreas Dilger 2014-07-08 20:41 ` [PATCH 2/3] tune2fs: remove_journal_device(): use the correct block to find jsb Azat Khuzhin 2014-07-09 4:48 ` Andreas Dilger 2014-07-08 20:41 ` [PATCH 3/3] tune2fs: update journal super block when changing UUID for fs Azat Khuzhin 2014-07-09 20:38 ` Andreas Dilger 2014-07-10 6:03 ` Azat Khuzhin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.