network lock method for checkpoint/restore defaults to iptables

[adrianreber]

I think PR #1613 is not helpful as it is. With this change crun always defaults to iptables as network lock method (because if nothing is set the parameter will always be 0 (which is iptables based locking)). This breaks podman container restore on CentOS 10. On CentOS 10 I am switching CRIU to default to the nftables locking backend. With PR #1613, if nothing is specified, crun forces the iptables locking backend.

If the user does not select a specific locking backend crun should not explicitly set the parameter as it overwrites the defaults from CRIU.

Please revert #1613 or do not force a certain locking backend. Maybe default to -1 and skip telling CRIU about the locking backend if the parameter is -1. Would be good to have a fix in CentOS 10 also as soon as possible.

[rst0git]

CC: @danishprakash

It is also worth noting the following pull request with a fix for network locking with nftables: checkpoint-restore/criu#2550

[giuseppe]

should we default to CRIU_NETWORK_LOCK_SKIP or do you suggest something like the following?

diff --git a/src/libcrun/criu.c b/src/libcrun/criu.c
index 2e4e0d9d..614dc35f 100644
--- a/src/libcrun/criu.c
+++ b/src/libcrun/criu.c
@@ -647,7 +647,7 @@ libcrun_container_checkpoint_linux_criu (libcrun_container_status_t *status, lib
     libcriu_wrapper->criu_set_manage_cgroups_mode (cr_options->manage_cgroups_mode);
   libcriu_wrapper->criu_set_manage_cgroups (true);
 
-  if (libcriu_wrapper->criu_set_network_lock)
+  if (libcriu_wrapper->criu_set_network_lock && cr_options->network_lock_method)
     libcriu_wrapper->criu_set_network_lock (cr_options->network_lock_method);
 
   ret = libcriu_wrapper->criu_dump ();

[adrianreber]

No, CRIU_NETWORK_LOCK_SKIP would also be wrong.

Looking closer at the code I am confused it works at all. Ah, the return value is not checked. Setting it to 0 should return an error. There is also a bug in CRIU I just saw.

Your patch does sound like a good idea. 0 is not a valid value for the network locking. Currently it has to be > 0. Yes, your fix sounds like the right approach. I thought it might be more complicated.

Return value checking would have helped here, but most of the CRIU library function return codes are not checked currently (which I am probably also to blame for).

[giuseppe]

opened a PR: #1628