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.