[libvirt] [PATCH] qemu: panic device: check for invalid address type
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) { virCommandAddArgList(cmd, "-device", "pvpanic", NULL); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("panic device only works with ISA address type")); + goto error; } } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", -- 1.9.3
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
On Mon, Sep 08, 2014 at 01:39:37PM +0200, Martin Kletzander wrote:
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
Erik agreed to the changes privately, so I pushed the patch. I also added a link to the public BZ this is supposed to fix: https://bugzilla.redhat.com/show_bug.cgi?id=1138125 Martin
participants (2)
-
Erik Skultety -
Martin Kletzander