Re: [libvirt] [PATCH v2 2/2] qemu: implement debugcon-isa chardev

On Mon, 2019-02-11 at 08:53 +0000, Nikolay Shirokovskiy wrote: > 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 Okay, I see now why you'd want to be able to print one and not the other. However, right now if you have an address such as <address type='isa' iobase='0x402'/> it will be formatted as iobase=0x402,irq=0x0 and I don't quite see how we could stop formatting the unassigned attribute without breaking compatibility with that behavior. Unless of course we can prove that QEMU always defaults to 0x0 for both when not specified...

Another interesting fact I've noticed is that, unlike what happens for usb-serial, pci-serial and spapr-vio-serial, if you have <serial type='pty'> <target type='isa-serial'/> </serial> an appropriate <address> element will *not* be added by libvirt. That also seems wrong, but again unless QEMU defaults to 0x0 for those options I don't think we can quite fix it :(

-- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list