[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