[Devel,3/3] net: do iptables restore in ve0

Submitted by Stanislav Kinsburskiy on July 21, 2017, 7:42 a.m.

Details

Message ID 20170721074203.16840.97378.stgit@skinsbursky-vz7.qa.sw.ru
State New
Series "criu: restore iptables in VE#0"
Headers show

Commit Message

Stanislav Kinsburskiy July 21, 2017, 7:42 a.m.
This is needed to be able to restore container vwith disabled netfilter.
When netfilter is disabled, its denties and operations are forbiddedn in CTs
VE.
Thus we need to switch to VE#0. This is done by forking a child, which
switches to VE#0, does the task and exits.

https://jira.sw.ru/browse/PSBM-58574

Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
---
 criu/net.c |   25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/criu/net.c b/criu/net.c
index 9986310..6b2385c 100644
--- a/criu/net.c
+++ b/criu/net.c
@@ -34,6 +34,7 @@ 
 #include "kerndat.h"
 #include "util.h"
 #include "external.h"
+#include "crtools.h"
 
 #include "protobuf.h"
 #include "images/netdev.pb-c.h"
@@ -1301,11 +1302,33 @@  static int run_iptables_tool(char *def_cmd, int fdin, int fdout)
 	return ret;
 }
 
-static int iptables_tool_restore(char *def_cmd, int fdin)
+static int __iptables_tool_restore(char *def_cmd, int fdin)
 {
+	if (join_ve(root_item->pid->real, false))
+		return -1;
+
 	return run_iptables_tool(def_cmd, fdin, -1);
 }
 
+static int iptables_tool_restore(char *def_cmd, int fdin)
+{
+	int child, status;
+
+	child = fork();
+	if (child < 0) {
+		pr_perror("failed to fork");
+		return -1;
+	} else if (!child)
+		_exit(__iptables_tool_restore(def_cmd, fdin));
+
+	if (waitpid(child, &status, 0) != child) {
+		pr_err("failed to collect child %d\n", child);
+		return -1;
+	}
+	return WIFEXITED(status) ? WEXITSTATUS(status) : -1;
+
+}
+
 static int iptables_tool_dump(char *def_cmd, int fdout)
 {
 	return run_iptables_tool(def_cmd, -1, fdout);

Comments

Andrey Vagin July 24, 2017, 3:34 p.m.
On Fri, Jul 21, 2017 at 10:42:03AM +0300, Stanislav Kinsburskiy wrote:
> This is needed to be able to restore container vwith disabled netfilter.
> When netfilter is disabled, its denties and operations are forbiddedn in CTs
> VE.
> Thus we need to switch to VE#0. This is done by forking a child, which
> switches to VE#0, does the task and exits.
> 
> https://jira.sw.ru/browse/PSBM-58574
> 
> Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
> ---
>  criu/net.c |   25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/criu/net.c b/criu/net.c
> index 9986310..6b2385c 100644
> --- a/criu/net.c
> +++ b/criu/net.c
> @@ -34,6 +34,7 @@
>  #include "kerndat.h"
>  #include "util.h"
>  #include "external.h"
> +#include "crtools.h"
>  
>  #include "protobuf.h"
>  #include "images/netdev.pb-c.h"
> @@ -1301,11 +1302,33 @@ static int run_iptables_tool(char *def_cmd, int fdin, int fdout)
>  	return ret;
>  }
>  
> -static int iptables_tool_restore(char *def_cmd, int fdin)
> +static int __iptables_tool_restore(char *def_cmd, int fdin)
>  {
> +	if (join_ve(root_item->pid->real, false))

Do you switch here into VE0? Could you add a comment? It is not obvious
that root_item in VE0 at this moment.
> +		return -1;
> +
>  	return run_iptables_tool(def_cmd, fdin, -1);
>  }
>  
> +static int iptables_tool_restore(char *def_cmd, int fdin)
> +{
> +	int child, status;
> +
> +	child = fork();
> +	if (child < 0) {
> +		pr_perror("failed to fork");
> +		return -1;
> +	} else if (!child)
> +		_exit(__iptables_tool_restore(def_cmd, fdin));
> +
> +	if (waitpid(child, &status, 0) != child) {
> +		pr_err("failed to collect child %d\n", child);
> +		return -1;
> +	}
> +	return WIFEXITED(status) ? WEXITSTATUS(status) : -1;
> +
> +}
> +
>  static int iptables_tool_dump(char *def_cmd, int fdout)
>  {
>  	return run_iptables_tool(def_cmd, -1, fdout);
> 
> _______________________________________________
> Devel mailing list
> Devel@openvz.org
> https://lists.openvz.org/mailman/listinfo/devel
Kirill Gorkunov July 24, 2017, 3:38 p.m.
On Mon, Jul 24, 2017 at 08:34:52AM -0700, Andrei Vagin wrote:
> >  
> > -static int iptables_tool_restore(char *def_cmd, int fdin)
> > +static int __iptables_tool_restore(char *def_cmd, int fdin)
> >  {
> > +	if (join_ve(root_item->pid->real, false))
> 
> Do you switch here into VE0? Could you add a comment? It is not obvious
> that root_item in VE0 at this moment.

root_item is NOT in ve0 at this moment, the @false argument points
to enter the ve0.

int join_ve(pid_t pid, bool veX)
Andrey Vagin July 24, 2017, 3:45 p.m.
On Mon, Jul 24, 2017 at 06:38:15PM +0300, Cyrill Gorcunov wrote:
> On Mon, Jul 24, 2017 at 08:34:52AM -0700, Andrei Vagin wrote:
> > >  
> > > -static int iptables_tool_restore(char *def_cmd, int fdin)
> > > +static int __iptables_tool_restore(char *def_cmd, int fdin)
> > >  {
> > > +	if (join_ve(root_item->pid->real, false))
> > 
> > Do you switch here into VE0? Could you add a comment? It is not obvious
> > that root_item in VE0 at this moment.
> 
> root_item is NOT in ve0 at this moment, the @false argument points
> to enter the ve0.
> 
> int join_ve(pid_t pid, bool veX)

Maybe we have to use join_ve0() here?
Andrey Vagin July 24, 2017, 3:46 p.m.
On Mon, Jul 24, 2017 at 06:38:15PM +0300, Cyrill Gorcunov wrote:
> On Mon, Jul 24, 2017 at 08:34:52AM -0700, Andrei Vagin wrote:
> > >  
> > > -static int iptables_tool_restore(char *def_cmd, int fdin)
> > > +static int __iptables_tool_restore(char *def_cmd, int fdin)
> > >  {
> > > +	if (join_ve(root_item->pid->real, false))
> > 
> > Do you switch here into VE0? Could you add a comment? It is not obvious
> > that root_item in VE0 at this moment.
> 
> root_item is NOT in ve0 at this moment, the @false argument points
> to enter the ve0.
> 
> int join_ve(pid_t pid, bool veX)

Does pid have to be 0 in this case?
Stanislav Kinsburskiy July 24, 2017, 3:52 p.m.
24.07.2017 18:46, Andrei Vagin пишет:
> On Mon, Jul 24, 2017 at 06:38:15PM +0300, Cyrill Gorcunov wrote:
>> On Mon, Jul 24, 2017 at 08:34:52AM -0700, Andrei Vagin wrote:
>>>>  
>>>> -static int iptables_tool_restore(char *def_cmd, int fdin)
>>>> +static int __iptables_tool_restore(char *def_cmd, int fdin)
>>>>  {
>>>> +	if (join_ve(root_item->pid->real, false))
>>>
>>> Do you switch here into VE0? Could you add a comment? It is not obvious
>>> that root_item in VE0 at this moment.
>>
>> root_item is NOT in ve0 at this moment, the @false argument points
>> to enter the ve0.
>>
>> int join_ve(pid_t pid, bool veX)
> 
> Does pid have to be 0 in this case?
> 

Maybe interface to this helper should be reworked?
Kirill Gorkunov July 24, 2017, 4:03 p.m.
On Mon, Jul 24, 2017 at 06:52:31PM +0300, Stanislav Kinsburskiy wrote:
> 
> >> int join_ve(pid_t pid, bool veX)
> > 
> > Does pid have to be 0 in this case?
> > 
> 
> Maybe interface to this helper should be reworked? 

In criu/cr_restore.c

#define join_veX(pid)	join_ve(pid, true)
#define join_ve0(pid)	join_ve(pid, false)

Just move it to the header, and that;s all

	Cyrill