[PATCH 1/2] NetTestSlave: fix restoration of system configuration

[Ondrej Lichtner]

From: Ondrej Lichtner <olichtne(a)redhat.com&gt;

When the NetTestController code was refactored and several methods were
moved to the new class Machine, part of the code of run_command was
forgotten.

The system configuration data was moved to be stored in the slave
application, however for that the method _update_system_config has to be
called and that wasn't happening.

This commit fixes that. The fix also changes the behaviour of the
system_config command- it will not be running in the background anymore,
instead it is run in the main lnst-slave application, just like the
wait/intr/kill commands.

Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com&gt;
---
 lnst/Common/NetTestCommand.py |  5 +++--
 lnst/Slave/NetTestSlave.py    | 11 +++++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/lnst/Common/NetTestCommand.py b/lnst/Common/NetTestCommand.py
index 527764e..8dc9562 100644
--- a/lnst/Common/NetTestCommand.py
+++ b/lnst/Common/NetTestCommand.py
@@ -80,7 +80,8 @@ class NetTestCommand:
         return self._finished
 
     def run(self):
-        if isinstance(self._cmd_cls, NetTestCommandControl):
+        if isinstance(self._cmd_cls, NetTestCommandControl) or \
+           isinstance(self._cmd_cls, NetTestCommandSystemConfig):
             return self._cmd_cls.run()
 
         self._read_pipe, self._write_pipe = multiprocessing.Pipe()
@@ -321,7 +322,7 @@ class NetTestCommandSystemConfig(NetTestCommandGeneric):
 
         res = {"passed": True}
         res["res_data"] = res_data
-        self.set_result(res)
+        return res
 
 class NetTestCommandControl(NetTestCommandGeneric):
     def __init__(self, command_context, command):
diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py
index 9b39c0a..d2dd484 100644
--- a/lnst/Slave/NetTestSlave.py
+++ b/lnst/Slave/NetTestSlave.py
@@ -207,6 +207,7 @@ class SlaveMethods:
                 logging.warn("Unable to restore '%s' config option!",
option)
                 return False
 
+        self._system_config = {}
         return True
 
     def run_command(self, command):
@@ -218,6 +219,16 @@ class SlaveMethods:
             res = cmd.run()
             if not cmd.forked():
                 self._command_context.del_cmd(cmd)
+
+            if command["type"] == "system_config":
+                if res["passed"]:
+                   
self._update_system_config(res["res_data"]["options"],
+                                               command["persistent"])
+                else:
+                    err = "Error occured while setting system "\
+                          "configuration (%s)" % res["err_msg"]
+                    logging.error(err)
+
             return res
         except:
             log_exc_traceback()
-- 
1.8.1.4

[PATCH 2/2] RecipeParse: restrict bg_id usage

[Ondrej Lichtner]

From: Ondrej Lichtner <olichtne(a)redhat.com&gt;

The previous commit changed the execution of the system_config command,
which now cannot run in the background as it would break the
application. This commit adds a check to the recipe parser that will not
allow bg_id to be used in commands of type system_config, wait, intr and
kill.

This commit also updates the recipe schema accordingly.

Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com&gt;
---
 lnst/Controller/RecipeParse.py |  5 +++++
 recipe-schema.rng              | 48 ++++++++++++++++++++++++++++++++----------
 2 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/lnst/Controller/RecipeParse.py b/lnst/Controller/RecipeParse.py
index 8cac525..d2d1171 100644
--- a/lnst/Controller/RecipeParse.py
+++ b/lnst/Controller/RecipeParse.py
@@ -458,6 +458,11 @@ class CommandParse(LnstParser):
         if self._has_attribute(node, "bg_id"):
             command["bg_id"] = self._get_attribute(node, "bg_id")
 
+            if command["type"] in ["system_config", "wait",
"intr", "kill"]:
+                msg = "Command %s cannot be run in the background!" \
+                        % command["type"]
+                raise XmlProcessingError(msg, node)
+
         if self._has_attribute(node, "desc"):
             command["desc"] = self._get_attribute(node, "desc")
 
diff --git a/recipe-schema.rng b/recipe-schema.rng
index aa6d375..57445b4 100644
--- a/recipe-schema.rng
+++ b/recipe-schema.rng
@@ -319,7 +319,8 @@
                 <ref name="ctl_wait"/>
                 <ref name="exec"/>
                 <ref name="system_config"/>
-                <ref name="other_commands"/>
+                <ref name="test"/>
+                <ref name="control_commands"/>
             </choice>
 
             <optional>
@@ -386,10 +387,6 @@
         </attribute>
 
         <optional>
-            <attribute name="bg_id"/>
-        </optional>
-
-        <optional>
             <attribute name="value"/>
         </optional>
 
@@ -421,12 +418,45 @@
         </optional>
     </define>
 
-    <define name="other_commands">
+    <define name="test">
+        <attribute name="machine_id"/>
+
+        <attribute name="type">
+            <value>test</value>
+        </attribute>
+
+        <optional>
+            <attribute name="bg_id"/>
+        </optional>
+
+        <optional>
+            <attribute name="value"/>
+        </optional>
+
+        <optional>
+            <attribute name="timeout">
+                <data type="integer"/>
+            </attribute>
+        </optional>
+
+        <optional>
+            <attribute name="option"/>
+        </optional>
+
+        <optional>
+            <attribute name="desc"/>
+        </optional>
+
+        <optional>
+            <attribute name="source"/>
+        </optional>
+    </define>
+
+    <define name="control_commands">
         <attribute name="machine_id"/>
 
         <attribute name="type">
             <choice>
-                <value>test</value>
                 <value>wait</value>
                 <value>intr</value>
                 <value>kill</value>
@@ -434,10 +464,6 @@
         </attribute>
 
         <optional>
-            <attribute name="bg_id"/>
-        </optional>
-
-        <optional>
             <attribute name="value"/>
         </optional>
 
-- 
1.8.1.4

[Jiri Pirko]

Fri, May 31, 2013 at 01:15:41PM CEST, olichtne(a)redhat.com wrote:
> From: Ondrej Lichtner <olichtne(a)redhat.com&gt;
>
> When the NetTestController code was refactored and several methods were
> moved to the new class Machine, part of the code of run_command was
> forgotten.
>
> The system configuration data was moved to be stored in the slave
> application, however for that the method _update_system_config has to be
> called and that wasn't happening.
>
> This commit fixes that. The fix also changes the behaviour of the
> system_config command- it will not be running in the background anymore,
> instead it is run in the main lnst-slave application, just like the
> wait/intr/kill commands.
>
> Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com&gt;
> ---
>  lnst/Common/NetTestCommand.py |  5 +++--
>  lnst/Slave/NetTestSlave.py    | 11 +++++++++++
>  2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/lnst/Common/NetTestCommand.py b/lnst/Common/NetTestCommand.py
> index 527764e..8dc9562 100644
> --- a/lnst/Common/NetTestCommand.py
> +++ b/lnst/Common/NetTestCommand.py
> @@ -80,7 +80,8 @@ class NetTestCommand:
>          return self._finished
>  
>      def run(self):
> -        if isinstance(self._cmd_cls, NetTestCommandControl):
> +        if isinstance(self._cmd_cls, NetTestCommandControl) or \
> +           isinstance(self._cmd_cls, NetTestCommandSystemConfig):
>              return self._cmd_cls.run()
>  
>          self._read_pipe, self._write_pipe = multiprocessing.Pipe()
> @@ -321,7 +322,7 @@ class NetTestCommandSystemConfig(NetTestCommandGeneric):
>  
>          res = {"passed": True}
>          res["res_data"] = res_data
> -        self.set_result(res)
> +        return res
>  
>  class NetTestCommandControl(NetTestCommandGeneric):
>      def __init__(self, command_context, command):
> diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py
> index 9b39c0a..d2dd484 100644
> --- a/lnst/Slave/NetTestSlave.py
> +++ b/lnst/Slave/NetTestSlave.py
> @@ -207,6 +207,7 @@ class SlaveMethods:
>                  logging.warn("Unable to restore '%s' config option!",
> option)
>                  return False
>  
> +        self._system_config = {}
>          return True
>  
>      def run_command(self, command):
> @@ -218,6 +219,16 @@ class SlaveMethods:
>              res = cmd.run()
>              if not cmd.forked():
>                  self._command_context.del_cmd(cmd)
> +
> +            if command["type"] == "system_config":
> +                if res["passed"]:
> +                   
> self._update_system_config(res["res_data"]["options"],
> +                                               command["persistent"])
> +                else:
> +                    err = "Error occured while setting system "\
> +                          "configuration (%s)" % res["err_msg"]
> +                    logging.error(err)
> +
>              return res
>          except:
>              log_exc_traceback()
> -- 
> 1.8.1.4
>
> _______________________________________________
> LNST-developers mailing list
> LNST-developers(a)lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/lnst-developers 

both applied. Thanks