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

Radek Pazdera rpazdera at redhat.com
Mon Aug 13 08:32:46 UTC 2012


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"? 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 ...

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