| Message ID | 20231208190911.102879-7-crosa@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | for-8.3 tests/avocado: prep for Avocado 103.0 LTS | expand |
On 8/12/23 20:09, 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. Having cache assets modified is a design issue. We should assume the cache directory as read-only. > 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(-)
On Mon, Dec 11, 2023 at 11:32 AM Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > On 8/12/23 20:09, 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. > > Having cache assets modified is a design issue. We should assume > the cache directory as read-only. > I agree those files should not be modified, but I wonder if you thought about any solution to this? Given that the same user writes (downloads) those files, do you think setting file permissions between the download and the use of the files should be done? That can make the management of the cache (such as pruning it) either require undoing the restriction or being done by a super user. Anyway, just curious. Regards, - Cleber.
On 1/8/24 05:30, Cleber Rosa wrote: > On Mon, Dec 11, 2023 at 11:32 AM Philippe Mathieu-Daudé > <philmd@linaro.org> wrote: >> >> On 8/12/23 20:09, 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. >> >> Having cache assets modified is a design issue. We should assume >> the cache directory as read-only. >> > > I agree those files should not be modified, but I wonder if you > thought about any solution to this? Given that the same user writes > (downloads) those files, do you think setting file permissions between > the download and the use of the files should be done? We want to share a cachedir on development hosts with multiple developers. OK to alter a downloaded file before adding it to the cache; but then once a file is added/hashed it shouldn't be modified IMO. So far this directory is group=RW but we like the ability to track a read-only directory (like owned by a particular user) and adding missing assets to current user cachedir, to avoid duplication of files and waste of network transfer. > That can make the management of the cache (such as pruning it) either > require undoing the restriction or being done by a super user. > > Anyway, just curious. > > Regards, > - Cleber. >
On Thu, Aug 1, 2024 at 8:57 AM Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > I agree those files should not be modified, but I wonder if you > > thought about any solution to this? Given that the same user writes > > (downloads) those files, do you think setting file permissions between > > the download and the use of the files should be done? > > We want to share a cachedir on development hosts with multiple > developers. OK to alter a downloaded file before adding it to > the cache; but then once a file is added/hashed it shouldn't be > modified IMO. > I was asking more in terms of what to do before/after the test. When it comes to this type of setup, Avocado's cache was designed to support this use case. You can provide multiple cache dirs in the configuration, and some (the first ones, ideally) can be RO (life NFS mounts). But this is hardly something that can be configured without proper user input, so this is not present in the generic "make check-avocado". > So far this directory is group=RW but we like the ability to track > a read-only directory (like owned by a particular user) and adding > missing assets to current user cachedir, to avoid duplication of > files and waste of network transfer. > That can be done in avocado.conf, something like: [datadir.paths] cache_dirs = ['/path/that/is/ro/because/owned/by/someone/else', '/home/cleber/avocado/data/cache'] The asset library will take care of trying to find assets in the RO directories, while writing to the RW ones. Hope this helps, - Cleber.
On 1/8/24 17:17, Cleber Rosa wrote: > On Thu, Aug 1, 2024 at 8:57 AM Philippe Mathieu-Daudé <philmd@linaro.org> wrote: >>> I agree those files should not be modified, but I wonder if you >>> thought about any solution to this? Given that the same user writes >>> (downloads) those files, do you think setting file permissions between >>> the download and the use of the files should be done? >> >> We want to share a cachedir on development hosts with multiple >> developers. OK to alter a downloaded file before adding it to >> the cache; but then once a file is added/hashed it shouldn't be >> modified IMO. >> > > I was asking more in terms of what to do before/after the test. When > it comes to this type of setup, Avocado's cache was designed to > support this use case. You can provide multiple cache dirs in the > configuration, and some (the first ones, ideally) can be RO (life NFS > mounts). > > But this is hardly something that can be configured without proper > user input, so this is not present in the generic "make > check-avocado". > >> So far this directory is group=RW but we like the ability to track >> a read-only directory (like owned by a particular user) and adding >> missing assets to current user cachedir, to avoid duplication of >> files and waste of network transfer. >> > > That can be done in avocado.conf, something like: > > [datadir.paths] > cache_dirs = ['/path/that/is/ro/because/owned/by/someone/else', > '/home/cleber/avocado/data/cache'] > > The asset library will take care of trying to find assets in the RO > directories, while writing to the RW ones. Great, thanks!
diff --git a/tests/avocado/kvm_xen_guest.py b/tests/avocado/kvm_xen_guest.py index ec4052a1fe..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 - 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,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):
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(-)