bump version to 0.2.5HAL_0_2_5

bump version to 0.2.5
sleep 1000ms instead of 500ms before sending signal - we really need to
    update to latest libsysfs in hald to avoid the race - this is soo ugly
fix possible memory violation
call linux_class_block_removed() just before destroying the HalDevice obj
new function
add prototype for linux_class_block_removed()
new funtion
add prototype for ds_device_find_multiple_by_key_value_string()
Make this signal handler *a lot* safer by only setting a flag a timer can
    pick up (detect_media): properly handle situation when user removes
    media where partitions are mounted on (media_detect_timer_handler):
    check sigio_etc_changed (force_unmount_of_all_childs): newfunction
    (force_unmount): new function (linux_class_block_removed): new function

11 files changed, 339 insertions, 33 deletions

diff --git a/ChangeLog b/ChangeLog
index 1a965379..fe4ac15f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,40 @@
+2004-01-16  David Zeuthen  <david@fubar.dk>
+
+	* packaging/fedora/hal.spec (Version): bump version to 0.2.5
+
+	* configure.in: bump version to 0.2.5
+
+	* tools/linux/hal_hotplug.c (main): sleep 1000ms instead of 500ms
+	before sending signal - we really need to update to latest libsysfs
+	in hald to avoid the race - this is soo ugly
+
+	* hald/main.c (property_atomic_update_end): fix possible memory
+	violation
+
+	* hald/linux/linux_osspec.c (handle_hotplug): call 
+	linux_class_block_removed() just before destroying the HalDevice obj
+
+	* hald/linux/linux_class_block.c (linux_class_block_removed): new
+	function
+
+	* hald/linux/linux_class_block.h: add prototype for 
+	linux_class_block_removed()
+
+	* hald/device_store.c 
+	(ds_device_find_multiple_by_key_value_string): new funtion
+
+	* hald/device_store.h: add prototype for
+	ds_device_find_multiple_by_key_value_string()
+
+	* hald/linux/linux_class_block.c (sigio_handler): Make this signal
+	handler *a lot* safer by only setting a flag a timer can pick up
+	(detect_media): properly handle situation when user removes media
+	where partitions are mounted on
+	(media_detect_timer_handler): check sigio_etc_changed
+	(force_unmount_of_all_childs): newfunction
+	(force_unmount): new function
+	(linux_class_block_removed): new function
+
 2004-01-14  David Zeuthen  <david@fubar.dk>
 
 	* hald/linux/linux_class_block.c (detect_media): Doh, leaked a
diff --git a/Doxyfile b/Doxyfile
index 326fd723..8db1ec15 100644
--- a/Doxyfile
+++ b/Doxyfile
@@ -23,7 +23,7 @@ PROJECT_NAME           = HAL
 # This could be handy for archiving the generated documentation or 
 # if some version control system is used.
 
-PROJECT_NUMBER         = 0.2.4
+PROJECT_NUMBER         = 0.2.5
 
 # The OUTPUT_DIRECTORY tag is used to specify the (relative or absolute) 
 # base path where the generated documentation will be put. 
diff --git a/configure.in b/configure.in
index ab8e4782..6e9e4e60 100644
--- a/configure.in
+++ b/configure.in
@@ -1,8 +1,8 @@
 dnl Process this file with autoconf to produce a configure script.
 
 AC_PREREQ(2.57)
-AC_INIT(hal, 0.2.4, david@fubar.dk)
-AM_INIT_AUTOMAKE(hal, 0.2.4)
+AC_INIT(hal, 0.2.5, david@fubar.dk)
+AM_INIT_AUTOMAKE(hal, 0.2.5)
 AM_CONFIG_HEADER(config.h)
 AM_MAINTAINER_MODE
 
diff --git a/hald/device_store.c b/hald/device_store.c
index 545703f9..3d25b334 100644
--- a/hald/device_store.c
+++ b/hald/device_store.c
@@ -459,8 +459,68 @@ void ds_device_async_find_by_key_value_string(const char* key,
 
 }
 
+/** Find one or more devices by requiring a specific key to assume string
+ *  value. 
+ *
+ *  @param  key                 key of the property
+ *  @param  value               value of the property
+ *  @param  only_gdl            only search in the gdl
+ *  @param  num_results         pointer to where number of results are stored
+ *  @return                     Array of pointers to #HalDevice object, 
+ *                              terminated by #NULL, or #NULL if no such 
+ *                              devices exist. 
+ *                              Caller is supposed to free this with free()
+ */
+HalDevice** ds_device_find_multiple_by_key_value_string(const char* key, 
+                                                        const char* value,
+                                                        dbus_bool_t only_gdl,
+                                                        int* num_results)
+{
+    int type;
+    int num_devices;
+    HalDevice* device;
+    HalDevice** devices;
+    HalDeviceIterator iter_device;
+
+    /** @todo FIXME HACK XXX HERE_BE_DRAGONS this is an ugly hack, and a waste
+     *  to have max 1024 devices */
+    devices = xmalloc(sizeof(HalDevice*)*1024);
+    num_devices = 0;
+
+    for(ds_device_iter_begin(&iter_device);
+        ds_device_iter_has_more(&iter_device);
+        ds_device_iter_next(&iter_device))
+    {
+        device = ds_device_iter_get(&iter_device);
+
+        if( only_gdl && !device->in_gdl )
+            continue;
+
+        type = ds_property_get_type(device, key);
+        if( type==DBUS_TYPE_STRING )
+        {
+            if( strcmp(ds_property_get_string(device, key),
+                       value)==0 )
+                devices[num_devices++] = device;
+        }
+    }
+
+    if( num_devices==0 )
+    {
+        free(devices);
+        return NULL;
+    }
+    else
+    {
+        if( num_results!=NULL )
+            *num_results = num_devices;
+        return devices;
+    }
+}
+
 /** Find a device by requiring a specific key to assume string value. If 
- *  multiple devices meet this criteria then the result is undefined.
+ *  multiple devices meet this criteria then the result is undefined. Use
+ *  ds_device_find_multiple_by_key_value_string() instead.
  *
  *  @param  key                 key of the property
  *  @param  value               value of the property
@@ -1414,7 +1474,7 @@ void ds_add_capability(HalDevice* device, const char* capability)
     }
     else
     {
-        if( strstr(caps, capability)==NULL )
+        if( !ds_query_capability(device, capability) )
         {
             snprintf(buf, MAX_CAP_SIZE, "%s %s", caps, capability);
             ds_property_set_string(device, "info.capabilities", buf);
@@ -1440,6 +1500,9 @@ dbus_bool_t ds_query_capability(HalDevice* device, const char* capability)
     caps = ds_property_get_string(device, "info.capabilities");
     if( caps!=NULL )
     {
+        /** @todo FIXME this is clearly borken - consider properties foo
+         *  and foobar - you cannot add foo if foobar already exist 
+         */
         if( strstr(caps, capability)!=NULL )
             return TRUE;
     }
diff --git a/hald/device_store.h b/hald/device_store.h
index d4771972..d0dadd16 100644
--- a/hald/device_store.h
+++ b/hald/device_store.h
@@ -155,6 +155,11 @@ typedef void (*DSAsyncFindDeviceCB)(HalDevice* result,
 
 HalDevice* ds_device_find(const char* udi);
 
+HalDevice** ds_device_find_multiple_by_key_value_string(const char* key, 
+                                                        const char* value,
+                                                        dbus_bool_t only_gdl,
+                                                        int* num_results);
+
 HalDevice* ds_device_find_by_key_value_string(const char* key, 
                                               const char* value,
                                               dbus_bool_t only_gdl);
diff --git a/hald/linux/linux_class_block.c b/hald/linux/linux_class_block.c
index c590fbc6..fcefdc82 100644
--- a/hald/linux/linux_class_block.c
+++ b/hald/linux/linux_class_block.c
@@ -46,6 +46,7 @@
 #include <linux/fcntl.h>
 #include <linux/kdev_t.h>
 #include <linux/cdrom.h>
+#include <linux/fs.h>
 
 #include "../logger.h"
 #include "../device_store.h"
@@ -816,7 +817,7 @@ static void etc_mtab_process_all_block_devices(dbus_bool_t force)
                     "%s mounted at %s, major:minor=%d:%d, fstype=%s, udi=%s",
                     mp->device, mp->mount_point, mp->major, mp->minor, 
                     mp->fs_type, d->udi));
-                
+
                 property_atomic_update_begin();
 
                 existing_block_device = ds_property_get_string(d,
@@ -882,19 +883,21 @@ static void etc_mtab_process_all_block_devices(dbus_bool_t force)
 }
 
 
+/** Will be set to true by the SIGIO handler */
+static dbus_bool_t sigio_etc_changed = FALSE;
+
 /** Signal handler for watching /etc
  *
  *  @param  sig                 Signal number
  */
 static void sigio_handler(int sig)
 {
-    HAL_INFO(("Directory /etc changed"));
-
-    /** @todo FIXME: It's evil to sleep in a signal handler, yes? */
-    usleep(250*1000);
+    /* Set a variable instead of handling it now - this is *much* safer
+     * since this handler must be very careful - man signal for more 
+     * information 
+     */
 
-    /* don't force reloading of /etc/mtab */
-    etc_mtab_process_all_block_devices(FALSE);
+    sigio_etc_changed = TRUE;
 }
 
 /** Find udev root directory (e.g. '/udev/') by invoking '/sbin/udev -r'.
@@ -959,6 +962,165 @@ void linux_class_block_init()
     get_udev_root();
 }
 
+/** Force unmount of a patition. Must have block.volume=1 and valid
+ *  block.device
+ *
+ *  @param  d                   Device
+ */
+static void force_unmount(HalDevice* d)
+{
+    const char* device_file;
+    const char* device_mount_point;
+    const char* umount_argv[4] = {"/bin/umount", "-l", NULL, NULL};
+    char* umount_stdout;
+    char* umount_stderr;
+    int umount_exitcode;
+
+    device_file = ds_property_get_string(d, "block.device");
+    device_mount_point = ds_property_get_string(d, "block.mount_point");
+            
+    umount_argv[2] = device_file;
+            
+    if( ds_property_exists(d, "block.is_volume") &&
+        ds_property_get_bool(d, "block.is_volume") &&
+        device_mount_point!=NULL && 
+        strlen(device_mount_point)>0 )
+    {
+        HAL_INFO(("attempting /bin/umount -l %s", device_file));
+                                                
+        /* invoke umount */
+        if( g_spawn_sync("/",
+                         umount_argv,
+                         NULL,
+                         0,
+                         NULL,
+                         NULL,
+                         &umount_stdout,
+                         &umount_stderr,
+                         &umount_exitcode,
+                         NULL)!=TRUE )
+        {
+            HAL_ERROR(("Couldn't invoke /bin/umount"));
+        }
+        
+        if( umount_exitcode!=0 )
+        {
+            HAL_INFO(("/bin/umount returned %d", umount_exitcode));
+        }
+        else
+        {
+            /* Tell clients we are going to unmount so they close
+             * can files - otherwise this unmount is going to stall
+             *
+             * One candidate for catching this would be FAM - the
+             * File Alteration Monitor
+             *
+             * Lazy unmount been in Linux since 2.4.11, so we're
+             * homefree (but other OS'es might not support this)
+             */
+            HAL_INFO(("Goint to emit BlockForcedUnmountPartition('%s', '%s', TRUE)",
+                      device_file, device_mount_point));
+            emit_condition(
+                d, "BlockForcedUnmountPartition",
+                DBUS_TYPE_STRING, device_file,
+                DBUS_TYPE_STRING, device_mount_point,
+                DBUS_TYPE_BOOLEAN, TRUE,
+                DBUS_TYPE_INVALID);
+
+            /* Woohoo, have to change block.mount_point *afterwards*, other
+             * wise device_mount_point points to garbage and D-BUS throws
+             * us off the bus, in fact it's doing exiting with code 1
+             * for us - not nice
+             */
+            property_atomic_update_begin();
+            ds_property_set_string(d, "block.mount_point", "");
+            ds_property_set_string(d, "block.fstype", "");
+            ds_property_set_bool(d, "block.is_mounted", FALSE);
+            property_atomic_update_end();
+        }
+    }
+}
+
+/** Unmount all partitions that stems from this block device. Must have
+ *  block.is_volume==0
+ *
+ *  @param  d                   Device
+ */
+static void force_unmount_of_all_childs(HalDevice* d)
+{
+    int fd;
+    int num_childs;
+    const char* device_file;
+    HalDevice* child;
+    HalDevice** childs;
+
+    device_file = ds_property_get_string(d, "block.device");
+
+    childs = ds_device_find_multiple_by_key_value_string("info.parent",
+                                                         d->udi, 
+                                                         TRUE,
+                                                         &num_childs);
+    if( childs!=NULL )
+    {
+        int n;
+
+        for(n=0; n<num_childs; n++)
+        {
+            child = childs[n];
+
+            force_unmount(child);
+                        
+        } /* for all childs */
+        
+        free(childs);
+        
+        HAL_INFO(("Rereading partition table for %s", device_file));
+        fd = open(device_file, O_RDONLY|O_NONBLOCK);
+        if( fd!=-1 )
+        {
+            ioctl(fd, BLKRRPART);
+        }
+        close(fd);
+        
+        /* All this work should generate hotplug events to actually
+         * remove the child devices 
+         */
+
+        /* Finally, send a single signal on the device - this
+         * is useful for desktop policy clients such as g-v-m
+         * such that only a single annoying "dude, you need to
+         * *stop* the device before pulling it out" popup is
+         * displayed */
+        HAL_INFO(("Goint to emit BlockForcedUnmount('%s')",
+                  device_file));
+        emit_condition(d, "BlockForcedUnmount",
+                       DBUS_TYPE_STRING, device_file,
+                       DBUS_TYPE_INVALID);
+        
+    } /* childs!=NULL */
+}
+
+
+
+/** Called when this device is about to be removed
+ *
+ *  @param  d                   Device
+ */
+void linux_class_block_removed(HalDevice* d)
+{
+    if( ds_property_exists(d, "block.is_volume") )
+    {
+        if( ds_property_get_bool(d, "block.is_volume") )
+        {
+            force_unmount(d);
+        }
+        else
+        {
+            force_unmount_of_all_childs(d);
+        }
+    }
+}
+
 /** Check for media on a block device that is not a volume
  *
  *  @param  d                   Device to inspect; can be any device, but
@@ -970,7 +1132,9 @@ void linux_class_block_init()
 static dbus_bool_t detect_media(HalDevice* d)
 {
     int fd;
+    dbus_bool_t is_cdrom;
     const char* device_file;
+    HalDevice* child;
 
     /* need to be in GDL, need to have block.deve and 
      * have block.is_volume==FALSE 
@@ -985,29 +1149,44 @@ static dbus_bool_t detect_media(HalDevice* d)
     if( device_file==NULL )
         return FALSE;
     
-    /* This is sufficient to invoke hotplug remove and hotplug add
-     * for partitions on the device; neat!
-     */
-    fd = open(device_file, O_RDONLY|O_NONBLOCK);
+    /* we do special treatment for optical discs */
+    is_cdrom = ds_property_exists(d, "storage.media") &&
+        strcmp(ds_property_get_string(d, "storage.media"), "cdrom")==0 &&
+        ds_property_get_bool(d, "storage.cdrom.support_media_changed");
 
-    if( fd==-1 )
+    if( !is_cdrom )
     {
-        /* open failed */
-        HAL_WARNING(("open(\"%s\", O_RDONLY|O_NONBLOCK) failed, "
-                     "errno=%d", device_file, errno));
-        return FALSE;
-    }
+        fd = open(device_file, O_RDONLY);
+        
+        if( fd==-1 )
+        {
+            /* open failed */
+            HAL_WARNING(("open(\"%s\", O_RDONLY) failed, "
+                         "errno=%d", device_file, errno));
 
-    /* special treatment for optical discs */
-    if( ds_property_exists(d, "storage.media") &&
-        strcmp(ds_property_get_string(d, "storage.media"), "cdrom")==0 &&
-        ds_property_get_bool(d, "storage.cdrom.support_media_changed")
-        )
+            if( errno==ENOMEDIUM )
+            {
+                force_unmount_of_all_childs(d);
+            }
+
+        }
+
+    } /* device is not an optical drive */
+    else
     {
         int drive;
-        HalDevice* child;
         dbus_bool_t got_disc = FALSE;
-            
+
+        fd = open(device_file, O_RDONLY|O_NONBLOCK);
+
+        if( fd==-1 )
+        {
+            /* open failed */
+            HAL_WARNING(("open(\"%s\", O_RDONLY|O_NONBLOCK) failed, "
+                         "errno=%d", device_file, errno));
+            return FALSE;
+        }
+        
         drive = ioctl(fd, CDROM_DRIVE_STATUS, CDSL_CURRENT);
         switch( drive )
         {
@@ -1134,6 +1313,19 @@ static gboolean media_detect_timer_handler(gpointer data)
             break;
     }
 
+    /* check if the SIGIO signal handler delivered something to us */
+    if( sigio_etc_changed )
+    {
+        /* acknowledge we got it */
+        sigio_etc_changed = FALSE;
+
+        HAL_INFO(("Directory /etc changed"));
+        /* don't force reloading of /etc/mtab */
+        etc_mtab_process_all_block_devices(FALSE);
+    }
+
+    HAL_INFO(("exiting"));
+
     return TRUE;
 }
 
diff --git a/hald/linux/linux_class_block.h b/hald/linux/linux_class_block.h
index df2788ad..10b56cc7 100644
--- a/hald/linux/linux_class_block.h
+++ b/hald/linux/linux_class_block.h
@@ -31,7 +31,9 @@
 void visit_class_device_block(const char* path, 
                               struct sysfs_class_device *class_device);
 
-void linux_class_block_check_if_ready_to_add();
+void linux_class_block_removed(HalDevice* d);
+
+void linux_class_block_check_if_ready_to_add(HalDevice* d);
 
 void linux_class_block_init();
 void linux_class_block_detection_done();
diff --git a/hald/linux/linux_osspec.c b/hald/linux/linux_osspec.c
index 82873fc0..f2b47b48 100644
--- a/hald/linux/linux_osspec.c
+++ b/hald/linux/linux_osspec.c
@@ -504,6 +504,9 @@ static DBusHandlerResult handle_hotplug(DBusConnection* connection,
             }
             else
             {
+                if( strcmp(subsystem, "block")==0 )
+                    linux_class_block_removed(d);
+
                 HAL_INFO(("Removing classdevice @ sysfspath %s, udi %s", 
                           sysfs_devpath, d->udi));
                 ds_device_destroy(d);
diff --git a/hald/main.c b/hald/main.c
index 6b5179da..2c4233e0 100644
--- a/hald/main.c
+++ b/hald/main.c
@@ -37,6 +37,7 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <errno.h>
+#include <signal.h>
 
 #include <dbus/dbus.h>
 #include <dbus/dbus-glib.h>
@@ -1743,6 +1744,7 @@ static PendingUpdate* pending_updates_head = NULL;
 void property_atomic_update_end()
 {
     PendingUpdate* pu_iter = NULL;
+    PendingUpdate* pu_iter_next = NULL;
     PendingUpdate* pu_iter2 = NULL;
 
     --atomic_count;
@@ -1760,10 +1762,12 @@ void property_atomic_update_end()
 
         for(pu_iter=pending_updates_head; 
             pu_iter!=NULL; 
-            pu_iter=pu_iter->next)
+            pu_iter=pu_iter_next)
         {
             int num_updates_this;
 
+            pu_iter_next = pu_iter->next;
+
             if( pu_iter->device==NULL )
                 goto have_processed;
 
diff --git a/packaging/fedora/hal.spec b/packaging/fedora/hal.spec
index bd43cbe2..434f77d1 100644
--- a/packaging/fedora/hal.spec
+++ b/packaging/fedora/hal.spec
@@ -4,7 +4,7 @@
 
 Summary: Hardware Abstraction Layer
 Name: hal
-Version: 0.2.4
+Version: 0.2.5
 Release: 1
 URL: http://www.freedesktop.org/software/hal/
 Source0: %{name}-%{version}.tar.gz
diff --git a/tools/linux/hal_hotplug.c b/tools/linux/hal_hotplug.c
index 26d016b0..eec5e8c6 100644
--- a/tools/linux/hal_hotplug.c
+++ b/tools/linux/hal_hotplug.c
@@ -103,7 +103,7 @@ int main(int argc, char* argv[], char* envp[])
     /* Do some sleep here so the kernel have time to publish it's
      * stuff in sysfs
      */
-    usleep(500*1000);
+    usleep(1000*1000);
 
     if ( !dbus_connection_send(sysbus_connection, message, NULL) )
         return 1;