diff mbox series

tests/vm: do not specify -bios option

Message ID 20220616083025.116902-1-pbonzini@redhat.com
State New
Headers show
Series tests/vm: do not specify -bios option | expand

Commit Message

Paolo Bonzini June 16, 2022, 8:30 a.m. UTC
When running from the build tree, the executable is able to find
the BIOS on its own; when running from the source tree, a firmware
blob should already be installed and there is no guarantee that
the one in the source tree works with the QEMU that is being used for
the installation.

Just remove the -bios option, since it is unnecessary and in fact
there are other x86 VM tests that do not bother specifying it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/vm/fedora  | 1 -
 tests/vm/freebsd | 1 -
 tests/vm/netbsd  | 1 -
 tests/vm/openbsd | 1 -
 4 files changed, 4 deletions(-)

Comments

Daniel P. Berrangé June 16, 2022, 8:44 a.m. UTC | #1
On Thu, Jun 16, 2022 at 10:30:25AM +0200, Paolo Bonzini wrote:
> When running from the build tree, the executable is able to find
> the BIOS on its own; when running from the source tree, a firmware
> blob should already be installed and there is no guarantee that
> the one in the source tree works with the QEMU that is being used for
> the installation.

I think there is interaction with

  --firmwarepath=/usr/share/qemu-firmware:/usr/share/ipxe/qemu:/usr/share/seavgabios:/usr/share/seabios:/usr/share/sgabios

as the firmware path listed there will be searched before the local
build tree.

I agree with your desire to remove the -bios option, but I'm not
sure it is entirely safe unless logic in qemu_find_file is fixed
to ignore the global search path when running from the build
tree.

> Just remove the -bios option, since it is unnecessary and in fact
> there are other x86 VM tests that do not bother specifying it.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/vm/fedora  | 1 -
>  tests/vm/freebsd | 1 -
>  tests/vm/netbsd  | 1 -
>  tests/vm/openbsd | 1 -
>  4 files changed, 4 deletions(-)
> 
> diff --git a/tests/vm/fedora b/tests/vm/fedora
> index 92b78d6e2c..12eca919a0 100755
> --- a/tests/vm/fedora
> +++ b/tests/vm/fedora
> @@ -79,7 +79,6 @@ class FedoraVM(basevm.BaseVM):
>          self.exec_qemu_img("create", "-f", "qcow2", img_tmp, self.size)
>          self.print_step("Booting installer")
>          self.boot(img_tmp, extra_args = [
> -            "-bios", "pc-bios/bios-256k.bin",
>              "-machine", "graphics=off",
>              "-device", "VGA",
>              "-cdrom", iso
> diff --git a/tests/vm/freebsd b/tests/vm/freebsd
> index 805db759d6..cd1fabde52 100755
> --- a/tests/vm/freebsd
> +++ b/tests/vm/freebsd
> @@ -95,7 +95,6 @@ class FreeBSDVM(basevm.BaseVM):
>  
>          self.print_step("Booting installer")
>          self.boot(img_tmp, extra_args = [
> -            "-bios", "pc-bios/bios-256k.bin",
>              "-machine", "graphics=off",
>              "-device", "VGA",
>              "-cdrom", iso
> diff --git a/tests/vm/netbsd b/tests/vm/netbsd
> index 45aa9a7fda..aa883ec23c 100755
> --- a/tests/vm/netbsd
> +++ b/tests/vm/netbsd
> @@ -86,7 +86,6 @@ class NetBSDVM(basevm.BaseVM):
>  
>          self.print_step("Booting installer")
>          self.boot(img_tmp, extra_args = [
> -            "-bios", "pc-bios/bios-256k.bin",
>              "-machine", "graphics=off",
>              "-cdrom", iso
>          ])
> diff --git a/tests/vm/openbsd b/tests/vm/openbsd
> index 13c8254214..6f1b6f5b98 100755
> --- a/tests/vm/openbsd
> +++ b/tests/vm/openbsd
> @@ -82,7 +82,6 @@ class OpenBSDVM(basevm.BaseVM):
>  
>          self.print_step("Booting installer")
>          self.boot(img_tmp, extra_args = [
> -            "-bios", "pc-bios/bios-256k.bin",
>              "-machine", "graphics=off",
>              "-device", "VGA",
>              "-cdrom", iso
> -- 
> 2.36.1
> 
> 

With regards,
Daniel
Paolo Bonzini June 16, 2022, 10:14 a.m. UTC | #2
On 6/16/22 10:44, Daniel P. Berrangé wrote:
> On Thu, Jun 16, 2022 at 10:30:25AM +0200, Paolo Bonzini wrote:
>> When running from the build tree, the executable is able to find
>> the BIOS on its own; when running from the source tree, a firmware
>> blob should already be installed and there is no guarantee that
>> the one in the source tree works with the QEMU that is being used for
>> the installation.
> I think there is interaction with
> 
>    --firmwarepath=/usr/share/qemu-firmware:/usr/share/ipxe/qemu:/usr/share/seavgabios:/usr/share/seabios:/usr/share/sgabios
> 
> as the firmware path listed there will be searched before the local
> build tree.
> 
> I agree with your desire to remove the -bios option, but I'm not
> sure it is entirely safe unless logic in qemu_find_file is fixed
> to ignore the global search path when running from the build
> tree.

Isn't this the same for any other invocation of QEMU, for example in qtest?

I admit I didn't think of the interaction with --firmwarepath, but "if 
it hurts, don't do it" might apply here.  That is, install compatible 
firmware to the path _before_ trying to use a QEMU that specifies that path.

Thanks,

Paolo
Daniel P. Berrangé June 16, 2022, 10:29 a.m. UTC | #3
On Thu, Jun 16, 2022 at 12:14:01PM +0200, Paolo Bonzini wrote:
> On 6/16/22 10:44, Daniel P. Berrangé wrote:
> > On Thu, Jun 16, 2022 at 10:30:25AM +0200, Paolo Bonzini wrote:
> > > When running from the build tree, the executable is able to find
> > > the BIOS on its own; when running from the source tree, a firmware
> > > blob should already be installed and there is no guarantee that
> > > the one in the source tree works with the QEMU that is being used for
> > > the installation.
> > I think there is interaction with
> > 
> >    --firmwarepath=/usr/share/qemu-firmware:/usr/share/ipxe/qemu:/usr/share/seavgabios:/usr/share/seabios:/usr/share/sgabios
> > 
> > as the firmware path listed there will be searched before the local
> > build tree.
> > 
> > I agree with your desire to remove the -bios option, but I'm not
> > sure it is entirely safe unless logic in qemu_find_file is fixed
> > to ignore the global search path when running from the build
> > tree.
> 
> Isn't this the same for any other invocation of QEMU, for example in qtest?

Yes, quite likely in fact, so a pre-existing widespread problem and
thus not a reason to avoid this proposed change.

> I admit I didn't think of the interaction with --firmwarepath, but "if it
> hurts, don't do it" might apply here.  That is, install compatible firmware
> to the path _before_ trying to use a QEMU that specifies that path.

I'm mostly thinking of downstream distro developers where they have an
installed QEMU but are working on a build from a different QEMU version.
We can address this separately though

With regards,
Daniel
Daniel P. Berrangé June 16, 2022, 10:29 a.m. UTC | #4
On Thu, Jun 16, 2022 at 10:30:25AM +0200, Paolo Bonzini wrote:
> When running from the build tree, the executable is able to find
> the BIOS on its own; when running from the source tree, a firmware
> blob should already be installed and there is no guarantee that
> the one in the source tree works with the QEMU that is being used for
> the installation.
> 
> Just remove the -bios option, since it is unnecessary and in fact
> there are other x86 VM tests that do not bother specifying it.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/vm/fedora  | 1 -
>  tests/vm/freebsd | 1 -
>  tests/vm/netbsd  | 1 -
>  tests/vm/openbsd | 1 -
>  4 files changed, 4 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
Thomas Huth June 17, 2022, 8:51 a.m. UTC | #5
On 16/06/2022 10.30, Paolo Bonzini wrote:
> When running from the build tree, the executable is able to find
> the BIOS on its own; when running from the source tree, a firmware
> blob should already be installed and there is no guarantee that
> the one in the source tree works with the QEMU that is being used for
> the installation.
> 
> Just remove the -bios option, since it is unnecessary and in fact
> there are other x86 VM tests that do not bother specifying it.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   tests/vm/fedora  | 1 -
>   tests/vm/freebsd | 1 -
>   tests/vm/netbsd  | 1 -
>   tests/vm/openbsd | 1 -
>   4 files changed, 4 deletions(-)
> 
> diff --git a/tests/vm/fedora b/tests/vm/fedora
> index 92b78d6e2c..12eca919a0 100755
> --- a/tests/vm/fedora
> +++ b/tests/vm/fedora
> @@ -79,7 +79,6 @@ class FedoraVM(basevm.BaseVM):
>           self.exec_qemu_img("create", "-f", "qcow2", img_tmp, self.size)
>           self.print_step("Booting installer")
>           self.boot(img_tmp, extra_args = [
> -            "-bios", "pc-bios/bios-256k.bin",
>               "-machine", "graphics=off",
>               "-device", "VGA",
>               "-cdrom", iso
> diff --git a/tests/vm/freebsd b/tests/vm/freebsd
> index 805db759d6..cd1fabde52 100755
> --- a/tests/vm/freebsd
> +++ b/tests/vm/freebsd
> @@ -95,7 +95,6 @@ class FreeBSDVM(basevm.BaseVM):
>   
>           self.print_step("Booting installer")
>           self.boot(img_tmp, extra_args = [
> -            "-bios", "pc-bios/bios-256k.bin",
>               "-machine", "graphics=off",
>               "-device", "VGA",
>               "-cdrom", iso
> diff --git a/tests/vm/netbsd b/tests/vm/netbsd
> index 45aa9a7fda..aa883ec23c 100755
> --- a/tests/vm/netbsd
> +++ b/tests/vm/netbsd
> @@ -86,7 +86,6 @@ class NetBSDVM(basevm.BaseVM):
>   
>           self.print_step("Booting installer")
>           self.boot(img_tmp, extra_args = [
> -            "-bios", "pc-bios/bios-256k.bin",
>               "-machine", "graphics=off",
>               "-cdrom", iso
>           ])
> diff --git a/tests/vm/openbsd b/tests/vm/openbsd
> index 13c8254214..6f1b6f5b98 100755
> --- a/tests/vm/openbsd
> +++ b/tests/vm/openbsd
> @@ -82,7 +82,6 @@ class OpenBSDVM(basevm.BaseVM):
>   
>           self.print_step("Booting installer")
>           self.boot(img_tmp, extra_args = [
> -            "-bios", "pc-bios/bios-256k.bin",
>               "-machine", "graphics=off",
>               "-device", "VGA",
>               "-cdrom", iso

Reviewed-by: Thomas Huth <thuth@redhat.com>
diff mbox series

Patch

diff --git a/tests/vm/fedora b/tests/vm/fedora
index 92b78d6e2c..12eca919a0 100755
--- a/tests/vm/fedora
+++ b/tests/vm/fedora
@@ -79,7 +79,6 @@  class FedoraVM(basevm.BaseVM):
         self.exec_qemu_img("create", "-f", "qcow2", img_tmp, self.size)
         self.print_step("Booting installer")
         self.boot(img_tmp, extra_args = [
-            "-bios", "pc-bios/bios-256k.bin",
             "-machine", "graphics=off",
             "-device", "VGA",
             "-cdrom", iso
diff --git a/tests/vm/freebsd b/tests/vm/freebsd
index 805db759d6..cd1fabde52 100755
--- a/tests/vm/freebsd
+++ b/tests/vm/freebsd
@@ -95,7 +95,6 @@  class FreeBSDVM(basevm.BaseVM):
 
         self.print_step("Booting installer")
         self.boot(img_tmp, extra_args = [
-            "-bios", "pc-bios/bios-256k.bin",
             "-machine", "graphics=off",
             "-device", "VGA",
             "-cdrom", iso
diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index 45aa9a7fda..aa883ec23c 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -86,7 +86,6 @@  class NetBSDVM(basevm.BaseVM):
 
         self.print_step("Booting installer")
         self.boot(img_tmp, extra_args = [
-            "-bios", "pc-bios/bios-256k.bin",
             "-machine", "graphics=off",
             "-cdrom", iso
         ])
diff --git a/tests/vm/openbsd b/tests/vm/openbsd
index 13c8254214..6f1b6f5b98 100755
--- a/tests/vm/openbsd
+++ b/tests/vm/openbsd
@@ -82,7 +82,6 @@  class OpenBSDVM(basevm.BaseVM):
 
         self.print_step("Booting installer")
         self.boot(img_tmp, extra_args = [
-            "-bios", "pc-bios/bios-256k.bin",
             "-machine", "graphics=off",
             "-device", "VGA",
             "-cdrom", iso