Re: [Utopia] [patch] mount/unmount when starting/stopping

[David Zeuthen <david fubar dk>]

Hi,

On Tue, 2004-08-17 at 10:11 -0400, Joe Shaw wrote:
> Hi,
> 
> On Tue, 2004-08-17 at 15:42 +0200, David Zeuthen wrote:
> > +/** List of UDI's for volumes mounted when starting up */
> > +static GSList *mount_ignore_udi_list;
> > +
> > +/** List of UDI's of all volumes mounted during the lifetime of the program */
> > +static GSList *all_mounted_volumes;
> 
> You need to assign these to NULL.
> 

Thanks for catching this, my bad.

> > +               for (i=all_mounted_volumes; i != NULL; i = g_slist_next (i)) {
> > +                       if (strcmp (udi, (const char *)i->data) == 0) {
> > +                               g_free (i->data);
> > +                               all_mounted_volumes = 
> > +                                       g_slist_remove_link (
> > +                                               all_mounted_volumes, i);
> > +                               break;
> > +                       }
> > +               }
> 
> I think this will cause problems because if you remove the link that
> you're on, then the next for node i will be NULL 

Well, we're breaking out anyway..

> and the loop will
> terminate.  Also since i isn't freed, you'll be leaking the node.  I
> think you'd need to do something like:
> 
> for (i = all_mounted_volumes; i != NULL; i = next) {
> 	next = g_slist_next (i);
> 
> 	...
> 
> 	all_mounted_volumes = g_slist_delete_link (all_mounted_volumes, i);
> }
> 
> Ditto for mount_ignore_udi_list.  Other than those, it looks good to me.
> 

Yeah. I've attached the third patch with these changes.

Cheers,
David

Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/gnome-volume-manager/ChangeLog,v
retrieving revision 1.87
diff -u -p -r1.87 ChangeLog
--- ChangeLog	13 Aug 2004 20:58:23 -0000	1.87
+++ ChangeLog	17 Aug 2004 14:24:57 -0000
@@ -1,3 +1,24 @@
+2004-08-17  David Zeuthen  <david fubar dk>
+
+	* src/manager.c
+	(gvm_device_mount): Mount synchronously so we know whether it
+	fails and can report
+	(gvm_device_unmount): New function
+	(hal_property_modified): Respect the list of ignored UDI's.
+	Maintain the list of volumes that have been mounted during our
+	lifetime
+	(hal_device_condition): Don't listen for the EjectPressed
+	condition; HAL no longer emits it (it only works on a tiny
+	fraction of devices)
+	(mount_all): New function
+	(unmount_all): New function
+	(handle_sigterm): New function
+	(sigterm_iochn_data): New function
+	(gvm_die): New function
+	(main): Setup handler for 'die' signal. Setup SIGTERM handler and
+	create a pipe and integrate this pipe into the mainloop. Invoke
+	mount_all
+
 2004-08-13  Adam Roben  <aroben fas harvard edu>
 
 	* src/manager.c: storage.drive_type belongs to the media's parent in
Index: src/manager.c
===================================================================
RCS file: /cvs/gnome/gnome-volume-manager/src/manager.c,v
retrieving revision 1.28
diff -u -p -r1.28 manager.c
--- src/manager.c	13 Aug 2004 20:58:23 -0000	1.28
+++ src/manager.c	17 Aug 2004 14:24:58 -0000
@@ -21,6 +21,7 @@
 #include <dbus/dbus.h>
 #include <dbus/dbus-glib.h>
 #include <libhal.h>
+#include <signal.h>
 
 #include "gvm.h"
 
@@ -47,11 +48,18 @@
 #define warn(fmt,arg...) g_warning("%s/%d: " fmt,__FILE__,__LINE__,##arg)
 
 #define BIN_MOUNT	"/bin/mount"	/* what we mount with */
+#define BIN_UMOUNT	"/bin/umount"	/* what we unmount with */
 #define NAUTILUS_COMMAND "/usr/bin/nautilus -n --no-desktop %m"
 
 static struct gvm_configuration config;
 static LibHalContext *hal_ctx;
 
+/** List of UDI's for volumes mounted when starting up */
+static GSList *mount_ignore_udi_list = NULL;
+
+/** List of UDI's of all volumes mounted during the lifetime of the program */
+static GSList *all_mounted_volumes = NULL;
+
 /*
  * gvm_load_config - synchronize gconf => config structure
  */
@@ -381,21 +389,55 @@ out:
  *
  * Note that this requires that the given device node is in /etc/fstab.  This
  * is intentional.
+ *
+ * @return TRUE iff the mount was succesful
  */
-static void
+static gboolean
 gvm_device_mount (char *device)
 {
 	char *argv[3];
 	GError *error = NULL;
+	gint exit_status;
 
 	argv[0] = BIN_MOUNT;
 	argv[1] = device;
 	argv[2] = NULL;
 
-	g_spawn_async (g_get_home_dir (), argv, NULL, 0, NULL,
-		       NULL, NULL, &error);
-	if (error)
+	if (!g_spawn_sync (g_get_home_dir (), argv, NULL, 0, NULL,
+			   NULL, NULL, NULL, &exit_status, &error)) {
+		warn ("failed to exec " BIN_MOUNT ": %s\n", error->message);
+		return FALSE;
+	}
+
+	return exit_status == 0;
+}
+
+/*
+ * gvm_device_unmount - use BIN_UMOUNT to unmount the given device node.
+ *
+ * Note that this requires that the given device node is in /etc/fstab.  This
+ * is intentional.
+ *
+ * @return TRUE iff the mount was succesful
+ */
+static gboolean
+gvm_device_unmount (char *device)
+{
+	char *argv[3];
+	GError *error = NULL;
+	gint exit_status;
+
+	argv[0] = BIN_UMOUNT;
+	argv[1] = device;
+	argv[2] = NULL;
+
+	if (!g_spawn_sync (g_get_home_dir (), argv, NULL, 0, NULL,
+			   NULL, NULL, NULL, &exit_status, &error)) {
 		warn ("failed to exec " BIN_MOUNT ": %s\n", error->message);
+		return FALSE;
+	}
+
+	return exit_status == 0;
 }
 
 /*
@@ -688,16 +730,61 @@ hal_property_modified (LibHalContext *ct
 		       dbus_bool_t is_removed __attribute__((__unused__)), 
 		       dbus_bool_t is_added __attribute__((__unused__)))
 {
-	if (!g_strcasecmp (key, "volume.is_mounted")) {
-		dbus_bool_t val;
+	dbus_bool_t val;
+	GSList *i;
+	GSList *next;
+
+	if (g_strcasecmp (key, "volume.is_mounted") != 0)
+		return;
+	
+	val = hal_device_get_property_bool (hal_ctx, udi, key);
+	if (val == TRUE) {
+		gboolean ignore_mount;
+
+		dbg ("Mounted: %s\n", udi);
+
+		/* add to list of all volumes mounted during lifetime */
+		all_mounted_volumes = g_slist_append (all_mounted_volumes,
+						      g_strdup (udi));
+
+		ignore_mount = FALSE;
+		for (i = mount_ignore_udi_list; i != NULL; i = next) {
+			next = g_slist_next (i);
+
+			if (strcmp (udi, (const char *)i->data) == 0) {
+				ignore_mount = TRUE;
+				g_free (i->data);
+				mount_ignore_udi_list = 
+					g_slist_delete_link (
+						mount_ignore_udi_list, i);
+				break;
+			}
+		}
+		
 		
-		val = hal_device_get_property_bool (hal_ctx, udi, key);
-		if (val == TRUE) {
-			dbg ("Mounted: %s\n", udi);
+		/* only autorun if not in ignore list, cf.
+		 * function mount_all
+			 */
+		if (!ignore_mount)
 			gvm_device_autorun (udi);
-		} else
-			dbg ("Unmounted: %s\n", udi);
-	}	
+
+	} else {
+		dbg ("Unmounted: %s\n", udi);
+
+		/* remove from list of all volumes mounted during lifetime */
+
+		for (i=all_mounted_volumes; i != NULL; i = next) {
+			next = g_slist_next (i);
+			if (strcmp (udi, (const char *)i->data) == 0) {
+				g_free (i->data);
+				all_mounted_volumes = 
+					g_slist_delete_link (
+						all_mounted_volumes, i);
+				break;
+			}
+		}
+
+	}
 }
 
 /** Invoked when a device in the GDL emits a condition that cannot be
@@ -714,29 +801,6 @@ hal_device_condition (LibHalContext *ctx
 		      const char *condition_name __attribute__((__unused__)),
 		      DBusMessage * message __attribute__((__unused__)))
 {
-	if (!g_strcasecmp (condition_name, "EjectPressed")) {		
-		char *argv[3];
-		GError *error = NULL;
-		char *device;
-
-		device = hal_device_get_property_string (ctx, udi,
-							 "block.device");
-		if (!device)
-			warn ("cannot get block.device\n");
-		else if (config.eject_command) {
-			argv[0] = config.eject_command;
-			argv[1] = device;
-			argv[2] = NULL;
-
-			g_spawn_async (g_get_home_dir (), argv, NULL, 0, NULL,
-				       NULL, NULL, &error);
-			if (error)
-				warn ("failed to exec %s: %s\n",
-				      config.eject_command, error->message);
-		}
-
-		hal_free_string (device);
-	}
 }
 
 /** Invoked by libhal for integration with our mainloop. 
@@ -793,6 +857,151 @@ gvm_do_hal_init (LibHalFunctions *functi
 	return ctx;
 }
 
+/** Attempt to mount all volumes; should be called on startup.
+ *
+ *  @param  ctx                 LibHal context
+ */
+static void mount_all (LibHalContext *ctx)
+{
+	int i;
+	int num_volumes;
+	char **volumes;
+	char *udi;
+	char *device_file;
+
+	if (!config.automount_media)
+		return;
+
+	volumes = hal_find_device_by_capability (ctx, "volume", &num_volumes);
+	for (i = 0; i < num_volumes; i++) {
+		udi = volumes [i];
+
+		/* don't attempt to mount already mounted volumes */
+		if (!hal_device_property_exists (ctx, udi, 
+						"volume.is_mounted") ||
+		    hal_device_get_property_bool (ctx, udi, 
+						  "volume.is_mounted"))
+			continue;
+
+		/* only mount if the block device got a sensible filesystem */
+		if (!hal_device_property_exists (ctx, udi, 
+						"volume.is_filesystem") ||
+		    !hal_device_get_property_bool (ctx, udi, 
+						   "volume.is_filesystem"))
+			continue;
+
+		device_file = hal_device_get_property_string (ctx, udi,
+							      "block.device");
+
+		if (device_file != NULL ) {
+
+			dbg ("mount_all: mounting %s\n", device_file);
+
+			if (gvm_device_mount (device_file)) {
+				/* yay, it worked; add to list of
+				 * udi's not to autorun from hal_property_
+				 * changed
+				 */
+
+				mount_ignore_udi_list = 
+					g_slist_append (mount_ignore_udi_list,
+							g_strdup (udi));
+
+			}
+
+			hal_free_string (device_file);
+		} else {
+			warn ("no device_file for udi=%s\n", udi);
+		}
+	}
+
+	hal_free_string_array (volumes);
+}
+
+/* Unmount all volumes that were mounted during the lifetime of this
+ * g-v-m instance
+ *
+ *  @param  ctx                 LibHal context
+ */
+static void
+unmount_all (LibHalContext *ctx)
+{
+	GSList *i;
+	char *device_file;
+	char *udi;
+
+	dbg ("unmounting all volumes that we saw mounted in our life\n");
+
+	for (i = all_mounted_volumes; i != NULL; i = g_slist_next (i)) {
+
+		udi = i->data;
+
+		device_file = hal_device_get_property_string (ctx, udi,
+							      "block.device");
+		if (device_file != NULL ) {
+
+			dbg ("unmount_all: unmounting %s\n", device_file);
+
+			gvm_device_unmount (device_file);
+			hal_free_string (device_file);
+		} else {
+			warn ("no device_file for udi=%s\n", udi);
+		}
+	}
+}
+
+
+static int sigterm_unix_signal_pipe_fds[2];
+static GIOChannel *sigterm_iochn;
+
+static void 
+handle_sigterm (int value)
+{
+	static char marker[1] = {'S'};
+
+	/* write a 'S' character to the other end to tell about
+	 * the signal. Note that 'the other end' is a GIOChannel thingy
+	 * that is only called from the mainloop - thus this is how we
+	 * defer this since UNIX signal handlers are evil
+	 *
+	 * Oh, and write(2) is indeed reentrant */
+	write (sigterm_unix_signal_pipe_fds[1], marker, 1);
+}
+
+static gboolean
+sigterm_iochn_data (GIOChannel *source, 
+		    GIOCondition condition, 
+		    gpointer user_data)
+{
+	GError *err = NULL;
+	gchar data[1];
+	gsize bytes_read;
+
+	/* Empty the pipe */
+	if (G_IO_STATUS_NORMAL != 
+	    g_io_channel_read_chars (source, data, 1, &bytes_read, &err)) {
+		warn ("Error emptying callout notify pipe: %s",
+		      err->message);
+		g_error_free (err);
+		goto out;
+	}
+
+	dbg ("Received SIGTERM, initiating shutdown\n");
+	unmount_all (hal_ctx);
+	gtk_main_quit();
+
+out:
+	return TRUE;
+}
+
+static void
+gvm_die (GnomeClient *client, gpointer user_data) 
+{
+	dbg ("Received 'die', initiating shutdown\n");
+	unmount_all (hal_ctx);
+	gtk_main_quit ();
+}
+
 int
 main (int argc, char *argv[])
 {
@@ -822,7 +1031,7 @@ main (int argc, char *argv[])
 	}
 
 	gtk_signal_connect (GTK_OBJECT (client), "die",
-			    GTK_SIGNAL_FUNC (gtk_main_quit), NULL);
+			    GTK_SIGNAL_FUNC (gvm_die), NULL);
 
 	hal_ctx = gvm_do_hal_init (&hal_functions);
 	if (!hal_ctx)
@@ -830,6 +1039,22 @@ main (int argc, char *argv[])
 
 	gvm_init_config ();
 
+	/* SIGTERM handling via pipes  */
+	if (pipe (sigterm_unix_signal_pipe_fds) != 0) {
+		warn ("Could not setup pipe, errno=%d", errno);
+		return 1;
+	}
+	sigterm_iochn = g_io_channel_unix_new (sigterm_unix_signal_pipe_fds[0]);
+	if (sigterm_iochn == NULL) {
+		warn ("Could not create GIOChannel");
+		return 1;
+	}
+	g_io_add_watch (sigterm_iochn, G_IO_IN, sigterm_iochn_data, NULL);
+	signal (SIGTERM, handle_sigterm);
+
+
+	mount_all (hal_ctx);
+
 	gtk_main ();
 
 	return 0;