Pino Toscano
2014-Apr-01 16:37 UTC
[Libguestfs] [PATCH] src/launch: improve the addition of the no-hpet option
Since HPET is specific to x86, we can safely add it its option only on
x86 and x86_64 when creating the libvirt XML (no more hitting the
launching failures due to that on other architectures).
Regarding the direct qemu launch, since qemu 1.1 (which is our current
minimum) "-ho-hpet" appears in the help only where actually supported,
so we could just checking for it and adding it only if present. This
should fix the architecture issues on this backend as well.
---
src/launch-direct.c | 12 +++---------
src/launch-libvirt.c | 6 ++++--
2 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/src/launch-direct.c b/src/launch-direct.c
index 37bf144..bb06c9e 100644
--- a/src/launch-direct.c
+++ b/src/launch-direct.c
@@ -471,15 +471,9 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
/* These are recommended settings, see RHBZ#1053847. */
ADD_CMDLINE ("-rtc");
ADD_CMDLINE ("driftfix=slew");
-#if !defined(__arm__) && !defined(__aarch64__) &&
!defined(__powerpc__)
- /* qemu-system-arm and qemu-system-ppc64 advertises the -no-hpet option
- * but if you try to use it, it usefully says:
- * "Option no-hpet not supported for this target".
- * Cheers qemu developers. How many years have we been asking for
- * capabilities? Could be 3 or 4 years, I forget.
- */
- ADD_CMDLINE ("-no-hpet");
-#endif
+ if (qemu_supports (g, data, "-no-hpet")) {
+ ADD_CMDLINE ("-no-hpet");
+ }
if (data->qemu_version_major < 1 ||
(data->qemu_version_major == 1 && data->qemu_version_minor
<= 2))
ADD_CMDLINE ("-no-kvm-pit-reinjection");
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index 8899b1b..4eba851 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -1042,9 +1042,11 @@ construct_libvirt_xml_cpu (guestfs_h *g,
} end_element ();
/* libvirt has a bug (RHBZ#1066145) where it adds the -no-hpet
- * flag on ARM & ppc64.
+ * flag on ARM & ppc64 (and possibly any architecture).
+ * Since hpet is specific to x86 & x86_64 anyway, just add it only
+ * for those architectures.
*/
-#if !defined(__arm__) && !defined(__aarch64__) &&
!defined(__powerpc__)
+#if defined(__i386__) || defined(__x86_64__)
start_element ("timer") {
attribute ("name", "hpet");
attribute ("present", "no");
--
1.9.0
Richard W.M. Jones
2014-Apr-01 17:55 UTC
[Libguestfs] [PATCH] src/launch: improve the addition of the no-hpet option
On Tue, Apr 01, 2014 at 06:37:40PM +0200, Pino Toscano wrote:> Since HPET is specific to x86, we can safely add it its option only on > x86 and x86_64 when creating the libvirt XML (no more hitting the > launching failures due to that on other architectures). > > Regarding the direct qemu launch, since qemu 1.1 (which is our current > minimum) "-ho-hpet" appears in the help only where actually supported, > so we could just checking for it and adding it only if present. This > should fix the architecture issues on this backend as well. > --- > src/launch-direct.c | 12 +++--------- > src/launch-libvirt.c | 6 ++++-- > 2 files changed, 7 insertions(+), 11 deletions(-) > > diff --git a/src/launch-direct.c b/src/launch-direct.c > index 37bf144..bb06c9e 100644 > --- a/src/launch-direct.c > +++ b/src/launch-direct.c > @@ -471,15 +471,9 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) > /* These are recommended settings, see RHBZ#1053847. */ > ADD_CMDLINE ("-rtc"); > ADD_CMDLINE ("driftfix=slew"); > -#if !defined(__arm__) && !defined(__aarch64__) && !defined(__powerpc__) > - /* qemu-system-arm and qemu-system-ppc64 advertises the -no-hpet option > - * but if you try to use it, it usefully says: > - * "Option no-hpet not supported for this target". > - * Cheers qemu developers. How many years have we been asking for > - * capabilities? Could be 3 or 4 years, I forget. > - */ > - ADD_CMDLINE ("-no-hpet"); > -#endif > + if (qemu_supports (g, data, "-no-hpet")) { > + ADD_CMDLINE ("-no-hpet"); > + } > if (data->qemu_version_major < 1 || > (data->qemu_version_major == 1 && data->qemu_version_minor <= 2)) > ADD_CMDLINE ("-no-kvm-pit-reinjection"); > diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c > index 8899b1b..4eba851 100644 > --- a/src/launch-libvirt.c > +++ b/src/launch-libvirt.c > @@ -1042,9 +1042,11 @@ construct_libvirt_xml_cpu (guestfs_h *g, > } end_element (); > > /* libvirt has a bug (RHBZ#1066145) where it adds the -no-hpet > - * flag on ARM & ppc64. > + * flag on ARM & ppc64 (and possibly any architecture). > + * Since hpet is specific to x86 & x86_64 anyway, just add it only > + * for those architectures. > */ > -#if !defined(__arm__) && !defined(__aarch64__) && !defined(__powerpc__) > +#if defined(__i386__) || defined(__x86_64__) > start_element ("timer") { > attribute ("name", "hpet"); > attribute ("present", "no");Looks OK in recent QEMU, therefore ACK. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top