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(a)redhat.com>
>
> 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