[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