[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 17/21] conf: Add target type and model for pl011



On Fri, Nov 24, 2017 at 11:26:32AM +0100, Andrea Bolognani wrote:
> On Thu, 2017-11-23 at 17:43 +0100, Pavel Hrdina wrote:
> > On Tue, Nov 21, 2017 at 05:42:27PM +0100, Andrea Bolognani wrote:
> > > We can finally introduce a specific target model for the pl011 device
> > > used by mach-virt guests, which means isa-serial will no longer show
> > > up to confuse users.
> > > 
> > > We make sure migration works in both directions by interpreting the
> > > isa-serial target type, or the lack of target type, appropriately
> > > when parsing the guest XML, and skipping the newly-introduced type
> > > when formatting if for migration. We also verify that pl011 is not
> > > used for non-mach-virt guests and add a bunch of test cases.
> > > 
> > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=151292
> > 
> > I'm not sure that 'system' is a good name for serial type for mach-virt,
> > it kind of feels like too generic name.  Can we use the "apb-serial" for
> > mach-virt?  I understand that the "apb" name might not be well known but
> > at least people that work closely with ARM machines have a chance to
> > understand it better than "system" name.
> 
> I was discussing the naming issue with a QEMU developer focusing on
> aarch64 and he admitted never hearing about "APB" at all ;)
> 
> Apparently that's a really low-level implementation detail, which is
> several layer down from a serial device.
> 
> I know "system" is extremely generic and I don't like it either
> because of that, but it's what the bus is commonly called among
> people working on mach-virt, so it's probably the only reasonable
> option :(
> 
> > Either way, we should document
> > it better than just listing it as a valid value for type, at least
> > mention that it's valid for ARM/mach-virt machine and that it's a system
> > bus or something like that.  The standalone "system" doesn't make it
> > clear what it actually is.
> 
> Do you mean in the code (in which case, where exactly?) or in the
> documentation? Usually we just list the available values without
> much of an explanation, and I kinda like that because it means we
> don't need to duplicate in writing the kind of checks that we have
> already implemented in code, but in this case the options should
> change infrequently enough and be specific enough that mentioning
> at least some of the constraints in the documentation as well.

In documentation, the "system" name is meaningless without mentioning
that it is a system bus for mach-virt.  It might be clear to mach-virt
people but for majority users it will be confusing.  The implementation
is useless to users as well :).

Pavel

Attachment: signature.asc
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]