[libvirt] [PATCH] qemu: Copy console definition from serial

[Jan Kiszka]

On 2012-03-09 19:11, Jan Kiszka wrote:
> On 2012-03-09 19:02, Daniel P. Berrange wrote:
>> On Fri, Mar 09, 2012 at 06:58:06PM +0100, Jan Kiszka wrote:
>>> On 2012-03-09 18:53, Daniel P. Berrange wrote:
>>>> On Fri, Mar 09, 2012 at 06:48:42PM +0100, Jan Kiszka wrote:
>>>>> On 2011-11-16 14:14, Michal Privoznik wrote:
>>>>>> Now, when we support multiple consoles per domain,
>>>>>> the vm->def->console[0] can still remain an alias
>>>>>> for vm->def->serial[0]; However, we need to copy
>>>>>> it's source definition as well otherwise we'll regress
>>>>>> on virDomainOpenConsole.
>>>>>
>>>>> ...
>>>>>
>>>>>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>>>>>> index 2882ef8..e0b1824 100644
>>>>>> --- a/src/qemu/qemu_process.c
>>>>>> +++ b/src/qemu/qemu_process.c
>>>>>> @@ -1163,11 +1163,22 @@ qemuProcessFindCharDevicePTYs(virDomainObjPtr vm,
>>>>>>  
>>>>>>      for (i = 0 ; i < vm->def->nconsoles ; i++) {
>>>>>>          virDomainChrDefPtr chr = vm->def->consoles[i];
>>>>>> -        if (chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY &&
>>>>>> -            chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) {
>>>>>> -            if ((ret = qemuProcessExtractTTYPath(output, &offset,
>>>>>> -                                                 &chr->source.data.file.path)) != 0)
>>>>>> +        /* For historical reasons, console[0] can be just an alias
>>>>>> +         * for serial[0]; That's why we need to update it as well */
>>>>>> +        if (i == 0 && vm->def->nserials &&
>>>>>> +            chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
>>>>>> +            chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) {
>>>>>> +            if ((ret = virDomainChrSourceDefCopy(&chr->source,
>>>>>> +                                                 &((vm->def->serials[0])->source))) != 0)
>>>>>>                  return ret;
>>>>>> +            chr->source.type = VIR_DOMAIN_CHR_TYPE_PTY;
>>>>>
>>>>> This unconditional setting of TYPE_PTY breaks serial on stdio (we use
>>>>> this to easily fold guest into host logs). Can you explain why the
>>>>> copied source.type of serial[0] is not always correct? Or are we already
>>>>> in the wrong branch for a
>>>>>
>>>>>     <serial type='stdio'></serial>
>>>>>
>>>>> configuration?
>>>>
>>>> Yeah I think this is a bug. The first serial element should match the
>>>> first console exactly, with targetType==serial. We shouldn't be
>>>> forcing it to type=pty
>>>
>>> So, if vm->def->serials[0])->source.type != VIR_DOMAIN_CHR_TYPE_PTY, we
>>> should skip this fixup branch?
>>
>> No, we should do the fix for all types. serial[0] and console[0] should
>> be identical, unless the console[0] targetType != TARGET_TYPE_SERIAL
> 
> Well, libvirt is alien to me, and at this point I'm lost in fixups. :)
> Do you mean simply dropping the source.type = TYPE_PTY line?
> 

Ping?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux