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

[Deepti B Kalakeri]

Hi Kaitlin,

Please see my comments inline.

Thanks and Regards,
Deepti.

Kaitlin Rupert wrote:
>> +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.
I had verified the test case that I had submitted yesterday with Xen , 
XenFV, KVM.

>
> 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?
>
I have added a new function xml_get_net_bridge() which is gets the 
bridge information if any with the domain.
> 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".
>
ok I will take care of this.
> Thanks!