+++ This bug was initially created as a clone of Bug #585108 +++

Description of problem:
As summary.

Version-Release number of selected component (if applicable):
virt-manager-0.8.4-1.el6.noarch
python-virtinst-0.500.3-1.el6.noarch
libvirt-0.8.0-3.el6.x86_64
libvirt-python-0.8.0-3.el6.x86_64

How reproducible:
Always

Steps to Reproduce:
1.Edit -> Host Details..., and Click on the 'Networks Interfaces' tab.
2.Click on the '+' symbol on the bottom left to configure a new network interface. 
3.Select 'Bond' in the 'Interface type' drop down list. Hit 'Forward'.
4.In the 'Name' field, enter 'bond0'.
5.Click on check box 'Activate now' with checked.
6.In the 'Choose interface to bond' field, clicking on check box 'eth0' with checked. Hit 'Finish'.
  
Actual results:
Error creating interface: '<type 'exceptions.RuntimeError'> Could not define interface: XML description for could not get interface XML description (netcf: XML invalid - Element interface failed to validate content
) is not well formed or invalid
Traceback (most recent call last):
  File "/usr/share/virt-manager/virtManager/createinterface.py", line 1156, in do_install
    self.interface.install(meter, create=activate)
  File "/usr/lib/python2.6/site-packages/virtinst/Interface.py", line 242, in install
    raise RuntimeError(_("Could not define interface: %s" % str(e)))
RuntimeError: Could not define interface: XML description for could not get interface XML description (netcf: XML invalid - Element interface failed to validate content
) is not well formed or invalid
'

Expected results:
The interface 'bond0' will be successfully created in the network interfaces list.

--- Additional comment from crobinso on 2010-04-30 10:06:22 EDT ---

Can you please attach the output of virt-manager --debug when reproducing?

--- Additional comment from nzhang on 2010-05-04 01:47:04 EDT ---

I have 2 ethernet on my machine, using eth1 with static ip.

# virt-manager --debug
2010-05-04 09:37:25,207 (virt-manager:161): Application startup
2010-05-04 09:37:25,342 (engine:338): About to connect to uris ['qemu:///system']
2010-05-04 09:37:25,376 (engine:628): window counter incremented to 1
2010-05-04 09:37:25,430 (connection:836): Scheduling background open thread for qemu:///system
2010-05-04 09:37:25,431 (connection:981): Background thread is running
2010-05-04 09:37:25,530 (connection:1019): Background open thread complete, scheduling notify
2010-05-04 09:37:25,531 (connection:1024): Notifying open result
2010-05-04 09:37:25,553 (connection:1032): qemu:///system capabilities:
<capabilities>

  <host>
    <cpu>
      <arch>x86_64</arch>
      <model>core2duo</model>
      <topology sockets='1' cores='2' threads='1'/>
      <feature name='lahf_lm'/>
      <feature name='xtpr'/>
      <feature name='cx16'/>
      <feature name='tm2'/>
      <feature name='est'/>
      <feature name='vmx'/>
      <feature name='ds_cpl'/>
      <feature name='pbe'/>
      <feature name='tm'/>
      <feature name='ht'/>
      <feature name='ss'/>
      <feature name='acpi'/>
      <feature name='ds'/>
    </cpu>
    <migration_features>
      <live/>
      <uri_transports>
        <uri_transport>tcp</uri_transport>
      </uri_transports>
    </migration_features>
    <topology>
      <cells num='1'>
        <cell id='0'>
          <cpus num='2'>
            <cpu id='0'/>
            <cpu id='1'/>
          </cpus>
        </cell>
      </cells>
    </topology>
    <secmodel>
      <model>selinux</model>
      <doi>0</doi>
    </secmodel>
  </host>

  <guest>
    <os_type>hvm</os_type>
    <arch name='i686'>
      <wordsize>32</wordsize>
      <emulator>/usr/libexec/qemu-kvm</emulator>
      <machine>rhel6.0.0</machine>
      <machine>pc-0.12</machine>
      <machine>pc-0.11</machine>
      <machine>pc-0.10</machine>
      <machine>isapc</machine>
      <machine canonical='rhel6.0.0'>pc</machine>
      <machine>rhel5.5.0</machine>
      <machine>rhel5.4.4</machine>
      <machine>rhel5.4.0</machine>
      <domain type='qemu'>
      </domain>
      <domain type='kvm'>
        <emulator>/usr/libexec/qemu-kvm</emulator>
      </domain>
    </arch>
    <features>
      <cpuselection/>
      <pae/>
      <nonpae/>
      <acpi default='on' toggle='yes'/>
      <apic default='on' toggle='no'/>
    </features>
  </guest>

  <guest>
    <os_type>hvm</os_type>
    <arch name='x86_64'>
      <wordsize>64</wordsize>
      <emulator>/usr/libexec/qemu-kvm</emulator>
      <machine>rhel6.0.0</machine>
      <machine>pc-0.12</machine>
      <machine>pc-0.11</machine>
      <machine>pc-0.10</machine>
      <machine>isapc</machine>
      <machine canonical='rhel6.0.0'>pc</machine>
      <machine>rhel5.5.0</machine>
      <machine>rhel5.4.4</machine>
      <machine>rhel5.4.0</machine>
      <domain type='qemu'>
      </domain>
      <domain type='kvm'>
        <emulator>/usr/libexec/qemu-kvm</emulator>
      </domain>
    </arch>
    <features>
      <cpuselection/>
      <acpi default='on' toggle='yes'/>
      <apic default='on' toggle='no'/>
    </features>
  </guest>

</capabilities>

2010-05-04 09:37:25,915 (connection:212): Using libvirt API for netdev enumeration
2010-05-04 09:37:25,917 (connection:251): Using libvirt API for mediadev enumeration
2010-05-04 09:37:29,140 (engine:628): window counter incremented to 2
2010-05-04 09:37:57,542 (Interface:237): Creating interface 'bond0' with xml:
<interface type='bond' name='bond0'>
  <start mode='none'/>
  <protocol family="ipv4">
    <ip address="192.168.0.1" prefix="24"/>
  </protocol>
  <bond>
    <interface type='ethernet' name='eth1'>
      <mac address='00:15:17:62:ae:e8'/>
      <protocol family='ipv6'>
        <ip address='fe80::215:17ff:fe62:aee8' prefix='64'/>
      </protocol>
    </interface>
  </bond>
</interface>

2010-05-04 09:37:57,587 (error:86): Uncaught Error: Error creating interface: 'Could not define interface: XML description for could not get interface XML description (netcf: XML invalid - Element interface failed to validate content
) is not well formed or invalid' : Error creating interface: '<type 'exceptions.RuntimeError'> Could not define interface: XML description for could not get interface XML description (netcf: XML invalid - Element interface failed to validate content
) is not well formed or invalid
Traceback (most recent call last):
  File "/usr/share/virt-manager/virtManager/createinterface.py", line 1156, in do_install
    self.interface.install(meter, create=activate)
  File "/usr/lib/python2.6/site-packages/virtinst/Interface.py", line 242, in install
    raise RuntimeError(_("Could not define interface: %s" % str(e)))
RuntimeError: Could not define interface: XML description for could not get interface XML description (netcf: XML invalid - Element interface failed to validate content
) is not well formed or invalid
'
2010-05-04 09:38:04,710 (engine:632): window counter decremented to 1
2010-05-04 09:38:06,382 (engine:632): window counter decremented to 0
2010-05-04 09:38:06,384 (engine:641): Exiting app normally.
Unhandled exception in thread started by <bound method Thread.__bootstrap of <Thread(Tick thread, stopped daemon 139813051336464)>>

--- Additional comment from crobinso on 2010-05-04 15:48:57 EDT ---

Well, the XML looks valid to me as far as I can tell, and libvirt doesn't seem to have any difficulties digesting it, so reassigning to netcf. The error could be virt-manager but I'll let the knowledgeable guys chime in.

--- Additional comment from lutter on 2010-05-04 20:41:24 EDT ---

The XML is actually not valid. If you run it through xmllint (e.g. saving it in /tmp/bond.xml and running 'xmllint -relaxng /usr/share/netcf/xml/interface.rng /tmp/bond.xml'), there's a number of errors in the XML.

The problems with the XML are
1. the bond element needs a bonding mode
2. the child of the bond element has to be either a miimon or arpmon element
3. you can't specify an IP inside the bonded interface

For example, this XML validates:

<interface type='bond' name='bond0'>
  <start mode='none'/>
  <protocol family="ipv4">
    <ip address="192.168.0.1" prefix="24"/>
  </protocol>
  <bond mode='balance-rr'>
    <miimon freq="100"/>
    <interface type='ethernet' name='eth1'>
      <mac address='00:15:17:62:ae:e8'/>
    </interface>
  </bond>
</interface>

Assigning back to virt-manager ;)

--- Additional comment from crobinso on 2010-05-13 10:59:18 EDT ---

The IP inside <bond> is the result of us just copying the full <interface> XML from eth1 and sticking it as a child of <bond>. I was hoping netcf could just throw out what it doesn't need rather than requiring a client to generate a minimal XML element: seems like extra difficulty for API users.

miimon or arpmon shouldn't be required: they aren't required in bonding initscripts as far as I can tell, they are opt in functionality. Omitting these should be fine.

initscripts also have a default value for bonding mode. If the user doesn't specify one, netcf should just defer to the system default. We explictly show this in the virt-manager UI: the default value we show is 'System default', so if the user doesn't know anything or care about bonding modes, they can know they are deferring to common case. The alternative here is to hardcode the system default into virt-manager/virtinst which doesn't sound optimal.

David, Laine, any thoughts on this? Basically I think the schema should be relaxed, and some notion added that an API user can defer to the system default.

--- Additional comment from laine on 2010-05-18 23:50:39 EDT ---

It looks like lutter wasn't Cc'ed at the time Cole asked the question in Comment 7, so I just added him.

If it can be verified that miimon/arpmon aren't required, and if making them optional doesn't create and backward compatibility problems, I wouldn't oppose making them optional.

As far as allowing the IP address inside <bond>, I don't think that's a good idea.

--- Additional comment from crobinso on 2010-05-19 11:04:01 EDT ---

> As far as allowing the IP address inside <bond>, I don't think that's a good
> idea.    

Why not? I'm having trouble thinking of a downside. The change would simplify the lives of API users, shouldn't add any user confusion, and presumably should be an easy change since aren't you guys validating against RNG?

If netcf doesn't change, then the libvirt netcf driver needs to be changed, since this isn't the libvirt way of doing things (throw out unknown/unneeded XML). netcf shouldn't be hamstrung by libvirt behavior, but libvirt should use the same XML handling semantics for interface XML that it uses for all other drivers.

--- Additional comment from crobinso on 2010-06-15 12:17:58 EDT ---

Reassigning to netcf. I think that's where this should be fixed, and I have posted netcf patches and tests upstream:

Make arp/mii info optional:
https://fedorahosted.org/pipermail/netcf-devel/2010-June/000419.html

Accept unused child interace XML:
https://fedorahosted.org/pipermail/netcf-devel/2010-June/000418.html

The former sounds uncontroversial.

If the latter is rejected for netcf, then this bug should be assigned to libvirt, since libvirt is happily accepting the XML virt-manager is generating, when it should be duplicating the netcf parsing functionality. Ideally in that case, libvirt would do the sanitization for us.

In general, I think netcf's mode of explicitly validating against the XML schema doesn't play well with libvirt's way of doing things. New libvirt XML elements are ignored by older libvirt versions, netcf won't work that way. Maybe netcf should offer a 'define' flag to not validate against the schema, which libvirt could then use.

--- Additional comment from laine on 2010-06-27 23:38:28 EDT ---

The first of the two patches listed above has been pushed to upstream netcf, and will be in the next release. The 2nd patch would be inconsistent with netcf' validation, but according to my tests (with the xml given above) is not required when working via libvirt's API, because libvirt already strips out the extra/unrecognized parts of the XML (as is libvirt's tradition).

So another netcf release, and rebase for RHEL6 should solve this problem.

--- Additional comment from laine on 2010-06-29 18:09:01 EDT ---

Here is the email of the patch posted upstream:

https://fedorahosted.org/pipermail/netcf-devel/2010-June/000419.html

Upstream commit is: 

commit 6d760fe7119e17965ed5bcfaba3be10cb2f78b22
Author: Cole Robinson <crobinso>
Date:   Mon Jun 14 17:25:30 2010 -0400

    rng: bond: Make miimon/arpmon optional
    
    Initscripts have defaults for these values, so if mii or arp isn't
    explicitly specified, just defer to system defaults.
    
    Add test files for implicit bonding defaults.