[PATCH V4 0/4] kdumpctl: force rebuild when dump target is modified

[Pratyush Anand]

When dump target is re-formatted then its UUID changes. These patches add
support to recognize such changes and then force initramfs rebuild, if
dracut argument has an UUID based root location.

Changes since v3:
- "$ret" unquoted
- No message for "$system_modified" != "0"
- image_time is now global
- is_dump_target_modified has been renamed as is_fs_uuid_changed

Changes since v2:
-- now is_system_modified returns 2 in case of invalid modification, 1 in
case of valid modification and 0 in case of no modification.
-- file modification check has been moved to is_system_modified now
-- variables have been put under "" in if condition check

Changes since v1:
-- Local variable is defined in shell function  now
-- raw target does not take persistent names "by-uuid" now
-- dracut arguments grep is more robust
-- When no UUID then warn and return

Pratyush Anand (4):
  mkdumprd: do not lookup in by-uuid dirs for raw device's persistent
    name
  kdumpctl: force rebuild in case of dynamic system modification
  kdumpctl: Move file modification check logic in is_system_modified()
  kdumpctl: force rebuild in case of file system UUID changes

kdumpctl | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++-------------
 mkdumprd |  14 +++++--
 2 files changed, 111 insertions(+), 30 deletions(-)

[PATCH V4 1/4] mkdumprd: do not lookup in by-uuid dirs for raw device's persistent name

[Pratyush Anand]

raw devices are not mounted and also does not need to contain any
filesystem. So they may have UUIDs(when formatted) and may not have UUIDs
when raw. Therefore, do not look for persistent names by-uuid for raw
devices.

Signed-off-by: Pratyush Anand 
Suggested-by: Dave Young 
---
 mkdumprd | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/mkdumprd b/mkdumprd
index 6e3d9757f3d8..ad40d2872f98 100644
--- a/mkdumprd
+++ b/mkdumprd
@@ -30,14 +30,20 @@ perror() {
 }

get_persistent_dev() {
-    local i _tmp _dev
+    local i _tmp _dev _lookup_dirs

_dev=$(udevadm info --query=name --name="$1" 2>/dev/null)
     [ -z "$_dev" ] && {
         perror_exit "Kernel dev name of $1 is not found."
     }

-    for i in /dev/mapper/* /dev/disk/by-uuid/* /dev/disk/by-id/*; do
+    if [[ $2 = "raw" ]];then
+	_lookup_dirs="/dev/mapper/* /dev/disk/by-id/*"
+    else
+	_lookup_dirs="/dev/mapper/* /dev/disk/by-uuid/* /dev/disk/by-id/*"
+    fi
+
+    for i in $_lookup_dirs; do
         _tmp=$(udevadm info --query=name --name="$i" 2>/dev/null)
         if [ "$_tmp" = "$_dev" ]; then
             echo $i
@@ -138,7 +144,7 @@ to_mount() {
     _mntopts="$_target $_fstype $_options"
     #for non-nfs _dev converting to use udev persistent name
     if [ -b "$_source" ]; then
-        _pdev="$(get_persistent_dev $_source)"
+        _pdev="$(get_persistent_dev $_source $_fstype)"
         if [ $? -ne 0 ]; then
             return 1
         fi
@@ -565,7 +571,7 @@ do
         dd if=$config_val count=1 of=/dev/null > /dev/null 2>&1 || {
             perror_exit "Bad raw disk $config_val"
         }
-        _praw=$(get_persistent_dev $config_val)
+        _praw=$(get_persistent_dev $config_val "raw")
         if [ $? -ne 0 ]; then
             exit 1
         fi

[PATCH V4 2/4] kdumpctl: force rebuild in case of dynamic system modification

[Pratyush Anand]

There could be some dynamic system modification, which may affect kdump
kernel boot process. In such situation initramfs must be rebulit on the
basis of changes.
Since most of these checking methods will use information from
TARGET_INITRD, therefore check for its existence in common code.

Signed-off-by: Pratyush Anand 
---
 kdumpctl | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/kdumpctl b/kdumpctl
index ef18a2d4f6ce..41dcf6473e24 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -327,10 +327,21 @@ setup_target_initrd()
    fi
 }

+# returns 0 if system is not modified
+# returns 1 if system is modified
+# returns 2 if system modification is invalid
+is_system_modified()
+{
+	[[ -f $TARGET_INITRD ]] || return 1
+
+	return 0
+}
+
 check_rebuild()
 {
    local extra_modules modified_files=""
    local _force_rebuild force_rebuild="0"
+	local ret system_modified="0"
    local initramfs_has_fadump

check_boot_dir
@@ -388,6 +399,14 @@ check_rebuild()
    	fi
    done

+	is_system_modified
+	ret=$?
+	if [ $ret -eq 2 ]; then
+		return 1
+	elif [ $ret -eq 1 ];then
+		system_modified="1"
+	fi
+
    #check if target initrd has fadump support
    if [ "$DEFAULT_DUMP_MODE" = "fadump" ] && [ -f "$TARGET_INITRD" ]; then
    	initramfs_has_fadump=`lsinitrd -m $TARGET_INITRD | grep ^kdumpbase$ | wc -l`
@@ -399,6 +418,8 @@ check_rebuild()
    	echo "$TARGET_INITRD has no fadump support"
    elif [ "$force_rebuild" != "0" ]; then
    	echo -n "Force rebuild $TARGET_INITRD"; echo
+	elif [ "$system_modified" != "0" ]; then
+		:
    elif [ -n "$modified_files" ]; then
    	echo "Detected change(s) in the following file(s):"
    	echo -n "  "; echo "$modified_files" | sed 's/\s/\n  /g'

[PATCH V4 3/4] kdumpctl: Move file modification check logic in is_system_modified()

[Pratyush Anand]

Relevant kdump files are also part of system. Therefore, moving logic of
file modification checking in is_system_modified() function now.

Signed-off-by: Pratyush Anand 
---
 kdumpctl | 69 +++++++++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 42 insertions(+), 27 deletions(-)

diff --git a/kdumpctl b/kdumpctl
index 41dcf6473e24..1cba00b9dc0f 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -14,6 +14,7 @@ FADUMP_ENABLED_SYS_NODE="/sys/kernel/fadump_enabled"
 FADUMP_REGISTER_SYS_NODE="/sys/kernel/fadump_registered"
 #kdump shall be the default dump mode
 DEFAULT_DUMP_MODE="kdump"
+image_time=0

. /lib/kdump/kdump-lib.sh

@@ -275,7 +276,6 @@ check_config()
 # return list of modified file for fence_kdump modified in Pacemaker cluster
 get_pcs_cluster_modified_files()
 {
-	local image_time=$1
    local time_stamp
    local modified_files

@@ -327,19 +327,59 @@ setup_target_initrd()
    fi
 }

+is_files_modified()
+{
+	local modified_files=""
+
+	#also rebuild when Pacemaker cluster conf is changed and fence kdump is enabled.
+	modified_files=$(get_pcs_cluster_modified_files)
+
+	EXTRA_BINS=`grep ^kdump_post $KDUMP_CONFIG_FILE | cut -d\  -f2`
+	CHECK_FILES=`grep ^kdump_pre $KDUMP_CONFIG_FILE | cut -d\  -f2`
+	EXTRA_BINS="$EXTRA_BINS $CHECK_FILES"
+	CHECK_FILES=`grep ^extra_bins $KDUMP_CONFIG_FILE | cut -d\  -f2-`
+	EXTRA_BINS="$EXTRA_BINS $CHECK_FILES"
+	files="$KDUMP_CONFIG_FILE $kdump_kernel $EXTRA_BINS /etc/fstab"
+
+	check_exist "$files" && check_executable "$EXTRA_BINS"
+	[ $? -ne 0 ] && return 2
+
+	for file in $files; do
+		time_stamp=`stat -c "%Y" $file`
+		if [ "$time_stamp" -gt "$image_time" ]; then
+			modified_files="$modified_files $file"
+		fi
+	done
+	if [ -n "$modified_files" ]; then
+		echo "Detected change(s) in the following file(s):"
+		echo -n "  "; echo "$modified_files" | sed 's/\s/\n  /g'
+		return 1
+	fi
+
+	return 0
+}
+
 # returns 0 if system is not modified
 # returns 1 if system is modified
 # returns 2 if system modification is invalid
 is_system_modified()
 {
+	local ret
+
    [[ -f $TARGET_INITRD ]] || return 1

+	is_files_modified
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		return $ret
+	fi
+
    return 0
 }

check_rebuild()
 {
-	local extra_modules modified_files=""
+	local extra_modules
    local _force_rebuild force_rebuild="0"
    local ret system_modified="0"
    local initramfs_has_fadump
@@ -375,30 +415,8 @@ check_rebuild()
    #since last build of the image file
    if [ -f $TARGET_INITRD ]; then
    	image_time=`stat -c "%Y" $TARGET_INITRD 2>/dev/null`
-	else
-		image_time=0
    fi

-	#also rebuild when Pacemaker cluster conf is changed and fence kdump is enabled.
-	modified_files=$(get_pcs_cluster_modified_files $image_time)
-
-	EXTRA_BINS=`grep ^kdump_post $KDUMP_CONFIG_FILE | cut -d\  -f2`
-	CHECK_FILES=`grep ^kdump_pre $KDUMP_CONFIG_FILE | cut -d\  -f2`
-	EXTRA_BINS="$EXTRA_BINS $CHECK_FILES"
-	CHECK_FILES=`grep ^extra_bins $KDUMP_CONFIG_FILE | cut -d\  -f2-`
-	EXTRA_BINS="$EXTRA_BINS $CHECK_FILES"
-	files="$KDUMP_CONFIG_FILE $kdump_kernel $EXTRA_BINS /etc/fstab"
-
-	check_exist "$files" && check_executable "$EXTRA_BINS"
-	[ $? -ne 0 ] && return 1
-
-	for file in $files; do
-		time_stamp=`stat -c "%Y" $file`
-		if [ "$time_stamp" -gt "$image_time" ]; then
-			modified_files="$modified_files $file"
-		fi
-	done
-
    is_system_modified
    ret=$?
    if [ $ret -eq 2 ]; then
@@ -420,9 +438,6 @@ check_rebuild()
    	echo -n "Force rebuild $TARGET_INITRD"; echo
    elif [ "$system_modified" != "0" ]; then
    	:
-	elif [ -n "$modified_files" ]; then
-		echo "Detected change(s) in the following file(s):"
-		echo -n "  "; echo "$modified_files" | sed 's/\s/\n  /g'
    else
    	return 0
    fi

[PATCH V4 4/4] kdumpctl: force rebuild in case of file system UUID changes

[Pratyush Anand]

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:
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 
---
 kdumpctl | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/kdumpctl b/kdumpctl
index 1cba00b9dc0f..f7e3af8733f5 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -359,6 +359,39 @@ is_files_modified()
    return 0
 }

+is_fs_uuid_changed()
+{
+	local _target _dracut_args _uuid
+
+	_target=$(egrep "^ext[234]|^xfs|^btrfs" /etc/kdump.conf)
+
+	#if dump target does not exist then do not rebuild
+	[[ -n "$_target" ]] || return 0
+
+	_dracut_args=$(lsinitrd $TARGET_INITRD | grep "^Arguments:" | head -1)
+	if [[ -z "$_dracut_args" ]];then
+		echo "Warning: No dracut arguments found in initrd"
+		return 0
+	fi
+	#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)
+	_uuid=$(blkid $_target | awk -F"UUID=" '{print $2}' | cut -d '"' -f 2)
+	if [[ -z "$_uuid" ]];then
+		echo "Warning: UUID is not present"
+		return 1
+	fi
+
+	echo $_dracut_args  | grep $_uuid &> /dev/null
+	#if dracut argument and _target have same uuid, then also do not rebuild
+	[[ $? -ne 0 ]] || return 0
+
+	echo "Detected change in File System UUID"
+	return 1
+}
+
 # returns 0 if system is not modified
 # returns 1 if system is modified
 # returns 2 if system modification is invalid
@@ -374,6 +407,12 @@ is_system_modified()
    	return $ret
    fi

+	is_fs_uuid_changed
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		return $ret
+	fi
+
    return 0
 }

[PATCH V4 4/4] kdumpctl: force rebuild in case of file system UUID changes

[Baoquan He]

On 04/20/16 at 03:37pm, 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:
> 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
>
> kdumpctl | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/kdumpctl b/kdumpctl
> index 1cba00b9dc0f..f7e3af8733f5 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -359,6 +359,39 @@ is_files_modified()
>    return 0
>  }
>
> +is_fs_uuid_changed()
> +{
> local _target _dracut_args _uuid
> _target=$(egrep "^ext[234]|^xfs|^btrfs" /etc/kdump.conf)
> #if dump target does not exist then do not rebuild
> [[ -n "$_target" ]] || return 0

If a disk is mounted on /var/crash, and is changed from /dev/diskA to
/dev/diskB. This change is skipped. But checking this seems complicated
in kdumpctl. Maybe just leave it now.
> _dracut_args=$(lsinitrd $TARGET_INITRD | grep "^Arguments:" | head -1)
> if [[ -z "$_dracut_args" ]];then
> echo "Warning: No dracut arguments found in initrd"
> return 0
> fi
> #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)
> _uuid=$(blkid $_target | awk -F"UUID=" '{print $2}' | cut -d '"' -f 2)
> if [[ -z "$_uuid" ]];then
> echo "Warning: UUID is not present"
> return 1
> fi
> echo $_dracut_args  | grep $_uuid &> /dev/null
> #if dracut argument and _target have same uuid, then also do not rebuild
> [[ $? -ne 0 ]] || return 0
> echo "Detected change in File System UUID"
> return 1
>
> +}
>
> # returns 0 if system is not modified
>  # returns 1 if system is modified
>  # returns 2 if system modification is invalid
> @@ -374,6 +407,12 @@ is_system_modified()
>    	return $ret
>    fi
> is_fs_uuid_changed
> ret=$?
> if [ $ret -ne 0 ]; then
> return $ret
> fi
> return 0
>
> }
>
> -- 
> 2.5.0
> _______________________________________________
> kexec mailing list
> [e-mail]
> http://lists.fedoraproject.org/admin/lists/[e-mail]

[PATCH V4 4/4] kdumpctl: force rebuild in case of file system UUID changes

[Pratyush Anand]

On 21/04/2016:09:58:34 AM, Baoquan He wrote:
> If a disk is mounted on /var/crash, and is changed from /dev/diskA to
> /dev/diskB. This change is skipped. But checking this seems complicated
> in kdumpctl. Maybe just leave it now.

Thanks for bringing it.

I think this is a good point. Will send a follow-up patch for review which will
apply on top of this series.

~Pratyush

[PATCH V4 4/4] kdumpctl: force rebuild in case of file system UUID changes

[Xunlei Pang]

On 2016/04/20 at 18:07, 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:
> 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
>
> kdumpctl | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/kdumpctl b/kdumpctl
> index 1cba00b9dc0f..f7e3af8733f5 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -359,6 +359,39 @@ is_files_modified()
>    return 0
>  }
>
> +is_fs_uuid_changed()
> +{
> local _target _dracut_args _uuid
> _target=$(egrep "^ext[234]|^xfs|^btrfs" /etc/kdump.conf)
> #if dump target does not exist then do not rebuild
> [[ -n "$_target" ]] || return 0
> _dracut_args=$(lsinitrd $TARGET_INITRD | grep "^Arguments:" | head -1)
> if [[ -z "$_dracut_args" ]];then
> echo "Warning: No dracut arguments found in initrd"
> return 0
> fi
> #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

May be "[[ $? -ne 0 ]] && return 0" is more readable, but that's ok :-)

Additionally, for device mappers, if they got reformatted, because the persistent name
is from "/dev/mapper" not from "/dev/disk/by-uuid", will "return 0" here, not rebuilt.
Should we handle this?

Regards,
Xunlei
> _target=$(echo $_target | cut -d ' ' -f 2)
> _uuid=$(blkid $_target | awk -F"UUID=" '{print $2}' | cut -d '"' -f 2)
> if [[ -z "$_uuid" ]];then
> echo "Warning: UUID is not present"
> return 1
> fi
> echo $_dracut_args  | grep $_uuid &> /dev/null
> #if dracut argument and _target have same uuid, then also do not rebuild
> [[ $? -ne 0 ]] || return 0

Ditto, "[[ $? -eq 0 ]] && return 0" is more readable, but that's ok :-)
> echo "Detected change in File System UUID"
> return 1
>
> +}
>
> # returns 0 if system is not modified
>  # returns 1 if system is modified
>  # returns 2 if system modification is invalid
> @@ -374,6 +407,12 @@ is_system_modified()
>    	return $ret
>    fi
> is_fs_uuid_changed
> ret=$?
> if [ $ret -ne 0 ]; then
> return $ret
> fi
> return 0
>
> }

[PATCH V4 4/4] kdumpctl: force rebuild in case of file system UUID changes

[Pratyush Anand]

Hi Xunlei,

Thanks for the review.

On Wed, Apr 27, 2016 at 2:28 PM, Xunlei Pang  wrote:
> On 2016/04/20 at 18:07, 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:
> > 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
> >
> > kdumpctl | 39 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> >
> > diff --git a/kdumpctl b/kdumpctl
> > index 1cba00b9dc0f..f7e3af8733f5 100755
> > --- a/kdumpctl
> > +++ b/kdumpctl
> > @@ -359,6 +359,39 @@ is_files_modified()
> >       return 0
> >  }
> >
> > +is_fs_uuid_changed()
> > +{
> > local _target _dracut_args _uuid
> > _target=$(egrep "^ext[234]|^xfs|^btrfs" /etc/kdump.conf)
> > #if dump target does not exist then do not rebuild
> > [[ -n "$_target" ]] || return 0
> > _dracut_args=$(lsinitrd $TARGET_INITRD | grep "^Arguments:" | head -1)
> > if [[ -z "$_dracut_args" ]];then
> >         echo "Warning: No dracut arguments found in initrd"
> >         return 0
> > fi
> > #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
>
> May be "[[ $? -ne 0 ]] && return 0" is more readable, but that's ok :-)

OK..Will use above format from next time.
> Additionally, for device mappers, if they got reformatted, because the persistent name
> is from "/dev/mapper" not from "/dev/disk/by-uuid", will "return 0" here, not rebuilt.
> Should we handle this?

we do not need to rebuild in that case. Persistent name would be valid
and so dracut will create valid root mount point.

~Pratyush

[PATCH V4 4/4] kdumpctl: force rebuild in case of file system UUID changes

[Xunlei Pang]

On 2016/04/27 at 17:23, Pratyush Anand wrote:
> Hi Xunlei,
>
> Thanks for the review.
>
> On Wed, Apr 27, 2016 at 2:28 PM, Xunlei Pang  wrote:
> > On 2016/04/20 at 18:07, 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:
> > > 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
> > >
> > > kdumpctl | 39 +++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 39 insertions(+)
> > >
> > > diff --git a/kdumpctl b/kdumpctl
> > > index 1cba00b9dc0f..f7e3af8733f5 100755
> > > --- a/kdumpctl
> > > +++ b/kdumpctl
> > > @@ -359,6 +359,39 @@ is_files_modified()
> > >       return 0
> > >  }
> > >
> > > +is_fs_uuid_changed()
> > > +{
> > > local _target _dracut_args _uuid
> > > _target=$(egrep "^ext[234]|^xfs|^btrfs" /etc/kdump.conf)
> > > #if dump target does not exist then do not rebuild
> > > [[ -n "$_target" ]] || return 0
> > > _dracut_args=$(lsinitrd $TARGET_INITRD | grep "^Arguments:" | head -1)
> > > if [[ -z "$_dracut_args" ]];then
> > >         echo "Warning: No dracut arguments found in initrd"
> > >         return 0
> > > fi
> > > #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
> >
> > May be "[[ $? -ne 0 ]] && return 0" is more readable, but that's ok :-)
>
> OK..Will use above format from next time.
> > Additionally, for device mappers, if they got reformatted, because the persistent name
> > is from "/dev/mapper" not from "/dev/disk/by-uuid", will "return 0" here, not rebuilt.
> > Should we handle this?
>
> we do not need to rebuild in that case. Persistent name would be valid
> and so dracut will create valid root mount point.

Yes, but if a device mapper device is reformatted to be a different filesystem,
its uuid is changed(after remount, its fstype changes), then user restart kdump,
shouldn't we trigger rebuilding to update its mount fstype passed to dracut?

Regards,
Xunlei
> ~Pratyush

[PATCH V4 4/4] kdumpctl: force rebuild in case of file system UUID changes

[Pratyush Anand]

On Wed, Apr 27, 2016 at 4:56 PM, Xunlei Pang  wrote:
> On 2016/04/27 at 17:23, Pratyush Anand wrote:
> > > Additionally, for device mappers, if they got reformatted, because the persistent name
> > > is from "/dev/mapper" not from "/dev/disk/by-uuid", will "return 0" here, not rebuilt.
> > > Should we handle this?
> >
> > we do not need to rebuild in that case. Persistent name would be valid
> > and so dracut will create valid root mount point.
>
> Yes, but if a device mapper device is reformatted to be a different filesystem,
> its uuid is changed(after remount, its fstype changes), then user restart kdump,
> shouldn't we trigger rebuilding to update its mount fstype passed to dracut?

Yes, I agree if above could be a valid scenario then we need to
rebuild in that case.  I am not sure how to generate above scenario.
If you can help with a scenario for generating above issue,  I can
cook a patch to resolve. Moreever, should we also not required to
change the filesystem type in kdump.conf?

This is what my understanding is:

-- When we pass filesystem type and target device (like ext4
/dev/sdb1) in kdump.conf, then kexec-tools passes --mount  as
'/dev/disk/by-uuid/...'. So, whenever we will reformat even with
different filesystem, UUID will change and there will be a rebuild.
-- If we do not pass filesystem type, but "dump path" (like
/var/crash)  mounts a device other than the device which mounts root,
then also kexec-tools passes --mount  as '/dev/disk/by-uuid/...'. So,
in that case as well it will rebuild after reformatting.
-- kexec-tools does not pass --mount or --device  when both "dump
path" and root path mounts same device.

~Pratyush

[PATCH V4 4/4] kdumpctl: force rebuild in case of file system UUID changes

[Xunlei Pang]

On 2016/04/27 at 20:10, Pratyush Anand wrote:
> On Wed, Apr 27, 2016 at 4:56 PM, Xunlei Pang  wrote:
> > On 2016/04/27 at 17:23, Pratyush Anand wrote:
> > > > Additionally, for device mappers, if they got reformatted, because the persistent name
> > > > is from "/dev/mapper" not from "/dev/disk/by-uuid", will "return 0" here, not rebuilt.
> > > > Should we handle this?
> > >
> > > we do not need to rebuild in that case. Persistent name would be valid
> > > and so dracut will create valid root mount point.
> >
> > Yes, but if a device mapper device is reformatted to be a different filesystem,
> > its uuid is changed(after remount, its fstype changes), then user restart kdump,
> > shouldn't we trigger rebuilding to update its mount fstype passed to dracut?
>
> Yes, I agree if above could be a valid scenario then we need to
> rebuild in that case.  I am not sure how to generate above scenario.
> If you can help with a scenario for generating above issue,  I can
> cook a patch to resolve. Moreever, should we also not required to
> change the filesystem type in kdump.conf?
>
> This is what my understanding is:
>
> -- When we pass filesystem type and target device (like ext4
> /dev/sdb1) in kdump.conf, then kexec-tools passes --mount  as
> '/dev/disk/by-uuid/...'. So, whenever we will reformat even with
> different filesystem, UUID will change and there will be a rebuild.
> -- If we do not pass filesystem type, but "dump path" (like
> /var/crash)  mounts a device other than the device which mounts root,
> then also kexec-tools passes --mount  as '/dev/disk/by-uuid/...'. So,
> in that case as well it will rebuild after reformatting.
> -- kexec-tools does not pass --mount or --device  when both "dump
> path" and root path mounts same device.

For example, I just tested on my laptop:
cat /etc/kdump.conf
#raw /dev/vg/lv_kdump
#ext4 /dev/vg/lv_kdump
#xfs /dev/mapper/rhel-root
#ext4 UUID=03138356-5e61-4ab3-b58e-27507ac41937
#nfs 10.66.129.115:/export/nfs
#ssh [e-mail]
#sshkey /root/.ssh/kdump_id_rsa
path /var/crash

Initailly it was mounted as:
/dev/mapper/rhel-swap on /var/crash type ext2 (rw,relatime,seclabel)

Then I unmounted /var/crash and changed /dev/mapper/rhel-swap to a ext4 using mkfs.ext4:
/dev/mapper/rhel-swap on /var/crash type ext4 (rw,relatime,seclabel,data=ordered)

After that, I did kdumpctl restart, it didn't trigger the rebuilding.

Actually, though "/dev/mapper/rhel-swap" device mapper name was not changed,
but its(dm-1's) uuid (and filesystem)  has been changed.

Regards,
Xunlei
> ~Pratyush

[PATCH V4 4/4] kdumpctl: force rebuild in case of file system UUID changes

[Pratyush Anand]

On Wed, Apr 27, 2016 at 6:10 PM, Xunlei Pang  wrote:
> On 2016/04/27 at 20:10, Pratyush Anand wrote:
> > On Wed, Apr 27, 2016 at 4:56 PM, Xunlei Pang  wrote:
> > > On 2016/04/27 at 17:23, Pratyush Anand wrote:
> > > > > Additionally, for device mappers, if they got reformatted, because the persistent name
> > > > > is from "/dev/mapper" not from "/dev/disk/by-uuid", will "return 0" here, not rebuilt.
> > > > > Should we handle this?
> > > >
> > > > we do not need to rebuild in that case. Persistent name would be valid
> > > > and so dracut will create valid root mount point.
> > >
> > > Yes, but if a device mapper device is reformatted to be a different filesystem,
> > > its uuid is changed(after remount, its fstype changes), then user restart kdump,
> > > shouldn't we trigger rebuilding to update its mount fstype passed to dracut?
> >
> > Yes, I agree if above could be a valid scenario then we need to
> > rebuild in that case.  I am not sure how to generate above scenario.
> > If you can help with a scenario for generating above issue,  I can
> > cook a patch to resolve. Moreever, should we also not required to
> > change the filesystem type in kdump.conf?
> >
> > This is what my understanding is:
> >
> > -- When we pass filesystem type and target device (like ext4
> > /dev/sdb1) in kdump.conf, then kexec-tools passes --mount  as
> > '/dev/disk/by-uuid/...'. So, whenever we will reformat even with
> > different filesystem, UUID will change and there will be a rebuild.
> > -- If we do not pass filesystem type, but "dump path" (like
> > /var/crash)  mounts a device other than the device which mounts root,
> > then also kexec-tools passes --mount  as '/dev/disk/by-uuid/...'. So,
> > in that case as well it will rebuild after reformatting.
> > -- kexec-tools does not pass --mount or --device  when both "dump
> > path" and root path mounts same device.
>
> For example, I just tested on my laptop:
> cat /etc/kdump.conf
> #raw /dev/vg/lv_kdump
> #ext4 /dev/vg/lv_kdump
> #xfs /dev/mapper/rhel-root
> #ext4 UUID=03138356-5e61-4ab3-b58e-27507ac41937
> #nfs 10.66.129.115:/export/nfs
> #ssh [e-mail]
> #sshkey /root/.ssh/kdump_id_rsa
> path /var/crash
>
> Initailly it was mounted as:
> /dev/mapper/rhel-swap on /var/crash type ext2 (rw,relatime,seclabel)
>
> Then I unmounted /var/crash and changed /dev/mapper/rhel-swap to a ext4 using mkfs.ext4:
> /dev/mapper/rhel-swap on /var/crash type ext4 (rw,relatime,seclabel,data=ordered)
>
> After that, I did kdumpctl restart, it didn't trigger the rebuilding.
>
> Actually, though "/dev/mapper/rhel-swap" device mapper name was not changed,
> but its(dm-1's) uuid (and filesystem)  has been changed.

Got It :-)

Thanks for the creating a scenario. Will send a follow-up patch to resolve it.

~Pratyush

[PATCH V4 4/4] kdumpctl: force rebuild in case of file system UUID changes

[Pratyush Anand]

On Wed, Apr 27, 2016 at 8:35 PM, Pratyush Anand  wrote:
> On Wed, Apr 27, 2016 at 6:10 PM, Xunlei Pang  wrote:
> > For example, I just tested on my laptop:
> > cat /etc/kdump.conf
> > #raw /dev/vg/lv_kdump
> > #ext4 /dev/vg/lv_kdump
> > #xfs /dev/mapper/rhel-root
> > #ext4 UUID=03138356-5e61-4ab3-b58e-27507ac41937
> > #nfs 10.66.129.115:/export/nfs
> > #ssh [e-mail]
> > #sshkey /root/.ssh/kdump_id_rsa
> > path /var/crash
> >
> > Initailly it was mounted as:
> > /dev/mapper/rhel-swap on /var/crash type ext2 (rw,relatime,seclabel)
> >
> > Then I unmounted /var/crash and changed /dev/mapper/rhel-swap to a ext4 using mkfs.ext4:
> > /dev/mapper/rhel-swap on /var/crash type ext4 (rw,relatime,seclabel,data=ordered)
> >
> > After that, I did kdumpctl restart, it didn't trigger the rebuilding.
> >
> > Actually, though "/dev/mapper/rhel-swap" device mapper name was not changed,
> > but its(dm-1's) uuid (and filesystem)  has been changed.
>
> Got It :-)
>
> Thanks for the creating a scenario. Will send a follow-up patch to resolve it.

In stead of sending as follow-up patch, I merged the changes in V5.

~Pratyush

[Dave Young]

Hi, Pratyush

On 04/20/16 at 03:37pm, Pratyush Anand wrote:
> When dump target is re-formatted then its UUID changes. These patches add
> support to recognize such changes and then force initramfs rebuild, if
> dracut argument has an UUID based root location.
>
> Changes since v3:
> "$ret" unquoted
> No message for "$system_modified" != "0"
> image_time is now global
> is_dump_target_modified has been renamed as is_fs_uuid_changed

Another nitpick in patch 4/4:
+        is_fs_uuid_changed
+        ret=$?
+        if [ $ret -ne 0 ]; then
+                return $ret
+        fi
+

Since only checking -ne 0 so no need to use $ret, directly use $? is ok.

If no other comments I can merge it one week later with a manually modification.

For the whole series:

Reviewed-by: Dave Young

Thanks
Dave

[Dave Young]

On 04/21/16 at 09:18am, Dave Young wrote:
> Hi, Pratyush
>
> On 04/20/16 at 03:37pm, Pratyush Anand wrote:
> > When dump target is re-formatted then its UUID changes. These patches add
> > support to recognize such changes and then force initramfs rebuild, if
> > dracut argument has an UUID based root location.
> >
> > Changes since v3:
> > "$ret" unquoted
> > No message for "$system_modified" != "0"
> > image_time is now global
> > is_dump_target_modified has been renamed as is_fs_uuid_changed
>
> Another nitpick in patch 4/4:
>    is_fs_uuid_changed
>    ret=$?
>    if [ $ret -ne 0 ]; then
>            return $ret
>    fi
>
> Since only checking -ne 0 so no need to use $ret, directly use $? is ok.

Oops, $ret is needed because you return the value, so please ignore above comment..
> If no other comments I can merge it one week later with a manually modification.
>
> For the whole series:
>
> Reviewed-by: Dave Young
>
> Thanks
> Dave
> _______________________________________________
> kexec mailing list
> [e-mail]
> http://lists.fedoraproject.org/admin/lists/[e-mail]