From tateishi.katsuyuki ＠ oss.ntt.co.jp  Wed Jun 30 17:36:05 2010
From: tateishi.katsuyuki ＠ oss.ntt.co.jp (TATEISHI Katsuyuki)
Date: Wed, 30 Jun 2010 17:36:05 +0900 (JST)
Subject: [Ultramonkey-l7-develop 624] Re: [SCM] ultramonkey-l7-v2
 (ultramonkey-l7) branch, tproxy, created. v2.1.3-0-4-g11d8515
In-Reply-To: <4C29AC9D.80301@nttcom.co.jp>
References: <1277119636.943984.28856.nullmailer@users.sourceforge.jp>
	<20100629.153002.1018962831995038949.tateishi.katsuyuki@oss.ntt.co.jp>
	<4C29AC9D.80301@nttcom.co.jp>
Message-ID: <20100630.173605.1100045365675771277.tateishi.katsuyuki@oss.ntt.co.jp>

立石です。お疲れさまです。

Kohei TANUMA <tanuma.kouhei ＠ nttcom.co.jp>-san wrote:

>> 1. 新たに stdio.h をインクルードしているヘッダがいくつかあり
>>    ますが、コンパイル時にsnprintfまわりでエラーがでることに対
>>    する修正ですよね？ tproxy 関係でないのでブランチ（すくなく
>>    ともコミット）をわけたほうがいいと思います。(gccの新版対応
>>    でしょうか)
> 
> stdio.h は上記理由であってます。
> これ、いまから分ける際にスマートな git 操作はどんな感じに
> なるんでしょうか。
> 前に以下のような流れでやって大変だった記憶があるので…。
> 
>   git diff OLD_COMMIT
>   diff 修正
>   git checkout OLD_COMMIT
>   patch
>   git commit
>   patch
>   git commit
>   git push

今回の場合は現在 tproxy の HEAD をチェックアウトしているとして、

git br tproxy.old       (もとのブランチを保存)
git reset HEAD^         (HEADを一つ前のコミットに戻す)
git add include/l7vs.h include/l7vs_iomuxlist.h ... (snpintf関連の変更のみ持っているファイル)
git add -p include/l7vs_dest.h (snpintf関連以外の変更も持っているファイルを対話処理 *1)
git diff                (snprintf関連の変更が残っていない(ちゃんとaddされた)ことを確認)
git diff --cached       (snprintf関連の変更だけaddされていることを確認)
git commit -m "snprintf fix" (addした分をコミット)
git br fix-snprintf-err (今作ったコミットを先頭とするブランチを作っておく)
git add -u		(残りの変更をadd)
git commit -m "tproxy"  (コミット)
git diff tproxy.old     (もとのブランチと比較して差分がないことを確認)

が楽ですかね。

 *1 add -p ではhunkごとに対話処理で git add するかどうかを
  gitに教えることができます。詳細は git-add(1) 参照。

なお、今回は snprintf の修正に tproxy の機能追加が依存して
いるので、 snprintf のコミットに続けて tproxy のコミットを作
っていることに注意してください。

依存関係が存在せず、二つのブランチに完全に分けないといけない
場合(多くの場合はこっちだと思います)、

git br fix-snprintf-err

以降で

git stash                    (tproxy関連の変更をよけておく)
git co -b tproxy.new master  (共通祖先(ここではmasterがポイントしてると仮定)から新しく分岐)
git stash pop                (よけた変更をもとに戻す。コンフリクトする場合もあるかも)
git diff
git add -u
git ci -m "tproxy"
git merge snprintf-fix       (分離したブランチと一旦マージ)
git diff tproxy.old          (もとのブランチと比較して差分がないことを確認)
git reset --hard HEAD^       (比較のためのマージをなかったことに)

とかやると楽です。(stashしなくてもいける場合もあるかも)

いずれにしろポイントは
   1. reset して部分的な add で1回のコミットを調整しつつ履歴を合成
  (2. 必要なら最後にmergeして)
   3. diffで再構成前のブランチと比較して漏れがないことを確認
  (4. mergeしてたらresetでマージをなかったことにする)
というところです。

コミットの粒度には問題がなくて、特定のコミット群を別ブランチ
に分離したい場合は"reset & 部分的なadd"の代わりに
"format-patch & am"あるいは cherry-pick, rebase -i とかでコミッ
ト単位で履歴を合成していくと楽です。この場合も最後のチェック
のやり方は同じです。

> そもそもローカルで細かくコミットしておかないとダメですかね？

ひとつの意味を持った塊でコミットするのが良いと思います。
例えばあるコミットひとつにバグフィックスと機能追加が混じって
いるようであれば分離すべきです。

が、機能追加の作業中に修正したい内容を見つけてしまうというの
はよくあることだと思います。そのときは

git commit -a (仮でコミットしておく)
git co -b fixbranch master (バグフィクス用ブランチを作成)
vi ...
git commit -a (バグフィックスを終了)
git co @{-1}  (直前のブランチに戻る)
git reset HEAD^ (仮のコミットをなかったことに)

のように、すぐさま別ブランチで直しておけば精神衛生上いいかも
しれません。機能追加がバグフィクスに依存している場合とかは、
どうマージするのかとかいろいろ考えるところはありますが・・・


あと、gitk --all 重要です。
 * gitk --all でブランチやコミットの相互関係を眺めながら作
   業すると自分が何をやっているか把握しやすいです。
 * reset や branch -D でどのブランチHEADからも到達できなくなっ
   たコミットもリロード(Ctrl-F5)しない限り分かるので、無理や
   りもとのコミットにresetしたりbranchで名前をつけたりとかで
   リカバーでき、reset --hardとかブランチ再構成とかが怖くなく
   なります。

>> 7. src/l7vsadm_main.c の -m オプションって必要ですかね？(-t
>>    だけじゃダメ？) 必要な場合は・・・
> 
> これは、いまさら感があるのですが LVS にあわせるために -m も
> 用意しました。
> -m はモジュール指定のオプションで使われているので
> かなり微妙とか思いつつも、既にいろいろなオプション文字が
> 重複してるので、もういいかと実装に至りました。
> やっぱり、いらないですかね…？

私はいらないと思います。

それに L7 の masq と LVS の masq って意味が違いません？
むしろ L7 の tproxy のほうが LVS の masq に近い気がします。

他の方も何かご意見があれば聞いてみたいです。

>> 10. sorry サーバも tproxy を選択できるとうれしいです。(今は
>>     masq 決め打ちですよね？)
> 
> これはそのとおりなんですよね…。
> 指定の仕方が思いつかなくて諦めてしまいました。
> sorry サーバ用の tproxy 専用のオプションを用意するか、
> -b の sorry サーバ指定オプションを小細工するか、
> l7vsadm -A|E の場合の -t オプションは sorry サーバに
> 対するものとして処理するか、などどれもいまいちで…。

「sorry サーバ用の tproxy 専用のオプションを用意する」でいい
と思います。

そのとき -t が使えない、というのであれば、リアルサーバの
tproxy指定も含めてtproxy指定は -T にするとか。あるいは
tProxyということで -p にするとか。

> とにかくオプションについては、重複とか自前処理とか
> ちょっと厳しいと思います。

ちょっとコードを眺めました。
触りたくないと思いました。

> ipvsadm -Ln の変わりに l7vsadm -Ln って打つと
> エラーになるし…。（-L がそもそもないですが…）
> モジュールのオプションもヘルプ表示で出てこないから
> 何が指定できるかわからないし…。
> この辺のコマンドは iptables に似せて作ってるんだから
>   iptables -j REJECT -h
> みたいに
>   l7vsadm -m sessionless -h
> とかが出来たらいいんだけどなぁ…。

l7vsadm や l7directord は廃止して l7vsd は静的なコンフィグだ
けで動作するようにすればオプションの解釈とか気にしなくてよく
なりますw

真面目な話、 v2 でオプション解析をリファクタリングするのは危
険だと思います。
# v3...もなさそうですしv4とか？そのへんまで先だと
# l7vsadm/l7directord 廃止が現実的かも

--
TATEISHI Katsuyuki <tateishi.katsuyuki ＠ oss.ntt.co.jp>