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.