[PATCH 08/12] NetTestParse: Adding support for libvirt-integration

Jiri Pirko jpirko at redhat.com
Mon Aug 13 08:56:38 UTC 2012


Mon, Aug 13, 2012 at 10:32:46AM CEST, rpazdera at redhat.com wrote:
>On 08/10/2012 01:42 PM, Jiri Pirko wrote:
>>Fri, Aug 10, 2012 at 01:22:56PM CEST, rpazdera at redhat.com wrote:
>>>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).
>>Anyway, you specify here that you want "libvirt" to connect this device
>>to some "bridge". So the name seems to be appropriate.
>
>Right, I see your point now. From this perspective it makes sense.
>
>It seemed to me as if libvirt would somehow create the bridge (which
>is possible too, but we don't want that).
>
>>
>>Or do you see some broader use for this attribute that would not be
>>related to "libvirt" ?
>
>It could be used in broader terms (to add physical interfaces to these
>virtual bridges, so the virtual hosts can communicate with them - the
>"single bare-metal server to many virtual clients test scenario" I talked
>about at the meeting. But I think we agreed that LNST shouldn't do that.
>That it could be confused with the bridges for libvirt.
>
>You have to setup the wires yourself anyway, so bridging the physical
>interface is a part of the manual setup you have to do (in case you chose
>to perform such complicated tests).
>
>
>
>I'm thinking, what about making it "virt_create" and "virt_bridge"?

Again, would seem misleading.

>It's a little
>shorter and it will hide the dependency on libvirt from the recipes. Maybe
>even change "libvirt_domain" to something else.
>
>Now if libvirt team decides to change the project name, we will have to
>change all the recipes. But it's just a thought, it's not very likely
>to happen
>anytime soon ...

Nope.

>
>>>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