Re: [PATCH 07/13] tests/avocado/kvm_xen_guest.py: cope with asset RW requirements

[Daniel P . Berrangé]

On Fri, Jul 26, 2024 at 09:44:32AM -0400, Cleber Rosa wrote:
> Some of these tests actually require the root filesystem image,
> obtained through Avocado's asset feature and kept in a common cache
> location, to be writable.
> 
> This makes a distinction between the tests that actually have this
> requirement and those who don't.  The goal is to be as safe as
> possible, avoiding causing cache misses (because the assets get
> modified and thus need to be dowloaded again) while avoid copying the
> root filesystem backing file whenever possible.
> 
> This also allow these tests to be run in parallel with newer Avocado
> versions.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/avocado/kvm_xen_guest.py | 27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/avocado/kvm_xen_guest.py b/tests/avocado/kvm_xen_guest.py
> index 318fadebc3..d73fa888ef 100644
> --- a/tests/avocado/kvm_xen_guest.py
> +++ b/tests/avocado/kvm_xen_guest.py
> @@ -10,6 +10,7 @@
>  # SPDX-License-Identifier: GPL-2.0-or-later
>  
>  import os
> +import shutil
>  
>  from qemu.machine import machine
>  
> @@ -43,7 +44,7 @@ def get_asset(self, name, sha1):
>          return self.fetch_asset(name=f"qemu-kvm-xen-guest-{name}",
>                                  locations=(url), asset_hash=sha1)
>  
> -    def common_vm_setup(self):
> +    def common_vm_setup(self, readwrite=False):
>          # We also catch lack of KVM_XEN support if we fail to launch
>          self.require_accelerator("kvm")
>  
> @@ -56,11 +57,19 @@ def common_vm_setup(self):
>                                            
> "367962983d0d32109998a70b45dcee4672d0b045")
>          self.rootfs = self.get_asset("rootfs.ext4",
>                                       
> "f1478401ea4b3fa2ea196396be44315bab2bb5e4")
> +        if readwrite:
> +            dest = os.path.join(self.workdir, os.path.basename(self.rootfs))
> +            shutil.copy(self.rootfs, dest)
> +            self.rootfs = dest

This is a very expensive way of creating a writable disk. Better to
avoid adding this 'readwrite' parameter at all, and instead create
a throwaway qcow2 overlay for the image for all tests. That ensures
writability for everything in a cheap manner.

>  
> -    def run_and_check(self):
> +    def run_and_check(self, readwrite=False):
> +        if readwrite:
> +            drive = f"file={self.rootfs},if=none,format=raw,id=drv0"
> +        else:
> +            drive = 
> f"file={self.rootfs},if=none,readonly=on,format=raw,id=drv0"
>          self.vm.add_args('-kernel', self.kernel_path,
>                           '-append', self.kernel_params,
> -                         '-drive',  
> f"file={self.rootfs},if=none,snapshot=on,format=raw,id=drv0",
> +                         '-drive',  drive,
>                           '-device', 'xen-disk,drive=drv0,vdev=xvda',
>                           '-device', 'virtio-net-pci,netdev=unet',
>                           '-netdev', 'user,id=unet,hostfwd=:127.0.0.1:0-:22')
> @@ -90,11 +99,11 @@ def test_kvm_xen_guest(self):
>          :avocado: tags=kvm_xen_guest
>          """
>  
> -        self.common_vm_setup()
> +        self.common_vm_setup(True)
>  
>          self.kernel_params = (self.KERNEL_DEFAULT +
>                                ' xen_emul_unplug=ide-disks')
> -        self.run_and_check()
> +        self.run_and_check(True)
>          self.ssh_command('grep xen-pirq.*msi /proc/interrupts')
>  
>      def test_kvm_xen_guest_nomsi(self):
> @@ -102,11 +111,11 @@ def test_kvm_xen_guest_nomsi(self):
>          :avocado: tags=kvm_xen_guest_nomsi
>          """
>  
> -        self.common_vm_setup()
> +        self.common_vm_setup(True)
>  
>          self.kernel_params = (self.KERNEL_DEFAULT +
>                                ' xen_emul_unplug=ide-disks pci=nomsi')
> -        self.run_and_check()
> +        self.run_and_check(True)
>          self.ssh_command('grep xen-pirq.* /proc/interrupts')
>  
>      def test_kvm_xen_guest_noapic_nomsi(self):
> @@ -114,11 +123,11 @@ def test_kvm_xen_guest_noapic_nomsi(self):
>          :avocado: tags=kvm_xen_guest_noapic_nomsi
>          """
>  
> -        self.common_vm_setup()
> +        self.common_vm_setup(True)
>  
>          self.kernel_params = (self.KERNEL_DEFAULT +
>                                ' xen_emul_unplug=ide-disks noapic pci=nomsi')
> -        self.run_and_check()
> +        self.run_and_check(True)
>          self.ssh_command('grep xen-pirq /proc/interrupts')
>  
>      def test_kvm_xen_guest_vapic(self):
> -- 
> 2.45.2
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|