Skip to content

[th/checkpoint-preserve-external-ports] preserve external ports during checkpoint rollback

Thomas Haller requested to merge th/checkpoint-preserve-external-ports into main

When we have a bridge interface with ports attached externally (that is, not by NetworkManager itself), then it can make sense that during checkpoint rollback we want to keep those ports attached.

During rollback, we may need to deactivate the bridge device and re-activate it. Implement this, by setting a flag before deactivating, which prevents external ports to be detached. The flag gets cleared, when the device state changes to activated (the following activation) or unmanaged.

This is an ugly solution, for several reasons.

For one, NMDevice tracks its ports in the port_info_lst_head. But what it does is really ugly. There is no easy concept to understand what it actually tacks. For example, it tracks externally added interfaces (nm_device_sys_iface_state_is_external()) that are attached. But it also tracks interfaces that we want to attach (but which are not yet actually enslaved). So it's not clear what this list contains. And when we skip the change of the port states during nm_device_master_release_slaves_all(), it's not really clear what that means. It's ugly, but probably correct enough. What would be better, if we had a clear purpose of what the lists (or several lists) mean. E.g. a list of all ports that are currently, physically attached vs. a list of ports we want to attach.

Another problem is that we attach state on the device ("activation_state_preserve_external_ports"), which should longer there during the deactivation and reactivation. How can we be sure that we don't leave that flag dangling there, and that the desired activation is the next one? In practice, it's probably fine because we clear the flag on various state changes.

Also, we only implement this for bridges. I think this is where it makes the most sense. And after all, it's an odd thing to leave there unknown things during a rollback (unknown, because we have no knowledge about why these ports are attached and what to do with them).

Also, we don't remember the ports that were attached when the checkpoint was created. Instead, we preserve all ports that are attached during rollback. That seems more useful and easier to implement.

Also, we do this now by default and introduce a flag to get the previous behavior.

https://bugzilla.redhat.com/show_bug.cgi?id=2035519 #909 (closed)

Merge request reports