On Sun, 2015-05-10 at 17:50 -0400, Cole Robinson wrote:
<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.
I really like the alternative approach you suggested: it's the same
feature after all, so the XML should be the same regardless of how it's
actually implemented for the specific machine type.
I've reworked my patch accordingly, I'm going to send a v2 in a few
minutes. Please let me know what you think of it.
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.
I agree. I feel it belongs to a different commit, though.
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
My mistake. Thanks both to you and Daniel for pointing that out.
Cheers.
--
Andrea Bolognani <abologna(a)redhat.com>