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

Re: [libvirt] [PATCH] domain_conf: Include the correct console alias



On 02.07.2013 13:27, Daniel P. Berrange wrote:
> On Fri, Jun 28, 2013 at 07:34:07PM +0200, Michal Privoznik wrote:
>> On 28.06.2013 17:01, Daniel P. Berrange wrote:
>>> On Fri, Jun 28, 2013 at 04:45:08PM +0200, Michal Privoznik wrote:
>>>> On 28.06.2013 15:55, Daniel P. Berrange wrote:
>>>>> On Fri, Jun 28, 2013 at 03:31:17PM +0200, Michal Privoznik wrote:
>>>>>> For some crazy backward compatibility, a console can by just an alias to
>>>>>> a serial device. This is detected in the XML formating function which
>>>>>> takes the values to format from corresponding serial device. Including
>>>>>> the device alias. This results in wrong alias being written into the XML
>>>>>> definition:
>>>>>>
>>>>>>     <console type='pty' tty='/dev/pts/5'>
>>>>>>       ...
>>>>>>       <alias name='serial0'/>
>>>>>>     </console>
>>>>>>
>>>>>> While holding the correct alias still in the memory, it doesn't matter.
>>>>>> However, it starts to matter as soon as libvirtd is restarted and the
>>>>>> (incorrect) alias is read from status file.
>>>
>>> I don't actually see this problem at all.
>>>
>>> Starting with a guest containing
>>>
>>>     <serial type='pty'>
>>>       <target port='0'/>
>>>     </serial>
>>>     <console type='pty'>
>>>       <target type='serial' port='0'/>
>>>     </console>
>>>
>>> After virsh start, it contains
>>>
>>>     <serial type='pty'>
>>>       <source path='/dev/pts/7'/>
>>>       <target port='0'/>
>>>       <alias name='serial0'/>
>>>     </serial>
>>>     <console type='pty' tty='/dev/pts/7'>
>>>       <source path='/dev/pts/7'/>
>>>       <target type='serial' port='0'/>
>>>       <alias name='serial0'/>
>>>     </console>
>>>
>>>
>>> The XML in $XDG_RUNTIME_DIR/libvirt/qemu/run/vm1.xml contains
>>> exactly the same.
>>>
>>> If i now kill libvirtd and restart it, the XML is still
>>> reported correctly.
>>>
>>> How do you reproduce the bug ?
>>
>> The problem is, the internal array (vm->def->consoles) doesn't contain
>> just 'consoleX' but 'serialX' too after libvirtd restarts. I've noticed
>> this while testing my chardev hotplug patches. If user wants to hotplug
>> a console, I need to iterate over vm->def->consoles to find the next
>> free X to generate the alias. I am using qemuDomainDeviceAliasIndex for
>> that. The function takes alias prefix as one of its arguments. However,
>> if the prefix doesn't match, an error is returned. And since the aliases
>> vary over libvirtd restarts I think that's the problem. Of course, I can
>> workaround it in my patches, but c'mon we want the libvirt source code
>> to be hack-less, don't we? :)
> 
> Ok, but that's just an internal issue not visible to the user. Your
> proposed patches are changing the user visible XML data for the alias
> name which is wrong.  So you need to find a way to address it without
> causing a change in the user visible XML, which is correct as it is
> now.
> 
> Daniel
> 

My first version didn't change the visible part of XML, just internal
alias representation instead.

Michal


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