| Message ID | 20220614015044.2501806-4-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/5] tests/qemu-iotests: hotfix for 307, 223 output | expand |
On 14/06/2022 03.50, John Snow wrote: > If the initial setup fails, you've permanently altered the state of the > downloaded image in an unknowable way. Use 'cp' like our other test > setup scripts do. > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > tests/vm/centos | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/vm/centos b/tests/vm/centos > index 5c7bc1c1a9a..be4f6ff2f14 100755 > --- a/tests/vm/centos > +++ b/tests/vm/centos > @@ -34,7 +34,7 @@ class CentosVM(basevm.BaseVM): > def build_image(self, img): > cimg = self._download_with_cache("https://cloud.centos.org/centos/8/x86_64/images/CentOS-8-GenericCloud-8.3.2011-20201204.2.x86_64.qcow2") > img_tmp = img + ".tmp" > - subprocess.check_call(["ln", "-f", cimg, img_tmp]) > + subprocess.check_call(['cp', '-f', cimg, img_tmp]) I wonder whether it would make sense to use "qemu-img create -b" instead to save some disk space? Anyway, your patch is certainly already an improvement, so: Reviewed-by: Thomas Huth <thuth@redhat.com>
On Tue, Jun 14, 2022 at 12:40 AM Thomas Huth <thuth@redhat.com> wrote: > > On 14/06/2022 03.50, John Snow wrote: > > If the initial setup fails, you've permanently altered the state of the > > downloaded image in an unknowable way. Use 'cp' like our other test > > setup scripts do. > > > > Signed-off-by: John Snow <jsnow@redhat.com> > > --- > > tests/vm/centos | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tests/vm/centos b/tests/vm/centos > > index 5c7bc1c1a9a..be4f6ff2f14 100755 > > --- a/tests/vm/centos > > +++ b/tests/vm/centos > > @@ -34,7 +34,7 @@ class CentosVM(basevm.BaseVM): > > def build_image(self, img): > > cimg = self._download_with_cache("https://cloud.centos.org/centos/8/x86_64/images/CentOS-8-GenericCloud-8.3.2011-20201204.2.x86_64.qcow2") > > img_tmp = img + ".tmp" > > - subprocess.check_call(["ln", "-f", cimg, img_tmp]) > > + subprocess.check_call(['cp', '-f', cimg, img_tmp]) > > I wonder whether it would make sense to use "qemu-img create -b" instead to > save some disk space? > > Anyway, your patch is certainly already an improvement, so: > > Reviewed-by: Thomas Huth <thuth@redhat.com> I wondered the same, but decided to keep a smaller series this time around. VM tests already use a lot of space, so I doubt this is adding new constraints that didn't exist before. A more rigorous overhaul may be in order, but not right now. (It looks like the config file stuff to override defaults is not necessarily rigorously respected by the different installer recipes.) I think the caching of the fully set-up image needs work, too. In practice we leave the image sitting around, but we seem to always rebuild it no matter what, so it's not that useful. There's a few things that can be done here to drastically speed up some things, but... later. --js
diff --git a/tests/vm/centos b/tests/vm/centos index 5c7bc1c1a9a..be4f6ff2f14 100755 --- a/tests/vm/centos +++ b/tests/vm/centos @@ -34,7 +34,7 @@ class CentosVM(basevm.BaseVM): def build_image(self, img): cimg = self._download_with_cache("https://cloud.centos.org/centos/8/x86_64/images/CentOS-8-GenericCloud-8.3.2011-20201204.2.x86_64.qcow2") img_tmp = img + ".tmp" - subprocess.check_call(["ln", "-f", cimg, img_tmp]) + subprocess.check_call(['cp', '-f', cimg, img_tmp]) self.exec_qemu_img("resize", img_tmp, "50G") self.boot(img_tmp, extra_args = ["-cdrom", self.gen_cloud_init_iso()]) self.wait_ssh()
If the initial setup fails, you've permanently altered the state of the downloaded image in an unknowable way. Use 'cp' like our other test setup scripts do. Signed-off-by: John Snow <jsnow@redhat.com> --- tests/vm/centos | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)