Bug 547045

Summary: libvirt can generate bogus node device XML
Product: [Fedora] Fedora Reporter: Eike Hein <hein>
Component: libvirtAssignee: Daniel Veillard <veillard>
Status: CLOSED UPSTREAM QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: 12CC: berrange, clalance, crobinso, hbrock, itamar, jforbes, sven, veillard, virt-maint
Target Milestone: ---Keywords: Triaged
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-02-10 04:37:54 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On:    
Bug Blocks: 546327    
Attachments:
Description Flags
virt-manager log showing Python backtrace
none
virsh nodedev-list --tree output.
none
for n in `virsh nodedev-list`; do virsh nodedev-dumpxml $n; done > nodedev-xml.log 2>&1 output. none

Description Eike Hein 2009-12-13 05:50:47 EST
Created attachment 377970 [details]
virt-manager log showing Python backtrace

Description of problem:
On a x86_64 Fedora 12 box with updates and updates-testing enabled and upgraded to the latest versions of everything, virt-manager fails to list the VMs on the local system. $HOME/.virt-manager/virt-manager.log contains a Python traceback showing a script stumbling over an XML parsing problem; the log file is attached to this bug report.

Downgrading to virt-manager-0.8.0-7.fc12.noarch from updates fixes the problem again.

Version-Release number of selected component (if applicable):
Name       : virt-manager
Arch       : noarch
Version    : 0.8.1
Release    : 3.fc12
Size       : 4.8 M
Repo       : installed
From repo  : updates-testing
Summary    : Virtual Machine Manager

How reproducible:
Always

Steps to Reproduce:
1. Start libvirtd.
2. Start virt-manager.

Actual results:
No VMs below "localhost (QEMU)".

Expected results:
Seeing my VMs below "localhost (QEMU)" as I used to with older versions of virt-manager such as virt-manager-0.8.0-7.fc12.noarch.
Comment 1 Cole Robinson 2009-12-13 12:03:27 EST
Huh, interesting. Backtrace is:

[Sun, 13 Dec 2009 11:44:08 virt-manager 15216] ERROR (virt-manager:162) Traceback (most recent call last):
  File "/usr/share/virt-manager/virtManager/connection.py", line 931, in _open_notify
    self.tick()
  File "/usr/share/virt-manager/virtManager/connection.py", line 1331, in tick
    (oldNodedevs, newNodedevs, self.nodedevs) = self._update_nodedevs()
  File "/usr/share/virt-manager/virtManager/connection.py", line 1196, in _update_nodedevs
    check_obj(name)
  File "/usr/share/virt-manager/virtManager/connection.py", line 1183, in check_obj
    vdev = virtinst.NodeDeviceParser.parse(obj.XMLDesc(0))
  File "/usr/lib/python2.6/site-packages/virtinst/NodeDeviceParser.py", line 443, in parse
    return _util.parse_node_helper(xml, "device", _parse_func)
  File "/usr/lib/python2.6/site-packages/virtinst/_util.py", line 406, in parse_node_helper
    raise exec_class("%s\n%s" % (e, error.msg))
ValueError: xmlReadMemory() failed
Entity: line 12: parser error : PCDATA invalid Char value 16
    <description> ̄￯メ|￴</description>
                           ^
Entity: line 12: parser error : PCDATA invalid Char value 18
    <description> ̄￯メ|￴</description>

ccing DV since this is an error coming from libxml.

Have you manually added a <description> field to your VMs? How did you do so? (manually editting /etc/libvirt xml files or using virsh edit)? Can you provide 'virsh dumpxml' of any VM with a description field?
Comment 2 Cole Robinson 2009-12-13 12:04:19 EST
(In reply to comment #1)

> Have you manually added a <description> field to your VMs? How did you do so?
> (manually editting /etc/libvirt xml files or using virsh edit)? Can you provide
> 'virsh dumpxml' of any VM with a description field?  

That should say, provide 'virsh dumpxml' of ALL VMs with a description field.
Comment 3 Eike Hein 2009-12-13 12:31:25 EST
My VM (the plural in the initial report was erroneous) doesn't have a description field :-).

[root@ehw1 /etc/libvirt 44K]# ls qemu/
Gentoo.xml  networks
[root@ehw1 /etc/libvirt 44K]# grep -inR "description" *
[root@ehw1 /etc/libvirt 44K]#
Comment 4 Cole Robinson 2009-12-13 13:05:07 EST
*slaps forehead*, sorry, I misread the backtrace. Something is going on here and preventing us from parsing node device XML.

Can you provide the output of (run as root):

virsh nodedev-list --tree

for n in `virsh nodedev-list`; do virsh nodedev-dumpxml $n; done > nodedev-xml.log 2>&1
Comment 5 Eike Hein 2009-12-13 13:35:16 EST
Created attachment 378045 [details]
virsh nodedev-list --tree output.
Comment 6 Eike Hein 2009-12-13 13:36:15 EST
Created attachment 378046 [details]
for n in `virsh nodedev-list`; do virsh nodedev-dumpxml $n; done > nodedev-xml.log 2>&1 output.
Comment 7 Cole Robinson 2009-12-13 13:54:17 EST
Thanks for the quick response. The offending output is:

<device>
  <name>usb_device_5e3_70e_000000325491_if0</name>
  <parent>usb_device_5e3_70e_000000325491</parent>
  <driver>
    <name>usb-storage</name>
  </driver>
  <capability type='usb'>
    <number>0</number>
    <class>8</class>
    <subclass>6</subclass>
    <protocol>80</protocol>
    <description> ̄￯メ|￴</description>
  </capability>
</device>

Seems like libvirt shouldn't be generating XML that xmlReadMemory will choke on. Reassigning to libvirt. DV, any ideas how to properly handle this? Is there some function we can use to santize the description string, or check for invalid data and just throw it out when dumping the XML?
Comment 8 Eike Hein 2009-12-13 14:04:03 EST
> Thanks for the quick response.

Thanks for working on a Sunday ;-).
Comment 9 Daniel Berrange 2009-12-13 14:48:02 EST
libvirt has a function for escaping XML strings that we clearly forgot to use here. In addition though we probably need to check where this garbage is coming from, because in this particular case we probably shouldn't be including the element at all.
Comment 10 Cole Robinson 2009-12-15 14:01:51 EST
FYI, virt-manager-0.8.2 in F12 updates-testing should no longer fail here: we catch the error and just move along. The libvirt bug still remains, but at least virt-manager should be usable now.

You can get this version by doing

yum --enablerepo=updates-testing update virt-manager
Comment 11 Eike Hein 2009-12-15 14:10:10 EST
Looks like my mirror hasn't synced the latest push yet; will retry later or tomorrow (CET here).
Comment 12 Eike Hein 2009-12-16 09:20:51 EST
0.8.2 indeed works fine for me, thanks!

Since this was reassigned to libvirt due to the underlying issue, I won't close from my end.
Comment 13 Daniel Veillard 2010-02-10 04:37:54 EST
The escaping problem in libvirt has now been pushed upstream,

  thanks !

Daniel