uboot-script-gen fails to process a zstd-compressed initramdisk, exiting
with:
Wrong file type initrd.img. It should be cpio archive, exiting.

Extend the existing approach to also check zstd.

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
 scripts/uboot-script-gen | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
index fc63702..db2c011 100755
--- a/scripts/uboot-script-gen
+++ b/scripts/uboot-script-gen
@@ -567,6 +567,7 @@ function check_compressed_file_type()
 {
     local filename=$1
     local type="$2"
+    local file_type
 
     if [ ! -f $filename ]
     then
@@ -574,13 +575,17 @@ function check_compressed_file_type()
         cleanup_and_return_err
     fi
 
-    file -L $filename | grep "gzip compressed data" &> /dev/null
-    if test $? == 0
-    then
+    file_type=$( file -L $filename )
+    if echo "$file_type" | grep -q "gzip compressed data" ; then
         local tmp=`mktemp`
         tmp_files+=($tmp)
         cat $filename | gunzip > $tmp
         filename=$tmp
+    elif echo "$file_type" | grep -q "Zstandard compressed data" ; then
+        local tmp=`mktemp`
+        tmp_files+=($tmp)
+        zstdcat $filename > $tmp
+        filename=$tmp
     fi
     check_file_type $filename "$type"
 }
-- 
2.34.1

Hi Jason

On 17/12/2024 21:19, Jason Andryuk wrote:
> uboot-script-gen fails to process a zstd-compressed initramdisk, exiting
> with:
> Wrong file type initrd.img. It should be cpio archive, exiting.
>
> Extend the existing approach to also check zstd.
>
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> ---
>   scripts/uboot-script-gen | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> index fc63702..db2c011 100755
> --- a/scripts/uboot-script-gen
> +++ b/scripts/uboot-script-gen
> @@ -567,6 +567,7 @@ function check_compressed_file_type()
>   {
>       local filename=$1
>       local type="$2"
> +    local file_type
>   
>       if [ ! -f $filename ]
>       then
> @@ -574,13 +575,17 @@ function check_compressed_file_type()
>           cleanup_and_return_err
>       fi
>   
> -    file -L $filename | grep "gzip compressed data" &> /dev/null
> -    if test $? == 0
> -    then
> +    file_type=$( file -L $filename )
> +    if echo "$file_type" | grep -q "gzip compressed data" ; then
>           local tmp=`mktemp`
>           tmp_files+=($tmp)
>           cat $filename | gunzip > $tmp
>           filename=$tmp
> +    elif echo "$file_type" | grep -q "Zstandard compressed data" ; then
> +        local tmp=`mktemp`
> +        tmp_files+=($tmp)
> +        zstdcat $filename > $tmp

I think you need to list zstd in |prog_req
|

|See 
https://gitlab.com/xen-project/imagebuilder/-/blob/master/scripts/uboot-script-gen?ref_type=heads#L5|

|Also you need to include this as a part of the dockerfiles like|

|https://gitlab.com/xen-project/xen/-/blob/staging/automation/tests-artifacts/qemu-system-aarch64/6.0.0-arm64v8.dockerfile?ref_type=heads|

|https://gitlab.com/xen-project/xen/-/blob/staging/automation/tests-artifacts/alpine/3.18-arm64v8.dockerfile?ref_type=heads
|

> +        filename=$tmp
>       fi
>       check_file_type $filename "$type"
>   }
- Ayan

On Mon, 30 Dec 2024, Ayan Kumar Halder wrote:
> Hi Jason
> 
> On 17/12/2024 21:19, Jason Andryuk wrote:
> > uboot-script-gen fails to process a zstd-compressed initramdisk, exiting
> > with:
> > Wrong file type initrd.img. It should be cpio archive, exiting.
> > 
> > Extend the existing approach to also check zstd.
> > 
> > Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> > ---
> >   scripts/uboot-script-gen | 11 ++++++++---
> >   1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> > index fc63702..db2c011 100755
> > --- a/scripts/uboot-script-gen
> > +++ b/scripts/uboot-script-gen
> > @@ -567,6 +567,7 @@ function check_compressed_file_type()
> >   {
> >       local filename=$1
> >       local type="$2"
> > +    local file_type
> >         if [ ! -f $filename ]
> >       then
> > @@ -574,13 +575,17 @@ function check_compressed_file_type()
> >           cleanup_and_return_err
> >       fi
> >   -    file -L $filename | grep "gzip compressed data" &> /dev/null
> > -    if test $? == 0
> > -    then
> > +    file_type=$( file -L $filename )
> > +    if echo "$file_type" | grep -q "gzip compressed data" ; then
> >           local tmp=`mktemp`
> >           tmp_files+=($tmp)
> >           cat $filename | gunzip > $tmp
> >           filename=$tmp
> > +    elif echo "$file_type" | grep -q "Zstandard compressed data" ; then
> > +        local tmp=`mktemp`
> > +        tmp_files+=($tmp)
> > +        zstdcat $filename > $tmp
> 
> I think you need to list zstd in |prog_req
> |
> 
> |See
> https://gitlab.com/xen-project/imagebuilder/-/blob/master/scripts/uboot-script-gen?ref_type=heads#L5|
> 
> |Also you need to include this as a part of the dockerfiles like|
> 
> |https://gitlab.com/xen-project/xen/-/blob/staging/automation/tests-artifacts/qemu-system-aarch64/6.0.0-arm64v8.dockerfile?ref_type=heads|
> 
> |https://gitlab.com/xen-project/xen/-/blob/staging/automation/tests-artifacts/alpine/3.18-arm64v8.dockerfile?ref_type=heads

Ayan makes a good point. Given that zstdcat is only used if images are
provided in zstd format, and given that we have been using ImageBuilder
without zstd for years now, I am tempted to consider zstd optional,
rather than required. We could leave it out of prog_req. If we add it to
prog_req, ImageBuilder will refuse to start if zstd is not installed,
and, like Ayan wrote, we would have to add zstd to the Dockerfiles.

So in conclusion I think it is best to go with this patch as is:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>