Re: [hatari-devel] Re: Hatari SCSI Driver patch

[Uwe Seimet <Uwe.Seimet@xxxxxxxxx>]

Hi,

Should be better now, please see attachment.

Take care

Uwe

> Hi,
> 
> torstai 22 lokakuu 2015 12:31:18 Uwe Seimet kirjoitti:
> > The attached diffs add libudev support to the SCSI Driver, on systems
> > where libudev is available.
> 
> I think that your CMake stuff has the issue that it checks udev header
> and library separately, but doesn't check that both are present.
> Both are needed to get a working binary.
> 
> (If header is missing, linking udev is redundant.  If library
> is missing, compiling stuff with udev header fails to linking
> error.)
> 
> Besides note in readme.txt about libudev, IMHO note about
> it should be output at the end of the CMakeLists.txt file,
> like is done with the other libraries.
> 
> Could you send an updated version?
> 
> 
> 	- Eero
> 
> PS. Nicolas, SCSI drv adds only couple of KB to Hatari
> size even with libudev support:
> --------------
> $ size src/CMakeFiles/hatari.dir/nf_scsidrv.c.o
>    text    data     bss     dec     hex filename
>    4918       0     640    5558    15b6 
> src/CMakeFiles/hatari.dir/nf_scsidrv.c.o
> --------------
> 
> (Real executable size increase can be less than
> the object file code/data/bss size.)
> 
> 
> 

diff -r 9fd8a83bd51e CMakeLists.txt
--- a/CMakeLists.txt	Sun Oct 25 19:30:08 2015 +0100
+++ b/CMakeLists.txt	Wed Oct 28 08:38:37 2015 +0100
@@ -135,6 +135,11 @@
 	set(HAVE_CAPSIMAGE 1)
 endif(CAPSIMAGE_FOUND)
 
+find_package(Udev)
+if(UDEV_FOUND)
+	set(HAVE_UDEV 1)
+endif(UDEV_FOUND)
+
 # ################
 # CPP Definitions:
 # ################
@@ -356,6 +361,12 @@
   message( "  - capsimage :\tv${CAPSIMAGE_VERSION} not found, install it to use .IPF, .RAW and .CTR disk images" )
 endif(CAPSIMAGE_FOUND)
 
+if(UDEV_FOUND)
+  message( "  - udev :\tfound, required by SCSI Driver on systems with udev support" )
+else()
+  message( "  - udev :\tnot found, not required by SCSI Driver on systems without udev support" )
+endif(UDEV_FOUND)
+
 message( "" )
 
 
diff -r 9fd8a83bd51e cmake/FindUdev.cmake
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/cmake/FindUdev.cmake	Wed Oct 28 08:38:37 2015 +0100
@@ -0,0 +1,15 @@
+
+if(UDEV_INCLUDE_DIR)
+  # Already in cache, be silent
+  set(UDEV_FIND_QUIETLY TRUE)
+endif(UDEV_INCLUDE_DIR)
+
+find_path(UDEV_INCLUDE_DIR udev)
+
+find_library(UDEV_LIBRARY NAMES udev)
+
+include(FindPackageHandleStandardArgs)
+find_package_handle_standard_args(UDEV DEFAULT_MSG
+                                  UDEV_LIBRARY UDEV_INCLUDE_DIR)
+
+mark_as_advanced(UDEV_LIBRARY UDEV_INCLUDE_DIR)
diff -r 9fd8a83bd51e cmake/config-cmake.h
--- a/cmake/config-cmake.h	Sun Oct 25 19:30:08 2015 +0100
+++ b/cmake/config-cmake.h	Wed Oct 28 08:38:37 2015 +0100
@@ -99,3 +99,6 @@
 
 /* Define to 1 to enable trace logs - undefine to slightly increase speed */
 #cmakedefine ENABLE_TRACING 1
+
+/* Define to 1 if udev support is available */
+#cmakedefine HAVE_UDEV 1
diff -r 9fd8a83bd51e src/CMakeLists.txt
--- a/src/CMakeLists.txt	Sun Oct 25 19:30:08 2015 +0100
+++ b/src/CMakeLists.txt	Wed Oct 28 08:38:37 2015 +0100
@@ -147,6 +147,10 @@
 	target_link_libraries(hatari ${CAPSIMAGE_LIBRARY})
 endif(CAPSIMAGE_FOUND)
 
+if(UDEV_FOUND)
+	target_link_libraries(hatari ${UDEV_LIBRARY})
+endif(UDEV_FOUND)
+
 if(WIN32)
 	# Needed for socket() on Windows
 	target_link_libraries(hatari ws2_32)
diff -r 9fd8a83bd51e src/nf_scsidrv.c
--- a/src/nf_scsidrv.c	Sun Oct 25 19:30:08 2015 +0100
+++ b/src/nf_scsidrv.c	Wed Oct 28 08:38:37 2015 +0100
@@ -16,9 +16,13 @@
 
 #if defined(__linux__)
 
+#include "config.h"
 #include <stdlib.h>
 #include <unistd.h>
 #include <fcntl.h>
+#ifdef HAVE_UDEV
+#include <libudev.h>
+#endif
 #include <sys/ioctl.h>
 #include <scsi/sg.h>
 #include "stMemory.h"
@@ -47,6 +51,13 @@
 
 static HANDLE_META_DATA handle_meta_data[SCSI_MAX_HANDLES];
 
+#ifdef HAVE_UDEV
+static struct udev *udev;
+static struct udev_monitor *mon;
+static int fd;
+static fd_set udevFds;
+static struct timeval tv;
+#endif
 
 static Uint32 read_stack_long(Uint32 *stack)
 {
@@ -73,19 +84,54 @@
     STMemory_WriteWord(addr, value);
 }
 
-static void set_error(Uint32 handle, Uint32 errnum)
+static void set_error(Uint32 handle, int errbit)
 {
     Uint32 i;
     for(i = 0; i < SCSI_MAX_HANDLES; i++)
     {
         if(handle != i && handle_meta_data[i].fd &&
-           handle_meta_data[i].id_lo == handle_meta_data[handle].id_lo)
+            handle_meta_data[i].id_lo == handle_meta_data[handle].id_lo)
         {
-            handle_meta_data[i].error = errnum;
+            handle_meta_data[i].error |= errbit;
         }
     }
 }
 
+#ifdef HAVE_UDEV
+// udev-based check for media change
+static void check_mchg_udev(void)
+{
+    FD_ZERO(&udevFds);
+    FD_SET(fd, &udevFds);
+
+    int ret = select(fd + 1, &udevFds, 0, 0, &tv);
+    if(ret > 0 && FD_ISSET(fd, &udevFds))
+    {
+        struct udev_device *dev = udev_monitor_receive_device(mon);
+        const char *dev_type = udev_device_get_devtype(dev);
+        const char *action = udev_device_get_action(dev);
+        if(!strcmp("disk",  dev_type) && !strcmp("change", action))
+        {
+            LOG_TRACE(TRACE_SCSIDRV, ": %s was changed",
+                      udev_device_get_devnode(dev));
+
+            // TODO Determine sg device name from block device name
+            // and only report media change for the actually affected device
+            
+            // cErrMediach for all open handles
+            Uint32 i;
+            for(i = 0; i < SCSI_MAX_HANDLES; i++)
+            {
+                if(handle_meta_data[i].fd)
+                {
+                    handle_meta_data[i].error |= 1;
+                }
+            }
+        }
+    }
+}
+#endif
+
 static int check_device_file(Uint32 id)
 {
     char device_file[16];
@@ -149,6 +195,24 @@
 
 static int scsidrv_open(Uint32 stack)
 {
+#ifdef HAVE_UDEV
+    if(!udev)
+    {
+        udev = udev_new();
+        if(!udev) {
+            return -1;
+        }
+
+        mon = udev_monitor_new_from_netlink(udev, "udev");
+        udev_monitor_filter_add_match_subsystem_devtype(mon, "block", NULL);
+        udev_monitor_enable_receiving(mon);
+        fd = udev_monitor_get_fd(mon);
+
+        tv.tv_sec = 0;
+        tv.tv_usec = 0;
+    }
+#endif
+
     Uint32 handle = read_stack_long(&stack);
     Uint32 id = read_stack_long(&stack);
 
@@ -316,16 +380,22 @@
         return GEMDOS_ENHNDL;
     }
 
+    int errbit = 1 << errnum;
+
     if(rwflag)
     {
-        set_error(handle, errnum);
+        set_error(handle, errbit);
 
-        return errnum;
+        return 0;
     }
     else
     {
-        int status = handle_meta_data[handle].error;
-        handle_meta_data[handle].error = 0;
+#ifdef HAVE_UDEV
+        check_mchg_udev();
+#endif
+
+        int status = handle_meta_data[handle].error & errbit;
+        handle_meta_data[handle].error &= ~errbit;
 
         return status;
     }