[PATCH 13/35] monitor: disable nvmf-autoconnect udev rules in autoconnect mode

[Hannes Reinecke]

On 1/26/21 9:33 PM, mwilck at suse.com wrote:
> From: Martin Wilck <mwilck at suse.com>
> 
> If autoconnect is enabled, disable the respective udev rules
> by symlinking /run/udev/rules.d to /dev/null, in order to avoid
> the connections being set up by the monitor and the udev workers
> at the same time. This is probably the preferred mode of operation
> for the monitor.
> 
> Users can override this by copying 70-nvmf-autoconnect.rules
> from /usr/lib/udev/rules.d to /etc/udev/rules.d (/etc/udev/rules.d
> takes precedence over /run/udev/rules.d).
> 
> If the symlink can't be created for some reason, autoconnect will
> be disabled. There is  only one exception: If
> /run/udev/rules.d/70-nvmf-autoconnect.rules already points to
> /dev/null at startup, autoconnect can be left on, but the symlink
> isn't removed on exit.
> 
> Signed-off-by: Martin Wilck <mwilck at suse.com>
> ---
>   monitor.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 73 insertions(+), 2 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index ecf3be2..2a906db 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -17,14 +17,18 @@
>   
>   #include <stddef.h>
>   #include <stdio.h>
> +#include <stdlib.h>
>   #include <unistd.h>
>   #include <errno.h>
>   #include <libudev.h>
>   #include <signal.h>
>   #include <time.h>
> +#include <limits.h>
>   #include <syslog.h>
> +#include <sys/stat.h>
>   #include <sys/epoll.h>
>   
> +#include "common.h"
>   #include "nvme-status.h"
>   #include "util/argconfig.h"
>   #include "monitor.h"
> @@ -33,6 +37,7 @@
>   
>   static struct monitor_config {
>   	bool autoconnect;
> +	bool skip_udev_on_exit;
>   } mon_cfg;
>   
>   static struct udev *udev;
> @@ -45,6 +50,8 @@ static void close_ptr(int *p)
>   	}
>   }
>   
> +CLEANUP_FUNC(char)
> +
>   static void cleanup_monitor(struct udev_monitor **pmon)
>   {
>   	if (*pmon) {
> @@ -174,12 +181,64 @@ static int monitor_main_loop(struct udev_monitor *monitor)
>   	return ret;
>   }
>   
> +static const char autoconnect_rules[] = "/run/udev/rules.d/70-nvmf-autoconnect.rules";
> +
> +static int monitor_disable_udev_rules(void)
> +{
> +	CLEANUP(char, path) = strdup(autoconnect_rules);
> +	char *s1, *s2;
> +	int rc;
> +
> +	if (!path)
> +		return -ENOMEM;
> +
> +	s2 = strrchr(path, '/');
> +	for (s1 = s2 - 1; s1 > path && *s1 != '/'; s1--);
> +
> +	*s2 = *s1 = '\0';
> +	rc = mkdir(path, 0755);
> +	if (rc == 0 || errno == EEXIST) {
> +		*s1 = '/';
> +		rc = mkdir(path, 0755);
> +		if (rc == 0 || errno == EEXIST) {
> +			*s2 = '/';
> +			rc = symlink("/dev/null", path);
> +		}
> +	}
> +	if (rc) {
> +		if (errno == EEXIST) {
> +			char target[PATH_MAX];
> +
> +			if (readlink(path, target, sizeof(target)) != -1 &&
> +			    !strcmp(target, "/dev/null")) {
> +				log(LOG_INFO,
> +				    "symlink %s -> /dev/null exists already\n",
> +				    autoconnect_rules);
> +				return 1;
> +			}
> +		}
> +		log(LOG_ERR, "error creating %s: %m\n", autoconnect_rules);
> +	} else
> +		log(LOG_INFO, "created %s\n", autoconnect_rules);
> +
> +	return rc ? (errno ? -errno : -EIO) : 0;
> +}
> +
> +static void monitor_enable_udev_rules(void)
> +{
> +	if (unlink(autoconnect_rules) == -1 && errno != ENOENT)
> +		log(LOG_ERR, "error removing %s: %m\n", autoconnect_rules);
> +	else
> +		log(LOG_INFO, "removed %s\n", autoconnect_rules);
> +}
> +
>   static int monitor_parse_opts(const char *desc, int argc, char **argv)
>   {
>   	bool quiet = false;
>   	bool verbose = false;
>   	bool debug = false;
> -	int ret;
> +	int ret = 0;
> +
>   	OPT_ARGS(opts) = {
>   		OPT_FLAG("autoconnect",    'A', &mon_cfg.autoconnect, "automatically connect newly discovered controllers"),
>   		OPT_FLAG("silent",         'S', &quiet,               "log level: silent"),
> @@ -198,7 +257,17 @@ static int monitor_parse_opts(const char *desc, int argc, char **argv)
>   		log_level = LOG_INFO;
>   	if (debug)
>   		log_level = LOG_DEBUG;
> -
> +	if (mon_cfg.autoconnect) {
> +		ret = monitor_disable_udev_rules();
> +		if (ret < 0) {
> +			mon_cfg.autoconnect = false;
> +			log(LOG_WARNING, "autoconnect disabled\n");
> +			ret = 0;
> +		} else if (ret > 0) {
> +			mon_cfg.skip_udev_on_exit = true;
> +			ret = 0;
> +		}
> +	}
>   	return ret;
>   }
>   
> @@ -221,6 +290,8 @@ int aen_monitor(const char *desc, int argc, char **argv)
>   		udev_monitor_unref(monitor);
>   	}
>   	udev = udev_unref(udev);
> +	if (mon_cfg.autoconnect && !mon_cfg.skip_udev_on_exit)
> +		monitor_enable_udev_rules();
>   out:
>   	return nvme_status_to_errno(ret, true);
>   }
> 
What on earth ...

No way.
We should _not_ reference individual files or mess with udev rules from 
a program. If a file needs to be modified, modify it.
But do not try to modify a file from a program context, unless the 
program itself has created it.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare at suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer