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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxxx>
>>> ---
>>>   * 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 ;)

>>
>> Martin
>>
>> -- 
>> libvir-list mailing list
>> libvir-list@xxxxxxxxxx
>> https://www.redhat.com/mailman/listinfo/libvir-list
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]