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

[Daniel P. Berrange]

On Tue, Jul 02, 2013 at 01:49:35PM +0200, Michal Privoznik wrote:
> 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.

Hmm, ok the commit message was a bit misleading - it looked like you
were saying the XML was wrong & the patch was changing it.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|