Blob Blame History Raw
usbscsi: Switch from lockdev to flock

Currently there are 2 problems with the lockdev usage in usbscsi:
1) It breaks usbscsi completely due to a missing symbol when dlopening
   usbscsi.so, which is caused by libgphoto2_port/usbscsi/Makefile-files not
   adding $(SERIAL_LIBS) to usbscsi_la_LIBADD
2) lockdev uses /var/lock/lockdev, which by default is:
   drwxrwxr-x. 2 root lock 40 Sep 19 22:49 /var/lock/lockdev
   So despite our udev rules, gphoto using apps need to run as
   root (or group lockdev) to be able to work with usbscsi port devices

I've decided to fix 2) by moving to flock, lockdev makes sense for serial
ports, since other programs may be trying to access them at the same time,
for usbscsi however we only need to coordinate with other apps also using
libgphoto2, and flock then suffices, is much simpler and does not have
the rights issues of lockdev. This fix for 2), also fixes 1) by simply no
longer needing lockdev.

Index: libgphoto2_port/usbscsi/linux.c
===================================================================
--- libgphoto2_port/usbscsi/linux.c	(revision 14108)
+++ libgphoto2_port/usbscsi/linux.c	(working copy)
@@ -1,6 +1,6 @@
 /* SCSI commands to USB Mass storage devices port library for Linux
  * 
- *   Copyright (c) 2010 Hans de Goede <hdegoede@redhat.com>
+ *   Copyright (c) 2010-2012 Hans de Goede <hdegoede@redhat.com>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU Lesser General Public License as published by
@@ -19,6 +19,7 @@
 #include "config.h"
 #include <gphoto2/gphoto2-port-library.h>
 
+#include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
@@ -29,6 +30,7 @@
 #ifdef HAVE_LIMITS_H
 # include <limits.h>
 #endif
+#include <sys/file.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #ifdef HAVE_SYS_PARAM_H
@@ -40,9 +42,6 @@
 #ifdef HAVE_SCSI_SG_H
 # include <scsi/sg.h>
 #endif
-#ifdef HAVE_LOCKDEV
-#  include <lockdev.h>
-#endif
 
 #include <gphoto2/gphoto2-port-result.h>
 #include <gphoto2/gphoto2-port-log.h>
@@ -80,62 +79,37 @@
 }
 
 static int
-gp_port_usbscsi_lock (GPPort *port, const char *path)
+gp_port_usbscsi_lock (GPPort *port)
 {
-#ifdef HAVE_LOCKDEV
-	int pid;
-
 	gp_log (GP_LOG_DEBUG, "gphoto2-port-usbscsi",
-		"Trying to lock '%s'...", path);
+		"Trying to lock '%s'...", port->settings.usbscsi.path);
 
-	pid = dev_lock (path);
-	if (pid) {
-		if (port) {
-			if (pid > 0)
-				gp_port_set_error (port, _("Device '%s' is "
-					"locked by pid %d"), path, pid);
-			else
-				gp_port_set_error (port, _("Device '%s' could "
-					"not be locked (dev_lock returned "
-					"%d)"), path, pid);
+	if (flock(port->pl->fd, LOCK_EX | LOCK_NB) != 0) {
+		switch (errno) {
+		case EWOULDBLOCK:
+			gp_port_set_error (port,
+				_("Device '%s' is locked by another app."),
+				port->settings.usbscsi.path);
+			return GP_ERROR_IO_LOCK;
+		default:
+			gp_port_set_error (port,
+				_("Failed to lock '%s' (%m)."),
+				port->settings.usbscsi.path);
+			return GP_ERROR_IO;
 		}
-		return GP_ERROR_IO_LOCK;
 	}
-#else
-# ifdef __GCC__
-#  warning No locking library found. 
-#  warning You will run into problems if you use
-#  warning gphoto2 with a usbscsi picframe in 
-#  warning combination with Konqueror (KDE) or Nautilus (GNOME).
-#  warning This will *not* concern USB cameras.
-# endif
-#endif
 
 	return GP_OK;
 }
 
 static int
-gp_port_usbscsi_unlock (GPPort *port, const char *path)
+gp_port_usbscsi_unlock (GPPort *port)
 {
-#ifdef HAVE_LOCKDEV
-	int pid;
-
-	pid = dev_unlock (path, 0);
-	if (pid) {
-		if (port) {
-			if (pid > 0)
-				gp_port_set_error (port, _("Device '%s' could "
-					"not be unlocked as it is locked by "
-					"pid %d."), path, pid);
-			else
-				gp_port_set_error (port, _("Device '%s' could "
-					"not be unlocked (dev_unlock "
-					"returned %d)"), path, pid);
-		}
-		return GP_ERROR_IO_LOCK;
+	if (flock(port->pl->fd, LOCK_UN) != 0) {
+		gp_port_set_error (port, _("Failed to unlock '%s' (%m)."),
+				   port->settings.usbscsi.path);
+		return GP_ERROR_IO;
 	}
-#endif /* !HAVE_LOCKDEV */
-
 	return GP_OK;
 }
 
@@ -271,34 +245,36 @@
 	const int max_tries = 5;
 	const char *path = port->settings.usbscsi.path;
 
-	result = gp_port_usbscsi_lock (port, path);
-	if (result != GP_OK) {
-		for (i = 0; i < max_tries; i++) {
-			result = gp_port_usbscsi_lock (port, path);
-			if (result == GP_OK)
-				break;
-			gp_log (GP_LOG_DEBUG, "gphoto2-port-usbscsi",
-				"Failed to get a lock, trying again...");
-			sleep (1);
-		}
-		CHECK (result)
-	}
 	port->pl->fd = open (path, O_RDWR);
 	if (port->pl->fd == -1) {
-		gp_port_usbscsi_unlock (port, path);
 		gp_port_set_error (port, _("Failed to open '%s' (%m)."), path);
 		return GP_ERROR_IO;
 	}
 
-	return GP_OK;
+	result = gp_port_usbscsi_lock (port);
+	for (i = 0; i < max_tries && result == GP_ERROR_IO_LOCK; i++) {
+		gp_log (GP_LOG_DEBUG, "gphoto2-port-usbscsi",
+			"Failed to get a lock, trying again...");
+		sleep (1);
+		result = gp_port_usbscsi_lock (port);
+	}
+	if (result != GP_OK) {
+		close (port->pl->fd);
+		port->pl->fd = -1;
+	}
+	return result;
 }
 
 static int
 gp_port_usbscsi_close (GPPort *port)
 {
+	int result;
+
 	if (!port || port->pl->fd == -1)
 		return GP_OK;
 
+	result = gp_port_usbscsi_unlock (port);
+
 	if (close (port->pl->fd) == -1) {
 		gp_port_set_error (port, _("Could not close "
 			"'%s' (%m)."), port->settings.usbscsi.path);
@@ -306,10 +282,7 @@
 	}
 	port->pl->fd = -1;
 
-	CHECK (gp_port_usbscsi_unlock (port,
-					port->settings.usbscsi.path))
-
-	return GP_OK;
+	return result;
 }
 
 static int gp_port_usbscsi_send_scsi_cmd (GPPort *port, int to_dev, char *cmd,