CP-35551: create files to replace block device minor ids

Signed-off-by: Mark Syms <[email protected]>

Author: MarkSymsCtx

control/Makefile.am

@@ -2,6 +2,7 @@
 AM_CFLAGS  = -Wall
 AM_CFLAGS += -Werror
 AM_CFLAGS += $(if $(GCOV),-fprofile-dir=/tmp/coverage/blktap/control -fprofile-arcs -ftest-coverage)
+AM_CFLAGS += -g
 
 AM_CPPFLAGS  = -D_GNU_SOURCE
 AM_CPPFLAGS += -DTAPCTL

control/tap-ctl-allocate.c

@@ -34,18 +34,12 @@
 
 #include <stdio.h>
 #include <errno.h>
-#include <fcntl.h>
 #include <stdlib.h>
 #include <unistd.h>
 #include <string.h>
-#include <getopt.h>
-#include <libgen.h>
 #include <sys/file.h>
 #include <sys/stat.h>
-#include <sys/sysmacros.h>
 #include <sys/types.h>
-#include <sys/ioctl.h>
-#include <linux/major.h>
 
 #include "tap-ctl.h"
 #include "blktap.h"
@@ -62,7 +56,7 @@ tap_ctl_prepare_directory(const char *dir)
 
 	name = strdup(dir);
 	if (!name)
-		return ENOMEM;
+		return -errno;
 
 	start = name;
 
@@ -73,8 +67,8 @@ tap_ctl_prepare_directory(const char *dir)
 
 		err = mkdir(name, 0700);
 		if (err && errno != EEXIST) {
+			err = -errno;
 			PERROR("mkdir %s", name);
-			err = errno;
 			EPRINTF("mkdir failed with %d\n", err);
 			break;
 		}
@@ -116,19 +110,72 @@ tap_ctl_check_environment(void)
 }
 
 static int
-tap_ctl_allocate_device(int *minor, char **devname)
+tap_ctl_allocate_minor(int *minor, char **minor_name)
 {
+	char *path = NULL;
+	struct stat st_buf;
+	int err, id, st, f, fid;
+
 	*minor = -1;
-	if (!devname)
-		return EINVAL;
 
-	/* TO-DO: get this from a file based resource */
-	*minor = 1;
-	return 0;
+	f = open(BLKTAP2_NP_RUN_DIR, O_RDONLY);
+	if (f == -1) {
+		err = -errno;
+		EPRINTF("Failed to open runtime directory %d\n", errno);
+		return err;
+	}
+
+	/* The only way this can fail is with an EINTR or ENOLCK*/
+	err = flock(f, LOCK_EX);
+	if (err == -1) {
+		err = -errno;
+		EPRINTF("Failed to lock runtime directory %d\n", errno);
+		return err;
+	}
+
+	for (id=0; id<MAX_ID; id++) {
+		err = asprintf(&path, "%s/tapdisk-%d", BLKTAP2_NP_RUN_DIR, id);
+		if (err == -1) {
+			err = -errno;
+			goto out;
+		}
+
+		st = stat(path, &st_buf);
+		if (st == 0) {
+			/* Already exists */
+			free(path);
+			path = NULL;
+			continue;
+		}
+		if (errno != ENOENT) {
+			err = -errno;
+			free(path);
+			goto out;
+		}
+
+		fid = open(path, O_CREAT | O_WRONLY, 0600);
+		if (fid == -1) {
+			err = -errno;
+			EPRINTF("Failed to create ID file %s, %d\n", path, errno);
+			free(path);
+			goto out;
+		}
+		close(fid);
+
+		*minor = id;
+		*minor_name = path;
+		break;
+	}
+
+	err = 0;
+out:
+	flock(f, LOCK_UN);
+	close(f);
+	return err;
 }
 
 int
-tap_ctl_allocate(int *minor, char **devname)
+tap_ctl_allocate(int *minor, char **minor_name)
 {
 	int err;
 
@@ -140,7 +187,7 @@ tap_ctl_allocate(int *minor, char **devname)
 		return err;
 	}
 
-	err = tap_ctl_allocate_device(minor, devname);
+	err = tap_ctl_allocate_minor(minor, minor_name);
 	if (err) {
 		EPRINTF("tap-ctl allocate failed to allocate device");
 		return err;

control/tap-ctl-free.c

@@ -37,28 +37,68 @@
 #include <fcntl.h>
 #include <stdlib.h>
 #include <unistd.h>
-#include <getopt.h>
-#include <sys/ioctl.h>
+#include <sys/file.h>
 
 #include "tap-ctl.h"
 #include "blktap.h"
 
 int
 tap_ctl_free(const int minor)
 {
-	/* TO-DO: Take the lock and remove the associated marker file */
-	/* int fd, err; */
+	char *path = NULL;
+	int mfd = -1, fd, err;
 
-	/* fd = open(BLKTAP2_CONTROL_DEVICE, O_RDONLY); */
-	/* if (fd == -1) { */
-	/* 	EPRINTF("failed to open control device: %d\n", errno); */
-	/* 	return errno; */
-	/* } */
+	fd = open(BLKTAP2_NP_RUN_DIR, O_RDONLY);
+	if (fd == -1) {
+		err = -errno;
+		EPRINTF("Failed to open runtime directory %d\n", errno);
+		return err;
+	}
 
-	/* err = ioctl(fd, BLKTAP2_IOCTL_FREE_TAP, minor); */
-	/* err = (err == -1) ? -errno : 0; */
-	/* close(fd); */
+	/* The only way this can fail is with an EINTR or ENOLCK*/
+	err = flock(fd, LOCK_EX);
+	if (err == -1) {
+		err = -errno;
+		EPRINTF("Failed to lock runtime directory %d\n", errno);
+		return err;
+	}
 
-	/* return err; */
-	return 0;
+	err = asprintf(&path, "%s/tapdisk-%d", BLKTAP2_NP_RUN_DIR, minor);
+	if (err == -1) {
+		err = -errno;
+		goto out;
+	}
+	err = 0;
+
+	/* Non-Blocking lock to check it's not in use */
+	mfd = open(path, O_RDONLY);
+	if (mfd == -1) {
+		err = -errno;
+		EPRINTF("Failed to open marker file %s, %d, err=%d\n",
+			path, minor, errno);
+		goto out;
+	}
+
+	err = flock(mfd, LOCK_EX | LOCK_NB);
+	if (err == -1) {
+		err = -errno;
+		EPRINTF("Unable to lock marker file %s, err = %d\n",
+			path, errno);
+		goto out;
+	}
+
+	unlink(path);
+
+out:
+	if (path)
+		free(path);
+
+	if (mfd != -1) {
+		flock(mfd, LOCK_UN);
+		close(mfd);
+	}
+
+	flock(fd, LOCK_UN);
+	close(fd);
+	return err;
 }

drivers/tapdisk-control.c

@@ -633,6 +633,13 @@ tapdisk_control_attach_vbd(struct tapdisk_ctl_conn *conn,
 		goto out;
 	}
 
+	/* Lock the marker file to prevent freeing in use*/
+	err = tapdisk_vbd_lock(vbd);
+	if (err) {
+		ERR(err, "Failed to lock VBD marker file for %d\n", minor);
+		goto fail_vbd;
+	}
+
 	tapdisk_server_add_vbd(vbd);
 
 out:
@@ -645,6 +652,10 @@ tapdisk_control_attach_vbd(struct tapdisk_ctl_conn *conn,
 	}
 
 	return err;
+
+fail_vbd:
+	free(vbd);
+	goto out;
 }
 
 static int
@@ -669,6 +680,9 @@ tapdisk_control_detach_vbd(struct tapdisk_ctl_conn *conn,
 		goto out;
 	}
 
+	/* Unlock marker file */
+	tapdisk_vbd_unlock(vbd);
+
 	if (list_empty(&vbd->images)) {
 		tapdisk_server_remove_vbd(vbd);
 		tapdisk_vbd_free(vbd);

drivers/tapdisk-vbd.c

@@ -43,6 +43,7 @@
 #include <sys/ioctl.h>
 #include <sys/stat.h>
 #include <sys/types.h>
+#include <sys/file.h>
 
 #include "debug.h"
 #include "libvhd.h"
@@ -664,6 +665,55 @@ tapdisk_vbd_open_vdi(td_vbd_t *vbd, const char *name, td_flag_t flags, int prt_d
 	return err;
 }
 
+static int
+open_vbd_marker(int id)
+{
+   char *path = NULL;
+   int err, fid;
+
+   err = asprintf(&path, "%s/tapdisk-%d", BLKTAP2_NP_RUN_DIR, id);
+   if (err == -1) {
+       return -errno;
+   }
+
+   fid = open(path, O_RDONLY, 0600);
+   if (fid == -1) {
+       err = -errno;
+       EPRINTF("Failed to open VBD marker file for %d\n", id);
+       goto out;
+   }
+
+   return fid;
+
+out:
+   if (path) {
+       free(path);
+   }
+   return err;
+}
+
+void tapdisk_vbd_unlock(td_vbd_t *vbd)
+{
+   flock(vbd->lock_fd, LOCK_UN);
+   close(vbd->lock_fd);
+}
+
+int tapdisk_vbd_lock(td_vbd_t *vbd)
+{
+   int fid;
+
+   fid = open_vbd_marker(vbd->uuid);
+   if (fid < 0) {
+       /* Already logged */
+       return -1;
+   }
+
+   vbd->lock_fd = fid;
+   flock(vbd->lock_fd, LOCK_EX);
+
+   return 0;
+}
+
 /*
 int
 tapdisk_vbd_open(td_vbd_t *vbd, const char *name,
@@ -741,6 +791,7 @@ tapdisk_vbd_shutdown(td_vbd_t *vbd)
 		vbd->kicked);
 
 	tapdisk_vbd_close_vdi(vbd);
+	tapdisk_vbd_unlock(vbd);
 	tapdisk_server_remove_vbd(vbd);
 	tapdisk_vbd_free(vbd);
 
@@ -844,12 +895,6 @@ tapdisk_vbd_retry_needed(td_vbd_t *vbd)
 		 list_empty(&vbd->new_requests));
 }
 
-int
-tapdisk_vbd_lock(td_vbd_t *vbd)
-{
-	return 0;
-}
-
 int
 tapdisk_vbd_quiesce_queue(td_vbd_t *vbd)
 {

drivers/tapdisk-vbd.h

@@ -165,6 +165,8 @@ struct td_vbd_handle {
 	struct td_vbd_encryption   encryption;
 
 	bool                       watchdog_warned;
+
+	int                        lock_fd;
 };
 
 #define tapdisk_vbd_for_each_request(vreq, tmp, list)	                \
@@ -206,6 +208,9 @@ int tapdisk_vbd_open_vdi(td_vbd_t * vbd, const char *params, td_flag_t flags,
         int prt_devnum);
 void tapdisk_vbd_close_vdi(td_vbd_t *);
 
+int tapdisk_vbd_lock(td_vbd_t *);
+void tapdisk_vbd_unlock(td_vbd_t *);
+
 int tapdisk_vbd_queue_request(td_vbd_t *, td_vbd_request_t *);
 void tapdisk_vbd_forward_request(td_request_t);
 

include/blktap.h

@@ -37,4 +37,7 @@
 #define BLKTAP2_CONTROL_SOCKET         "ctl"
 #define BLKTAP2_ENOSPC_SIGNAL_FILE     "/run/tapdisk-enospc"
 
+/* Maximum number of possible minor ids, to match old kernel definition */
+#define MAX_ID  16384
+
 #endif /* _TD_BLKTAP_H_ */

mockatests/control/Makefile.am

@@ -15,6 +15,7 @@ test_control_LDFLAGS = -lcmocka
 test_control_LDFLAGS += -static-libtool-libs
 # Would be good to use the cmocka malloc wraps but looks like maybe strdup doesn't call malloc
 #test_control_LDFLAGS += -Wl,--wrap=malloc,--wrap=free
+test_control_LDFLAGS += -Wl,--wrap=stat
 test_control_LDFLAGS += -Wl,--wrap=socket,--wrap=connect,--wrap=read,--wrap=select,--wrap=write,--wrap=fdopen
 test_control_LDFLAGS += -Wl,--wrap=open,--wrap=ioctl,--wrap=close,--wrap=access,--wrap=mkdir,--wrap=flock,--wrap=unlink,--wrap=__xmknod
 #test_control_LDFLAGS += -Wl,--wrap=execl,--wrap=waitpid