On 05/07/2015 12:40 PM, Andrea Bolognani wrote: > The guest firmware already provides the same functionality, so we can > just safely drop the <panic/> element from the domain definition. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1182388 <devices><panic> is about a specific device though, which qemu ppc doesn't support. Even if the firmware provides the logical feature (getting PANICKED notifications from qemu), it doesn't support the device or any explicit qemu CLI config, so IMO it's correct to reject <panic>. Apps/users will just have to take that into account, along with all the other XML differences for x86 vs ppc64. An alternative could be the unconditionally add <panic> to ppc64 XML since the logical feature is always available... but you'd probably have to invent a new <address> scheme or something Instead I think it's just a documentation patch. Another thing from that bug: The error message 'your QEMU is too old to support pvpanic' isn't accurate for non-x86, so it's probably better to change it to 'this QEMU binary does not support pvpanic' or similar, maybe look at existing error messages to find a better example. Another comment below > --- > src/conf/domain_conf.c | 17 +++++++----- > .../qemuxml2argv-pseries-panic.args | 7 +++++ > .../qemuxml2argv-pseries-panic.xml | 30 ++++++++++++++++++++++ > tests/qemuxml2argvtest.c | 2 ++ > .../qemuxml2xmlout-pseries-panic.xml | 29 +++++++++++++++++++++ > tests/qemuxml2xmltest.c | 1 + > 6 files changed, 80 insertions(+), 6 deletions(-) > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic.args > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic.xml > create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic.xml > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 4cd36a1..a7d4efa 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -15478,13 +15478,18 @@ virDomainDefParseXML(xmlDocPtr xml, > goto error; > } > if (n > 0) { > - virDomainPanicDefPtr panic = > - virDomainPanicDefParseXML(nodes[0]); > - if (!panic) > - goto error; > + /* Ignore the panic device on pSeries, as the guest > + * firmware already provides the same functionality */ > + if (!(ARCH_IS_PPC64(def->os.arch) && > + STRPREFIX(def->os.machine, "pseries"))) { > + virDomainPanicDefPtr panic = > + virDomainPanicDefParseXML(nodes[0]); > + if (!panic) > + goto error; > > - def->panic = panic; > - VIR_FREE(nodes); > + def->panic = panic; > + VIR_FREE(nodes); > + } > } > FWIW, since this is hypervisor specific, this type of stuff shouldn't be in the (intended to be) generic domain_conf.c. Check out qemu_domain.c:qemuDomainDefPostParse instead - Cole -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list