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
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 <[email protected]>
Signed-off-by: Azat Khuzhin <[email protected]>
---
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
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 <[email protected]>
---
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
Signed-off-by: Azat Khuzhin <[email protected]>
---
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
On Jul 8, 2014, at 2:41 PM, Azat Khuzhin <[email protected]> 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
On Tue, Jul 08, 2014 at 05:30:10PM -0600, Andreas Dilger wrote:
> On Jul 8, 2014, at 2:41 PM, Azat Khuzhin <[email protected]> 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
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 <[email protected]>
---
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
The patch looks good, and you can add:
Reviewed-by: Andreas Dilger <[email protected]>
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 <[email protected]> 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 <[email protected]>
> ---
> 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
>
Looks good. You can add:
Reviewed-by: Andreas Dilger <[email protected]>
Cheers, Andreas
> On Jul 8, 2014, at 14:41, Azat Khuzhin <[email protected]> wrote:
>
> Signed-off-by: Azat Khuzhin <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <[email protected]>
>
> 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 <[email protected]> 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 <[email protected]>
> > ---
> > 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
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 <[email protected]>
> >
> > 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 <[email protected]> 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 <[email protected]>
> > > ---
> > > 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <[email protected]>
> > >
> > > 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 <[email protected]> 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 <[email protected]>
> > > > ---
> > > > 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 [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Respectfully
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 <[email protected]>
---
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
On Jul 9, 2014, at 1:39 PM, Azat Khuzhin <[email protected]> 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 <[email protected]>
Reviewed-by: Andreas Dilger <[email protected]>
> ---
> 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
On Jul 8, 2014, at 2:41 PM, Azat Khuzhin <[email protected]> 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 <[email protected]>
> Signed-off-by: Azat Khuzhin <[email protected]>
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 <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers, Andreas
On Wed, Jul 09, 2014 at 02:38:19PM -0600, Andreas Dilger wrote:
>
> On Jul 8, 2014, at 2:41 PM, Azat Khuzhin <[email protected]> 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 <[email protected]>
> > Signed-off-by: Azat Khuzhin <[email protected]>
>
> 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 <[email protected]>
Thanks!
--
Respectfully
Azat Khuzhin