Re: [Libvirt-cim] [PATCH] [TEST][Resubmitting]Adding 05_RAPF_err.py to verify RAPF

> +def setup_env(): > + vsxml_info = None > + if virt == "Xen": > + test_disk = "xvda" > + else: + test_disk = "hda" > + > + virt_xml = get_class(virt) > + vsxml_info = virt_xml(test_dom, vcpus = test_vcpus, mac = test_mac, > disk = test_disk) > + bridge = vsxml_info.set_vbridge(server) I agree with Dan's comment on the first version of this patch: + if virt != 'XenFV': + bridge = vsxml.set_vbridge(server) Having this as a special case is confusing. However, the new version of the patch has a bug. This test works fine for Xen, but it does not work for KVM or XenFV. Please be sure to run the test against all of the platforms the test supports before submitting.

The reason this fails for KVM and XenFV is because the bridge is already defined, and the call to set_vbridge() adds another bridge tag to the XML. The XML ends up looking something like: <interface type="bridge"> <mac address="00:11:22:33:44:aa"/> <source bridge="testbridge"/> <source bridge="virbr0"/> </interface> I'd recommend adding a function to vxml.py that allows you to check whether the bridge value is already set. There's a fair number of set_ functions in vxml.py, so I think it would make sense to add this as a get_ function. You can probably utilize get_node() and toprettyxml() to return the value of the bridge. If a bridge isn't set, you could return None. Thoughts?

Also, instead of using [Resubmitting] in the subject, please use a version number. The libvirt-cim project uses this convention - it's helpful in determining which patch is the most recent version. The subject of the next revision of this patch could be "[TEST] #3 Adding 05_RAPF_err.py to verify RAPF".

Thanks!