Re: [PATCH v2 2/2] fadump: preserve file modification time to help with hardlinking

[Hari Bathini]

On 04/11/22 4:21 pm, Philipp Rudo wrote:
> Hi Hari,
>
> just a small nit.

Thanks for the review, Philipp.
> On Mon, 31 Oct 2022 15:42:21 +0530
> Hari Bathini  wrote:
> > With commit fa9201b2 ("fadump: isolate fadump initramfs image within
> > the default one"), initramfs image gets to hold two images, one for
> > production kernel boot purpose and the other for capture kernel boot.
> > Most files are common among the two images. Retain file modification
> > time to replace duplicate files with hardlinks and save space. Also,
> > avoid unnecessarily compressing fadump image that is decompressed
> > immediately anyway.
> > Signed-off-by: Hari Bathini
> >
> > mkfadumprd | 10 ++++++----
> >   1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/mkfadumprd b/mkfadumprd
> > index 5587ccf..4a4b98f 100644
> > --- a/mkfadumprd
> > +++ b/mkfadumprd
> > @@ -40,14 +40,16 @@ touch "$MKFADUMPRD_TMPDIR/fadump.initramfs"
> >   ddebug "rebuild fadump initrd: $FADUMP_INITRD $DEFAULT_INITRD $KDUMP_KERNELVER"
> >   # Don't use squash for capture image or default image as it negatively impacts
> >   # compression ratio and increases the size of the initramfs image.
> > -if ! $MKDUMPRD "$FADUMP_INITRD" -i "$MKFADUMPRD_TMPDIR/fadump.initramfs" /etc/fadump.initramfs --omit squash; then
> > +# Don't compress the capture image as uncompressed image is needed immediately.
> > +# Also, early microcode would not be needed here.
> > +if ! $MKDUMPRD "$FADUMP_INITRD" -i "$MKFADUMPRD_TMPDIR/fadump.initramfs" /etc/fadump.initramfs --omit squash --no-compress --no-early-microcode; then
> >    perror_exit "mkfadumprd: failed to build image with dump capture support"
> >   fi
> >
> > -### Unpack the initramfs having dump capture capability
> > +### Unpack the initramfs having dump capture capability retaining previous file modification time.
> > +# This helps in saving space by hardlinking identical files.
> >   mkdir -p "$MKFADUMPRD_TMPDIR/fadumproot"
> > -if ! (pushd "$MKFADUMPRD_TMPDIR/fadumproot" > /dev/null && lsinitrd --unpack "$FADUMP_INITRD" &&
> > popd > /dev/null); then
> >
> > +if ! (cpio -id --preserve-modification-time --quiet -D "$MKFADUMPRD_TMPDIR/fadumproot" < "$FADUMP_INITRD"); then
>
> without changing the directories I don't think you need the subshell
> here.

True.

Coiby, can you take care of this while merging (assuming you are ok with
the changes). Do let me know if you are expecting a respin from me.

Thanks
Hari