On Mon, Jun 26, 2017 at 10:10:55AM -0400, Cole Robinson wrote: > On 06/24/2017 10:07 PM, Andrea Bolognani wrote: > > On Sat, 2017-06-24 at 16:07 +0200, Christoffer Dall wrote: > >> At this point I'm a little confused about how to proceed here. Would > >> you like further evidence of an environment that reproduces the issue > >> with console and the isa bus, with additional logic added to this > >> patch to fix that, or should we get this patch merged and fix the > >> other issue separately? > > > > We can merge the patch without further changes to it, as > > it fixes part of the issues that prevent the feature to work. > > > > Actually, I just added > > > > Reviewed-by: Andrea Bolognani <abologna@xxxxxxxxxx> > > > > and pushed it :) > > It may fix part of the issue but it likely breaks all existing aarch64 -M virt > libvirt VMs. My VM created by virt-manager has: > > <serial type='pty'> > <target port='0'/> > </serial> > <console type='pty'> > <target type='serial' port='0'/> > </console> > > And after this patch fails with: > > error: Failed to start domain fedora25-aarch64 > error: internal error: process exited while connecting to monitor: > 2017-06-26T13:55:34.726293Z qemu-system-aarch64: -chardev pty,id=charserial0: > char device redirected to /dev/pts/5 (label charserial0) > 2017-06-26T13:55:34.782121Z qemu-system-aarch64: -device > isa-serial,chardev=charserial0,id=serial0: No 'ISA' bus found for device > 'isa-serial' > > There's a few things going on here: > > - Internally libvirt thinks that by default <serial> is isa-serial > - For arm qemu though it's actually a pl011. This is a platform device and as > such there isn't any current way to use -chardev with it AFAIK. There is, -chardev pty,id=chardev0,logfile=/my/log/file \ -serial chardev:chardev0 > - However -chardev will work for pci-serial device, which is <serial><target > type='serial'/> This works, but would require the guest kernel to have console=ttyS0 on its command line, and adds an unnecessary extra serial device to the guest, as it already has the PL011. > > So for one, we should change SupportsChardev to also check the devices target > type. The test suite likely needs to be extended to add QEMU_CAPS_CHARDEV for > all arm/aarch64 test cases, to demonstrate the change. And describe the > existing logic by adding a TARGET_TYPE_PL011 which is filled in as the default > for arm/aarch64, and then we can key off that in the code. > > After that I think the options are either: > > 1) Openstack/other tools specifies target type='pci' for PCIe machvirt, to > make <log> work > > 2) Change the libvirt serial default to target type=pci for PCIe machvirt. Not > sure if they are operationally equivalent for all cases we care about though. > Maybe we could make some kind of adaptive default like 'if PCIe machvirt && > serial device has <log> specified (or other features) then default target > type=pci', but maybe that's too magic 3) magic, but magic that enables the PL011 to be the one and only default serial console device for mach-virt. The magic I advocate using is the -serial 'chardev:<chardev>' shown above. > > Would also be nice to have the libvirt output XML always list the serial > target type which would clear up some of this confusion, but might cause > migration XML compat issues for other archs > > In the interim though I think this patch should be reverted. Reverted, or quickly completed. It's not wrong, but apparently it needs to also ensure pci-serial is chosen for ARM guest targets. Thanks, drew -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list