Re: [PATCH v2 1/3] qemu: add panic device support for S390

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/27/2016 05:02 PM, Andrea Bolognani wrote:
On Fri, 2016-04-15 at 10:20 +0200, Boris Fiuczynski wrote:
If a panic device is being defined without a model in a domain
the default value is always overwritten with model ISA. An ISA
bus does not exist on S390 and therefore specifying a panic device
results in an unsupported configuration.
Since the S390 architecture inherently provides a crash detection
capability the panic device should be defined in the domain xml.

This patch adds an s390 panic device model and prevents setting a
device address on it.

Signed-off-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Cornelia Huck <cornelia.huck@xxxxxxxxxx>
---
   docs/formatdomain.html.in     |  7 ++++++-
   docs/schemas/domaincommon.rng |  1 +
   src/conf/domain_conf.c        |  3 ++-
   src/conf/domain_conf.h        |  1 +
   src/qemu/qemu_command.c       | 21 ++++++++++++++++++++-
   src/qemu/qemu_domain.c        |  2 ++
   6 files changed, 32 insertions(+), 3 deletions(-)

Sorry for taking so long to reply.

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index c2955eb..10c27fb 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6242,6 +6242,9 @@ qemu-kvm -net nic,model=? /dev/null
         For pSeries guests, this feature is always enabled since it's
         implemented by the guest firmware, thus libvirt automatically
         adds the <code>panic</code> element to the domain XML.
+      For S390 guests, this feature is always enabled since it's an
+      integral part of the S390 architecture, thus libvirt automatically
+      adds the <code>panic</code> element to the domain XML.

Shouldn't that be "S/390" instead of "S390"?
I rather stick with the generic term S390 and fix the mismatching occurrences in the document accordingly in a "tiny" separate patch.


Just like we use "pSeries". Same for the commit message and
in the remaining patches.

This paragraph should also be merged with the existing one, as
they're basically identical. Tweak the existing text as needed.
OK


@@ -9034,7 +9053,7 @@ qemuBuildPanicCommandLine(virCommandPtr cmd,
               if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) {
                   virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                                  _("the QEMU binary does not support the "
-                                 "panic device"));
+                                 "ISA panic device"));

This should be a separate commit. A very tiny one ;)
Sure


Everything else looks good.

--
Andrea Bolognani
Software Engineer - Virtualization Team



--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]