[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [Grml-devel] Grml.org patch bomb


Christian Hofstaedtler wrote (11 Sep 2011 22:01:03 GMT) :
>>   16_nodhcp.patch
>>   27_support_static_ip.patch

> This patch is required to support standard ip= style IP configuration,
> as supported by the kernel and klibc. Unfortunately the code in
> initramfs-tools isn't flexible enough so we couldn't reuse it.

> Also the patch simplifies the code quite a bit.

Thanks for the explanation. Christian and other grml folks, there are
a few things that need to be improved before I'm happy to merge these
two patches in:

  1. The rationale for get_ipconfig_para, and the way this function
     works, should be minimally documented. Two lines of inline
     comments should be enough.

  2. The fact standard ip= parameters can now be used must be
     documented in the manpage.

  3. Your patch seems to remove support for ip=[frommedia]. If this is
     intended, please explain why, and update the manpage accordingly.
     If this is not intended, how about getting the deleted code back?

Other than that, I've reviewed these patches quite carefully and they
seem right to me. Not only do they add support for standard ip=
arguments (and allow using it for multiple devices, btw), but they
generally improve the robustness and maintainability of the
do_netsetup code. However, I'm far from having a shard global view of
the live-boot networking -related code, so I'd welcome other reviews
before we merge these patches.

  intrigeri <intrigeri@boum.org>
  | GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc
  | OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc
  | Did you exchange a walk on part in the war
  | for a lead role in the cage?

Reply to: