[libvirt] [PATCH v2 1/1] Backcompt for console devices in virDomainDeviceInfoIterate

[Li Zhang]

On Wed, Sep 12, 2012 at 4:43 PM, Martin Kletzander <mkletzan at redhat.com> wrote:
> On 09/12/2012 10:32 AM, Osier Yang wrote:
>> On 2012年09月12日 16:06, Martin Kletzander wrote:
>>> On 09/11/2012 04:57 AM, Li Zhang wrote:
>>>> Histrically, the first<console>  element is treated as the
>>>
>>> s/Histrically/Historically/
>>>
>>>> alias of a<serial>  device. In the virDomainDeviceInfoIterate,
>>>> This situation is not considered. It still handles the first<console>
>>>> element as another devices, which means that for console[0] with
>>>> serial targetType, it calls callback function another time.
>>>> It will cause the problem of address conflicts when assigning
>>>> spapr-vio address for serial device on pSeries guest.
>>>>
>>>> For pSeries guest, the serial configuration in the xml file
>>>> is as the following:
>>>>           <serial type='pty'>
>>>>                 <target port='0'/>
>>>>                 <address type='spapr-vio'/>
>>>>            </serial>
>>>>
>>>> Console configuration is default, the dumped xml file is as the
>>>> following:
>>>>     <serial type='pty'>
>>>>        <source path='/dev/pts/5'/>
>>>>        <target port='0'/>
>>>>        <alias name='serial0'/>
>>>>        <address type='spapr-vio' reg='0x30000000'/>
>>>>      </serial>
>>>>      <console type='pty' tty='/dev/pts/5'>
>>>>        <source path='/dev/pts/5'/>
>>>>        <target type='serial' port='0'/>
>>>>        <alias name='serial0'/>
>>>>        <address type='spapr-vio' reg='0x30000000'/>
>>>>      </console>
>>>>
>>>> It shows that the<console>  device is the alias of serial device.
>>>> So its address is the same as the serial device. When dectecting
>>>
>>> s/dectecting/detecting/
>>>
>>>> the conflicts in the qemuAssignSpaprVIOAddress the first console
>>>> and the serial device conflicts because virDomainDeviceInfoIterate()
>>>> still handle these as two different devices, and in the
>>>> qemuAssignSpaprVIOAddress(),
>>>> it will compare these two devices' addressed. If they have same address,
>>>> it will report address confilict error.
>>>
>>> s/confilict/conflict/
>>>
>>>>
>>>> So this patch is to handle the first console which targetType is serial
>>>> as the alias of serial device to avoid address conflicts error reported.
>>>>
>>>
>>> I'm wondering if this is the place we want to fix it. If somebody wants
>>> to use the iterate function for something else, they must accept that
>>> the fact that the first serial console will be skipped. OTOH it is
>>> common (and the only supported way) to have the serial console.
>>>
>>>> Signed-off-by: Li Zhang<zhlcindy at linux.vnet.ibm.com>
>>>> ---
>>>>   * v1 ->  v2:
>>>>     Rebase v1 on latest version of libvirt.
>>>>
>>>>   src/conf/domain_conf.c |    3 +++
>>>>   1 files changed, 3 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>>> index 8952b69..0e71b06 100644
>>>> --- a/src/conf/domain_conf.c
>>>> +++ b/src/conf/domain_conf.c
>>>> @@ -2054,6 +2054,9 @@ int virDomainDeviceInfoIterate(virDomainDefPtr
>>>> def,
>>>>               return -1;
>>>>       }
>>>>       for (i = 0; i<  def->nconsoles ; i++) {
>>>> +        if ((STREQ(def->os.type, "hvm"))&&  i == 0&&
>>>> +            def->consoles[i]->targetType ==
>>>> VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL)
>>>> +            continue;
>>>>           device.data.chr = def->consoles[i];
>>>>           if (cb(def,&device,&def->consoles[i]->info, opaque)<  0)
>>>>               return -1;
>>>>
>>>
>>> I was thinkinf about fixing it in the callback function, configuration
>>> parser and so on, but this looks like the best solution, so ACK from me,
>>> is it OK if I push this with the address in Cc?
>>
>> It's the one listed in AUTHORS. So should be fine.
>>
>
> That's why I asked, because the one from 'From:' is not there. OK than,
> pushed with fixes, thanks both of you ;)
>
Oh, you have pushed. :)
Thanks.
>>>
>>> Martin
>>>
>>> --
>>> libvir-list mailing list
>>> libvir-list at redhat.com
>>> https://www.redhat.com/mailman/listinfo/libvir-list
>>
>



-- 

Best Regards
-Li