On Wed, 15 Nov 2023 at 11:40, Coiby Xu <coxu(a)redhat.com> wrote:
On Wed, Nov 15, 2023 at 10:54:36AM +0800, RuiRui Yang wrote:
>Hi Coiby,
>
>Thanks for the update! I still have some questions though, please see
>the inline comments.
Hi Dave,
Thanks for the quick response!
>
>On Tue, 14 Nov 2023 at 17:22, Coiby Xu <coxu(a)redhat.com> wrote:
>[...]
>>
>> +kdump_install_resolv_conf() {
>> + local _resolv_conf=/etc/resolv.conf
_nm_conf_dir=/etc/NetworkManager/conf.d
>> +
>> + # Some users may choose to manage /etc/resolve.conf manually [1]
>> + # by setting dns=none or use a symbolic link resolve.conf [2].
>> + # So resolve.conf should be installed to kdump initrd as well. To prevent
>> + # NM frome overwritting the user-configured resolve.conf in kdump initrd,
>> + # also set dns=none for NM.
>> + #
>> + # Note:
>> + # 1. When resolv.conf is managed by systemd-resolved.service, it could also
be a
>> + # symbolic link So exclude this case by teling if systemd-resolved is
enabled
>> + #
>> + # 2. It's harmless to bindly copy /etc/resolve.conf to the initrd
because
>s/bindly/blindly
Thanks for catching this typo!
>
>> + # by default in initramfs this file will be overwritten by
NetworkManager.
>> + #
>> + # [1]
https://bugzilla.gnome.org/show_bug.cgi?id=690404
>> + # [2]
https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/...
>> + systemctl -q is-enabled systemd-resolved && return 0
>> + inst "$(readlink -f "$_resolv_conf")"
"$_resolv_conf"
>> + if NetworkManager --print-config | grep -qs "^dns=none" || [[ -L
"$_resolv_conf" ]]; then
>
>What is the reason to check the [[ -L "$_resolv_conf" ]] here? it
>would be better to have the dns-none chunk to handle the dns-none case
>only and it will be easier to understand.
Because users could also self-manage /etc/resolve.conf via a symbolic
link. When resolve.conf is a symbolic file, NM won't touch resolve.conf
even without dns=none.
Hi Coiby, ok, thanks for the explanation.
It seems dracut "inst" will automatically install the original file
and the target symlink, if so as you said we do not need to add the
dns=none for this case and it will just work?
Not sure about 'inst "$(readlink -f "$_resolv_conf")"
"$_resolv_conf"', at least 'inst "$_resolv_conf"
"$_resolv_conf"'
works fine for me.
>
>> + echo "[main]\ndns=none" >
"$_nm_conf_dir"/90-dns-none.conf
>
>BTW, would this be better to add in dracut hostonly code?
dns=none is strongly coupled with the code before, so it doesn't seem to
ake sense to fragment the code? Btw, I'm not sure if dracut wants this
code because currently the only user is the kdump dracut module which is
still out of tree.
Hostonly exists before the kdump dracut module created, so there are
more users who will manually generate the initrd with host only mode
to do something although we can not have an example for this specific
case. I previously always tried to move code to dracut as much as
possible. But since you guys are maintaining the code now, I have no
strong opinion on this. Please just take what you think is better if
no any other objections.
--
Best regards,
Coiby