[PATCH 08/12] NetTestParse: Adding support for libvirt-integration
Radek Pazdera
rpazdera at redhat.com
Fri Aug 10 11:22:56 UTC 2012
On 08/10/2012 01:11 PM, Jiri Pirko wrote:
> Fri, Aug 10, 2012 at 12:49:11PM CEST, rpazdera at redhat.com wrote:
>> From: Radek Pazdera <rpazdera at redhat.com>
>>
>> This commit adds necessary changes to the parser in LNST in order
>> to support the libvirt device creation.
>>
>> Netdevice tags are now grouped under a <netdevices> parent tag
>> within <netmachineconfig>.
>>
>> On top of that a special mode for defining netdevices (called create)
>> was added. Devices defined in this mode are not present in the machine
>> and will be created on test execution.
>>
>> This mode is available only on virtual test machines created using
>> libvirt. Here's an exmaple config of such a machine:
>>
>> <machine id="1">
>> <netmachineconfig>
>> <info hostname="192.168.122.11" libvirt_domain="RHEL6"
>> rootpass="redhat"/>
>> <netdevices>
>> <create>
>> <netdevice network="tnet" phys_id="1" type="eth"/>
>> <netdevice network="tnet" phys_id="2" type="eth"/>
>> </create>
>
> This is ok and I love your work!
> But still I have some concerns about naming. I think that
> "create" and "tagret_bridge" might be misleading for users.
>
> How about to change it this way:
> "create" -> "libvirt_create"
Fair point, we discussed it yesterday and settled on create, because it
was shorter, but with libvirt_create it's more clear.
> "target_bridge" -> "libvirt_bridge"
On the other hand, I think, that this would be misleading. This attribute
is there as a possibility to bypass the default bridge, that is created
with the network and connect it to a different (possibly hand-configured
one). This option is useful only in cases when you want to mix manually
and dynamically configured devices (which should happen only in
several corner cases).
I don't know it target_bridge is the best name, but I wouldn't use
libvirt_bridge.
>
> This naming would tell everyone on a first look that this xml element and
> attribute are only valid for libvirt.
>
> Also, I think that there should be check for <info libvirt_domain="">
> attribute in case "libvirt_create" is detected.
>
> Also, I think that dev["dynamic"] = True/False should be uniform to
> "libvirt_create" somehow. How about:
>
> dev["create"] = None
> dev["create"] = "libvirt"
> dev["create"] = "whateverisneededinfuture"
>
> ?
>
>
>> </netdevices>
>> </netmachineconfig>
>> <netconfig>
>> <interface id="1" phys_id="1" type="eth">
>> <addresses>
>> <address value="192.168.100.226/24"/>
>> </addresses>
>> </interface>
>> <interface id="2" phys_id="2" type="eth">
>> <addresses>
>> <address value="192.168.100.240/24"/>
>> </addresses>
>> </interface>
>> </netconfig>
>> </machine>
>>
>> The dynamic devices are enclosed within the <create> tag. Their hwaddr
>> doesn't need to be specified (it will be generated if missing).
>>
>> Additionally, 'target_bridge' attribute can be added to these devices
>> to override the default LNST internal bridge, that these devices are
>> connected to.
>>
>> Signed-off-by: Radek Pazdera <rpazdera at redhat.com>
>> ---
>> NetTest/NetTestParse.py | 34 +++++++++++++++++++++++++++++++---
>> 1 files changed, 31 insertions(+), 3 deletions(-)
>>
>> diff --git a/NetTest/NetTestParse.py b/NetTest/NetTestParse.py
>> index 746342c..d1edf6b 100644
>> --- a/NetTest/NetTestParse.py
>> +++ b/NetTest/NetTestParse.py
>> @@ -118,7 +118,7 @@ class NetMachineConfigParse(RecipeParser):
>>
>> def parse(self, node):
>> scheme = {"info": self._info,
>> - "netdevice": self._netdevice}
>> + "netdevices": self._netdevices}
>> self._process_child_nodes(node, scheme)
>>
>> def _info(self, node, params):
>> @@ -127,6 +127,10 @@ class NetMachineConfigParse(RecipeParser):
>>
>> info["hostname"] = self._get_attribute(node, "hostname")
>>
>> + if self._has_attribute(node, "libvirt_domain"):
>> + info["libvirt_domain"] = self._get_attribute(node,
>> + "libvirt_domain")
>> +
>> if self._has_attribute(node, "rootpass"):
>> info["rootpass"] = self._get_attribute(node, "rootpass")
>>
>> @@ -138,18 +142,42 @@ class NetMachineConfigParse(RecipeParser):
>> self._trigger_event("machine_info_ready",
>> {"machine_id": self._machine_id})
>>
>> + def _netdevices(self, node, params):
>> + scheme = {"netdevice": self._netdevice,
>> + "create": self._create}
>> +
>> + new_params = {"dynamic": False}
>> + self._process_child_nodes(node, scheme, new_params)
>> +
>> + def _create(self, node, params):
>> + scheme = {"netdevice": self._netdevice}
>> +
>> + new_params = {"dynamic": True}
>> + self._process_child_nodes(node, scheme, new_params)
>> +
>> def _netdevice(self, node, params):
>> machine = self._machine
>> phys_id = self._get_attribute(node, "phys_id", int)
>>
>> dev = machine["netdevices"][phys_id] = {}
>> + dev["dynamic"] = params["dynamic"]
>> dev["type"] = self._get_attribute(node, "type")
>> dev["network"] = self._get_attribute(node, "network")
>> - dev["hwaddr"] = normalize_hwaddr(self._get_attribute(node, "hwaddr"))
>>
>> - if self._has_attribute(node, "name"):
>> + # hwaddr attribute is optional for dynamic devices,
>> + # but it is required by non-dynamic devices
>> + if not dev["dynamic"] or self._has_attribute(node, "hwaddr"):
>> + hwaddr = self._get_attribute(node, "hwaddr")
>> + dev["hwaddr"] = normalize_hwaddr(hwaddr)
>> +
>> + # name attribute is only valid when the device is not dynamic
>> + if not dev["dynamic"] and self._has_attribute(node, "name"):
>> dev["name"] = self._get_attribute(node, "name")
>>
>> + # bridge attribute is valid only when the device is dynamic
>> + if dev["dynamic"] and self._has_attribute(node, "target_bridge"):
>> + dev["target_bridge"] = self._get_attribute(node, "target_bridge")
>> +
>> self._trigger_event("netdevice_ready", {"machine_id": self._machine_id,
>> "dev_id": phys_id})
>>
>> --
>> 1.7.7.6
>>
>> _______________________________________________
>> LNST-developers mailing list
>> LNST-developers at lists.fedorahosted.org
>> https://lists.fedorahosted.org/mailman/listinfo/lnst-developers
More information about the LNST-developers
mailing list