Re: [libvirt] [PATCH] qemu: panic device: check for invalid address type

On Mon, Sep 08, 2014 at 12:27:23PM +0200, Erik Skultety wrote: >qemu now checks for invalid address type for a panic device, which is >currently implemented only to use ISA address type, thus rejecting >any other options, except for leaving XML attributes blank, in that case, >defaults are used (this behaviour remains the same from earlier verions). >--- > src/qemu/qemu_command.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > >diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c >index 1ca98fb..2611799 100644 >--- a/src/qemu/qemu_command.c >+++ b/src/qemu/qemu_command.c >@@ -9418,12 +9418,16 @@ qemuBuildCommandLine(virConnectPtr conn, > > if (def->panic) { > if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) { >- if (def->panic->info.addr.isa.iobase > 0) { >+ if (def->panic->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA) { > virCommandAddArg(cmd, "-device"); > virCommandAddArgFormat(cmd, "pvpanic,ioport=%d", > def->panic->info.addr.isa.iobase); >- } else { >+ } else if (def->panic->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { Long line. > virCommandAddArgList(cmd, "-device", "pvpanic", NULL); >+ } else { >+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >+ _("panic device only works with ISA address type")); Long line. I think it would express the problem more closely if you would do: s/only works/is supported only/ ACK with that and lines wrapped. I'll push it if you're OK with it. Martin