[OpenWrt-Devel] [PATCH] kernel/build: Fix SetNfsCmdline to work with new .config.set

[Florian Fainelli]

Le 10/02/2016 03:43, Petr Štetiar a écrit :
> Florian Fainelli <florian at openwrt.org> [2016-02-09 19:57:22]:
> 
>> Le 09/02/2016 12:16, Petr Štetiar a écrit :
>>> Petr Štetiar <ynezz at true.cz> [2016-02-09 21:04:14]:
>>>
>>>>>    define Kernel/SetNfsCmdline
>>>>>  	rm -f $(LINUX_DIR)/.config.prev
>>>>> -	mv $(LINUX_DIR)/.config $(LINUX_DIR)/.config.old
>>>>> -	grep -v "CONFIG_CMDLINE=" $(LINUX_DIR)/.config.old > $(LINUX_DIR)/.config
>>>>> -	grep "CONFIG_CMDLINE=" $(LINUX_DIR)/.config.old | cut -d\" -f2 | sed 's/root=\/dev\/\([a-z0-9]*\)\(.*\)/CONFIG_CMDLINE=\"root=\/dev\/nfs ip=dhcp\2\"/' >> $(LINUX_DIR)/.config
>>>>> +	mv $(LINUX_DIR)/.config.set $(LINUX_DIR)/.config.old
>>>>> +	grep -v "CONFIG_CMDLINE=" $(LINUX_DIR)/.config.old > $(LINUX_DIR)/.config.set
>>>>> +	grep "CONFIG_CMDLINE=" $(LINUX_DIR)/.config.old | cut -d\" -f2 | sed 's/root=\/dev\/\([a-z0-9]*\)\(.*\)/CONFIG_CMDLINE=\"root=\/dev\/nfs ip=dhcp\2\"/' >> $(LINUX_DIR)/.config.set
>>>>>    endef
>>>>
>>>> This cut/sed kung-fu needs some improvements:
>>>>
>>>>  * it doesn't work correctly in case of empty cmdline, CONFIG_CMDLINE=""
>>>>  * how to handle custom cmdline options? For example I don't want to use this
>>>>    hardcoded cmdline options, but use instead options provided by the bootloader
>>>
>>> Hm, now I'm wondering why is this macro needed at all. In case I'll set
>>> CONFIG_KERNEL_ROOT_NFS=y, then I can set CONFIG_CMDLINE accordingly also,
>>> right?
>>>
>>> Florian, what's your use case for this SetNfsCmdline macro, that you need to
>>> hardcode kernel cmdline options? Thanks.
>>
>> Not all platforms get their command-line from the bootloader, or Device
>> Tree, or whatever, some do actually hardcode the command-line into the
>> kernel, that's what motivated this change in the first place.
> 
> I see, but I'm not sure how to handle it properly so it works for other use
> cases as well. Maybe adding something like CONFIG_CMDLINE_NFSROOT option, but
> this seems like overdesigning to me...
> 
> FYI, I'm using this base-files-nfs[1] package for development over NFS for a
> few years already, currently on i.MX6 with u-boot like this:
> 
>   setenv nfsroot /opt/devel/openwrt-master.git/bin/imx6/rootfs
>   setenv nfsargs root=/dev/nfs nfsroot=${serverip}:${nfsroot} ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}:apalis:eth0:off nfsrootdebug
> 
> I've noticed, that you've added something similar to the OpenWRT config
> directly:
> 
>    buildroot: add options to build the kernel for NFS boot
> 
> And I've to test it yet - just wondering if we don't need to use similar
> approach to skip network initialization in case we boot over NFS as it's
> currently done in base-files-nfs[1] package.

We do need to skip network initialization yes, your commit looks good to
me, though we would want the default base-files to be nfsroot aware, not
a specific package (seems like you do this because of local
customization, that seems fine).

Grepping for "nfsroot" is not exactly failsafe AFAIR, since you can just
have "root=/dev/nfs" and that would be enough to trump the script. Even
better is probably to look at /proc/mounts.

> 
>> I am fine dropping this, since obviously; if you turn on the option you
>> know what you are doing, can you re-submit the patch you think is
>> appropriate?
> 
> From my point of view this macro SetNfsCmdline is not needed as you can
> specify CONFIG_CMDLINE directly in your config. If you think also, that we
> don't need this SetNfsCmdline macro, then you can just revert it yourself,
> right? :-)

I just reverted this in r48689
--
Florian
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel