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

Andrea Bolognani / Red Hat / Virtualization

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