On 5/11/22 05:44, Coiby Xu wrote:
> On Tue, May 10, 2022 at 02:08:58PM -0400, Dusty Mabe wrote:
>
>
>> On 5/10/22 04:31, Coiby Xu
wrote:
>>> Hi Dusty,
>>
>>> On Fri, May 06, 2022 at
10:08:56AM +0800, RuiRui Yang wrote:
>>>> Hi Dusty,
>>>
>>>> On Tue, 3 May 2022
at 22:43, Dusty Mabe <dusty(a)dustymabe.com> wrote:
>>>>
>>>>> Hi Team,
>>>>
>>>>> Editing
/etc/sysconfig/kdump via something like `sed` is error prone and also
>>>>> means I'll no longer get RPM updates to that file in the future.
It would be
>>>>> nice if we could have some sort of dropin configuration directly to
override
>>>>> default behavior (think systemd dropins) like a /etc/kdump.d/
directory.
>>>>
>>>>> Has there been
any considering for such a feature? Does it sound like a good
>>>>> or bad idea?
>>>
>>>> I would say if we
have no strong reason to do that, for the "irqpoll"
>>>> example we can just post a patch to change the default instead of
>>>> depending on users to do extra overriding. But anyway this is up to
>>>> the developers to decide, let's see how others think about it :)
>>
>>> I don't how much
work we need to support config dropins. But if you have
>>> the only use case of adding `ignition.firstboot` to the
>>> `KDUMP_COMMANDLINE_REMOVE=`, I would suggest simply changing the default
>>> /etc/sysconfig/kdump.
>
>> Yeah that's just one example. I'm sure there
are others. For example for
>>
https://bugzilla.redhat.com/show_bug.cgi?id=2080468 downstreams could put
>> in a config dropin to workaround that issue for now, rather than having to
>> use sed.
> Thanks for providing another example!
>
>> The config dropin model is
very powerful (think systemd) and I think there
>> are some libraries that make it easy to use them. That being said, it is some
>> investment so it doesn't come for free.
> One question comes to my mind today is could the config
dropin model
> could deal with a key with a list of values regarding get RPM updates to
> the /etc/sysconfig/kdump? Take KDUMP_COMMANDLINE_REMOVE for example, its
> current value is "hugepages hugepagesz slub_debug quiet log_buf_len
> swiotlb cma hugetlb_cma". If a user wants to append "irqpoll" to
> KDUMP_COMMANDLINE_REMOVE, does he/she has to provide a config dropin
> file which contains full "KDUMP_COMMANDLINE_REMOVE="hugepages slub_debug
> ...irqpoll" or he/she can use a syntax like
> "KDUMP_COMMANDLINE_REMOVE=irqpoll+" to inherit the default values?
> Because for the fist case, config dropin model doesn't bring the benefit
> of getting RPM updates automatically.
Thanks for responding and thinking about this critically. I think the answer
is that it depends on how we set things up. We definitely want to able to append
or remove items with an override and we also want to be able to keep the original
file(configuration) untouched so we don't lose RPM updates as you mentioned.
Some options:
- use a differing config file format to support this better
- come up with another syntax
- for example we can support adding to any list with bash += like
KDUMP_COMMANDLINE_REMOVE+=" ignition.firstboot"
but we can't really remove things from that list.. If we support '-' on
the front to denote cancelling out a
previous entry in the line. KDUMP_COMMANDLINE_REMOVE+=" -hugepages" would
end up with
KDUMP_COMMANDLINE_REMOVE="hugepages hugepagesz slub_debug quiet log_buf_len
swiotlb cma hugetlb_cma -hugepages"
which would then be processed as "KDUMP_COMMANDLINE_REMOVE="hugepagesz
slub_debug quiet log_buf_len swiotlb cma hugetlb_cma"
Luckily we aren't the first ones to contemplate this problem. I'm sure there's
a library that
probably already handles this for us.
Good to know that! Note this library may also need to be take care
of some special cases of kernel parameters. For example, some cases
have been documented in one of kdumpctl's functions
_update_kernel_arg_in_grub_etc_default,
# Note this function doesn't address the following cases,
# 1. The kernel ignores everything on the command line after a '--'. So
# simply adding the new entry to the end will fail if the cmdline
# contains a --.
# 2. If the value for a parameter contains spaces it can be quoted using
# double quotes, for example param="value with spaces". This will
# break the [^[:space:]\"] regex for the value.
# 3. Dashes and underscores in the parameter name are equivalent. So
# some_parameter and some-parameter are identical.
# 4. Some parameters, e.g. efivar_ssdt, can be given multiple times.
# 5. Some kernel parameters, e.g. quiet, doesn't have value
#
# $1: the name of the kernel command line parameter
# $2: new value. If empty, given parameter would be removed
_update_kernel_arg_in_grub_etc_default()