Feat/udev hotplug

[sohorx]

This pull request (PR) is the second part of the changes originating from #235 (Part 1: #263).

The objective of this PR is to reintroduce udev hotplug capabilities without causing any breaking changes.

Since ifupdown2 does not support concurrent calls, I had to implement a workaround.
I achieved this by using the flock command to wait and lock the file directly from the shell script. Then, I passed the fd to ifupdown2.

flock man page:

Locks created by flock() are associated with an open file
description (see open(2)).  This means that duplicate file
descriptors (created by, for example, fork(2) or dup(2)) refer to
the same lock, and this lock may be modified or released using
any of these file descriptors.  Furthermore, the lock is released
either by an explicit LOCK_UN operation on any of these duplicate
file descriptors, or when all such file descriptors have been
closed.

(Initially, I attempted to use fcntl for obtaining information, but the implementation diverged at some point.
https://stackoverflow.com/questions/29611352/what-is-the-difference-between-locking-with-fcntl-and-flock)

@aderumier, please feel free to create issues or provide comments. Your input is highly appreciated.

Here's what you can expect from these changes in terms of handling:

• The udev-add/remove will wait until no ifupdown2 process is running before using ifupdown2.
• If a manual ifup command is used while the udev rule is running (net_up/net_down), an error will be raised.
• The file lock must be released at the end of a udev rule call.

[sohorx]

After some verification the ifupdown2-hotplug should call [email protected] if systemd is running.

This mean ifup must be fixed first and used in ifupdown2-hotplug.

I'll mark this PR as Draft until this is done (it's working but not as it should).

[aderumier]

Hi, I'm super busy until the end of the month (currently working on the release of proxmox 8). I'll do tests and review in 2 weeks.

Thanks !

[OlivierB]

• https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=855598
• https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=988050

[OlivierB]

@sohorx You should remove hotplug managment in ifupdown2/sbin/start-networking.

• udev hotplug handles start (and stop on system shutdown).
• On a systemctl restart networking, hotplug interfaces are not put down. So, no need to restart them with start-networking.

[sohorx]

@sohorx You should remove hotplug managment in ifupdown2/sbin/start-networking.

• udev hotplug handles start (and stop on system shutdown).
• On a systemctl restart networking, hotplug interfaces are not put down. So, no need to restart them with start-networking.

I moved the default/networking env file handling in both the [email protected] and ifupdown2-hotplug to get close to the current approach (adding -v -d to our ifup ifdown on hotplug).

To avoid disrupting existing configurations, I had to reimplement the exclusions var to the ifupdown2-hotplug. However, this variable seems somewhat unnecessary as it excludes interfaces that are already filtered by the --allow=hotplug flag.

@julienfortin, I would appreciate your thoughts on this matter.

[julienfortin]

The PR is missing the following:

diff --git a/ifupdown2/ifupdown/main.py b/ifupdown2/ifupdown/main.py
index d5fe30f..8b08fd6 100644
--- a/ifupdown2/ifupdown/main.py
+++ b/ifupdown2/ifupdown/main.py
@@ -75,7 +75,7 @@ class Ifupdown2:
             self.init(stdin_buffer)
 
             try:
-                if self.op != 'query' and not utils.lockFile(lockfile):
+                if self.op != 'query' and not utils.lockFile(self.args.lockfile):
                     log.error("Another instance of this program is already running.")
                     return Status.Client.STATUS_ALREADY_RUNNING
             except FileExistsError as e:

I'm running some test on it, if you can add that change in the meantime that would be great

[sohorx]

The PR is missing the following:

diff --git a/ifupdown2/ifupdown/main.py b/ifupdown2/ifupdown/main.py
index d5fe30f..8b08fd6 100644
--- a/ifupdown2/ifupdown/main.py
+++ b/ifupdown2/ifupdown/main.py
@@ -75,7 +75,7 @@ class Ifupdown2:
             self.init(stdin_buffer)
 
             try:
-                if self.op != 'query' and not utils.lockFile(lockfile):
+                if self.op != 'query' and not utils.lockFile(self.args.lockfile):
                     log.error("Another instance of this program is already running.")
                     return Status.Client.STATUS_ALREADY_RUNNING
             except FileExistsError as e:

I'm running some test on it, if you can add that change in the meantime that would be great

Hi Julien,

As far as I can tell, there don't seem to be any issues with this line in the current state of the branch.

This is what I'm seeing in my branch:

              if self.op != 'query' and not utils.lockFile(self.args.lockfile):
                 log.error("Another instance of this program is already running.")
                 return Status.Client.STATUS_ALREADY_RUNNING

Am I overlooking anything?

[julienfortin]

@sohorx ohh yes I see, my bad, i applied your patch on top of my private branch which didn't contain the previous lockfile changes!