[libvirt] [PATCH] qemu: Fix specifying char devs for PPC

[Cole Robinson]

On 05/20/2014 01:25 AM, Hong-Hua.Yin at freescale.com wrote:
> Hi Cole,
> 
> Thanks for the comments.
> Exactly there were already test cases for both pseries and ppce500 machines.
> For example,
> 1) qemuxml2argv-pseries-basic.args: 	'-chardev spapr-vty' for pseries.
> 2) qemuxml2argv-ppc-dtb.args: 	 	'-serial pty' for ppce500.
> 

If there's already test cases, then what does the patch actually change? What
I'm suggesting is to add a test case that would fail before this patch, and
succeed afterwards, to demonstrate what is actually changing.

- Cole


> 
> 
>> -----Original Message-----
>> From: Cole Robinson [mailto:crobinso at redhat.com]
>> Sent: Monday, May 19, 2014 9:40 PM
>> To: Yin Olivia-R63875; libvir-list at redhat.com; zhlcindy at linux.vnet.ibm.com
>> Subject: Re: [PATCH] qemu: Fix specifying char devs for PPC
>>
>> On 05/19/2014 03:41 AM, Hong-Hua.Yin at freescale.com wrote:
>>> Hi Cole,
>>>
>>> This is a patch similar with your previous patch to fix for ARM.
>>> Do you have any comments on it?
>>>
>>> Cindy,
>>> Since you are the main contributor to QEMU driver on PPC, I'll also
>> appreciate your comments.
>>>
>>> Best Regards,
>>> Olivia
>>>
>>
>> Patch looks fine, but it should add a qemuxml2argv test case to validate
>> that it actually works. My original patch added test cases later in the
>> patch series, see 54a77c6df3c483864463f602c4c6f435d50bd79e
>>
>> - Cole
>>
>>>> -----Original Message-----
>>>> From: Yin Olivia-R63875
>>>> Sent: Friday, May 16, 2014 8:38 AM
>>>> To: Yin Olivia-R63875; libvir-list at redhat.com
>>>> Subject: RE: [PATCH] qemu: Fix specifying char devs for PPC
>>>>
>>>> Ping.
>>>>
>>>> This is a patch similar with ARM platforms.
>>>> http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=3a2beaee1d50dc96
>>>> 8171c5
>>>> 84ec2edcfdcb8fadde
>>>>
>>>> Right now on ppce500, chardev is not supported for the serial
>>>> console. So it uses the the legacy -serial option.
>>>>
>>>> Best Regards,
>>>> Olivia
>>>>
>>>>> -----Original Message-----
>>>>> From: Olivia Yin [mailto:Hong-Hua.Yin at freescale.com]
>>>>> Sent: Wednesday, May 14, 2014 6:47 PM
>>>>> To: libvir-list at redhat.com
>>>>> Cc: Yin Olivia-R63875
>>>>> Subject: [PATCH] qemu: Fix specifying char devs for PPC
>>>>>
>>>>> QEMU ppce500 board uses the old style -serial options.
>>>>>
>>>>> Other PPC boards don't give any way to explicitly wire in a -chardev
>>>>> except pseries which uses -device spapr-vty with -chardev.
>>>>>
>>>>> ---
>>>>>  src/qemu/qemu_capabilities.c | 10 +++++++---
>>>>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/src/qemu/qemu_capabilities.c
>>>>> b/src/qemu/qemu_capabilities.c index b491f58..fe5dd19 100644
>>>>> --- a/src/qemu/qemu_capabilities.c
>>>>> +++ b/src/qemu/qemu_capabilities.c
>>>>> @@ -3460,13 +3460,17 @@ virQEMUCapsSupportsChardev(virDomainDefPtr def,
>>>>>          !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE))
>>>>>          return false;
>>>>>
>>>>> -    if ((def->os.arch != VIR_ARCH_ARMV7L) && (def->os.arch !=
>>>>> VIR_ARCH_AARCH64))
>>>>> +    if ((def->os.arch != VIR_ARCH_ARMV7L) && (def->os.arch !=
>>>>> VIR_ARCH_AARCH64)
>>>>> +	 && (def->os.arch != VIR_ARCH_PPC) && (def->os.arch !=
>>>>> +VIR_ARCH_PPC64))
>>>>>          return true;
>>>>>
>>>>>      /* This may not be true for all ARM machine types, but at least
>>>>>       * the only supported non-virtio serial devices of vexpress and
>>>>> versatile
>>>>> -     * don't have the -chardev property wired up. */
>>>>> +     * don't have the -chardev property wired up.
>>>>> +     * For PPC machines, only pserial need -device spapr-vty with
>>>>> + -chardev */
>>>>>      return (chr->info.type ==
>>>>> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO
>>>>> ||
>>>>>              (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
>>>>> -             chr->targetType ==
>>>>> VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO));
>>>>> +             chr->targetType ==
>>>>> + VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO)
>>>>> ||
>>>>> +            (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
>>>>> +             chr->info.type ==
>>>>> + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO));
>>>>>  }
>>>>> --
>>>>> 1.8.5
>>>
>