On 06/29/17 at 10:24am, Bhupesh Sharma wrote:
Thanks for the review !
On Thu, Jun 29, 2017 at 7:46 AM, Dave Young <dyoung(a)redhat.com> wrote:
> Hi Bhupesh,
> On 06/07/17 at 02:20pm, Bhupesh Sharma wrote:
>> This patch introduces a new configuration file layout structure for the
>> kdump configuration files.
>> Currently we have two types of the kdump configuration files:
>> 1. One in /etc/kdump.conf (for which initrd rebuild is required), and
>> 2. Another in /etc/syconfig/kdump (for which initrd rebuild is not required).
>> In this patch, we create a single directory /etc/kdump/ and put both
>> the config files in there.
> How do we keep compatible with old /etc/kdump.conf if user has one in
Existing user config files (i.e. /etc/sysconfig/kdump and
/etc/kdump.conf) are saved as a backup files with the extension
.usrorig - this has been noted in the updated man page documentation
for the .conf file in this patchset.
Our code des the backup? If so it looks not good, we should not touch
it. Current behavior of rpm -U for kexec-tools is replacing original
kdump.conf with new one and move original one as kdump.conf.rpmnew
>> Currently we also don't have a mechanism to deliver new config options
>> in kdump.conf and kdump files, i.e.:
>> - During an upgrade we cannot upgrade /etc/kdump.conf or
>> /etc/sysconfig/kdump as the user might have modified it locally and
>> new options will never be visible or take affect.
>> So this patch proposes to ship default kdump configs in /lib/kdump
>> directory and these configuration files can be selectively overridden
>> by the user provided config files in /etc/kdump. So we now have config
>> files available in the following locations:
>> /lib/kdump/ and /etc/kdump/
>> This allows us to upgrade the configuration files inside /lib/kdump
>> during a package upgrade and also ship new default options without
>> breaking existing user configuration.
>> Commands and comments available in /lib/kdump/kdump.conf and
>> /etc/kdump/kdump.conf (or /lib/kdump/kdump and /etc/kdump/kdump)
>> are now handled using the following logic:
>> 1. If a unknown/deprecated command was found in user .conf, ignore it.
>> 2. If a non-commented command was found both in user .conf
>> and distro .conf, preserve the value specified in user.conf
>> 3. If a new comment or command is found in distro .conf,
>> preserve it.
>> The modified configuration files are automatically generated during
>> an upgrade and later on the user is free to invoke the script
>> /lib/kdump/kdump-create-config.sh to recreate the modified configuration
>> files (the user is expected to be hands-on with the kdump configuration
>> file layout structure in such a case).
>> In this patch the man page documentation source for kdump.conf has
>> also been updated to reflect the new layout structure.
>> Signed-off-by: Bhupesh Sharma <bhsharma(a)redhat.com>
>> fadump-howto.txt | 8 +-
>> kdump-create-config.sh | 373 +++++++++++++++++++++++++++++++++++++++++++++
>> kdump-lib.sh | 2 +-
>> kdump.conf.5 | 43 +++++-
>> kdumpctl | 21 +--
>> kexec-kdump-howto.txt | 18 +--
>> kexec-tools.spec | 30 ++--
>> live-image-kdump-howto.txt | 2 +-
>> mkdumprd.8 | 4 +-
>> 9 files changed, 455 insertions(+), 46 deletions(-)
>> create mode 100755 kdump-create-config.sh
> I'm surprised we need so many lines changes for config files location
> changes. It is not acceptable to use so many code only for config files.
The helper script is executed only then the .spec file is executed,
i.e. the package is installed. We need to take care of all the
possible combinations to ensure that the resulting conf files are
valid and adhere to the new options specified by the distribution conf
file and keep the modified values specified in the user conf file.
If you have any specific comments about the helper script, I think I
can work on the same ( although we are already on a v3 :) ).
Pratyush reported a issue with applying this v3 on the latest
kexec-tools (which I suspect is caused by the patch pending review for
a long time and the underlying code base shifting fast), so I intend
to send a v4 which can cleanly apply on the latest kexec-tools, so I
can also take up the changes which we agree upon.
> Seems dracut use a=b style in config files so dracut can just source the
> config files in different order to overwrite different variables. But I
> do not know if we can do that and still keep compatible with old config
> file in /etc/kdump.conf
The updated dracut config file layout is quite different from
kexec-tools, so it is not a apple-to-apple comparison. Also the
requirement of keeping two config files (one which needs a initrd
rebuild and other which doesn't) is quite unique to kexec-tools.
I understand the kexec-tools config files are different from dracut, but
it is still odd to have so much code for the config file handling. If
there is no good code we'd better to keep current config file until we
have a better solution.