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

[Boris Fiuczynski]

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 at linux.vnet.ibm.com>
>> Reviewed-by: Cornelia Huck <cornelia.huck at de.ibm.com>
>> ---
>>    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