diff mbox series

[v3,3/3] tools: disable building qemu-trad per default

Message ID 20210910055518.562-4-jgross@suse.com (mailing list archive)
State New, archived
Headers show
Series disable building of pv-grub and qemu-trad per default | expand

Commit Message

Jürgen Groß Sept. 10, 2021, 5:55 a.m. UTC 
Using qemu-traditional as device model is deprecated for some time now.

So change the default for building it to "disable". This will affect
ioemu-stubdom, too, as there is a direct dependency between the two.

Today it is possible to use a PVH/HVM Linux-based stubdom as device
model. Additionally using ioemu-stubdom isn't really helping for
security, as it requires to run a very old and potentially buggy qemu
version in a PV domain. This is adding probably more security problems
than it is removing by using a stubdom.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Acked-by: Ian Jackson <iwj@xenproject.org>
---
V2:
- new patch
---
 CHANGELOG.md         |  3 +++
 stubdom/configure    |  8 --------
 stubdom/configure.ac |  8 +-------
 tools/configure      | 17 ++---------------
 tools/configure.ac   | 13 +------------
 5 files changed, 7 insertions(+), 42 deletions(-)

Comments

Ian Jackson Nov. 3, 2021, 12:54 p.m. UTC | #1 
Juergen Gross writes ("[PATCH v3 3/3] tools: disable building qemu-trad per default"):
> Using qemu-traditional as device model is deprecated for some time now.
> 
> So change the default for building it to "disable". This will affect
> ioemu-stubdom, too, as there is a direct dependency between the two.
> 
> Today it is possible to use a PVH/HVM Linux-based stubdom as device
> model. Additionally using ioemu-stubdom isn't really helping for
> security, as it requires to run a very old and potentially buggy qemu
> version in a PV domain. This is adding probably more security problems
> than it is removing by using a stubdom.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> Acked-by: Ian Jackson <iwj@xenproject.org>

Now that the relevant osstest patches are in and stable, I revisited
this.  The downside risk to the release is very modest.  It might
cause some temporary test breakage but is very easily reverted.

The upside is that this will accelerate the total removal of qemu-trad
by about one release cycle.  That is highly desirable.

Release-Acked-by: Ian Jackson <iwj@xenproject.org>

And pushed to staging.

Ian.
Ian Jackson Nov. 3, 2021, 3:20 p.m. UTC | #2 
Ian Jackson writes ("Re: [PATCH v3 3/3] tools: disable building qemu-trad per default"):
> Juergen Gross writes ("[PATCH v3 3/3] tools: disable building qemu-trad per default"):
> > Using qemu-traditional as device model is deprecated for some time now.
> > 
> > So change the default for building it to "disable". This will affect
> > ioemu-stubdom, too, as there is a direct dependency between the two.
> > 
> > Today it is possible to use a PVH/HVM Linux-based stubdom as device
> > model. Additionally using ioemu-stubdom isn't really helping for
> > security, as it requires to run a very old and potentially buggy qemu
> > version in a PV domain. This is adding probably more security problems
> > than it is removing by using a stubdom.
> 
> Now that the relevant osstest patches are in and stable, I revisited
> this.  The downside risk to the release is very modest.  It might
> cause some temporary test breakage but is very easily reverted.
...
> And pushed to staging.

Unfortunately this broke the gitlab CI:

https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/1743723306

  === configuring in tools (/builds/xen-project/people/andyhhp/xen/tools)
  configure: running /bin/sh ./configure --disable-option-checking '--prefix=/usr/local'  '--enable-docs' '--with-system-seabios=/usr/share/seabios/bios.bin' '--with-system-ipxe=/usr/lib/ipxe/ipxe.pxe' '--disable-stubdom' '--with-extra-qemuu-configure-args="--disable-werror"' '--with-system-seabios=/bin/false' --cache-file=/dev/null --srcdir=.
  configure: WARNING: Setting CC, CFLAGS, LDFLAGS, LIBS, CPPFLAGS or CPP is not recommended, use PREPEND_INCLUDES, PREPEND_LIB, APPEND_INCLUDES and APPEND_LIB instead when possible.
  checking build system type... x86_64-pc-linux-musl
  ...
  checking for _FILE_OFFSET_BITS value needed for large files... no
  configure: error: Rombios is required to use IPXE
  configure: error: ./configure failed for tools

The configure arguments look coherent so this is surely a bug in the
configure script.

Looking at the code, it seems that there is code to enable rombios if
$enable_qemu_traditional, but nothing for ipxe.  This seems like a
bug, which was presumably masked by qemu-trad being enabled by
default.

With my RM hat on: Unfortunately I think this means this patch needs
to be reverted right away, at least for now, since it is causing a
regression.  I will do that now.

I would be open to reapplying it after the default for rombios is made
to depend on ipxe too, and ideally if we can see that the CI is happy
with the branch.

> The upside is that this will accelerate the total removal of qemu-trad
> by about one release cycle.  That is highly desirable.

So FTAOD although I am reverting this now, I am still in favour of it.

Thanks,
Ian.
diff mbox series

Patch

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e7107ac3de..e5ab49e779 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,6 +18,9 @@  The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
    or by passing "iommu=quarantine=scratch-page" on the hypervisor command line.
  - pv-grub stubdoms will no longer be built per default. In order to be able to use pv-grub
    configure needs to be called with "--enable-pv-grub" as parameter.
+ - qemu-traditional based device models (both, qemu-traditional and ioemu-stubdom) will
+   no longer be built per default. In order to be able to use those, configure needs to
+   be called with "--enable-qemu-traditional" as parameter.
 
 ## [4.15.0 UNRELEASED](https://xenbits.xen.org/gitweb/?p=xen.git;a=shortlog;h=RELEASE-4.15.0) - TBD
 
diff --git a/stubdom/configure b/stubdom/configure
index df31532abb..07b709f998 100755
--- a/stubdom/configure
+++ b/stubdom/configure
@@ -2286,14 +2286,6 @@  fi
 # Check whether --enable-qemu-traditional was given.
 if test "${enable_qemu_traditional+set}" = set; then :
   enableval=$enable_qemu_traditional;
-else
-
-    case "$host_cpu" in
-        i[3456]86|x86_64)
-           enable_qemu_traditional="yes";;
-        *) enable_qemu_traditional="no";;
-    esac
-
 fi
 
 if test "x$enable_qemu_traditional" = "xyes"; then :
diff --git a/stubdom/configure.ac b/stubdom/configure.ac
index a07a1edae5..e20d99edac 100644
--- a/stubdom/configure.ac
+++ b/stubdom/configure.ac
@@ -27,13 +27,7 @@  AX_STUBDOM_DEFAULT_ENABLE([xenstorepvh-stubdom], [xenstorepvh])
 AX_STUBDOM_CONDITIONAL([vtpm-stubdom], [vtpm])
 AX_STUBDOM_CONDITIONAL([vtpmmgr-stubdom], [vtpmmgr])
 
-AC_ARG_ENABLE([qemu-traditional],,,[
-    case "$host_cpu" in
-        i[[3456]]86|x86_64)
-           enable_qemu_traditional="yes";;
-        *) enable_qemu_traditional="no";;
-    esac
-])
+AC_ARG_ENABLE([qemu-traditional])
 AS_IF([test "x$enable_qemu_traditional" = "xyes"], [
     qemu_traditional=y],[
     qemu_traditional=n
diff --git a/tools/configure b/tools/configure
index 33814b24b3..8bf8fe75b8 100755
--- a/tools/configure
+++ b/tools/configure
@@ -1502,8 +1502,8 @@  Optional Features:
   --disable-seabios       Disable SeaBIOS (default is ENABLED)
   --disable-golang        Disable Go tools (default is ENABLED)
   --enable-qemu-traditional
-                          Enable qemu traditional device model, (DEFAULT is on
-                          for Linux or NetBSD x86, otherwise off)
+                          Enable qemu traditional device model, (DEFAULT is
+                          off)
   --enable-rombios        Enable ROMBIOS, (DEFAULT is on if qemu-traditional
                           is enabled, otherwise off)
   --disable-ipxe          Enable in-tree IPXE, (DEFAULT is on if rombios is
@@ -4287,19 +4287,6 @@  LINUX_BACKEND_MODULES="`eval echo $LINUX_BACKEND_MODULES`"
 # Check whether --enable-qemu-traditional was given.
 if test "${enable_qemu_traditional+set}" = set; then :
   enableval=$enable_qemu_traditional;
-else
-
-    case "$host_cpu" in
-        i[3456]86|x86_64)
-           enable_qemu_traditional="yes";;
-        *) enable_qemu_traditional="no";;
-    esac
-    case "$host_os" in
-        freebsd*)
-           enable_qemu_traditional="no";;
-    esac
-
-
 fi
 
 if test "x$enable_qemu_traditional" = "xyes"; then :
diff --git a/tools/configure.ac b/tools/configure.ac
index 6414fcbb44..a713fd34d6 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -120,18 +120,7 @@  AC_SUBST(LINUX_BACKEND_MODULES)
 
 AC_ARG_ENABLE([qemu-traditional],
     AS_HELP_STRING([--enable-qemu-traditional],
-                   [Enable qemu traditional device model, (DEFAULT is on for Linux or NetBSD x86, otherwise off)]),,[
-    case "$host_cpu" in
-        i[[3456]]86|x86_64)
-           enable_qemu_traditional="yes";;
-        *) enable_qemu_traditional="no";;
-    esac
-    case "$host_os" in
-        freebsd*)
-           enable_qemu_traditional="no";;
-    esac
-
-])
+                   [Enable qemu traditional device model, (DEFAULT is off)]))
 AS_IF([test "x$enable_qemu_traditional" = "xyes"], [
 AC_DEFINE([HAVE_QEMU_TRADITIONAL], [1], [Qemu traditional enabled])
     qemu_traditional=y],[