[PATCH v3 0/3] support tests against Fedora 35 or newer Cloud Base Images

[Coiby Xu]

This patch set enables the integration tests (i.e. the test cases in
tests/) for Fedora 35, 36 and 37 cloud base images.

v3:
 - use "unmount -R" to umount both the root and boot partition [Kairui]
	- add a code comment for the change of root node [Kairui]
	- fix the error of using the first partition as boot partition (the
			2nd partition is the boot parition) for Fedora >= 35
	- specify backing format for "qemu-img create" as required by newer qemu-img
	- specify the Fedora version for fedpkg sources

v2:
  - check LAST_PARTITION/root/root instead of LAST_PARTITION/root [Kairui]
  - improve commit message
	
Coiby Xu (3):
  tests: correctly mount the root and also the boot partitions for
    Fedora 35, 36 and rawhide Cloud Base Image
  tests: specify the backing format for the backing file when using
    qemu-img create
  tests: specify the Fedora version when running fedpkg sources

 tests/Makefile                  |  2 +-
 tests/scripts/image-init-lib.sh | 31 +++++++++++++++++++++++++++----
 2 files changed, 28 insertions(+), 5 deletions(-)

-- 
2.35.3

[PATCH v3 1/3] tests: correctly mount the root and also the boot partitions for Fedora 35, 36 and rawhide Cloud Base Image

[Coiby Xu]

Fedora 33 and 34 Cloud Base Images have only partition with the
following directory structure,
.
├── bin -> usr/bin
├── boot
├── dev
├── etc
├── home
├── root

By comparison, Fedora 35, 36 and 37 Cloud Base Images have multiple
partitions. The root partition which is the last partition has the
following directory,
.
├── home
└── root
    ├── bin -> usr/bin
    ├── boot
    ├── dev
    ├── etc
    ├── home
    ├── root

and the 2nd partition is the boot partition.

This patch address the above changes by mounting {LAST_PARTITION}/root as
to TEMP_ROOT and mount FIRST_PARTITION to TEMP_ROOT/boot. So the test
image can be built successfully.

Signed-off-by: Coiby Xu <coxu(a)redhat.com&gt;
---
 tests/scripts/image-init-lib.sh | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/tests/scripts/image-init-lib.sh b/tests/scripts/image-init-lib.sh
index 0a1524b..5995004 100644
--- a/tests/scripts/image-init-lib.sh
+++ b/tests/scripts/image-init-lib.sh
@@ -23,7 +23,7 @@ is_mounted()
 clean_up()
 {
 	for _mnt in ${MNTS[@]}; do
-		is_mounted $_mnt && $SUDO umount -f $_mnt
+		is_mounted $_mnt && $SUDO umount -f -R $_mnt
 	done
 
 	for _dev in ${DEVS[@]}; do
@@ -81,6 +81,21 @@ get_mountable_dev() {
 	fi
 }
 
+# get the separate boot partition
+# return the 2nd partition as boot partition
+get_mount_boot() {
+	local dev=$1 parts
+
+	$SUDO partprobe $dev && sync
+	parts="$(ls -1 ${dev}p*)"
+	if [ -n "$parts" ]; then
+		if [ $(echo "$parts" | wc -l) -gt 1 ]; then
+			echo "$parts" | head -2 | tail -1
+		fi
+	fi
+}
+
+
 prepare_loop() {
 	[ -n "$(lsmod | grep "^loop")" ] && return
 
@@ -133,7 +148,7 @@ image_lock()
 # Mount a device, will umount it automatially when shell exits
 mount_image() {
 	local image=$1 fmt
-	local dev mnt mnt_dev
+	local dev mnt mnt_dev boot root
 
 	# Lock the image just in case user run this script in parrel
 	image_lock $image
@@ -166,12 +181,20 @@ mount_image() {
 
 	$SUDO mount $mnt_dev $mnt
 	[ $? -ne 0 ] && perror_exit "failed to mount device
'$mnt_dev'"
+	boot=$(get_mount_boot "$dev")
+	if [[ -n "$boot" ]]; then
+		root=$(get_image_mount_root $image)
+		$SUDO mount $boot $root/boot
+		[ $? -ne 0 ] && perror_exit "failed to mount the bootable partition for
device '$mnt_dev'"
+	fi
 }
 
 get_image_mount_root() {
 	local image=$1
 	local root=${MNTS[$image]}
 
+	# Starting from Fedora 36, the root node is /root/root of the last partition
+	[ -d "$root/root/root" ] && root=$root/root
 	echo $root
 
 	if [ -z "$root" ]; then
@@ -213,7 +236,7 @@ run_in_image() {
 
 inst_in_image() {
 	local image=$1 src=$2 dst=$3
-	local root=${MNTS[$image]}
+	local root=$(get_image_mount_root $1)
 
 	$SUDO cp $src $root/$dst
 }
-- 
2.35.3

[PATCH v3 1/3] tests: correctly mount the root and also the boot partitions for Fedora 35, 36 and rawhide Cloud Base Image

[Philipp Rudo]

Hi Coiby,

this series doesn't seem to be merged yet. So let me add a few
comments/questions ;-)

On Fri, 13 May 2022 14:08:08 +0800
Coiby Xu <coxu(a)redhat.com&gt; wrote:

>  Fedora 33 and 34 Cloud Base Images have only partition with the
					      ^
					     one (?)

>  following directory structure,
>  .
>  ├── bin -> usr/bin
>  ├── boot
>  ├── dev
>  ├── etc
>  ├── home
>  ├── root
>  
>  By comparison, Fedora 35, 36 and 37 Cloud Base Images have multiple
>  partitions. The root partition which is the last partition has the
>  following directory,
>  .
>  ├── home
>  └── root
>      ├── bin -> usr/bin
>      ├── boot
>      ├── dev
>      ├── etc
>      ├── home
>      ├── root 
Just for my understanding, in this schema /root/home is the home
directory for root and /home for everybody else. Is that correct?

>  and the 2nd partition is the boot partition.
>  
>  This patch address the above changes by mounting {LAST_PARTITION}/root as
>  to TEMP_ROOT and mount FIRST_PARTITION to TEMP_ROOT/boot. So the test
>  image can be built successfully. 
That's a little bit confusing to me. When the boot partition is the 2nd
partition why is the variable called FIRST_PARTITION?

>  
>  Signed-off-by: Coiby Xu <coxu(a)redhat.com&gt;
>  ---
>   tests/scripts/image-init-lib.sh | 29 ++++++++++++++++++++++++++---
>   1 file changed, 26 insertions(+), 3 deletions(-)
>  
>  diff --git a/tests/scripts/image-init-lib.sh b/tests/scripts/image-init-lib.sh
>  index 0a1524b..5995004 100644
>  --- a/tests/scripts/image-init-lib.sh
>  +++ b/tests/scripts/image-init-lib.sh
>  @@ -23,7 +23,7 @@ is_mounted()
>   clean_up()
>   {
>   	for _mnt in ${MNTS[@]}; do
>  -		is_mounted $_mnt && $SUDO umount -f $_mnt
>  +		is_mounted $_mnt && $SUDO umount -f -R $_mnt
>   	done
>   
>   	for _dev in ${DEVS[@]}; do
>  @@ -81,6 +81,21 @@ get_mountable_dev() {
>   	fi
>   }
>   
>  +# get the separate boot partition
>  +# return the 2nd partition as boot partition
>  +get_mount_boot() {
>  +	local dev=$1 parts
>  +
>  +	$SUDO partprobe $dev && sync
>  +	parts="$(ls -1 ${dev}p*)"
>  +	if [ -n "$parts" ]; then
>  +		if [ $(echo "$parts" | wc -l) -gt 1 ]; then
>  +			echo "$parts" | head -2 | tail -1
>  +		fi
>  +	fi 
Hmmm... difficult...

Can we really rely on ls providing the right order here? If so why not
simply do 'ls -1 ${dev}p2' ?

Would it make sense to switch to something more powerful like lsblk?
For example we could use 

	lsblk -p -r -n -o NAME,MOUNTPOINT ${dev} | grep $root/boot

or 
	lsblk -p -r -n -o NAME,PARTLABEL ${dev} | grep efi

(what for non-efi systems, e.g. s390?).

Finally, you can drop the two if-blocks by simply piping to sed. For
example your original version could be rewritten to

	ls -1 ${dev}p* | sed -n 2p

which prints the second line or "" if it doesn't exist.

Thanks
Philipp

>  +}
>  +
>  +
>   prepare_loop() {
>   	[ -n "$(lsmod | grep "^loop")" ] && return
>   
>  @@ -133,7 +148,7 @@ image_lock()
>   # Mount a device, will umount it automatially when shell exits
>   mount_image() {
>   	local image=$1 fmt
>  -	local dev mnt mnt_dev
>  +	local dev mnt mnt_dev boot root
>   
>   	# Lock the image just in case user run this script in parrel
>   	image_lock $image
>  @@ -166,12 +181,20 @@ mount_image() {
>   
>   	$SUDO mount $mnt_dev $mnt
>   	[ $? -ne 0 ] && perror_exit "failed to mount device
> '$mnt_dev'"
>  +	boot=$(get_mount_boot "$dev")
>  +	if [[ -n "$boot" ]]; then
>  +		root=$(get_image_mount_root $image)
>  +		$SUDO mount $boot $root/boot
>  +		[ $? -ne 0 ] && perror_exit "failed to mount the bootable partition for
> device '$mnt_dev'"
>  +	fi
>   }
>   
>   get_image_mount_root() {
>   	local image=$1
>   	local root=${MNTS[$image]}
>   
>  +	# Starting from Fedora 36, the root node is /root/root of the last partition
>  +	[ -d "$root/root/root" ] && root=$root/root
>   	echo $root
>   
>   	if [ -z "$root" ]; then
>  @@ -213,7 +236,7 @@ run_in_image() {
>   
>   inst_in_image() {
>   	local image=$1 src=$2 dst=$3
>  -	local root=${MNTS[$image]}
>  +	local root=$(get_image_mount_root $1)
>   
>   	$SUDO cp $src $root/$dst
>   }

[PATCH v3 1/3] tests: correctly mount the root and also the boot partitions for Fedora 35, 36 and rawhide Cloud Base Image

[Coiby Xu]

Hi Philipp,

On Fri, Jul 29, 2022 at 05:33:57PM +0200, Philipp Rudo wrote:
> Hi Coiby,
>
> this series doesn't seem to be merged yet. So let me add a few
> comments/questions ;-) 
Thanks for reviewing the patch!

> On Fri, 13 May 2022 14:08:08 +0800
> Coiby Xu <coxu(a)redhat.com&gt; wrote:
>
> > Fedora 33 and 34 Cloud Base Images have only partition with the
> 					      ^
> 					     one (?) 
Nice catch!

> > following directory structure,
> > .
> > ├── bin -> usr/bin
> > ├── boot
> > ├── dev
> > ├── etc
> > ├── home
> > ├── root
> >
> > By comparison, Fedora 35, 36 and 37 Cloud Base Images have multiple
> > partitions. The root partition which is the last partition has the
> > following directory,
> > .
> > ├── home
> > └── root
> >     ├── bin -> usr/bin
> >     ├── boot
> >     ├── dev
> >     ├── etc
> >     ├── home
> >     ├── root
>
> Just for my understanding, in this schema /root/home is the home
> directory for root and /home for everybody else. Is that correct? 
/root/root is the home directory for root and /root/home is for
everybody else. And /home seems to be unused.

> > and the 2nd partition is the boot partition.
> >
> > This patch address the above changes by mounting {LAST_PARTITION}/root as
> > to TEMP_ROOT and mount FIRST_PARTITION to TEMP_ROOT/boot. So the test
> > image can be built successfully.
>
> That's a little bit confusing to me. When the boot partition is the 2nd
> partition why is the variable called FIRST_PARTITION? 
Sorry for the confusion! FIRST_PARTITION should be SECOND_PARTITION.

> >
> > Signed-off-by: Coiby Xu <coxu(a)redhat.com&gt;
> > ---
> >  tests/scripts/image-init-lib.sh | 29 ++++++++++++++++++++++++++---
> >  1 file changed, 26 insertions(+), 3 deletions(-)
> >
> > diff --git a/tests/scripts/image-init-lib.sh b/tests/scripts/image-init-lib.sh
> > index 0a1524b..5995004 100644
> > --- a/tests/scripts/image-init-lib.sh
> > +++ b/tests/scripts/image-init-lib.sh
> > @@ -23,7 +23,7 @@ is_mounted()
> >  clean_up()
> >  {
> >  	for _mnt in ${MNTS[@]}; do
> > -		is_mounted $_mnt && $SUDO umount -f $_mnt
> > +		is_mounted $_mnt && $SUDO umount -f -R $_mnt
> >  	done
> >
> >  	for _dev in ${DEVS[@]}; do
> > @@ -81,6 +81,21 @@ get_mountable_dev() {
> >  	fi
> >  }
> >
> > +# get the separate boot partition
> > +# return the 2nd partition as boot partition
> > +get_mount_boot() {
> > +	local dev=$1 parts
> > +
> > +	$SUDO partprobe $dev && sync
> > +	parts="$(ls -1 ${dev}p*)"
> > +	if [ -n "$parts" ]; then
> > +		if [ $(echo "$parts" | wc -l) -gt 1 ]; then
> > +			echo "$parts" | head -2 | tail -1
> > +		fi
> > +	fi
>
> Hmmm... difficult...
>
> Can we really rely on ls providing the right order here? If so why not
> simply do 'ls -1 ${dev}p2' ? 
Yes, I should have simply used "ls -1 ${dev}". However your suggestion
of using lsblk inspires me to find a more reliable method which also
works for RHEL cloud base image,

   if [[ $(lsblk -f ${dev}p2 -n -o LABEL 2> /dev/null) == boot ]]
      echo ${dev}p2
   fi

> Would it make sense to switch to something more powerful like lsblk?
> For example we could use
>
> 	lsblk -p -r -n -o NAME,MOUNTPOINT ${dev} | grep $root/boot 
Btw, the above command doesn't work for the cloud image case,

$ lsblk -p -r -n -o NAME,MOUNTPOINT /dev/nbd2
/dev/nbd2 
/dev/nbd2p1 
/dev/nbd2p2 
/dev/nbd2p3 
/dev/nbd2p4 
/dev/nbd2p5

> or
> 	lsblk -p -r -n -o NAME,PARTLABEL ${dev} | grep efi 
So is the above command.

> (what for non-efi systems, e.g. s390?). 
Currently, Fedora doesn't have s390 cloud image so we simply skip this
concern now.

> Finally, you can drop the two if-blocks by simply piping to sed. For
> example your original version could be rewritten to
>
> 	ls -1 ${dev}p* | sed -n 2p
>
> which prints the second line or "" if it doesn't exist.
>
> Thanks
> Philipp
>
> > +}
> > +
> > +
> >  prepare_loop() {
> >  	[ -n "$(lsmod | grep "^loop")" ] && return
> >
> > @@ -133,7 +148,7 @@ image_lock()
> >  # Mount a device, will umount it automatially when shell exits
> >  mount_image() {
> >  	local image=$1 fmt
> > -	local dev mnt mnt_dev
> > +	local dev mnt mnt_dev boot root
> >
> >  	# Lock the image just in case user run this script in parrel
> >  	image_lock $image
> > @@ -166,12 +181,20 @@ mount_image() {
> >
> >  	$SUDO mount $mnt_dev $mnt
> >  	[ $? -ne 0 ] && perror_exit "failed to mount device
> '$mnt_dev'"
> > +	boot=$(get_mount_boot "$dev")
> > +	if [[ -n "$boot" ]]; then
> > +		root=$(get_image_mount_root $image)
> > +		$SUDO mount $boot $root/boot
> > +		[ $? -ne 0 ] && perror_exit "failed to mount the bootable partition
> for device '$mnt_dev'"
> > +	fi
> >  }
> >
> >  get_image_mount_root() {
> >  	local image=$1
> >  	local root=${MNTS[$image]}
> >
> > +	# Starting from Fedora 36, the root node is /root/root of the last partition
> > +	[ -d "$root/root/root" ] && root=$root/root
> >  	echo $root
> >
> >  	if [ -z "$root" ]; then
> > @@ -213,7 +236,7 @@ run_in_image() {
> >
> >  inst_in_image() {
> >  	local image=$1 src=$2 dst=$3
> > -	local root=${MNTS[$image]}
> > +	local root=$(get_image_mount_root $1)
> >
> >  	$SUDO cp $src $root/$dst
> >  }
>  
-- 
Best regards,
Coiby

[PATCH v3 1/3] tests: correctly mount the root and also the boot partitions for Fedora 35, 36 and rawhide Cloud Base Image

[Philipp Rudo]

Hi Coiby,

On Tue, 2 Aug 2022 15:54:11 +0800
Coiby Xu <coxu(a)redhat.com&gt; wrote:

>  Hi Philipp,
>  
>  On Fri, Jul 29, 2022 at 05:33:57PM +0200, Philipp Rudo wrote:
>  >Hi Coiby,
>  >
>  >this series doesn't seem to be merged yet. So let me add a few
>  >comments/questions ;-)  
>  
>  Thanks for reviewing the patch! 
my pleasure.

>  
>  >
>  >On Fri, 13 May 2022 14:08:08 +0800
>  >Coiby Xu <coxu(a)redhat.com&gt; wrote:
>  >  
>  >> Fedora 33 and 34 Cloud Base Images have only partition with the  
>  >					      ^
>  >					     one (?)  
>  
>  Nice catch!
>  
>  >  
>  >> following directory structure,
>  >> .
>  >> ├── bin -> usr/bin
>  >> ├── boot
>  >> ├── dev
>  >> ├── etc
>  >> ├── home
>  >> ├── root
>  >>
>  >> By comparison, Fedora 35, 36 and 37 Cloud Base Images have multiple
>  >> partitions. The root partition which is the last partition has the
>  >> following directory,
>  >> .
>  >> ├── home
>  >> └── root
>  >>     ├── bin -> usr/bin
>  >>     ├── boot
>  >>     ├── dev
>  >>     ├── etc
>  >>     ├── home
>  >>     ├── root  
>  >
>  >Just for my understanding, in this schema /root/home is the home
>  >directory for root and /home for everybody else. Is that correct?  
>  
>  /root/root is the home directory for root and /root/home is for
>  everybody else. And /home seems to be unused. 
Thanks for the explanation! It's weird that there is this unused
/home... but well it's not our problem ;-)

>  
>  >  
>  >> and the 2nd partition is the boot partition.
>  >>
>  >> This patch address the above changes by mounting {LAST_PARTITION}/root as
>  >> to TEMP_ROOT and mount FIRST_PARTITION to TEMP_ROOT/boot. So the test
>  >> image can be built successfully.  
>  >
>  >That's a little bit confusing to me. When the boot partition is the 2nd
>  >partition why is the variable called FIRST_PARTITION?  
>  
>  Sorry for the confusion! FIRST_PARTITION should be SECOND_PARTITION.
>  
>  >  
>  >>
>  >> Signed-off-by: Coiby Xu <coxu(a)redhat.com&gt;
>  >> ---
>  >>  tests/scripts/image-init-lib.sh | 29 ++++++++++++++++++++++++++---
>  >>  1 file changed, 26 insertions(+), 3 deletions(-)
>  >>
>  >> diff --git a/tests/scripts/image-init-lib.sh b/tests/scripts/image-init-lib.sh
>  >> index 0a1524b..5995004 100644
>  >> --- a/tests/scripts/image-init-lib.sh
>  >> +++ b/tests/scripts/image-init-lib.sh
>  >> @@ -23,7 +23,7 @@ is_mounted()
>  >>  clean_up()
>  >>  {
>  >>  	for _mnt in ${MNTS[@]}; do
>  >> -		is_mounted $_mnt && $SUDO umount -f $_mnt
>  >> +		is_mounted $_mnt && $SUDO umount -f -R $_mnt
>  >>  	done
>  >>
>  >>  	for _dev in ${DEVS[@]}; do
>  >> @@ -81,6 +81,21 @@ get_mountable_dev() {
>  >>  	fi
>  >>  }
>  >>
>  >> +# get the separate boot partition
>  >> +# return the 2nd partition as boot partition
>  >> +get_mount_boot() {
>  >> +	local dev=$1 parts
>  >> +
>  >> +	$SUDO partprobe $dev && sync
>  >> +	parts="$(ls -1 ${dev}p*)"
>  >> +	if [ -n "$parts" ]; then
>  >> +		if [ $(echo "$parts" | wc -l) -gt 1 ]; then
>  >> +			echo "$parts" | head -2 | tail -1
>  >> +		fi
>  >> +	fi  
>  >
>  >Hmmm... difficult...
>  >
>  >Can we really rely on ls providing the right order here? If so why not
>  >simply do 'ls -1 ${dev}p2' ?  
>  
>  Yes, I should have simply used "ls -1 ${dev}". However your suggestion
>  of using lsblk inspires me to find a more reliable method which also
>  works for RHEL cloud base image,
>  
>     if [[ $(lsblk -f ${dev}p2 -n -o LABEL 2> /dev/null) == boot ]]
>        echo ${dev}p2
>     fi 
Originally I refrained from using LABEL as the way I understand it it
is more a scratchpad for users so they can remember which partition
contains what. So especially on systems with a custom partition scheme 
there is no guarantee that the LABEL on the boot partition contains
'boot'. That's why I thought MOUNTPOINT or PARTLABEL are better choices
(although when thinking about it PARTLABEL probably has the same
problem).

Anyway, on second thought this script is only run in a very specific
environment and isn't shipped to customers. So we don't have to be
generic but can take some shortcuts. So using LABEL should be fine.

But I'm not sure if the suggestion you made above makes sense.
Originally the function was used to return the boot partition of a
device (which happens to be the second partition). Now you are verifying
that the second partition is the boot partition.

Anyway when you are using the LABEL you can also use findfs to get the
partition. I don't know what findfs does when there are multiple
partitions with the same LABEL so I think it is better to pipe the
output to grep. I.e. this should work

	findfs LABEL=boot 2> /dev/null | grep $dev

Thanks
Philipp

>  
>  >
>  >Would it make sense to switch to something more powerful like lsblk?
>  >For example we could use
>  >
>  >	lsblk -p -r -n -o NAME,MOUNTPOINT ${dev} | grep $root/boot  
>  
>  Btw, the above command doesn't work for the cloud image case,
>  
>  $ lsblk -p -r -n -o NAME,MOUNTPOINT /dev/nbd2
>  /dev/nbd2 
>  /dev/nbd2p1 
>  /dev/nbd2p2 
>  /dev/nbd2p3 
>  /dev/nbd2p4 
>  /dev/nbd2p5
>  
>  >
>  >or
>  >	lsblk -p -r -n -o NAME,PARTLABEL ${dev} | grep efi  
>  
>  So is the above command.
>  
>  >
>  >(what for non-efi systems, e.g. s390?).  
>  
>  Currently, Fedora doesn't have s390 cloud image so we simply skip this
>  concern now.
>  
>  >
>  >Finally, you can drop the two if-blocks by simply piping to sed. For
>  >example your original version could be rewritten to
>  >
>  >	ls -1 ${dev}p* | sed -n 2p
>  >
>  >which prints the second line or "" if it doesn't exist.
>  >
>  >Thanks
>  >Philipp
>  >  
>  >> +}
>  >> +
>  >> +
>  >>  prepare_loop() {
>  >>  	[ -n "$(lsmod | grep "^loop")" ] && return
>  >>
>  >> @@ -133,7 +148,7 @@ image_lock()
>  >>  # Mount a device, will umount it automatially when shell exits
>  >>  mount_image() {
>  >>  	local image=$1 fmt
>  >> -	local dev mnt mnt_dev
>  >> +	local dev mnt mnt_dev boot root
>  >>
>  >>  	# Lock the image just in case user run this script in parrel
>  >>  	image_lock $image
>  >> @@ -166,12 +181,20 @@ mount_image() {
>  >>
>  >>  	$SUDO mount $mnt_dev $mnt
>  >>  	[ $? -ne 0 ] && perror_exit "failed to mount device
> '$mnt_dev'"
>  >> +	boot=$(get_mount_boot "$dev")
>  >> +	if [[ -n "$boot" ]]; then
>  >> +		root=$(get_image_mount_root $image)
>  >> +		$SUDO mount $boot $root/boot
>  >> +		[ $? -ne 0 ] && perror_exit "failed to mount the bootable
> partition for device '$mnt_dev'"
>  >> +	fi
>  >>  }
>  >>
>  >>  get_image_mount_root() {
>  >>  	local image=$1
>  >>  	local root=${MNTS[$image]}
>  >>
>  >> +	# Starting from Fedora 36, the root node is /root/root of the last partition
>  >> +	[ -d "$root/root/root" ] && root=$root/root
>  >>  	echo $root
>  >>
>  >>  	if [ -z "$root" ]; then
>  >> @@ -213,7 +236,7 @@ run_in_image() {
>  >>
>  >>  inst_in_image() {
>  >>  	local image=$1 src=$2 dst=$3
>  >> -	local root=${MNTS[$image]}
>  >> +	local root=$(get_image_mount_root $1)
>  >>
>  >>  	$SUDO cp $src $root/$dst
>  >>  }  
>  >  
>

[PATCH v3 1/3] tests: correctly mount the root and also the boot partitions for Fedora 35, 36 and rawhide Cloud Base Image

[Coiby Xu]

On Thu, Aug 04, 2022 at 06:27:46PM +0200, Philipp Rudo wrote:
> Hi Coiby,
>  
[...]
> > >Can we really rely on ls providing the right order here? If
> so why not
> > >simply do 'ls -1 ${dev}p2' ?
> >
> > Yes, I should have simply used "ls -1 ${dev}". However your suggestion
> > of using lsblk inspires me to find a more reliable method which also
> > works for RHEL cloud base image,
> >
> >    if [[ $(lsblk -f ${dev}p2 -n -o LABEL 2> /dev/null) == boot ]]
> >       echo ${dev}p2
> >    fi
>
> Originally I refrained from using LABEL as the way I understand it it
> is more a scratchpad for users so they can remember which partition
> contains what. So especially on systems with a custom partition scheme
> there is no guarantee that the LABEL on the boot partition contains 
> (although when thinking about it PARTLABEL probably has the same
> problem).
>
> Anyway, on second thought this script is only run in a very specific
> environment and isn't shipped to customers. So we don't have to be
> generic but can take some shortcuts. So using LABEL should be fine.
>
> But I'm not sure if the suggestion you made above makes sense.
> Originally the function was used to return the boot partition of a
> device (which happens to be the second partition). Now you are verifying
> that the second partition is the boot partition. 
For REHL9 cloud image, the second partition is an efi partition and
there is no separate boot partition. Verifying that the second partition
is the boot partition could avoid this problem.

> Anyway when you are using the LABEL you can also use findfs to get the
> partition. I don't know what findfs does when there are multiple
> partitions with the same LABEL so I think it is better to pipe the
> output to grep. I.e. this should work
>
> 	findfs LABEL=boot 2> /dev/null | grep $dev 
I can confirm it works, thanks for teaching me one more tip:)

-- 
Best regards,
Coiby

[PATCH v3 2/3] tests: specify the backing format for the backing file when using qemu-img create

[Coiby Xu]

New version of qemu-img requires specifying the backing format for the
backing file otherwise it will abort.

Signed-off-by: Coiby Xu <coxu(a)redhat.com&gt;
---
 tests/scripts/image-init-lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/scripts/image-init-lib.sh b/tests/scripts/image-init-lib.sh
index 5995004..4793fec 100644
--- a/tests/scripts/image-init-lib.sh
+++ b/tests/scripts/image-init-lib.sh
@@ -263,7 +263,7 @@ create_image_from_base_image() {
 	if [ "$image_fmt" != "raw" ]; then
 		if fmt_is_qcow2 "$image_fmt"; then
 			echo "Source image is qcow2, using snapshot..."
-			qemu-img create -f qcow2 -b $image $output
+			qemu-img create -f qcow2 -b $image -F qcow2 $output
 		else
 			perror_exit "Unrecognized base image format '$image_mnt'"
 		fi
-- 
2.35.3

[PATCH v3 3/3] tests: specify the Fedora version when running fedpkg sources

[Coiby Xu]

So fedpkg will fetch the sources that matches given Fedora version.

Signed-off-by: Coiby Xu <coxu(a)redhat.com&gt;
---
 tests/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile b/tests/Makefile
index 8fb38c7..05fda1d 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -42,7 +42,7 @@ all: $(TEST_ROOT)/output/test-base-image
 # to rebuild the rpm, currently use rpmbuild to have better control over the rpm building
process
 #
 $(KEXEC_TOOLS_RPM): $(KEXEC_TOOLS_SRC)
-	sh -c "cd .. && fedpkg sources"
+	sh -c "cd .. && fedpkg --release f$(RELEASE) sources"
 	@echo Rebuilding RPM due to modification of sources: $?
 	rpmbuild $(RPMDEFINE) -ba $(REPO)/$(SPEC)
 
-- 
2.35.3

[Philipp Rudo]

Hi Coiby,

except for the questions on the first patch the series looks fine by me.

Reviewed-by: Philipp Rudo <prudo(a)redhat.com&gt;

On Fri, 13 May 2022 14:08:07 +0800
Coiby Xu <coxu(a)redhat.com&gt; wrote:

>  This patch set enables the integration tests (i.e. the test cases in
>  tests/) for Fedora 35, 36 and 37 cloud base images.
>  
>  v3:
>   - use "unmount -R" to umount both the root and boot partition [Kairui]
>  	- add a code comment for the change of root node [Kairui]
>  	- fix the error of using the first partition as boot partition (the
>  			2nd partition is the boot parition) for Fedora >= 35
>  	- specify backing format for "qemu-img create" as required by newer qemu-img
>  	- specify the Fedora version for fedpkg sources
>  
>  v2:
>    - check LAST_PARTITION/root/root instead of LAST_PARTITION/root [Kairui]
>    - improve commit message
>  	
>  Coiby Xu (3):
>    tests: correctly mount the root and also the boot partitions for
>      Fedora 35, 36 and rawhide Cloud Base Image
>    tests: specify the backing format for the backing file when using
>      qemu-img create
>    tests: specify the Fedora version when running fedpkg sources
>  
>   tests/Makefile                  |  2 +-
>   tests/scripts/image-init-lib.sh | 31 +++++++++++++++++++++++++++----
>   2 files changed, 28 insertions(+), 5 deletions(-)
>

[Coiby Xu]

Hi Philipp,

On Fri, Jul 29, 2022 at 05:35:14PM +0200, Philipp Rudo wrote:
> Hi Coiby,
>
> except for the questions on the first patch the series looks fine by me.
>
> Reviewed-by: Philipp Rudo <prudo(a)redhat.com&gt; 
Thanks for reviewing the patch set! It has been merged with the issues
of the first patch resolved.

> On Fri, 13 May 2022 14:08:07 +0800
> Coiby Xu <coxu(a)redhat.com&gt; wrote:
>
> > This patch set enables the integration tests (i.e. the test cases in
> > tests/) for Fedora 35, 36 and 37 cloud base images.
> >
> > v3:
> >  - use "unmount -R" to umount both the root and boot partition [Kairui]
> > 	- add a code comment for the change of root node [Kairui]
> > 	- fix the error of using the first partition as boot partition (the
> > 			2nd partition is the boot parition) for Fedora >= 35
> > 	- specify backing format for "qemu-img create" as required by newer
> qemu-img
> > 	- specify the Fedora version for fedpkg sources
> >
> > v2:
> >   - check LAST_PARTITION/root/root instead of LAST_PARTITION/root [Kairui]
> >   - improve commit message
> > 	
> > Coiby Xu (3):
> >   tests: correctly mount the root and also the boot partitions for
> >     Fedora 35, 36 and rawhide Cloud Base Image
> >   tests: specify the backing format for the backing file when using
> >     qemu-img create
> >   tests: specify the Fedora version when running fedpkg sources
> >
> >  tests/Makefile                  |  2 +-
> >  tests/scripts/image-init-lib.sh | 31 +++++++++++++++++++++++++++----
> >  2 files changed, 28 insertions(+), 5 deletions(-)
> >
>  
-- 
Best regards,
Coiby

[Philipp Rudo]

Hi Coiby,

On Wed, 3 Aug 2022 21:23:21 +0800
Coiby Xu <coxu(a)redhat.com&gt; wrote:

>  Hi Philipp,
>  
>  On Fri, Jul 29, 2022 at 05:35:14PM +0200, Philipp Rudo wrote:
>  >Hi Coiby,
>  >
>  >except for the questions on the first patch the series looks fine by me.
>  >
>  >Reviewed-by: Philipp Rudo <prudo(a)redhat.com&gt;  
>  
>  Thanks for reviewing the patch set! It has been merged with the issues
>  of the first patch resolved. 
ok... should have read all messages in the thread first...

Anyway, it's only a small nit. When you like you can ignore the comment
I made for the first patch.

Thanks
Philipp

>  
>  >
>  >On Fri, 13 May 2022 14:08:07 +0800
>  >Coiby Xu <coxu(a)redhat.com&gt; wrote:
>  >  
>  >> This patch set enables the integration tests (i.e. the test cases in
>  >> tests/) for Fedora 35, 36 and 37 cloud base images.
>  >>
>  >> v3:
>  >>  - use "unmount -R" to umount both the root and boot partition
> [Kairui]
>  >> 	- add a code comment for the change of root node [Kairui]
>  >> 	- fix the error of using the first partition as boot partition (the
>  >> 			2nd partition is the boot parition) for Fedora >= 35
>  >> 	- specify backing format for "qemu-img create" as required by newer
> qemu-img
>  >> 	- specify the Fedora version for fedpkg sources
>  >>
>  >> v2:
>  >>   - check LAST_PARTITION/root/root instead of LAST_PARTITION/root [Kairui]
>  >>   - improve commit message
>  >> 	
>  >> Coiby Xu (3):
>  >>   tests: correctly mount the root and also the boot partitions for
>  >>     Fedora 35, 36 and rawhide Cloud Base Image
>  >>   tests: specify the backing format for the backing file when using
>  >>     qemu-img create
>  >>   tests: specify the Fedora version when running fedpkg sources
>  >>
>  >>  tests/Makefile                  |  2 +-
>  >>  tests/scripts/image-init-lib.sh | 31 +++++++++++++++++++++++++++----
>  >>  2 files changed, 28 insertions(+), 5 deletions(-)
>  >>  
>  >  
>