[libvirt] [PATCH v2 0/2] add debugcon-isa chardev guest interface

Ján Tomko jtomko at redhat.com
Thu Feb 14 11:33:34 UTC 2019


On Wed, Feb 13, 2019 at 05:18:55PM +0100, Andrea Bolognani wrote:
>On Wed, 2019-02-13 at 13:29 +0100, Ján Tomko wrote:
>> On Wed, Feb 13, 2019 at 10:59:28AM +0100, Andrea Bolognani wrote:
>> > On Wed, 2019-02-13 at 10:03 +0100, Ján Tomko wrote:
>> > > Also, the device will be given an iobase by QEMU, we should represent
>> > > that in the XML and fill in that default.
>> >
>> > If we can manage to implement it in a way that's both reliable and
>> > backwards compatible, then I'm absolutely in favor of doing that! The
>> > current behavior is clearly not great.
>>
>> We cannot if any proposal for improvement is met with "it's already
>> ugly so we might keep doing it that way"
>
>I enthusiastically backed a proposal for improvement literally in the
>paragraph you're replying to :)
>
>> >  <controller type='pci' model='pcie-root-port'>
>> >    <model name='pcie-root-port'/>
>> >  </controller>
>>
>> While having a model attribute and a model element is ugly, despite
>> exaclty matching QEMU's device, this is just a different way of saying
>> a generic device (e.g. isa-serial in my example above)
>
>There are no "generic" devices, only hypervisor-specific devices that
>present a similar interface to the guest but are completely separate
>and not interchangeable from the hypervisor's point of view.
>
>For controllers, we could have decided to use "intel-pcie-root-port"
>instead of "ioh3420" and then translate back and forth, but I guess
>at the time it was considered to be not worth doing, or perhaps the
>lack of redundancy resulted in the issue not being raised at all.
>
>> > Either way, my point is that while the current serial device XML is a
>> > bit redundant, at least it's mostly consistent
>>
>> consistent meaning it matches the QEMU devices?
>
>Yes, thus matching how controllers (and possibly other devices?)
>work. When I introduced the <model> element for serial devices,
>that's what I based the idea on, so it makes sense to me that we'll
>keep following the same semantics.
>

Nonsense.

>> BTW if you look at the title of this thread, it says 'debugcon-isa', while
>> the QEMU device is named 'isa-debugcon'.
>
>Clearly an oversight: if you look through the patches, you'll see
>that there is no 'debucon-isa' anywhere in the code.
>
>> > and its implementation
>> > is very simple;
>>
>> Ah, the beauty of using virDomainChrSerialTargetModelTypeToString
>> instead of qemuDomainChrSerialTargetModelTypeToString
>
>What about the beauty of not having to implement
>qemuDomainChrSerialTargetModelTypeToString() in the first place? :)
>

Given that your only compelling argument is "it's extra work" I went
ahead and done the work:
https://www.redhat.com/archives/libvir-list/2019-February/msg00860.html

Jano

>> > what you suggest doing would compromise both of those
>> > positive facts without allowing us to remove any redundancy from the
>> > existing scenarios, so I think it would overall represent a net
>> > negative.
>>
>> It allows us not to introduce more redundancy.
>
>I don't believe avoiding those four extra bytes is worth the extra
>code complexity and deviating from well-established semantics.
>
>-- 
>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/20190214/db3271a7/attachment-0001.sig>


More information about the libvir-list mailing list