[Ocfs2-tools-devel] tools patches review

[Sunil Mushran]

Joel,

The irc chat is below. In short, I need to pull out two patches.
Rest are sob-ed.

Sunil
============================================================

0001-o2cb-Handle-failure-to-register-heartbeat-mode.patch
0002-o2cb-Add-verbose-tracing-to-o2cb-register-cluster.patch
0003-o2cbtool-Disable-signals-when-starting-heartbeat.patch
**TODO**: Make block_signal() global later.

0004-mounted.ocfs2-Quick-detect-mode-made-more-efficient.patch
0005-mount.ocfs2-Fix-bugs-related-to-hard-ro-mounts.patch
0006-o2cbtool-Silence-usage-in-list-clusters.patch
0007-debugfs.ocfs2-fs_locks-command-understands-version-3.patch
0008-o2cb.init-Start-stop-o2hbmonitor.patch
0009-debugfs.ocfs2-Cleanup.patch
0010-debugfs.ocfs2-Add-command-stat_sysdir.patch
0011-debugfs.ocfs2-Condense-the-print-format-for-the-vari.patch
0012-extras-Add-utility-resize_slotmap.c.patch
0013-libocfs2-Move-metadata-block-type-detect-to-libocfs2.patch
0014-libocfs2-Added-generic-block-swap-functions.patch
**TODO**: Pull out patch14.

0015-debugfs.ocfs2-Fix-output-of-dxroot.patch
0016-libocfs2-Update-some-error-messages.patch
0017-debugfs-Fix-open_debugfs_file.patch
0018-debugfs-Add-command-net_stats.patch
0019-debugfs.ocfs2-Add-command-grpextents.patch
**TODO**: Pull out patch19 for now.

0020-o2cbtool-Fix-command-stop-heartbeat-and-unregister-c.patch
0021-Postamble-Remove-S-BIN_EXTRA-in-clean.patch
0022-manpages-Refreshes-manpages.patch
0023-o2hbmonitor-Add-manpage.patch
0024-o2cb-Add-manpage-for-cluster.conf.patch
0025-o2cb-Add-manpage-for-sysconfig-o2cb.patch
0026-o2cb-Refresh-manpage.patch
0027-ocfs2-Add-manpage.patch
0028-ocfs2-tools-Do-not-distrbute-old-doc-files.patch

===============================================================
20:30 *jlbec* Re: your block_signals change
20:34 *jlbec* Can you just move tunefs_block_signals() to libtools-internal (tools_block_signals()/tools_unblock_signals())
20:35 *jlbec* If you want, you can leave that for a later patch, because a lot of our code has the same block_signals stuff, and should all be moved to tools_block_signals().

20:36 *jlbec*>  yeah... i had wanted to do that but probably better handled separately
20:36 *jlbec*> will do so


20:44 *jlbec* Do you want to warn or anything if o2hbmonitor fails to start?


You mean openlog() fails? I'll add a print later.


20:48 *jlbec* Why do you need a slotmap resize utility, and why are you not using ocfs2_format_slot_map


*SUNIL* I built slotmap resize for a user to test a theory. Added it to extras.
*SUNIL* I want to be sure before tweaking ocfs2_format_slot_map.

20:50 *jlbec* NAK on the generic block swap functions
20:50 *jlbec* I like the clean detect function.
20:50 *jlbec* But no code, other than debugfs, should be guessing the type of the block.
20:50 *jlbec* Other code should know where it was looking and what type it expects.

*SUNIL* OK

20:51 *jlbec* (btw, I hate that oracle can't hit my regular email; some of this discussion belongs on-list)
20:52 *jlbec* (I even added my gmail address to the list, and it doesn't get all your patches either ???)
20:54 *jlbec* Actually, these are all formatted out of git, right?
20:55 *jlbec* Can you just do git format-patch --stdout?  I can then download the mailbox and read it in my email client....
20:55 *jlbec* (maybe for the future)

*SUNIL* Sure

20:56 *jlbec* like the net_stats output
21:00 *jlbec*> i need generic block swap fto fix fsck (metaecc)
21:01 *jlbec* grpextents is a poor name, and a poor API.  I don't think it should walk the chain.
21:01 *jlbec* Nor should "group".  You just asked "dump this group descriptor"
21:01 *jlbec* If you want traverse_chains, you start from the inode.
21:02 *jlbec* (or perhaps have the '-t|-T' options like we have for 'stat', but I think "don't traverse" should be the default)
21:02 *jlbec* I'm confused.
21:03 *jlbec* (about fsck)
21:03 *jlbec* Maybe it is a later patch I'll get to.
21:03 *jlbec*> no i don;t have that patch in as yet
21:03 *jlbec*> was in my work queue... need to look into that again
21:04 *jlbec* One more comment on grpextents ('group-free-ranges'?  don't care to have it toooo long, but grpextents is needlessly abbreviated and hard to read)
21:04 *jlbec* The output should be single columned
21:04 *jlbec* Otherwise it's impossible to parse
21:04 *jlbec* (I mean, one (# start end) tuple per line)
21:04 *jlbec*> single columned gets too long
21:04 *jlbec* I also don't know how # helps, given that they are at arbitrary lengths.
21:04 *jlbec* We have a pager.
21:05 *jlbec*> i could add a param to control the output
21:05 *jlbec* No, I think we should get it right.
21:05 *jlbec* Drop #; it is useless.
21:06 *jlbec* Perhaps <start>-<end>, <start>-<end>, ...?
21:07 *jlbec* Your example in the patch would look like
21:07 *jlbec* 6145-7494,7495-7884,7885-8249,..
21:08 *jlbec* You just linewrap when str + len(nextrange) > COLUMNS

*SUNIL* I'll pull out grpextents for now

21:10 *jlbec* cluster.conf.manpage++
21:11 *jlbec*> the length is the most important info
21:11 *jlbec*> start,end is hard to read
21:12 *jlbec* Ok, <start>+<length>,<start>+<length>,
21:12 *jlbec* It should be splittable by trivial scripts.
21:12 *jlbec* Btw, all the rest are sobbed except the comments I made here.
===============================================================