New watch handle mechanism for File_system session

File_system clients may now watch files and directories for changes by
opening a 'Watch_handle' rather than submitting a 'CONTENT_CHANGED'
packet to the server. When a change happens at a node with an open
Watch_handle a CONTENT_CHANGED packet will be sent from the server to
the client. This serializes registration with other handle operations
and separates I/O handle state from notification handle state.

Test at run/fs_rom_update.

Ref #1934

repos/os/include/file_system_session/client.h

@@ -5,7 +5,7 @@
  */
 
 /*
- * Copyright (C) 2012-2017 Genode Labs GmbH
+ * Copyright (C) 2012-2018 Genode Labs GmbH
  *
  * This file is part of the Genode OS framework, which is distributed
  * under the terms of the GNU Affero General Public License version 3.
@@ -81,6 +81,11 @@ class File_system::Session_client : public Genode::Rpc_client<Session>
 			return call<Rpc_node>(path);
 		}
 
+		Watch_handle watch(Path const &path) override
+		{
+			return call<Rpc_watch>(path);
+		}
+
 		void close(Node_handle node) override
 		{
 			call<Rpc_close>(node);

repos/os/include/file_system_session/connection.h

@@ -5,7 +5,7 @@
  */
 
 /*
- * Copyright (C) 2012-2017 Genode Labs GmbH
+ * Copyright (C) 2012-2018 Genode Labs GmbH
  *
  * This file is part of the Genode OS framework, which is distributed
  * under the terms of the GNU Affero General Public License version 3.
@@ -152,6 +152,12 @@ struct File_system::Connection : File_system::Connection_base
 		return _retry([&] () {
 			return Session_client::node(path); });
 	}
+
+	Watch_handle watch(Path const &path) override
+	{
+		return _retry([&] () {
+			return Session_client::watch(path); });
+	}
 };
 
 #endif /* _INCLUDE__FILE_SYSTEM_SESSION__CONNECTION_H_ */

repos/os/include/file_system_session/file_system_session.h

@@ -7,7 +7,7 @@
  */
 
 /*
- * Copyright (C) 2012-2017 Genode Labs GmbH
+ * Copyright (C) 2012-2018 Genode Labs GmbH
  *
  * This file is part of the Genode OS framework, which is distributed
  * under the terms of the GNU Affero General Public License version 3.
@@ -52,10 +52,19 @@ namespace File_system {
 		};
 	};
 
+	struct Watch : Node
+	{
+		struct Id : Node::Id
+		{
+			explicit Id(unsigned long value) : Node::Id { value } { };
+		};
+	};
+
 	typedef Node::Id      Node_handle;
 	typedef File::Id      File_handle;
 	typedef Directory::Id Dir_handle;
 	typedef Symlink::Id   Symlink_handle;
+	typedef Watch::Id     Watch_handle;
 
 	using Genode::size_t;
 
@@ -337,6 +346,23 @@ struct File_system::Session : public Genode::Session
 	 */
 	virtual Node_handle node(Path const &path) = 0;
 
+	/**
+	 * Watch a node for for changes.
+	 *
+	 * When changes are made to the node at this path a CONTENT_CHANGED
+	 * packet will be sent from the server to the client.
+	 *
+	 * \throw Lookup_failed      path lookup failed because one element
+	 *                           of 'path' does not exist
+	 * \throw Out_of_ram         server cannot allocate metadata
+	 * \throw Out_of_caps
+	 * \throw Unavailable        file-system is static or does not support
+	 *                           notifications
+	 *
+	 * The returned node handle is used to identify notification packets.
+	 */
+	virtual Watch_handle watch(Path const &path) = 0;
+
 	/**
 	 * Close file
 	 *
@@ -421,6 +447,9 @@ struct File_system::Session : public Genode::Session
 	GENODE_RPC_THROW(Rpc_node, Node_handle, node,
 	                 GENODE_TYPE_LIST(Lookup_failed, Out_of_ram, Out_of_caps),
 	                 Path const &);
+	GENODE_RPC_THROW(Rpc_watch, Watch_handle, watch,
+	                 GENODE_TYPE_LIST(Lookup_failed, Out_of_ram, Out_of_caps, Unavailable),
+	                 Path const &);
 	GENODE_RPC_THROW(Rpc_close, void, close,
 	                 GENODE_TYPE_LIST(Invalid_handle),
 	                 Node_handle);
@@ -444,7 +473,9 @@ struct File_system::Session : public Genode::Session
 	                                  Lookup_failed, Permission_denied, Unavailable),
 	                 Dir_handle, Name const &, Dir_handle, Name const &);
 
-	GENODE_RPC_INTERFACE(Rpc_tx_cap, Rpc_file, Rpc_symlink, Rpc_dir, Rpc_node,
+	GENODE_RPC_INTERFACE(Rpc_tx_cap,
+	                     Rpc_file, Rpc_symlink, Rpc_dir,
+	                     Rpc_node, Rpc_watch,
 	                     Rpc_close, Rpc_status, Rpc_control, Rpc_unlink,
 	                     Rpc_truncate, Rpc_move);
 };

repos/os/include/file_system_session/rpc_object.h

@@ -49,6 +49,13 @@ class File_system::Session_rpc_object : public Genode::Rpc_object<Session, Sessi
 		Genode::Capability<Tx> _tx_cap() { return _tx.cap(); }
 
 		Tx::Sink *tx_sink() { return _tx.sink(); }
+
+		/**
+		 * Default stub implementation
+		 */
+		Watch_handle watch(Path const &) override {
+			throw Unavailable(); }
+
 };
 
 #endif /* _INCLUDE__FILE_SYSTEM_SESSION__SERVER_H_ */

repos/os/run/fs_rom_update.run

@@ -1,7 +1,3 @@
-if {[have_spec arm]} {
-   assert_spec arm_v7
-}
-
 #
 # Build
 #
@@ -115,6 +111,6 @@ set boot_modules {
 
 build_boot_image $boot_modules
 
-append qemu_args " -nographic "
+append qemu_args " -nographic"
 
-run_genode_until forever
+run_genode_until {.*<config iteration="4" />.*} 60

repos/os/src/server/fs_rom/main.cc

@@ -1,11 +1,12 @@
 /*
  * \brief  Service that provides files of a file system as ROM sessions
  * \author Norman Feske
+ * \author Emery Hemingway
  * \date   2013-01-11
  */
 
 /*
- * Copyright (C) 2013-2017 Genode Labs GmbH
+ * Copyright (C) 2013-2018 Genode Labs GmbH
  *
  * This file is part of the Genode OS framework, which is distributed
  * under the terms of the GNU Affero General Public License version 3.
@@ -51,6 +52,7 @@ class Fs_rom::Rom_session_component : public  Rpc_object<Rom_session>,
 	private:
 
 		friend class List<Rom_session_component>;
+		friend class Packet_handler;
 
 		Env &_env;
 
@@ -65,9 +67,14 @@ class Fs_rom::Rom_session_component : public  Rpc_object<Rom_session>,
 		Path const _file_path;
 
 		/**
-		 * Handle of associated file
+		 * Wandering notification handle
 		 */
-		Constructible<File_system::File_handle> _file_handle { };
+		Constructible<File_system::Watch_handle> _watch_handle { };
+
+		/**
+		 * Handle of associated file opened during read loop
+		 */
+		File_system::File_handle _file_handle { ~0UL };
 
 		/**
 		 * Size of current version of the file
@@ -79,14 +86,6 @@ class Fs_rom::Rom_session_component : public  Rpc_object<Rom_session>,
 		 */
 		File_system::seek_off_t _file_seek = 0;
 
-		/**
-		 * Handle of currently watched compound directory
-		 *
-		 * The compund directory is watched only if the requested file could
-		 * not be looked up.
-		 */
-		Constructible<File_system::Dir_handle> _compound_dir_handle { };
-
 		/**
 		 * Dataspace exposed as ROM module to the client
 		 */
@@ -97,11 +96,6 @@ class Fs_rom::Rom_session_component : public  Rpc_object<Rom_session>,
 		 */
 		Signal_context_capability _sigh { };
 
-		/*
-		 * Exception
-		 */
-		struct Open_compound_dir_failed { };
-
 		/*
 		 * Version number used to track the need for ROM update notifications
 		 */
@@ -110,193 +104,126 @@ class Fs_rom::Rom_session_component : public  Rpc_object<Rom_session>,
 		Version _handed_out_version { ~0U };
 
 		/**
-		 * Open compound directory of specified file
+		 * Track if the session file or a directory is being watched
-		 *
-		 * \param walk_up  If set to true, the function tries to walk up the
-		 *                 hierarchy towards the root and returns the first
-		 *                 existing directory on the way. If set to false, the
-		 *                 function returns the immediate compound directory.
 		 */
-		File_system::Dir_handle _open_compound_dir(File_system::Session &fs,
+		bool _watching_file = false;
-		                                           Path const &path,
-		                                           bool walk_up)
-		{
-			using namespace File_system;
-
-			Path dir_path(path.base());
-
-			while (!path.equals("/")) {
-
-				dir_path.strip_last_element();
-
-				try { return fs.dir(dir_path.base(), false); }
-
-				catch (Invalid_handle)    { error(_file_path, ": invalid_handle"); }
-				catch (Invalid_name)      { error(_file_path, ": invalid_name"); }
-				catch (Lookup_failed)     { error(_file_path, ": lookup_failed"); }
-				catch (Permission_denied) { error(_file_path, ": permission_denied"); }
-				catch (Name_too_long)     { error(_file_path, ": name_too_long"); }
-				catch (No_space)          { error(_file_path, ": no_space"); }
-
-				/*
-				 * If the directory could not be opened, walk up the hierarchy
-				 * towards the root and try again.
-				 */
-				if (!walk_up) break;
-			}
-			throw Open_compound_dir_failed();
-		}
 
 		/*
 		 * Exception
 		 */
-		struct Open_file_failed { };
+		struct Watch_failed { };
 
 		/**
-		 * Open file with specified name at the file system
+		 * Watch the session ROM file or some parent directory
 		 */
-		File_system::File_handle _open_file(File_system::Session &fs,
+		void _open_watch_handle()
-		                                    Path const &path)
 		{
 			using namespace File_system;
 
+			_close_watch_handle();
+
+			Path watch_path(_file_path);
+
+			/* track if we can open the file or resort to a parent */
+			bool at_the_file = true;
+
 			try {
+				while (true) {
+					try {
+						_watch_handle.construct(_fs.watch(watch_path.base()));
+						_watching_file = at_the_file;
+						return;
+					}
+					catch (File_system::Lookup_failed) { }
+					catch (File_system::Unavailable) { }
 
-				Dir_handle dir = _open_compound_dir(fs, path, false);
+					if (watch_path == "/")
+						throw Watch_failed();
+					watch_path.strip_last_element();
 
-				try {
+					/* keep looping, but only directories will be opened */
-
+					at_the_file = false;
-					Handle_guard guard(fs, dir);
-
-					/* open file */
-					Path file_name(path.base());
-					file_name.keep_only_last_element();
-					return fs.file(dir, file_name.base() + 1,
-					               File_system::READ_ONLY, false);
 				}
-				catch (Invalid_handle)    { error(_file_path, ": Invalid_handle"); }
+			} catch (File_system::Out_of_ram) {
-				catch (Invalid_name)      { error(_file_path, ": invalid_name"); }
+				error("not enough RAM to watch '", watch_path, "'");
-				catch (Lookup_failed)     { error(_file_path, ": lookup_failed"); }
+			} catch (File_system::Out_of_caps) {
-				catch (Permission_denied) { error(_file_path, ": Permission_denied"); }
+				error("not enough caps to watch '", watch_path, "'");
-				catch (...)               { error(_file_path, ": unhandled error"); };
-
-				throw Open_file_failed();
-				
-			} catch (Open_compound_dir_failed) {
-				throw Open_file_failed();
 			}
+			throw Watch_failed();
 		}
 
-		void _register_for_compound_dir_changes()
+		void _close_watch_handle()
 		{
-			using namespace File_system;
+			if (_watch_handle.constructed()) {
-
+				_fs.close(*_watch_handle);
-			/* forget about the previously watched compound directory */
+				_watch_handle.destruct();
-			if (_compound_dir_handle.constructed()) {
-				_fs.close(*_compound_dir_handle);
-				_compound_dir_handle.destruct();
 			}
-
+			_watching_file = false;
-			try {
-				_compound_dir_handle.construct(_open_compound_dir(_fs, _file_path, true));
-
-				/* register for changes in compound directory */
-				_fs.tx()->submit_packet(File_system::Packet_descriptor(
-					*_compound_dir_handle,
-					File_system::Packet_descriptor::CONTENT_CHANGED));
-			}
-			catch (Open_compound_dir_failed) {
-				warning("could not track compound dir, giving up"); }
 		}
 
+		enum { UPDATE_OR_REPLACE = false, UPDATE_ONLY = true };
+
 		/**
-		 * Initialize '_file_ds' dataspace with file content
+		 * Fill dataspace with file content, return true if the
+		 * current dataspace is reused.
 		 */
-		void _update_dataspace()
+		bool _read_dataspace(bool update_only)
 		{
 			using namespace File_system;
 
-			/*
+			Genode::Path<PATH_MAX_LEN> dir_path(_file_path);
-			 * On each repeated call of this function, the dataspace is
+			dir_path.strip_last_element();
-			 * replaced with a new one that contains the most current file
+			Genode::Path<PATH_MAX_LEN> file_name(_file_path);
-			 * content. The dataspace is re-allocated if the new version
+			file_name.keep_only_last_element();
-			 * of the file has become bigger.
-			 */
-			try {
-				File_handle const file_handle = _open_file(_fs, _file_path);
-				File_system::file_size_t const new_file_size =
-					_fs.status(file_handle).size;
 
-				if (_file_ds.size() && (new_file_size > _file_size)) {
+			Dir_handle parent_handle = _fs.dir(dir_path.base(), false);
-					/* mark as invalid */
+			Handle_guard parent_guard(_fs, parent_handle);
-					_file_ds.realloc(&_env.ram(), 0);
-					_file_size = 0;
-					_file_seek = 0;
-				}
-				_fs.close(file_handle);
-			} catch (Open_file_failed) { }
 
-			/* close and then re-open the file */
+			/* the file handle is opened here... */
-			if (_file_handle.constructed()) {
+			_file_handle = _fs.file(
-				_fs.close(*_file_handle);
+				parent_handle, file_name.base() + 1,
-				_file_handle.destruct();
+				File_system::READ_ONLY, false);
-			}
+			Handle_guard file_guard(_fs, _file_handle);
+			/* ...but only for the lifetime of this procedure */
 
-			try {
+			_file_seek = 0;
-				_file_handle.construct(_open_file(_fs, _file_path));
+			_file_size = _fs.status(_file_handle).size;
-			} catch (Open_file_failed) { }
 
-			/*
+			if (_file_size > _file_ds.size()) {
-			 * If we got the file, we can stop paying attention to the
+				/* allocate new RAM dataspace according to file size */
-			 * compound directory.
+				if (update_only)
-			 */
+					return false;
-			if (_file_handle.constructed() && _compound_dir_handle.constructed()) {
-				_fs.close(*_compound_dir_handle);
-				_compound_dir_handle.destruct();
-			}
 
-			/* register for file changes */
-			if (_file_handle.constructed())
-				_fs.tx()->submit_packet(File_system::Packet_descriptor(
-					*_file_handle, File_system::Packet_descriptor::CONTENT_CHANGED));
-
-			size_t const file_size = _file_handle.constructed()
-			                       ? _fs.status(*_file_handle).size : 0;
-
-			/* allocate new RAM dataspace according to file size */
-			if (file_size > 0) {
 				try {
-					_file_seek = 0;
+					_file_ds.realloc(&_env.ram(), _file_size);
-					_file_ds.realloc(&_env.ram(), file_size);
-					_file_size = file_size;
 				} catch (...) {
-					error("couldn't allocate memory for file, empty result");
+					error("failed to allocate memory for ", _file_path);
-					return;
+					return false;
 				}
 			} else {
-				_register_for_compound_dir_changes();
+				memset(_file_ds.local_addr<char>(), 0x00, _file_ds.size());
-				return;
 			}
 
 			/* omit read if file is empty */
 			if (_file_size == 0)
-				return;
+				return false;
 
 			/* read content from file */
 			Tx_source &source = *_fs.tx();
 			while (_file_seek < _file_size) {
 				/* if we cannot submit then process acknowledgements */
-				if (source.ready_to_submit()) {
+				while (!source.ready_to_submit())
-					size_t chunk_size = min(_file_size - _file_seek,
+					_env.ep().wait_and_dispatch_one_io_signal();
-					                        source.bulk_buffer_size() / 2);
+
-					File_system::Packet_descriptor
+				size_t chunk_size = min(_file_size - _file_seek,
-						packet(source.alloc_packet(chunk_size),
+				                        source.bulk_buffer_size() / 2);
-						       *_file_handle,
+
-						       File_system::Packet_descriptor::READ,
+				File_system::Packet_descriptor
-						       chunk_size,
+					packet(source.alloc_packet(chunk_size), _file_handle,
-						       _file_seek);
+					       File_system::Packet_descriptor::READ,
-					source.submit_packet(packet);
+					       chunk_size, _file_seek);
-				}
+
+				source.submit_packet(packet);
 
 				/*
 				 * Process the global signal handler until we got a response
@@ -307,12 +234,45 @@ class Fs_rom::Rom_session_component : public  Rpc_object<Rom_session>,
 				while (_file_seek == orig_file_seek)
 					_env.ep().wait_and_dispatch_one_io_signal();
 			}
+
+			_handed_out_version = _curr_version;
+			return true;
+		}
+
+		bool _try_read_dataspace(bool update_only)
+		{
+			using namespace File_system;
+
+			try { _open_watch_handle(); }
+			catch (Watch_failed) { }
+
+			try { return _read_dataspace(update_only); }
+			catch (Lookup_failed)     { log(_file_path, " ROM file is missing"); }
+			catch (Invalid_handle)    { error(_file_path, ": invalid handle"); }
+			catch (Invalid_name)      { error(_file_path, ": invalid name"); }
+			catch (Permission_denied) { error(_file_path, ": permission denied"); }
+			catch (...)               { error(_file_path, ": unhandled error"); };
+
+			return false;
 		}
 
 		void _notify_client_about_new_version()
 		{
-			if (_sigh.valid() && _curr_version.value != _handed_out_version.value)
+			using namespace File_system;
-				Signal_transmitter(_sigh).submit();
+
+			if (_sigh.valid() && _curr_version.value != _handed_out_version.value) {
+
+				/* notify if the file is not empty */
+				try {
+					Node_handle file = _fs.node(_file_path.base());
+					Handle_guard g(_fs, file);
+					_file_size = _fs.status(file).size;
+				}
+				catch (...) { _file_size = 0; }
+
+				if (_file_size > 0)
+					Signal_transmitter(_sigh).submit();
+			}
 		}
 
 	public:
@@ -328,17 +288,15 @@ class Fs_rom::Rom_session_component : public  Rpc_object<Rom_session>,
 		 *                  creation time)
 		 */
 		Rom_session_component(Env &env,
-		                      File_system::Session &fs, const char *file_path)
+		                      File_system::Session &fs,
+		                      const char *file_path)
 		:
 			_env(env), _fs(fs),
 			_file_path(file_path),
 			_file_ds(env.ram(), env.rm(), 0) /* realloc later */
 		{
-			try {
+			try { _open_watch_handle(); }
-				_file_handle.construct(_open_file(_fs, _file_path));
+			catch (Watch_failed) { }
-			} catch (Open_file_failed) { }
-
-			_register_for_compound_dir_changes();
 		}
 
 		/**
@@ -346,33 +304,45 @@ class Fs_rom::Rom_session_component : public  Rpc_object<Rom_session>,
 		 */
 		~Rom_session_component()
 		{
-			/* close re-open the file */
+			_close_watch_handle();
-			if (_file_handle.constructed())
-				_fs.close(*_file_handle);
-
-			if (_compound_dir_handle.constructed())
-				_fs.close(*_compound_dir_handle);
 		}
 
-		using Sessions::Element::next;
-
 		/**
 		 * Return dataspace with up-to-date content of file
 		 */
 		Rom_dataspace_capability dataspace()
 		{
-			_update_dataspace();
+			using namespace File_system;
+
+			_try_read_dataspace(UPDATE_OR_REPLACE);
+
+			/* always serve a valid, even empty, dataspace */
+			if (_file_ds.size() < 1) {
+				_file_ds.realloc(&_env.ram(), 1);
+			}
+
 			Dataspace_capability ds = _file_ds.cap();
-			_handed_out_version = _curr_version;
 			return static_cap_cast<Rom_dataspace>(ds);
 		}
 
 		void sigh(Signal_context_capability sigh)
 		{
 			_sigh = sigh;
+
+			if (_sigh.valid()) {
+				try { _open_watch_handle(); }
+				catch (Watch_failed) { }
+			}
+
 			_notify_client_about_new_version();
 		}
 
+		/**
+		 * Update the current dataspace content
+		 */
+		bool update() override {
+			return _try_read_dataspace(UPDATE_ONLY); }
+
 		/**
 		 * If packet corresponds to this session then process and return true.
 		 *
@@ -383,25 +353,30 @@ class Fs_rom::Rom_session_component : public  Rpc_object<Rom_session>,
 			switch (packet.operation()) {
 
 			case File_system::Packet_descriptor::CONTENT_CHANGED:
+				if (!(packet.handle() == *_watch_handle))
+					return false;
 
-				_curr_version = Version { _curr_version.value + 1 };
+				if (!_watching_file) {
-
+					/* try and get closer to the file */
-				if ((_file_handle.constructed() && (*_file_handle == packet.handle())) ||
+					_open_watch_handle();
-				    (_compound_dir_handle.constructed() && (*_compound_dir_handle == packet.handle())))
-				{
-					_notify_client_about_new_version();
-					return true;
 				}
-				return false;
+
+				if (_watching_file) {
+					/* notify the client of the change */
+					_curr_version = Version { _curr_version.value + 1 };
+					_notify_client_about_new_version();
+				}
+				return true;
 
 			case File_system::Packet_descriptor::READ: {
 
-				if (!(_file_handle.constructed() && (*_file_handle == packet.handle())))
+				if (!(packet.handle() == _file_handle))
 					return false;
 
 				if (packet.position() > _file_seek || _file_seek >= _file_size) {
 					error("bad packet seek position");
 					_file_ds.realloc(&_env.ram(), 0);
+					_file_seek = 0;
 					return true;
 				}
 
@@ -412,9 +387,14 @@ class Fs_rom::Rom_session_component : public  Rpc_object<Rom_session>,
 				return true;
 			}
 
-			default:
+			case File_system::Packet_descriptor::WRITE:
-
+				warning("discarding strange WRITE acknowledgement");
-				error("discarding strange packet acknowledgement");
+				return true;
+			case File_system::Packet_descriptor::SYNC:
+				warning("discarding strange SYNC acknowledgement");
+				return true;
+			case File_system::Packet_descriptor::READ_READY:
+				warning("discarding strange READ_READY acknowledgement");
 				return true;
 			}
 			return false;

repos/os/src/server/ram_fs/main.cc

@@ -97,11 +97,7 @@ class Ram_fs::Session_component : public File_system::Session_rpc_object
 				break;
 
 			case Packet_descriptor::CONTENT_CHANGED: {
-				open_node.register_notify(*tx_sink());
+				Genode::error("CONTENT_CHANGED packets from clients have no effect");
-				Locked_ptr<Node> node { open_node.node() };
-				if (!node.valid())
-					return; 
-				node->notify_listeners();
 				return;
 			}
 
@@ -365,6 +361,24 @@ class Ram_fs::Session_component : public File_system::Session_rpc_object
 			return open_node->id();
 		}
 
+		Watch_handle watch(Path const &path) override
+		{
+			_assert_valid_path(path.string());
+
+			Node *node = _root.lookup(path.string() + 1);
+
+			Open_node *watcher = new (_alloc)
+				Open_node(node->weak_ptr(), _open_node_registry);
+
+			/*
+			 * like other open nodes, just the only
+			 * kind registered for notifications
+			 */
+			watcher->register_notify(*tx_sink());
+
+			return Watch_handle { watcher->id().value };
+		}
+
 		void close(Node_handle handle)
 		{
 			auto close_fn = [&] (Open_node &open_node) {