[Ocfs2-tools-devel] New patch set of backup superblock for ocfs2-tools

[Sunil Mushran]

patches for libocfs2 and mkfs look good.

debugfs has one boiler plate change.

tunefs has one serious flaw. rest are all boiler plate issues.

fsck am still thinking as to what all it should do. As in,
use a backup is obvious. What about refreshing the backups
as the last stage... after the disk is seen to be clean but
the backups are noticed to be off synch with main. something to
think about.


tunefs:
1.
+               { "backup-super", 0, 0, 'b'},
Don't waste the single letter for this. I mean, going forward
backup super will be the default. We can save 'b' for something
that will be used more often.

Instead do:
enum
{
  BACKUP_SUPER_OPTION = CHAR_MAX + 1,
};
...
            { "backup-super", 0, 0, BACKUP_SUPER_OPTION},
...
              case BACKUP_SUPER_OPTION:
                      opts.backup_super =  1;
...

2.
+               printf("Backing up superblock can't be applied "
+                       "with other options\n");

Change to:
               com_err(opts.progname, 0, "Cannot backup superblock along 
with other tasks");

3.
+               com_err(opts.progname, 0,
+                       "This volume already has backup superblocks");
Change to:
                com_err(opts.progname, 0,
                         "Volume has been enabled for Backup superblock");

4.
+               com_err(opts.progname, 0,
+                       "This volume has no space for backup superblocks");

Should this be an error?
I mean if the volume cannot hold backup supers, we could still set the
compat flag. Just like we do during mkfs.


5.
+                       com_err(opts.progname, 0, "block %"PRIu64
+                               " is already allocated.", blocks[i]);
Change to:
                        com_err(opts.progname, 0, "block %"PRIu64" is in 
use", blocks[i]);

6.
+               com_err(opts.progname, 0, "backup blocks check failed.");
+               com_err(opts.progname, 0, "Run debugfs.ocfs2 and use 
command"
+                       " 'icheck' to detect the block's owner.");
+               com_err(opts.progname,0, "Please delete (after backing 
up them)"
+                       " the files and try again.");
Change to:

                com_err(opts.progname, 0, "Cannot enable backup 
superblock as "
                         "backup blocks are in use");

We'll document the debugfs procedure in the manpage.

7.
+               com_err(opts.progname, ret, "while backuping superblock.");
Change to:
                com_err(opts.progname, ret, "while backing up superblock");

8.
+                       printf("Backuped Superblock.\n");
Change to:
                        printf("Backed up Superblock.\n");

9.
refresh_backup_super() is calling ocfs2_set_backup_super() which
is allocating space for the backup super too.

You may want to add a ocfs2_refresh_backup_super() in libocfs2
that just refreshes existing backup superblocks. This is the serious flaw
I was referring to... if am reading the code correctly.

10.
+                               printf("warning, superblock changed and 
failed"
+                                       " to synchronsize the backup 
blocks.");
If this happens, we should remove the backup super compat flag in
the superblock and let fsck reclaim the space before running tunefs again
to set it.


debugfs:
1.
+       g_print ("\t-s, --superblock <backup#>\tOpen the device using 
another superblock\n");
Change to:
       g_print ("\t-s, --superblock <backup#>\tOpen the device using a 
backup superblock\n");



tao.ma wrote:
> New patch set of backup superblock for ocfs2-tools.
> Thank sunil for all your advice about libocfs2, mkfs, tunefs and debugfs.
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> Ocfs2-tools-devel mailing list
> Ocfs2-tools-devel at oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-tools-devel
>