Re: [libvirt] [PATCH v2 2/2] qemu: implement debugcon-isa chardev
On 08.02.2019 18:02, Andrea Bolognani wrote:
On Thu, 2019-02-07 at 14:31 +0300, Nikolay Shirokovskiy wrote:
[...]
> @@ -393,9 +393,11 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
> info->addr.ccw.ssid,
> info->addr.ccw.devno);
> } else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA) {
> - virBufferAsprintf(buf, ",iobase=0x%x,irq=0x%x",
> - info->addr.isa.iobase,
> - info->addr.isa.irq);
> + if (info->addr.isa.iobase)
> + virBufferAsprintf(buf, ",iobase=0x%x",
info->addr.isa.iobase);
> +
> + if (info->addr.isa.irq)
> + virBufferAsprintf(buf, ",irq=0x%x", info->addr.isa.irq);
It's entirely unclear to me why you're doing this. Can you please
provide some explanation?
Both irq and iobase are optional and value reserved for "not specified" is 0.
However we pass this 0 value as it was set explicitly to qemu. This is odd.
For example if we have 2 isa-serials with addresses without irq then both
have irq=0. Is this meaningful configuration?
Specifying irq for debugcon just does not work:
error: internal error: qemu unexpectedly closed the monitor: 2019-02-11T08:33:00.460078Z
qemu-kvm: -device isa-debugcon,chardev=charserial0,id=serial0,iobase=0x402,irq=0x0:
Property '.irq' not found
Nikolay
Also note that this won't just affect isa-debugcon but also any
other device with an ISA address, so I really don't think you can
just go ahead and change it without breaking existing guests.
[...]
> +++ b/tests/qemuxml2argvtest.c
> @@ -1490,6 +1490,7 @@ mymain(void)
> QEMU_CAPS_DEVICE_ISA_SERIAL);
> DO_TEST("pci-serial-dev-chardev",
> QEMU_CAPS_DEVICE_PCI_SERIAL);
> + DO_TEST("isa-serial-debugcon", NONE);
>
> DO_TEST("channel-guestfwd", NONE);
> DO_TEST_CAPS_VER("channel-unix-guestfwd", "2.5.0");
If you had included this hunk in the previous commit, then the
QEMU command line would have been generated right away... I don't
think this belongs in a separate commit.