Hi Dave,
On 01/04/2016:04:55:39 PM, Dave Young wrote:
Hi, Pratyush
Thanks for the patches, see a few comments inline..
On 03/30/16 at 01:15pm, Pratyush Anand wrote:
> kdump also passes persistent device mapping as --mount or --device argument
> of dracut. However this persistent id (UUID) is changed if dump target is
> re-formated.
> kdumpctl must have a mechanism to recognise this modification, so that its
> service restart is able to rebuild initramfs.
>
> Testing:
> * When raw target is raid device, dracut argument is passed as --device
> '/dev/mapper/xxxxx', and in this case it does not force rebuild after
> reformatting. vmcore save was also fine.
> * When raw target is an ide device, dracut argument is passed as --device
> '/dev/disk/by-uuid/xxxxx', and in this case it does force rebuild after
> reformatting.
> * Similar test was also performed as ext4, xfs and btrfs file system target.
>
> One of the test case's detailed illustration:
> a) Attach an IDE device, lets say it is /dev/sdb1
> b) Format it as ext4
> # mkfs.ext4 /dev/sdb1
> # blkid /dev/sdb1
> /dev/sdb1: UUID="21c7baff-e35d-49c7-aa08-0fba4513f5bf"
TYPE="ext4"
> c) Mount it into /mnt and create a var/crash directory in it.
> # mount /dev/sdb1 /mnt;mkdir /mnt/var;mkdir /mnt/var/crash
> d) Add following line in /etc/kdump.conf
> ext4 /dev/sdb1
> e) Restart kdumpctl
> # kdumpctl restart
> f) crash
> # echo c > /proc/sysrq-trigger
> g) Here you will be able save vmcore with or without this patch.
> h) repeat step (b), (c), (e) and (f)
> i) Now you will be able to save vmcore only when you have this patch in
> your kexec-tools. Your initramfs will be rebuilt when you repeat step (e)
> after reformatting.
>
> Signed-off-by: Pratyush Anand <panand(a)redhat.com>
> ---
> kdumpctl | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/kdumpctl b/kdumpctl
> index 614a816e344c..060eed1233c5 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -327,10 +327,40 @@ setup_target_initrd()
> fi
> }
>
> +is_dump_target_modified()
> +{
> + target=$(egrep "^ext[234]|^xfs|^btrfs|^raw" /etc/kdump.conf)
All the vars should be local in the function
I thought, by default they are local. OK.Will improve in V2.
btw use the function below should be better:
_target=$(local_fs_dump_target)
raw is not necessary because we do not care the filesystem on it.
I had thought of using it, but did not use as
(1) We also need to take care of "raw". I agree it is filesystem independent,
but
UUID of a /dev/sdxn device changes after any type of formatting. So, even if
/etc/kdump.conf has raw entry, it will not work if disk is re-formatted.
(2) for minix, I am not sure if we need to handle it. It does not have UUID,
rather it has PARTUUID field which does not change after reformatting.
[root@localhost panand]# mkfs.minix /dev/sdb1
21856 inodes
65535 blocks
Firstdatazone=696 (696)
Zonesize=1024
Maxsize=268966912
[root@localhost panand]# blkid /dev/sdb1
/dev/sdb1: TYPE="minix" PARTUUID="3c66e385-01"
[root@localhost panand]# mkfs.minix /dev/sdb1
21856 inodes
65535 blocks
Firstdatazone=696 (696)
Zonesize=1024
Maxsize=268966912
[root@localhost panand]# blkid /dev/sdb1
/dev/sdb1: TYPE="minix" PARTUUID="3c66e385-01"
> +
> + #if dump target does not exist then do not rebuild
> + [ -n "$target" ] || return 0
> +
> + dracut_args=$(lsinitrd $TARGET_INITRD | grep "Arguments:")
grep "^Arguments:"|head -1 should be better
OK.
Is the "Arguments:" in lsinitrd output reliable? If Dracut changes the output
it will fail. Maybe we can add a sanity check about dracut_args, if it is nul
or something invalid (ie. without hostonly)then return 0 and give out a warning
OK.
> + #if --mount or --device is not by UUID, then also do not rebuild
> + echo $dracut_args | grep "by-uuid" &> /dev/null
> + [ $? -eq 0 ] || return 0
> +
> + target=$(echo $target | cut -d ' ' -f 2)
it will be not necessary using $(local_fs_dump_target) in previous code..
Yes, but $(local_fs_dump_target) might not be usable.
> + uuid=$(blkid $target | awk -F"UUID=" '{print $2}' | cut -d
'"' -f 2)
> + #if target does not have uuid, then also do not rebuild
> + [ -n $uuid ] || return 0
If it does not have uuid, there must be something wrong, should warning and return 1?
Initially I had thought of using is_dump_target_configured(), so had put this
line for "minix" and "nfs", but I think now this line is not needed,
or may be
what you say.
> +
> + echo $dracut_args | grep $uuid &> /dev/null
> + #if dracut argument and target have same uuid, then also do not rebuild
> + [ $? -ne 0 ] || return 0
> +
> + return 1
> +}
> +
> is_system_modified()
> {
> [[ -f $TARGET_INITRD ]] || return 1
>
> + is_dump_target_modified
> + if [ $? -ne 0 ]; then
> + echo "Detected change in dump target"
> + return 1
> + fi
> +
> return 0
> }
>
> --
> 2.5.0
> _______________________________________________
> kexec mailing list
> kexec(a)lists.fedoraproject.org
>
http://lists.fedoraproject.org/admin/lists/kexec@lists.fedoraproject.org
~Pratyush