Re: [osv-dev] [PATCH 1/2] add new C-wrappers to expose module API

2022-01-19 Thread Waldek Kozaczuk
Thanks for your review. I have addressed the first two of your suggestions.

On Wednesday, January 19, 2022 at 3:47:09 AM UTC-5 Nadav Har'El wrote:

>
> Looks good. Just a couple of nitpicks below.
>
> On Mon, Jan 17, 2022 at 7:55 AM Waldemar Kozaczuk  
> wrote:
>
>> 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 
>> ---
>>  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 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>>
>>  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 ) {
>> +for (auto  : 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(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;
>>
>
> Nitpick: you can simplify this by just returning "buf" unconditionally:
>
> if (buf) {
> std::copy(str.begin(), str.end(), buf);
> buf[len] = '\0';
> }
> return buf;
>
>  
>
>> +}
>> +}
>> +
>> +extern "C" OSV_MODULE_API
>> +int osv_get_all_threads(osv_thread** thread_arr, size_t *len) {
>> +using namespace std::chrono;
>> +std::vector threads;
>> +
>> +osv_thread thread;
>> +bool str_copy_error = false;
>> +sched::with_all_threads([&](sched::thread ) {
>> +thread.id = t.id();
>> +auto tcpu = t.tcpu();
>> +thread.cpu_id = tcpu ? tcpu->id : -1;
>> +thread.cpu_ms = 
>> duration_cast(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(static_cast(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 

Re: [osv-dev] [PATCH 1/2] add new C-wrappers to expose module API

2022-01-19 Thread Nadav Har'El
Looks good. Just a couple of nitpicks below.

On Mon, Jan 17, 2022 at 7:55 AM Waldemar Kozaczuk 
wrote:

> 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 
> ---
>  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 
> +#include 
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
>
>  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 ) {
> +for (auto  : 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(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;
>

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

if (buf) {
std::copy(str.begin(), str.end(), buf);
buf[len] = '\0';
}
return buf;



> +}
> +}
> +
> +extern "C" OSV_MODULE_API
> +int osv_get_all_threads(osv_thread** thread_arr, size_t *len) {
> +using namespace std::chrono;
> +std::vector threads;
> +
> +osv_thread thread;
> +bool str_copy_error = false;
> +sched::with_all_threads([&](sched::thread ) {
> +thread.id = t.id();
> +auto tcpu = t.tcpu();
> +thread.cpu_id = tcpu ? tcpu->id : -1;
> +thread.cpu_ms =
> duration_cast(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(static_cast(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-dev] [PATCH 1/2] add new C-wrappers to expose module API

2022-01-16 Thread 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 
---
 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 
+#include 
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 
 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 ) {
+for (auto  : 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(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 threads;
+
+osv_thread thread;
+bool str_copy_error = false;
+sched::with_all_threads([&](sched::thread ) {
+thread.id = t.id();
+auto tcpu = t.tcpu();
+thread.cpu_id = tcpu ? tcpu->id : -1;
+thread.cpu_ms = duration_cast(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(static_cast(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