[PATCH 1/2] add new C-wrappers to expose module API

[Waldemar Kozaczuk]

The commit af2d371a61f6ab1eb5a066a0c3e93230faf6611c introduced ability
to build OSv kernel with most symbols but subset of glibc hidden.
The regular Linux glibc apps should run fine on such kernel, but
unfortunately many unit tests and various internal OSv apps (so called
modules) do not as they have been coded to use many internal API
symbols. One such example is httpserver monitoring api module that
exposes various monitoring API REST endpoints.

At some point XLAB introduced C-wrappers API made of single C-style
osv_get_all_app_threads() functions. This patch enhances the C-wrappers API
by adding 9 more functions intended to be used by httpserver monitoring
api module.

Please note that new C-style API will open up access to relevant functionality
to new apps/modules implemented in languages different than C++.

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---
core/osv_c_wrappers.cc | 121 +++++++++++++++++++++++++++++++++++
include/osv/export.h | 3 +
include/osv/osv_c_wrappers.h | 105 +++++++++++++++++++++++++++++-
3 files changed, 228 insertions(+), 1 deletion(-)

diff --git a/core/osv_c_wrappers.cc b/core/osv_c_wrappers.cc
index 137f2c6f..dbda0613 100644
--- a/core/osv_c_wrappers.cc
+++ b/core/osv_c_wrappers.cc
@@ -1,12 +1,27 @@
+/*
+ * Copyright (C) 2022 Waldemar Kozaczuk
+ * Copyright (C) 2016 XLAB, d.o.o.
+ *
+ * This work is open source software, licensed under the terms of the
+ * BSD license as described in the LICENSE file in the top-level directory.
+ */

#include <osv/osv_c_wrappers.h>
+#include <osv/export.h>
#include <osv/debug.hh>
#include <osv/sched.hh>
#include <osv/app.hh>
+#include <osv/run.hh>
+#include <osv/version.hh>
+#include <osv/commands.hh>
+#include <osv/firmware.hh>
+#include <osv/hypervisor.hh>
+#include <vector>

using namespace osv;
using namespace sched;

+extern "C" OSV_MODULE_API
int osv_get_all_app_threads(pid_t tid, pid_t** tid_arr, size_t *len) {
thread* app_thread = tid==0? thread::current(): thread::find_by_id(tid);
if (app_thread == nullptr) {
@@ -28,3 +43,109 @@ int osv_get_all_app_threads(pid_t tid, pid_t** tid_arr, size_t *len) {
}
return 0;
}
+
+static void free_threads_names(std::vector<osv_thread> &threads) {
+ for (auto &t : threads) {
+ if (t.name) {
+ free(t.name);
+ }
+ }
+}
+
+static char* str_to_c_str(const std::string& str) {
+ auto len = str.size();
+ char *buf = static_cast<char*>(malloc(len + 1)); // This will be free()-ed in C world
+ if (buf) {
+ std::copy(str.begin(), str.end(), buf);
+ buf[len] = '\0';
+ return buf;
+ } else {
+ return nullptr;
+ }
+}
+
+extern "C" OSV_MODULE_API
+int osv_get_all_threads(osv_thread** thread_arr, size_t *len) {
+ using namespace std::chrono;
+ std::vector<osv_thread> threads;
+
+ osv_thread thread;
+ bool str_copy_error = false;
+ sched::with_all_threads([&](sched::thread &t) {
+ thread.id = t.id();
+ auto tcpu = t.tcpu();
+ thread.cpu_id = tcpu ? tcpu->id : -1;
+ thread.cpu_ms = duration_cast<milliseconds>(t.thread_clock()).count();
+ thread.switches = t.stat_switches.get();
+ thread.migrations = t.stat_migrations.get();
+ thread.preemptions = t.stat_preemptions.get();
+ thread.name = str_to_c_str(t.name());
+ if (!thread.name) {
+ str_copy_error = true;
+ }
+ thread.priority = t.priority();
+ thread.stack_size = t.get_stack_info().size;
+ thread.status = static_cast<osv_thread_status>(static_cast<int>(t.get_status()));
+ threads.push_back(thread);
+ });
+
+ if (str_copy_error) {
+ goto error;
+ }
+
+ *thread_arr = (osv_thread*)malloc(threads.size()*sizeof(osv_thread));
+ if (*thread_arr == nullptr) {
+ goto error;
+ }
+
+ std::copy(threads.begin(), threads.end(), *thread_arr);
+ *len = threads.size();
+ return 0;
+
+error:
+ free_threads_names(threads);
+ *len = 0;
+ return ENOMEM;
+}
+
+extern "C" OSV_MODULE_API
+char *osv_version() {
+ return str_to_c_str(osv::version());
+}
+
+extern "C" OSV_MODULE_API
+char *osv_cmdline() {
+ return str_to_c_str(osv::getcmdline());
+}
+
+extern "C" OSV_MODULE_API
+char *osv_hypervisor_name() {
+ return str_to_c_str(osv::hypervisor_name());
+}
+
+extern "C" OSV_MODULE_API
+char *osv_firmware_vendor() {
+ return str_to_c_str(osv::firmware_vendor());
+}
+
+extern "C" OSV_MODULE_API
+char *osv_processor_features() {
+ return str_to_c_str(processor::features_str());
+}
+
+extern char debug_buffer[DEBUG_BUFFER_SIZE];
+extern "C" OSV_MODULE_API
+const char *osv_debug_buffer() {
+ return debug_buffer;
+}
+
+extern "C" OSV_MODULE_API
+void osv_current_app_on_termination_request(void (*handler)()) {
+ osv::this_application::on_termination_request(handler);
+}
+
+extern bool verbose;
+extern "C" OSV_MODULE_API
+bool osv_debug_enabled() {
+ return verbose;
+}
diff --git a/include/osv/export.h b/include/osv/export.h
index c03659b8..b21ba561 100644
--- a/include/osv/export.h
+++ b/include/osv/export.h
@@ -32,6 +32,9 @@

// This is to expose some symbols in libsolaris.so
#define OSV_LIB_SOLARIS_API __attribute__((__visibility__("default")))
+//
+// This is to expose some OSv functions intended to be used by modules
+#define OSV_MODULE_API __attribute__((__visibility__("default")))

// In some very few cases, when source files are compiled without visibility
// flag in order to expose most symbols in the corresponding file, there are some specific
diff --git a/include/osv/osv_c_wrappers.h b/include/osv/osv_c_wrappers.h
index 94f07ad1..bb7d52a8 100644
--- a/include/osv/osv_c_wrappers.h
+++ b/include/osv/osv_c_wrappers.h
@@ -8,20 +8,123 @@
#ifndef INCLUDED_OSV_C_WRAPPERS_H
#define INCLUDED_OSV_C_WRAPPERS_H

+#include <limits.h>
#include <sys/types.h>
+#include <osv/mount.h>

#ifdef __cplusplus
extern "C" {
#endif

+enum osv_thread_status {
+ invalid,
+ prestarted,
+ unstarted,
+ waiting,
+ sending_lock,
+ running,
+ queued,
+ waking,
+ terminating,
+ terminated
+};
+
+struct osv_thread {
+ // 32-bit thread id
+ long id;
+
+ // CPU the thread is running on
+ long cpu_id;
+
+ // Total CPU time used by the thread (in milliseconds)
+ long cpu_ms;
+
+ // Number of times this thread was context-switched in
+ long switches;
+
+ // Number of times this thread was migrated between CPUs
+ long migrations;
+
+ // Number of times this thread was preempted (still runnable, but switched out)
+ long preemptions;
+
+ float priority;
+ long stack_size;
+
+ enum osv_thread_status status;
+
+ // Thread name
+ char* name;
+};
+
/*
Save in *tid_arr array TIDs of all threads from app which "owns" input tid/thread.
-*tid_arr is allocated with malloc, *len holds lenght.
+*tid_arr is allocated with malloc, *len holds length.
Caller is responsible to free tid_arr.
Returns 0 on success, error code on error.
*/
int osv_get_all_app_threads(pid_t tid, pid_t** tid_arr, size_t* len);

+/*
+Save in *thread_arr array info about all threads.
+*thread_arr is allocated with malloc, *len holds length.
+Caller is responsible to free thread_arr and thread names
+in osv_thread struct.
+Returns 0 on success, error code on error.
+*/
+int osv_get_all_threads(osv_thread** thread_arr, size_t *len);
+
+/*
+ * Return OSv version as C string. The returned C string is
+ * allocated with malloc and caller is responsible to free it
+ * if non null.
+ */
+char *osv_version();
+
+/*
+ * Return OSv command line C string. The returned C string is
+ * allocated with malloc and caller is responsible to free it
+ * if non null.
+ */
+char *osv_cmdline();
+
+/*
+ * Return hypervisor name as C string. The returned C string is
+ * allocated with malloc and caller is responsible to free it
+ * if non null.
+ */
+char *osv_hypervisor_name();
+
+/*
+ * Return firmware vendor as C string. The returned C string is
+ * allocated with malloc and caller is responsible to free it
+ * if non null.
+ */
+char *osv_firmware_vendor();
+
+/*
+ * Return processor features as C string. The returned C string is
+ * allocated with malloc and caller is responsible to free it
+ * if non null.
+ */
+char *osv_processor_features();
+
+/*
+ * Return pointer to OSv debug buffer.
+ */
+const char *osv_debug_buffer();
+
+/*
+ * Return true if OSv debug flag (--verbose) is enabled, otherwise return false.
+ */
+bool osv_debug_enabled();
+
+/*
+ * Pass a function pointer of a routine which will be invoked
+ * upon termination of the current app. Useful for resources cleanup.
+ */
+void osv_current_app_on_termination_request(void (*handler)());
+
#ifdef __cplusplus
}
#endif
--
2.31.1

[Waldemar Kozaczuk]

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---

Makefile | 2 +-
exported_symbols/osv_module_api.symbols | 10 ++++++++++
2 files changed, 11 insertions(+), 1 deletion(-)
create mode 100644 exported_symbols/osv_module_api.symbols

diff --git a/Makefile b/Makefile
index a0878787..2f249ee4 100644
--- a/Makefile
+++ b/Makefile
@@ -1970,7 +1970,7 @@ else
linker_archives_options = --whole-archive $(libstdc++.a) $(libgcc_eh.a) $(boost-libs) --no-whole-archive $(libgcc.a)
endif

-$(out)/version_script: exported_symbols/$(arch)/*.symbols
+$(out)/version_script: exported_symbols/*.symbols exported_symbols/$(arch)/*.symbols
$(call quiet, scripts/generate_version_script.sh $(out)/version_script, GEN version_script)

$(out)/loader.elf: $(stage1_targets) arch/$(arch)/loader.ld $(out)/bootfs.o $(loader_options_dep)
diff --git a/exported_symbols/osv_module_api.symbols b/exported_symbols/osv_module_api.symbols
new file mode 100644
index 00000000..2ba5050c
--- /dev/null
+++ b/exported_symbols/osv_module_api.symbols
@@ -0,0 +1,10 @@
+osv_cmdline
+osv_current_app_on_termination_request
+osv_debug_buffer
+osv_debug_enabled
+osv_firmware_vendor
+osv_get_all_app_threads
+osv_get_all_threads
+osv_hypervisor_name
+osv_processor_features
+osv_version
--
2.31.1

[Waldemar Kozaczuk]

When OSv kernel is built to hide most symbols but glibc ones, the
OSv applications like httpserver monitoring API can not function
corretly as they rely on number of internal C++ API.

This patch modifies httpserver monitoring API to stop using kernel
internal C++ API. It does so by replacing some of the calls to internal C++
symbols with new module C-style API symbols: for example, sched::with_all_threads()
with new osv_get_all_threads(). In other scenarios, we fall back to
standard glibc API: for example osv::current_mounts() is replaced with
getmntent_r() and related functions.

Finally, we link httpserver monitoring app with core/options.cc and
thus remove need to have those symbols exposed by the kernel.

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---

modules/httpserver-api/api/fs.cc | 42 ++++++++++----
modules/httpserver-api/api/hardware.cc | 23 +++++---
modules/httpserver-api/api/network.cc | 19 ++++--
modules/httpserver-api/api/os.cc | 67 +++++++++++++---------
modules/httpserver-api/global_server.cc | 34 +++++++----
modules/httpserver-api/global_server.hh | 5 ++
modules/httpserver-api/openssl-init.cc | 15 +++--
modules/httpserver-api/ssl_server.cc | 9 ++-
modules/httpserver-monitoring-api/Makefile | 6 +-
9 files changed, 150 insertions(+), 70 deletions(-)

diff --git a/modules/httpserver-api/api/fs.cc b/modules/httpserver-api/api/fs.cc
index 94eec77b..52e58c39 100644
--- a/modules/httpserver-api/api/fs.cc
+++ b/modules/httpserver-api/api/fs.cc
@@ -6,12 +6,12 @@
*/

#include "fs.hh"
-#include "osv/mount.h"
#include "json/formatter.hh"
#include "autogen/fs.json.hh"
#include <string>
#include <vector>
#include <sys/statvfs.h>
+#include <mntent.h>

namespace httpserver {

@@ -23,9 +23,9 @@ using namespace std;
using namespace json;
using namespace fs_json;

-static void fill_dfstat(DFStat& dfstat, const osv::mount_desc& mount, const struct statvfs& st) {
- dfstat.filesystem = mount.special;
- dfstat.mount = mount.path;
+static void fill_dfstat(DFStat& dfstat, mntent* mount, const struct statvfs& st) {
+ dfstat.filesystem = mount->mnt_fsname;
+ dfstat.mount = mount->mnt_dir;
dfstat.btotal = st.f_blocks;
dfstat.bfree = st.f_bfree;
dfstat.ftotal = st.f_files;
@@ -46,21 +46,31 @@ void init(routes& routes) {
getDFStats.set_handler("json",
[](const_req req)
{
- using namespace osv;
const std::string onemount = req.param.at("mount");
struct statvfs st;
httpserver::json::DFStat dfstat;
vector<httpserver::json::DFStat> dfstats;

- for (mount_desc mount : osv::current_mounts()) {
- if ((mount.type == "zfs" || mount.type == "rofs") && (onemount == "" || onemount == mount.path)) {
- if (statvfs(mount.path.c_str(),&st) != 0) {
+ FILE *mounts_fp = setmntent("/proc/mounts", "r");
+ if (!mounts_fp) {
+ throw server_error_exception("failed to get mounts information");
+ }
+
+ struct mntent* mount;
+ mntent mnt;
+ char strings[4096];
+ while ((mount = getmntent_r(mounts_fp, &mnt, strings, sizeof(strings)))) {
+ std::string fstype(mount->mnt_type);
+ if ((fstype == "zfs" || fstype == "rofs") && (onemount == "" || onemount == mount->mnt_dir)) {
+ if (statvfs(mount->mnt_dir,&st) != 0) {
+ endmntent(mounts_fp);
throw not_found_exception("mount does not exist");
}
fill_dfstat(dfstat, mount, st);
dfstats.push_back(dfstat);
}
};
+ endmntent(mounts_fp);

// checking if a specific file system was requested and if we found it
if (onemount != "" && dfstats.size() == 0) {
@@ -76,14 +86,24 @@ void init(routes& routes) {
httpserver::json::DFStat dfstat;
vector<httpserver::json::DFStat> res;

- for (osv::mount_desc mount : osv::current_mounts()) {
- if (mount.type == "zfs" || mount.type == "rofs") {
- if (statvfs(mount.path.c_str(),&st) == 0) {
+ FILE *mounts_fp = setmntent("/proc/mounts", "r");
+ if (!mounts_fp) {
+ throw server_error_exception("failed to get mounts information");
+ }
+
+ struct mntent* mount;
+ mntent mnt;
+ char strings[4096];
+ while ((mount = getmntent_r(mounts_fp, &mnt, strings, sizeof(strings)))) {
+ std::string fstype(mount->mnt_type);
+ if (fstype == "zfs" || fstype == "rofs") {
+ if (statvfs(mount->mnt_dir,&st) == 0) {
fill_dfstat(dfstat, mount, st);
res.push_back(dfstat);
}
}
}
+ endmntent(mounts_fp);
return res;
});

diff --git a/modules/httpserver-api/api/hardware.cc b/modules/httpserver-api/api/hardware.cc
index f023e394..fe069913 100644
--- a/modules/httpserver-api/api/hardware.cc
+++ b/modules/httpserver-api/api/hardware.cc
@@ -10,9 +10,8 @@
#include "autogen/hardware.json.hh"
#include "processor.hh"
#include "cpuid.hh"
-#include <osv/sched.hh>
-#include <osv/firmware.hh>
-#include <osv/hypervisor.hh>
+#include <osv/osv_c_wrappers.h>
+#include <sys/sysinfo.h>

namespace httpserver {

@@ -30,26 +29,36 @@ extern "C" void httpserver_plugin_register_routes(httpserver::routes* routes) {
}
#endif

+static std::string from_c_string(char *c_str) {
+ if (c_str) {
+ std::string str(c_str);
+ free(c_str);
+ return str;
+ } else {
+ return std::string();
+ }
+}
+
void init(routes& routes)
{
hardware_json_init_path("Hardware management API");

processorFeatures.set_handler([](const_req req)
{
- return processor::features_str();
+ return from_c_string(osv_processor_features());
});

processorCount.set_handler([](const_req req)
{
- return sched::cpus.size();
+ return get_nprocs();
});

firmware_vendor.set_handler([](const_req) {
- return osv::firmware_vendor();
+ return from_c_string(osv_firmware_vendor());
});

hypervisor_name.set_handler([](const_req) {
- return osv::hypervisor_name();
+ return from_c_string(osv_hypervisor_name());
});
}

diff --git a/modules/httpserver-api/api/network.cc b/modules/httpserver-api/api/network.cc
index 7d31faf8..595faf4c 100644
--- a/modules/httpserver-api/api/network.cc
+++ b/modules/httpserver-api/api/network.cc
@@ -10,7 +10,7 @@
#include "../libtools/network_interface.hh"
#include "exception.hh"
#include <vector>
-#include <osv/clock.hh>
+#include <time.h>

namespace httpserver {

@@ -57,13 +57,16 @@ void init(routes& routes)
network_json_init_path("Hardware management API");
network_json::listIfconfig.set_handler([](const_req req) {
vector<Interface> res;
- auto time = duration_cast<microseconds>
- (osv::clock::uptime::now().time_since_epoch()).count();
+ timespec time;
+ if (clock_gettime(CLOCK_BOOTTIME, &time)) {
+ return res;
+ }
+ auto time_mc = time.tv_sec * 1000000 + time.tv_nsec / 1000;
for (unsigned int i = 0; i <= number_of_interfaces(); i++) {
auto* ifp = get_interface_by_index(i);

if (ifp != nullptr) {
- res.push_back(get_interface(get_interface_name(ifp), ifp, time));
+ res.push_back(get_interface(get_interface_name(ifp), ifp, time_mc));
}
}
return res;
@@ -76,8 +79,12 @@ void init(routes& routes)
if (ifp == nullptr) {
throw not_found_exception(string("Interface ") + name + " not found");
}
- auto time = duration_cast<microseconds>(osv::clock::uptime::now().time_since_epoch()).count();
- return get_interface(name, ifp, time);
+ timespec time;
+ if (clock_gettime(CLOCK_BOOTTIME, &time)) {
+ throw not_found_exception("Failed to get time");
+ }
+ auto time_mc = time.tv_sec * 1000000 + time.tv_nsec / 1000;
+ return get_interface(name, ifp, time_mc);
});

network_json::getRoute.set_handler([](const_req req) {
diff --git a/modules/httpserver-api/api/os.cc b/modules/httpserver-api/api/os.cc
index 036f9162..38a662a7 100644
--- a/modules/httpserver-api/api/os.cc
+++ b/modules/httpserver-api/api/os.cc
@@ -6,23 +6,20 @@
*/

#include <sys/utsname.h>
+#include <sys/time.h>
#include "os.hh"
-#include "osv/version.hh"
#include "json/formatter.hh"
#include "autogen/os.json.hh"
#include <sys/sysinfo.h>
#include <time.h>
#include <osv/shutdown.hh>
#include <osv/power.hh>
-#include <osv/debug.hh>
-#include <osv/sched.hh>
#include <api/unistd.h>
#include <osv/commands.hh>
+#include <osv/osv_c_wrappers.h>
#include <algorithm>
#include "../java-base/balloon/balloon_api.hh"

-extern char debug_buffer[DEBUG_BUFFER_SIZE];
-
namespace httpserver {

namespace api {
@@ -39,6 +36,16 @@ extern "C" void httpserver_plugin_register_routes(httpserver::routes* routes) {
}
#endif

+static std::string from_c_string(char *c_str) {
+ if (c_str) {
+ std::string str(c_str);
+ free(c_str);
+ return str;
+ } else {
+ return std::string();
+ }
+}
+
void init(routes& routes)
{
os_json_init_path("OS core API");
@@ -48,7 +55,7 @@ void init(routes& routes)
});

os_version.set_handler([](const_req req) {
- return osv::version();
+ return from_c_string(osv_version());
});

os_vendor.set_handler([](const_req req) {
@@ -103,7 +110,7 @@ void init(routes& routes)
#endif

os_dmesg.set_handler([](const_req req) {
- return debug_buffer;
+ return osv_debug_buffer();
});

os_get_hostname.set_handler([](const_req req)
@@ -122,31 +129,39 @@ void init(routes& routes)
#endif

os_threads.set_handler([](const_req req) {
- using namespace std::chrono;
httpserver::json::Threads threads;
- threads.time_ms = duration_cast<milliseconds>
- (osv::clock::wall::now().time_since_epoch()).count();
+ timeval timeofday;
+ if (gettimeofday(&timeofday, nullptr)) {
+ return threads;
+ }
+ threads.time_ms = timeofday.tv_sec * 1000 + timeofday.tv_usec / 1000;
httpserver::json::Thread thread;
- sched::with_all_threads([&](sched::thread &t) {
- thread.id = t.id();
- thread.status = t.get_status();
- auto tcpu = t.tcpu();
- thread.cpu = tcpu ? tcpu->id : -1;
- thread.cpu_ms = duration_cast<milliseconds>(t.thread_clock()).count();
- thread.switches = t.stat_switches.get();
- thread.migrations = t.stat_migrations.get();
- thread.preemptions = t.stat_preemptions.get();
- thread.name = t.name();
- thread.priority = t.priority();
- thread.stack_size = t.get_stack_info().size;
- thread.status = t.get_status();
- threads.list.push(thread);
- });
+ osv_thread *osv_threads;
+ size_t threads_num;
+ if (!osv_get_all_threads(&osv_threads, &threads_num)) {
+ for (size_t i = 0; i < threads_num; i++) {
+ auto &t = osv_threads[i];
+ thread.id = t.id;
+ thread.status = t.status;
+ thread.cpu = t.cpu_id;
+ thread.cpu_ms = t.cpu_ms;
+ thread.switches = t.switches;
+ thread.migrations = t.migrations;
+ thread.preemptions = t.preemptions;
+ thread.name = t.name;
+ free(t.name);
+ thread.priority = t.priority;
+ thread.stack_size = t.stack_size;
+ thread.status = t.status;
+ threads.list.push(thread);
+ }
+ free(osv_threads);
+ }
return threads;
});

os_get_cmdline.set_handler([](const_req req) {
- return osv::getcmdline();
+ return from_c_string(osv_cmdline());
});

#if !defined(MONITORING)
diff --git a/modules/httpserver-api/global_server.cc b/modules/httpserver-api/global_server.cc
index 5cf1c62e..649e62cf 100644
--- a/modules/httpserver-api/global_server.cc
+++ b/modules/httpserver-api/global_server.cc
@@ -8,15 +8,14 @@
#include "global_server.hh"
#include "path_holder.hh"
#include <iostream>
-#include <osv/app.hh>
#include <fstream>
#include <dlfcn.h>
#include <boost/filesystem.hpp>
#include <boost/foreach.hpp>
#include "json/api_docs.hh"
-#include <osv/debug.h>
#include "transformers.hh"
#include <osv/options.hh>
+#include <osv/osv_c_wrappers.h>
#if !defined(MONITORING)
#include "yaml-cpp/yaml.h"
#else
@@ -41,6 +40,13 @@ global_server& global_server::get()
return *instance;
}

+void global_server::termination_handler() {
+ get().s->close();
+ for( auto plugin : get().plugins) {
+ dlclose(plugin);
+ }
+}
+
bool global_server::run(std::map<std::string,std::vector<std::string>>& _config)
{
if (get().s != nullptr) {
@@ -107,12 +113,7 @@ bool global_server::run(std::map<std::string,std::vector<std::string>>& _config)
auto port = get().config["port"][0];
get().s = new http::server::server(get().config, &get()._routes);

- osv::this_application::on_termination_request([&] {
- get().s->close();
- for( auto plugin : get().plugins) {
- dlclose(plugin);
- }
- });
+ osv_current_app_on_termination_request(termination_handler);

std::cout << "Rest API server running on port " << port << std::endl;
get().s->run();
@@ -121,6 +122,7 @@ bool global_server::run(std::map<std::string,std::vector<std::string>>& _config)

#if !defined(MONITORING)
void global_server::setup_file_mappings(const YAML::Node& file_mappings_node) {
+ auto debug_enabled = osv_debug_enabled();
for (auto node : file_mappings_node) {
const YAML::Node path = node["path"];
if (path && node["directory"]) {
@@ -130,7 +132,9 @@ void global_server::setup_file_mappings(const YAML::Node& file_mappings_node) {
new directory_handler(directory, new content_replace(content_replace_node.as<std::string>())) :
new directory_handler(directory);
_routes.add(GET, url(path.as<std::string>()).remainder("path"), handler);
- debug("httpserver: setup directory mapping: [%s] -> [%s]\n", path.as<std::string>().c_str(), directory.c_str());
+ if (debug_enabled) {
+ std::cout << "httpserver: setup directory mapping: [" << path.as<std::string>() << "] -> [" << directory << "]" << std::endl;
+ }
}
else if (path && node["file"]) {
const std::string file = node["file"].as<std::string>();
@@ -142,7 +146,9 @@ void global_server::setup_file_mappings(const YAML::Node& file_mappings_node) {
else {
_routes.add(GET, url(path.as<std::string>()).remainder("path"), handler);
}
- debug("httpserver: setup file mapping: [%s] -> [%s]\n", path.as<std::string>().c_str(), file.c_str());
+ if (debug_enabled) {
+ std::cout << "httpserver: setup file mapping: [" << path.as<std::string>() << "] -> [" << file << "]" << std::endl;
+ }
}
}
}
@@ -165,7 +171,9 @@ void global_server::setup_redirects(const YAML::Node& redirects_node) {
return "";
});
_routes.put(GET, path, redirect);
- debug("httpserver: setup redirect: [%s] -> [%s]\n", path.c_str(), target_path.c_str());
+ if (osv_debug_enabled()) {
+ std::cout << "httpserver: setup redirect: [" << path << "] -> [" << target_path << "]" << std::endl;
+ }
}
}
}
@@ -242,6 +250,8 @@ void global_server::load_plugin(const std::string& path)
}
plugins.push_back(plugin);
httpserver_plugin_register_routes(&_routes);
- debug("httpserver: loaded plugin from path: %s\n",path.c_str());
+ if (osv_debug_enabled()) {
+ std::cout << "httpserver: loaded plugin from path: " << path << std::endl;
+ }
}
}
diff --git a/modules/httpserver-api/global_server.hh b/modules/httpserver-api/global_server.hh
index e70b7854..587860fb 100644
--- a/modules/httpserver-api/global_server.hh
+++ b/modules/httpserver-api/global_server.hh
@@ -29,6 +29,11 @@ public:
*/
static global_server& get();

+ /**
+ * cleanup routine: shutdown server and close all plugins
+ */
+ static void termination_handler();
+
/**
* get the route object
* @return a reference to the route object
diff --git a/modules/httpserver-api/openssl-init.cc b/modules/httpserver-api/openssl-init.cc
index 8630d019..a1bed2bd 100644
--- a/modules/httpserver-api/openssl-init.cc
+++ b/modules/httpserver-api/openssl-init.cc
@@ -8,7 +8,7 @@
#include <iostream>
#include <fstream>
#include <mutex>
-#include <osv/debug.hh>
+#include <osv/osv_c_wrappers.h>
#include <assert.h>

#include <openssl/rand.h>
@@ -16,9 +16,16 @@

#include "openssl-init.hh"

+static void debug_line(const char *line) {
+ auto debug_enabled = osv_debug_enabled();
+ if (debug_enabled) {
+ std::cout << line << std::endl;
+ }
+}
+
static void seed_openssl()
{
- debug("Seeding OpenSSL...\n");
+ debug_line("Seeding OpenSSL...");

for (;;) {
char buf[32];
@@ -39,10 +46,10 @@ static void seed_openssl()
// fedora's openssl-1.0.1h-5.fc20 needs 48 bytes instead
// of 32, which is what is required by its upstream version
// (tag OpenSSL_1_0_1h). In general, we should make no assumptions.
- debug("Still not seeded, retrying\n");
+ debug_line("Still not seeded, retrying");
}

- debug("OpenSSL seeding done.\n");
+ debug_line("OpenSSL seeding done.");
}

void ensure_openssl_initialized()
diff --git a/modules/httpserver-api/ssl_server.cc b/modules/httpserver-api/ssl_server.cc
index a4f8ada2..a064bd6b 100644
--- a/modules/httpserver-api/ssl_server.cc
+++ b/modules/httpserver-api/ssl_server.cc
@@ -5,11 +5,12 @@

* BSD license as described in the LICENSE file in the top-level directory.

*/

-#include <osv/debug.hh>
+#include <osv/osv_c_wrappers.h>

#include "transport.hh"
#include "ssl_server.hh"
#include <openssl/ssl.h>
+#include <iostream>

namespace http {

@@ -83,8 +84,10 @@ void ssl_acceptor::do_accept(callback_t callback)
[this, socket, callback] (boost::system::error_code ec) {
if (ec) {
auto remote = socket->lowest_layer().remote_endpoint();
- debug("handshake with " + remote.address().to_string()
- + " failed: " + ec.message() + "\n");
+ if (osv_debug_enabled()) {
+ std::cout << "handshake with " << remote.address().to_string()
+ << " failed: " << ec.message() << std::endl;
+ }
}

if (!_tcp_acceptor.is_open()) {
diff --git a/modules/httpserver-monitoring-api/Makefile b/modules/httpserver-monitoring-api/Makefile
index e26fc06b..8ac415b7 100644
--- a/modules/httpserver-monitoring-api/Makefile
+++ b/modules/httpserver-monitoring-api/Makefile
@@ -45,7 +45,7 @@ all: $(module_out)/lib$(TARGET).so
$(src)/scripts/manifest_from_host.sh $(module_out)/lib$(TARGET).so > usr.manifest; \
fi

-$(module_out)/lib$(TARGET).so: $(JSON_OBJ_FILES) $(OBJ_FILES)
+$(module_out)/lib$(TARGET).so: $(JSON_OBJ_FILES) $(OBJ_FILES) $(module_out)/options.o
$(call quiet, $(CXX) $(CXXFLAGS) $(LDFLAGS) -shared $(STATIC_LIBS) -o $@ $^ $(DYN_LIBS), LINK $@)

ifneq ($(MAKECMDGOALS),clean)
@@ -64,6 +64,10 @@ $(JSON_OBJ_FILES): $(module_out)/autogen/%.o: autogen/%.cc
$(call very-quiet, mkdir -p $(module_out)/autogen)
$(call quiet, $(CXX) $(CXXFLAGS) -c -MMD -o $@ $<, CXX $@)

+$(module_out)/options.o: $(src)/core/options.cc
+ $(call very-quiet, mkdir -p $(module_out))
+ $(call quiet, $(CXX) $(CXXFLAGS) -c -MMD -o $@ $<, CXX $@)
+
clean:
$(call quiet, $(RM) -f $(TARGET), CLEAN)
$(call very-quiet, $(RM) -rf $(module_out))
--
2.31.1

[Nadav Har'El]

Looks good. Just a couple of nitpicks below.

Nitpick: you can simplify this by just returning "buf" unconditionally:

    if (buf) {

        std::copy(str.begin(), str.end(), buf);

        buf[len] = '\0';

    }

    return buf;

This enum probably needs to be identical to  the one in sched.hh.

We're not actively modifying this list, but in the past when we were more actively developing OSv, it did change.

So I think the least we should do is to put comments here and in sched.hh warning that these lists need to be kept in

sync. A more reliable - but uglier so probably we shouldn't do it - approach can be to have one header file that both

places include.

A thought (not critical): should we have a function to do this free? 

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/20220117055538.139407-1-jwkozaczuk%40gmail.com.

[Waldek Kozaczuk]

Thanks for your review. I have addressed the first two of your suggestions.

[Commit Bot]

From: Waldemar Kozaczuk <jwkoz...@gmail.com>
Committer: Waldemar Kozaczuk <jwkoz...@gmail.com>
Branch: master

add new C-wrappers to expose module API

The commit af2d371a61f6ab1eb5a066a0c3e93230faf6611c introduced ability
to build OSv kernel with most symbols but subset of glibc hidden.
The regular Linux glibc apps should run fine on such kernel, but
unfortunately many unit tests and various internal OSv apps (so called
modules) do not as they have been coded to use many internal API
symbols. One such example is httpserver monitoring api module that
exposes various monitoring API REST endpoints.

At some point XLAB introduced C-wrappers API made of single C-style
osv_get_all_app_threads() functions. This patch enhances the C-wrappers API
by adding 9 more functions intended to be used by httpserver monitoring
api module.

Please note that new C-style API will open up access to relevant functionality
to new apps/modules implemented in languages different than C++.

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>

---

diff --git a/core/osv_c_wrappers.cc b/core/osv_c_wrappers.cc

@@ -28,3 +43,107 @@ int osv_get_all_app_threads(pid_t tid, pid_t** tid_arr, size_t *len) {

}
return 0;
}
+
+static void free_threads_names(std::vector<osv_thread> &threads) {
+ for (auto &t : threads) {
+ if (t.name) {
+ free(t.name);
+ }
+ }
+}
+
+static char* str_to_c_str(const std::string& str) {
+ auto len = str.size();
+ char *buf = static_cast<char*>(malloc(len + 1)); // This will be free()-ed in C world
+ if (buf) {
+ std::copy(str.begin(), str.end(), buf);
+ buf[len] = '\0';
+ }

+ return buf;

--- a/include/osv/export.h
+++ b/include/osv/export.h
@@ -32,6 +32,9 @@

// This is to expose some symbols in libsolaris.so
#define OSV_LIB_SOLARIS_API __attribute__((__visibility__("default")))
+//
+// This is to expose some OSv functions intended to be used by modules
+#define OSV_MODULE_API __attribute__((__visibility__("default")))

// In some very few cases, when source files are compiled without visibility
// flag in order to expose most symbols in the corresponding file, there are some specific
diff --git a/include/osv/osv_c_wrappers.h b/include/osv/osv_c_wrappers.h

--- a/include/osv/osv_c_wrappers.h
+++ b/include/osv/osv_c_wrappers.h
@@ -8,20 +8,125 @@

#ifndef INCLUDED_OSV_C_WRAPPERS_H
#define INCLUDED_OSV_C_WRAPPERS_H

+#include <limits.h>
#include <sys/types.h>
+#include <osv/mount.h>

#ifdef __cplusplus
extern "C" {
#endif

+// This C enum should be kept in sync with the C++ enum class
+// sched::thread::status defined in sched.hh

+enum osv_thread_status {
+ invalid,
+ prestarted,
+ unstarted,
+ waiting,
+ sending_lock,
+ running,
+ queued,
+ waking,
+ terminating,
+ terminated

[Commit Bot]

From: Waldemar Kozaczuk <jwkoz...@gmail.com>
Committer: Waldemar Kozaczuk <jwkoz...@gmail.com>
Branch: master

add module API symbols to exported list

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>

---

diff --git a/Makefile b/Makefile

--- a/Makefile
+++ b/Makefile
@@ -1970,7 +1970,7 @@ else
linker_archives_options = --whole-archive $(libstdc++.a) $(libgcc_eh.a) $(boost-libs) --no-whole-archive $(libgcc.a)
endif

-$(out)/version_script: exported_symbols/$(arch)/*.symbols
+$(out)/version_script: exported_symbols/*.symbols exported_symbols/$(arch)/*.symbols
$(call quiet, scripts/generate_version_script.sh $(out)/version_script, GEN version_script)

$(out)/loader.elf: $(stage1_targets) arch/$(arch)/loader.ld $(out)/bootfs.o $(loader_options_dep)
diff --git a/exported_symbols/osv_module_api.symbols b/exported_symbols/osv_module_api.symbols

--- a/exported_symbols/osv_module_api.symbols

[Commit Bot]

From: Waldemar Kozaczuk <jwkoz...@gmail.com>
Committer: Waldemar Kozaczuk <jwkoz...@gmail.com>
Branch: master

httpserver-monitoring-api: stop using kernel internal C++ API

When OSv kernel is built to hide most symbols but glibc ones, the
OSv applications like httpserver monitoring API can not function
corretly as they rely on number of internal C++ API.

This patch modifies httpserver monitoring API to stop using kernel
internal C++ API. It does so by replacing some of the calls to internal C++
symbols with new module C-style API symbols: for example, sched::with_all_threads()
with new osv_get_all_threads(). In other scenarios, we fall back to
standard glibc API: for example osv::current_mounts() is replaced with
getmntent_r() and related functions.

Finally, we link httpserver monitoring app with core/options.cc and
thus remove need to have those symbols exposed by the kernel.

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>

---

diff --git a/modules/httpserver-api/api/fs.cc b/modules/httpserver-api/api/fs.cc

--- a/modules/httpserver-api/global_server.hh
+++ b/modules/httpserver-api/global_server.hh
@@ -29,6 +29,11 @@ public:
*/
static global_server& get();

+ /**
+ * cleanup routine: shutdown server and close all plugins
+ */
+ static void termination_handler();
+
/**
* get the route object
* @return a reference to the route object
diff --git a/modules/httpserver-api/openssl-init.cc b/modules/httpserver-api/openssl-init.cc

--- a/modules/httpserver-api/openssl-init.cc
+++ b/modules/httpserver-api/openssl-init.cc
@@ -8,17 +8,24 @@

#include <iostream>
#include <fstream>
#include <mutex>
-#include <osv/debug.hh>
+#include <osv/osv_c_wrappers.h>
#include <assert.h>

#include <openssl/rand.h>

#include <openssl/ssl.h>

#include "openssl-init.hh"

+static void debug_line(const char *line) {
+ auto debug_enabled = osv_debug_enabled();
+ if (debug_enabled) {
+ std::cout << line << std::endl;
+ }
+}
+
static void seed_openssl()
{
- debug("Seeding OpenSSL...\n");
+ debug_line("Seeding OpenSSL...");

for (;;) {
char buf[32];
@@ -39,10 +46,10 @@ static void seed_openssl()
// fedora's openssl-1.0.1h-5.fc20 needs 48 bytes instead
// of 32, which is what is required by its upstream version
// (tag OpenSSL_1_0_1h). In general, we should make no assumptions.
- debug("Still not seeded, retrying\n");
+ debug_line("Still not seeded, retrying");
}

- debug("OpenSSL seeding done.\n");
+ debug_line("OpenSSL seeding done.");
}

void ensure_openssl_initialized()
diff --git a/modules/httpserver-api/ssl_server.cc b/modules/httpserver-api/ssl_server.cc

--- a/modules/httpserver-api/ssl_server.cc
+++ b/modules/httpserver-api/ssl_server.cc
@@ -5,11 +5,12 @@

* BSD license as described in the LICENSE file in the top-level directory.

*/

-#include <osv/debug.hh>
+#include <osv/osv_c_wrappers.h>

#include "transport.hh"
#include "ssl_server.hh"
#include <openssl/ssl.h>
+#include <iostream>

namespace http {

@@ -83,8 +84,10 @@ void ssl_acceptor::do_accept(callback_t callback)
[this, socket, callback] (boost::system::error_code ec) {
if (ec) {
auto remote = socket->lowest_layer().remote_endpoint();
- debug("handshake with " + remote.address().to_string()
- + " failed: " + ec.message() + "\n");
+ if (osv_debug_enabled()) {
+ std::cout << "handshake with " << remote.address().to_string()
+ << " failed: " << ec.message() << std::endl;
+ }
}

if (!_tcp_acceptor.is_open()) {
diff --git a/modules/httpserver-monitoring-api/Makefile b/modules/httpserver-monitoring-api/Makefile