From 482576fabb1954143c667d7c949f4037a0121846 Mon Sep 17 00:00:00 2001 From: Emery Hemingway Date: Tue, 23 Aug 2016 17:05:37 +0200 Subject: [PATCH] server/fs_log: improve client isolation Use a seperate handle at each session. Use SEEK_TAIL to append messages to files. Increase packet buffer. Refactor to component framework. Fixes #1777 Issue #2060 --- repos/os/include/os/path.h | 14 +- repos/os/run/fs_log.run | 4 +- repos/os/src/server/fs_log/README | 5 +- repos/os/src/server/fs_log/log_file.h | 97 ----------- repos/os/src/server/fs_log/main.cc | 230 +++++++++++--------------- repos/os/src/server/fs_log/session.h | 138 ++++++++-------- repos/os/src/server/fs_log/target.mk | 2 +- tool/autopilot.list | 1 + 8 files changed, 184 insertions(+), 307 deletions(-) delete mode 100644 repos/os/src/server/fs_log/log_file.h diff --git a/repos/os/include/os/path.h b/repos/os/include/os/path.h index b358d50fd2..f25a30620b 100644 --- a/repos/os/include/os/path.h +++ b/repos/os/include/os/path.h @@ -341,8 +341,8 @@ class Genode::Path_base template -class Genode::Path : public Path_base { - +class Genode::Path : public Path_base +{ private: char _buf[MAX_LEN]; @@ -371,8 +371,16 @@ class Genode::Path : public Path_base { return *this; } + Path(Path const &other) : Path_base(_buf, sizeof(_buf), other._buf) { } + + Path& operator=(Path const &other) + { + Genode::strncpy(_buf, other._buf, MAX_LEN); + return *this; + } + template - Path& operator=(Path &other) + Path& operator=(Path const &other) { Genode::strncpy(_buf, other._buf, MAX_LEN); return *this; diff --git a/repos/os/run/fs_log.run b/repos/os/run/fs_log.run index fbf6e697cd..4f13d9dfad 100644 --- a/repos/os/run/fs_log.run +++ b/repos/os/run/fs_log.run @@ -20,6 +20,8 @@ set config { + + @@ -67,4 +69,4 @@ build_boot_image "core init ld.lib.so bomb timer vfs fs_log" append qemu_args " -nographic" -run_genode_until "Done\. Going to sleep\." $timeout +run_genode_until {.*\[0] Done\..*\n} $timeout diff --git a/repos/os/src/server/fs_log/README b/repos/os/src/server/fs_log/README index e6be366be8..4ec8750187 100644 --- a/repos/os/src/server/fs_log/README +++ b/repos/os/src/server/fs_log/README @@ -9,6 +9,9 @@ through session policy, as well the option to merge the logs of any session matching a given policy. When a merged policy label contains a trailing "->", the log filename takes the name of the next label element. +When a default-policy node specifies a merge, all sessions are merged into +the file "/log". + :Example configuration: ! ! @@ -16,7 +19,7 @@ trailing "->", the log filename takes the name of the next label element. ! ! ! -! +! ! ! diff --git a/repos/os/src/server/fs_log/log_file.h b/repos/os/src/server/fs_log/log_file.h deleted file mode 100644 index 3351e27121..0000000000 --- a/repos/os/src/server/fs_log/log_file.h +++ /dev/null @@ -1,97 +0,0 @@ -/* - * \brief File object shared between log sessions - * \author Emery Hemingway - * \date 2015-06-09 - */ - -/* - * Copyright (C) 2015 Genode Labs GmbH - * - * This file is part of the Genode OS framework, which is distributed - * under the terms of the GNU General Public License version 2. - */ - -#ifndef _FS_LOG__LOG_FILE_H_ -#define _FS_LOG__LOG_FILE_H_ - -/* Genode includes */ -#include -#include - -namespace Fs_log { - - using namespace Genode; - using namespace File_system; - - class Log_file; - -} - -class Fs_log::Log_file : public List::Element -{ - private: - - char _dir_path[ MAX_PATH_LEN]; - char _file_name[MAX_NAME_LEN]; - File_system::Session &_fs; - File_handle _handle; - seek_off_t _offset; - int _clients; - - public: - - /** - * Constructor - */ - Log_file(File_system::Session &fs, File_handle handle, - char const *dir_path, char const *file_name, - seek_off_t offset) - : - _fs(fs), _handle(handle), _offset(offset), _clients(0) - { - strncpy(_dir_path, dir_path, sizeof(_dir_path)); - strncpy(_file_name, file_name, sizeof(_file_name)); - } - - ~Log_file() { _fs.close(_handle); } - - bool match(char const *dir, char const *filename) const - { - return - (strcmp(_dir_path, dir, MAX_PATH_LEN) == 0) && - (strcmp(_file_name, filename, MAX_NAME_LEN) == 0); - } - - void incr() { ++_clients; } - void decr() { --_clients; } - int client_count() const { return _clients; } - - /** - * Write a log message to the packet buffer. - */ - size_t write(char const *msg, size_t msg_len) - { - File_system::Session::Tx::Source &source = *_fs.tx(); - - File_system::Packet_descriptor raw_packet; - if (!source.ready_to_submit()) - raw_packet = source.get_acked_packet(); - else - raw_packet = source.alloc_packet(Log_session::String::MAX_SIZE); - - File_system::Packet_descriptor - packet(raw_packet, - _handle, File_system::Packet_descriptor::WRITE, - msg_len, _offset); - - _offset += msg_len; - - char *buf = source.packet_content(packet); - memcpy(buf, msg, msg_len); - - source.submit_packet(packet); - return msg_len; - } -}; - -#endif diff --git a/repos/os/src/server/fs_log/main.cc b/repos/os/src/server/fs_log/main.cc index 7cfba57f57..c2f5811b4f 100644 --- a/repos/os/src/server/fs_log/main.cc +++ b/repos/os/src/server/fs_log/main.cc @@ -13,16 +13,17 @@ /* Genode includes */ #include -#include #include #include -#include -#include +#include #include +#include +#include +#include +#include #include /* Local includes */ -#include "log_file.h" #include "session.h" namespace Fs_log { @@ -31,35 +32,34 @@ namespace Fs_log { using namespace File_system; class Root_component; - struct Main; enum { - BLOCK_SIZE = Log_session::String::MAX_SIZE, + PACKET_SIZE = Log_session::String::MAX_SIZE, QUEUE_SIZE = File_system::Session::TX_QUEUE_SIZE, - TX_BUF_SIZE = BLOCK_SIZE * (QUEUE_SIZE*2 + 1) + TX_BUF_SIZE = PACKET_SIZE * (QUEUE_SIZE+2) }; typedef Genode::Path Path; } + class Fs_log::Root_component : public Genode::Root_component { private: - Allocator_avl _write_alloc; - File_system::Connection _fs; - List _log_files; + Genode::Env &_env; + Genode::Attached_rom_dataspace _config_rom { _env, "config" }; + Genode::Heap _heap { _env.ram(), _env.rm() }; + Allocator_avl _tx_alloc { &_heap }; + File_system::Connection _fs + { _env, _tx_alloc, "", "/", true, TX_BUF_SIZE }; - Log_file *lookup(char const *dir, char const *filename) - { - for (Log_file *file = _log_files.first(); file; file = file->next()) - if (file->match(dir, filename)) - return file; + void _update_config() { _config_rom.update(); } - return 0; - } + Genode::Signal_handler _config_handler + { _env.ep(), *this, &Root_component::_update_config }; protected: @@ -67,30 +67,29 @@ class Fs_log::Root_component : { using namespace File_system; - char dir_path[MAX_PATH_LEN]; + size_t ram_quota = + Arg_string::find_arg(args, "ram_quota").aligned_size(); + if (ram_quota < sizeof(Session_component)) + throw Root::Quota_exceeded(); + + Path dir_path; char file_name[MAX_NAME_LEN]; - dir_path[0] = '/'; - - bool truncate = false; Session_label const session_label = label_from_args(args); char const *label_str = session_label.string(); char const *label_prefix = ""; - - strncpy(dir_path+1, label_str, MAX_PATH_LEN-1); + bool truncate = false; try { - Session_policy policy(session_label); + Session_policy policy(session_label, _config_rom.xml()); truncate = policy.attribute_value("truncate", truncate); + bool merge = policy.attribute_value("merge", false); - if (policy.attribute_value("merge", false) - && policy.has_attribute("label_prefix")) { - - if (!dir_path[1]) { - Genode::error("cannot merge an empty policy label"); - throw Root::Unavailable(); - } - + /* only a match on 'label_prefix' can be merged */ + if (merge && policy.has_type("policy") + && (!(policy.has_attribute("label") + || policy.has_attribute("label_suffix")))) + { /* * split the label between what will be the log file * and what will be prepended to messages in the file @@ -100,103 +99,75 @@ class Fs_log::Root_component : if (strcmp(label_str+i, " -> ", 4)) continue; - dir_path[i+1] = '\0'; label_prefix = label_str+i+4; + { + char tmp[128]; + strncpy(tmp, label_str, min(sizeof(tmp), i+1)); + dir_path = path_from_label(tmp); + } break; } - } - } catch (Session_policy::No_policy_defined) { } + if (dir_path == "/") + dir_path = path_from_label(label_str); - - { - /* Parse out a directory and file name. */ - size_t len = strlen(dir_path); - size_t start = 1; - for (size_t i = 1; i < len;) { - /* Replace any slashes in label elements. */ - if (dir_path[i] == '/') dir_path[i] = '_'; - if (strcmp(" -> ", dir_path+i, 4) == 0) { - dir_path[i++] = '/'; - strncpy(dir_path+i, dir_path+i+3, MAX_PATH_LEN-i); - start = i; - i += 3; - } else ++i; + } else if (!policy.has_type("default-policy")) { + dir_path = path_from_label(label_str); } - /* Copy the remainder to the file name. */ - snprintf(file_name, MAX_NAME_LEN, "%s.log", dir_path+start); - - /* Terminate the directory path. */ - dir_path[(start == 1) ? start : start-1] = '\0'; - - /* Rewrite any slashes in the name. */ - for (char *p = file_name; *p; ++p) - if (*p == '/') *p = '_'; + } catch (Session_policy::No_policy_defined) { + dir_path = path_from_label(label_str); } - Log_file *file = lookup(dir_path, file_name); - if (!file) try { + if (dir_path == "/") { + strncpy(file_name, "log", sizeof(file_name)); + label_prefix = label_str; + } else { + dir_path.append(".log"); + strncpy(file_name, dir_path.last_element(), sizeof(file_name)); + dir_path.strip_last_element(); + dir_path.remove_trailing('/'); + } - Dir_handle dir_handle = ensure_dir(_fs, dir_path); + char const *errstr; + try { + + Dir_handle dir_handle = ensure_dir(_fs, dir_path.base()); Handle_guard dir_guard(_fs, dir_handle); File_handle handle; - seek_off_t offset = 0; try { handle = _fs.file(dir_handle, file_name, File_system::WRITE_ONLY, false); - if (truncate) + /* don't truncate at every new child session */ + if (truncate && (strcmp(label_prefix, "") == 0)) _fs.truncate(handle, 0); - else - offset = _fs.status(handle).size; } catch (File_system::Lookup_failed) { handle = _fs.file(dir_handle, file_name, File_system::WRITE_ONLY, true); } - file = new (env()->heap()) - Log_file(_fs, handle, dir_path, file_name, offset); - - _log_files.insert(file); - - } catch (Permission_denied) { - error(Path(file_name, dir_path), ": permission denied"); - - } catch (No_space) { - error("file system out of space"); - - } catch (Out_of_metadata) { - error("file system server out of metadata"); - - } catch (Invalid_name) { - error(Path(file_name, dir_path), ": invalid path"); - - } catch (Name_too_long) { - error(Path(file_name, dir_path), ": name too long"); - - } catch (...) { - error("cannot open log file ", Path(file_name, dir_path)); - throw; + return new (md_alloc()) Session_component(_fs, handle, label_prefix); } - if (!file) - throw Root::Unavailable(); + catch (Permission_denied) { + errstr = "permission denied"; } + catch (No_space) { + errstr = "file system out of space"; } + catch (Out_of_metadata) { + errstr = "file system server out of metadata"; } + catch (Invalid_name) { + errstr = "invalid path"; } + catch (Name_too_long) { + errstr = "name too long"; } + catch (...) { + errstr = "unhandled error"; } - if (*label_prefix) - return new (md_alloc()) Labeled_session_component(label_prefix, *file); - return new (md_alloc()) Unlabeled_session_component(*file); - } - - void _destroy_session(Session_component *session) - { - Log_file *file = session->file(); - destroy(md_alloc(), session); - if (file->client_count() < 1) { - _log_files.remove(file); - destroy(env()->heap(), file); - } + Genode::error("cannot open log file ", + (char const *)dir_path.base(), + ", ", errstr); + throw Root::Unavailable(); } public: @@ -204,39 +175,32 @@ class Fs_log::Root_component : /** * Constructor */ - Root_component(Server::Entrypoint &ep, Allocator &alloc) + Root_component(Genode::Env &env, Genode::Allocator &md_alloc) : - Genode::Root_component(&ep.rpc_ep(), &alloc), - _write_alloc(env()->heap()), - _fs(_write_alloc, TX_BUF_SIZE) - { } + Genode::Root_component(&env.ep().rpc_ep(), &md_alloc), + _env(env) + { + _config_rom.sigh(_config_handler); + + /* fill the ack queue with packets so sessions never need to alloc */ + File_system::Session::Tx::Source &source = *_fs.tx(); + for (int i = 0; i < QUEUE_SIZE-1; ++i) + source.submit_packet(source.alloc_packet(PACKET_SIZE)); + + env.parent().announce(env.ep().manage(*this)); + } }; -struct Fs_log::Main + +/*************** + ** Component ** + ***************/ + +Genode::size_t Component::stack_size() { return 4*1024*sizeof(long); } + +void Component::construct(Genode::Env &env) { - Server::Entrypoint &ep; - - Sliced_heap sliced_heap = { env()->ram_session(), env()->rm_session() }; - - Root_component root { ep, sliced_heap }; - - Main(Server::Entrypoint &ep) - : ep(ep) - { Genode::env()->parent()->announce(ep.manage(root)); } -}; - - -/************ - ** Server ** - ************/ - -namespace Server { - - char const* name() { return "fs_log_ep"; } - - size_t stack_size() { return 3*512*sizeof(long); } - - void construct(Entrypoint &ep) { static Fs_log::Main inst(ep); } - + static Genode::Sliced_heap sliced_heap { env.ram(), env.rm() }; + static Fs_log::Root_component root { env, sliced_heap }; } diff --git a/repos/os/src/server/fs_log/session.h b/repos/os/src/server/fs_log/session.h index 823bd84395..5e8668fb23 100644 --- a/repos/os/src/server/fs_log/session.h +++ b/repos/os/src/server/fs_log/session.h @@ -8,7 +8,7 @@ */ /* - * Copyright (C) 2015 Genode Labs GmbH + * Copyright (C) 2016 Genode Labs GmbH * * This file is part of the Genode OS framework, which is distributed * under the terms of the GNU General Public License version 2. @@ -21,98 +21,94 @@ #include #include #include - -/* Local includes */ -#include "log_file.h" +#include +#include namespace Fs_log { - class Session_component; - class Unlabeled_session_component; - class Labeled_session_component; + + enum { MAX_LABEL_LEN = 128 }; + + class Session_component; } -class Fs_log::Session_component : public Rpc_object -{ - protected: - - Log_file &_log_file; - - public: - - Session_component(Log_file &log_file) - : _log_file(log_file) { _log_file.incr(); } - - ~Session_component() { _log_file.decr(); } - - Log_file *file() const { return &_log_file; } - - virtual size_t write(String const &string) = 0; -}; - -class Fs_log::Unlabeled_session_component : public Session_component -{ - - public: - - /** - * Constructor - */ - Unlabeled_session_component(Log_file &log_file) - : Session_component(log_file) { } - - /***************** - ** Log session ** - *****************/ - - size_t write(Log_session::String const &msg) - { - if (!msg.valid_string()) { - Genode::error("corrupted string"); - return 0; - } - - char const *msg_str = msg.string(); - size_t msg_len = Genode::strlen(msg_str); - - return _log_file.write(msg_str, msg_len); - } -}; - -class Fs_log::Labeled_session_component : public Session_component +class Fs_log::Session_component : public Genode::Rpc_object { private: - char _label[Log_session::String::MAX_SIZE]; - size_t _label_len; + char _label_buf[MAX_LABEL_LEN]; + Genode::size_t const _label_len; + + File_system::Session &_fs; + File_system::File_handle const _handle; public: - /** - * Constructor - */ - Labeled_session_component(char const *label, Log_file &log_file) - : Session_component(log_file) + Session_component(File_system::Session &fs, + File_system::File_handle handle, + char const *label) + : + _label_len(Genode::strlen(label) ? Genode::strlen(label)+3 : 0), + _fs(fs), _handle(handle) { - snprintf(_label, sizeof(_label), "[%s] ", label); - _label_len = strlen(_label); + if (_label_len) + Genode::snprintf(_label_buf, MAX_LABEL_LEN, "[%s] ", label); } + ~Session_component() + { + _fs.sync(_handle); + _fs.close(_handle); + } + + /***************** ** Log session ** *****************/ - size_t write(Log_session::String const &msg) + Genode::size_t write(Log_session::String const &msg) { - if (!msg.valid_string()) { - Genode::error("corrupted string"); + using namespace Genode; + + if (!msg.is_valid_string()) { + Genode::error("received corrupted string"); return 0; } - char const *msg_str = msg.string(); - size_t msg_len = Genode::strlen(msg_str); + size_t msg_len = strlen(msg.string()); - _log_file.write(_label, _label_len); - return _log_file.write(msg_str, msg_len); + File_system::Session::Tx::Source &source = *_fs.tx(); + + File_system::Packet_descriptor packet( + source.get_acked_packet(), + _handle, File_system::Packet_descriptor::WRITE, + msg_len, File_system::SEEK_TAIL); + + char *buf = source.packet_content(packet); + + if (_label_len) { + memcpy(buf, _label_buf, _label_len); + + if (_label_len+msg_len > Log_session::String::MAX_SIZE) { + packet.length(msg_len); + source.submit_packet(packet); + + packet = File_system::Packet_descriptor( + source.get_acked_packet(), + _handle, File_system::Packet_descriptor::WRITE, + msg_len, File_system::SEEK_TAIL); + + buf = source.packet_content(packet); + + } else { + buf += _label_len; + packet.length(_label_len+msg_len); + } + } + + memcpy(buf, msg.string(), msg_len); + + source.submit_packet(packet); + return msg_len; } }; diff --git a/repos/os/src/server/fs_log/target.mk b/repos/os/src/server/fs_log/target.mk index 372c6082e8..6eee99668f 100644 --- a/repos/os/src/server/fs_log/target.mk +++ b/repos/os/src/server/fs_log/target.mk @@ -1,3 +1,3 @@ TARGET = fs_log SRC_CC = main.cc -LIBS = base config server +LIBS = base diff --git a/tool/autopilot.list b/tool/autopilot.list index c34733c49f..d47afb7185 100644 --- a/tool/autopilot.list +++ b/tool/autopilot.list @@ -67,3 +67,4 @@ rust xml_node fpu ds_ownership +fs_log