runtime shutdown errors: simplify, improve error specificity

* transport/control.c (*_cmd): Return -Ecodes rather than "-1" from
  file_operations callbacks.
* staprun/ctl.c (init_ctl_channel): Return distinct error codes.
* staprun/staprun.c (remove_module): Skip connection attempt to .ctl
  file; just do delete_module() with O_NONBLOCK.

3 files changed, 18 insertions, 19 deletions

diff --git a/runtime/staprun/ctl.c b/runtime/staprun/ctl.c
index 4597bf72..335006ec 100644
--- a/runtime/staprun/ctl.c
+++ b/runtime/staprun/ctl.c
@@ -24,11 +24,11 @@ int init_ctl_channel(const char *name, int verb)
 	} else {
 		old_transport = 1;
 		if (sprintf_chk(buf, "/proc/systemtap/%s/.cmd", name))
-			return -1;
+			return -2;
 	}
 
-	dbug(2, "Opening %s\n", buf);
 	control_channel = open(buf, O_RDWR);
+	dbug(2, "Opened %s (%d)\n", buf, control_channel);
 	if (control_channel < 0) {
 		if (verb) {
 			if (attach_mod && errno == ENOENT)
@@ -36,10 +36,10 @@ int init_ctl_channel(const char *name, int verb)
 			else
 				perr("Couldn't open control channel '%s'", buf);
 		}
-		return -1;
+		return -3;
 	}
 	if (set_clexec(control_channel) < 0)
-		return -1;
+		return -4;
 
 	return old_transport;
 }
@@ -47,6 +47,7 @@ int init_ctl_channel(const char *name, int verb)
 void close_ctl_channel(void)
 {
   if (control_channel >= 0) {
+          	dbug(2, "Closed ctl fd %d\n", control_channel);
 		close(control_channel);
 		control_channel = -1;
 	}
diff --git a/runtime/staprun/staprun.c b/runtime/staprun/staprun.c
index 7b4aba1c..c5651d9a 100644
--- a/runtime/staprun/staprun.c
+++ b/runtime/staprun/staprun.c
@@ -181,27 +181,25 @@ static int remove_module(const char *name, int verb)
 	int ret;
 	dbug(2, "%s\n", name);
 
+        (void) verb; /* XXX: ignore */
+
 	if (strcmp(name, "*") == 0) {
 		remove_all_modules();
 		return 0;
 	}
 
-	/* Call init_ctl_channel() which actually attempts an open()
-	 * of the control channel. This is better than using access() because
-	 * an open on an already open channel will fail, preventing us from attempting
-	 * to remove an in-use module.
-	 */
-	if (init_ctl_channel(name, 0) < 0) {
-		if (verb)
-			err("Error accessing systemtap module %s: %s\n", name, strerror(errno));
-		return 1;
-	}
-	close_ctl_channel();
+        /* We could call init_ctl_channel / close_ctl_channel here, as a heuristic
+           to determine whether the module is being used by some other stapio process.
+           However, delete_module() does basically the same thing. */
 
 	dbug(2, "removing module %s\n", name);
 	STAP_PROBE1(staprun, remove__module, name);
-	ret = delete_module (name, 0);
+	ret = delete_module (name, O_NONBLOCK);
 	if (ret != 0) {
+                /* XXX: maybe we should just accept this, with a
+                   diagnostic, but without an error.  Might it be
+                   possible for the same module to be started up just
+                   as we're shutting down?  */
 		err("Error removing module '%s': %s.\n", name, strerror(errno));
 		return 1;
 	}
diff --git a/runtime/transport/control.c b/runtime/transport/control.c
index 0e18bb8b..8712a8ad 100644
--- a/runtime/transport/control.c
+++ b/runtime/transport/control.c
@@ -59,7 +59,7 @@ static ssize_t _stp_ctl_write_cmd(struct file *file, const char __user *buf, siz
 #ifdef STP_BULKMODE
 		return count;
 #else
-		return -1;
+		return -EINVAL;
 #endif
 	case STP_RELOCATION:
           	_stp_do_relocation (buf, count);
@@ -150,7 +150,7 @@ static int _stp_ctl_write(int type, void *data, unsigned len)
 	/* get a buffer from the free pool */
 	bptr = _stp_mempool_alloc(_stp_pool_q);
 	if (unlikely(bptr == NULL))
-		return -1;
+		return -ENOMEM;
 
 	bptr->type = type;
 	memcpy(bptr->buf, data, len);
@@ -225,7 +225,7 @@ static ssize_t _stp_ctl_read_cmd(struct file *file, char __user *buf,
 static int _stp_ctl_open_cmd(struct inode *inode, struct file *file)
 {
 	if (_stp_ctl_attached)
-		return -1;
+		return -EBUSY;
 	_stp_attach();
 	return 0;
 }