On Mon, Feb 11, 2019 at 06:23:32PM +0100, Andrea Bolognani wrote:
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 foundOkay, 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...
Setting IRQ to 0 is nonsense for ISA devices, why would we need to stay compatible with that?
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 :(
IMO if the user does not fill it, they are okay with the default. And we should reflect the default in the XML (IOW fill it with what QEMU fills in right now). Jano
-- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: PGP signature
