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 qemuDomainChrSerialTargetModelTypeToStringWhat 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@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: PGP signature
