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

[Ján Tomko]

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 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...

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 at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190212/b62651de/attachment-0001.sig>