From acc636b1c8d942667e0be14f9d85876a5e2492cf Mon Sep 17 00:00:00 2001 From: John Keiser Date: Mon, 24 Mar 2014 15:39:25 -0700 Subject: [PATCH] Stop blocking ruby during long-running operations --- ext/lxc/lxc.c | 571 ++++++++++++++++++++------- test/test_helpers.rb | 27 ++ test/test_lxc_create_asynchronous.rb | 25 ++ test/test_lxc_created.rb | 11 +- test/test_lxc_long_running.rb | 228 +++++++++++ 5 files changed, 708 insertions(+), 154 deletions(-) create mode 100644 test/test_helpers.rb create mode 100644 test/test_lxc_create_asynchronous.rb create mode 100644 test/test_lxc_long_running.rb diff --git a/ext/lxc/lxc.c b/ext/lxc/lxc.c index 2d51776..b4bb86d 100644 --- a/ext/lxc/lxc.c +++ b/ext/lxc/lxc.c @@ -5,6 +5,7 @@ #include #include #include +#include #define SYMBOL(s) ID2SYM(rb_intern(s)) @@ -383,9 +384,23 @@ container_state(VALUE self) return rb_str_intern(rb_funcall(rb_state, rb_intern("downcase"), 0)); } +struct add_device_node_outside_gil_args +{ + struct container_data *data; + char *src_path; + char *dest_path; +}; + +static VALUE +add_device_node_outside_gil(void *args_void) +{ + struct add_device_node_outside_gil_args *args = (struct add_device_node_outside_gil_args *)args_void; + return INT2NUM( args->data->container->add_device_node(args->data->container, args->src_path, args->dest_path) ); +} + /* * call-seq: - * container.add_device_node(src_path, dst_path = src_path) + * container.add_device_node(src_path, dest_path = src_path) * * Adds a device node to the container. */ @@ -393,17 +408,16 @@ static VALUE container_add_device_node(int argc, VALUE *argv, VALUE self) { int ret; - char *src_path, *dst_path; - struct container_data *data; - VALUE rb_src_path, rb_dst_path; + VALUE rb_src_path, rb_dest_path; + struct add_device_node_outside_gil_args args; - rb_scan_args(argc, argv, "11", &rb_src_path, &rb_dst_path); - src_path = NIL_P(rb_src_path) ? NULL : StringValuePtr(rb_src_path); - dst_path = NIL_P(rb_dst_path) ? NULL : StringValuePtr(rb_dst_path); + rb_scan_args(argc, argv, "11", &rb_src_path, &rb_dest_path); + args.src_path = StringValuePtr(rb_src_path); + args.dest_path = NIL_P(rb_dest_path) ? NULL : StringValuePtr(rb_dest_path); - Data_Get_Struct(self, struct container_data, data); + Data_Get_Struct(self, struct container_data, args.data); - ret = data->container->add_device_node(data->container, src_path, dst_path); + ret = NUM2INT(rb_thread_blocking_region(add_device_node_outside_gil, &args, NULL, NULL)); if (!ret) rb_raise(Error, "unable to add device node"); @@ -593,6 +607,15 @@ err: return NULL; } +static VALUE lxc_wait_for_pid_status_without_gil(void* pid) +{ + return INT2NUM(lxc_wait_for_pid_status(*(pid_t*)pid)); +} +static void kill_pid_without_gil(void* pid) +{ + kill(*(pid_t*)pid, SIGKILL); +} + /* * call-seq: * container.attach(opts = {}, &block) @@ -650,7 +673,13 @@ container_attach(int argc, VALUE *argv, VALUE self) goto out; if (wait) { - ret = lxc_wait_for_pid_status(pid); + ret = NUM2INT( + rb_thread_blocking_region( + lxc_wait_for_pid_status_without_gil, + &pid, + kill_pid_without_gil, + &pid + )); /* handle case where attach fails */ if (WIFEXITED(ret) && WEXITSTATUS(ret) == 255) ret = -1; @@ -702,6 +731,30 @@ container_clear_config_item(VALUE self, VALUE rb_key) return self; } +struct clone_outside_gil_args +{ + struct container_data *data; + struct lxc_container *new_container; + char *name; + char *config_path; + int flags; + char *bdev_type; + char *bdev_data; + uint64_t new_size; + char **hook_args; +}; + +static VALUE +clone_outside_gil(void *args_void) +{ + struct clone_outside_gil_args *args = (struct clone_outside_gil_args *)args_void; + struct lxc_container *container = args->data->container; + args->new_container = container->clone(container, args->name, + args->config_path, args->flags, args->bdev_type, args->bdev_data, + args->new_size, args->hook_args); + return Qnil; +} + /* * call-seq: * container.clone(clone_name, opts = {}) @@ -719,27 +772,22 @@ container_clear_config_item(VALUE self, VALUE rb_key) static VALUE container_clone(int argc, VALUE *argv, VALUE self) { - int flags; - uint64_t new_size; - char *name, *config_path, *bdev_type, *bdev_data; - char **hook_args; - struct lxc_container *container, *new_container; - struct container_data *data; VALUE rb_name, rb_opts; VALUE rb_flags, rb_config_path, rb_bdev_type, rb_bdev_data; VALUE rb_new_size, rb_hook_args; VALUE rb_args[2]; + struct clone_outside_gil_args args; rb_scan_args(argc, argv, "11", &rb_name, &rb_opts); - name = StringValuePtr(rb_name); + args.name = StringValuePtr(rb_name); - config_path = NULL; - flags = 0; - bdev_type = NULL; - bdev_data = NULL; - new_size = 0; - hook_args = NULL; + args.config_path = NULL; + args.flags = 0; + args.bdev_type = NULL; + args.bdev_data = NULL; + args.new_size = 0; + args.hook_args = NULL; rb_config_path = Qnil; @@ -747,49 +795,65 @@ container_clone(int argc, VALUE *argv, VALUE self) Check_Type(rb_opts, T_HASH); rb_config_path = rb_hash_aref(rb_opts, SYMBOL("config_path")); if (!NIL_P(rb_config_path)) - config_path = StringValuePtr(rb_config_path); + args.config_path = StringValuePtr(rb_config_path); rb_flags = rb_hash_aref(rb_opts, SYMBOL("flags")); if (!NIL_P(rb_flags)) - flags = NUM2INT(rb_flags); + args.flags = NUM2INT(rb_flags); rb_bdev_type = rb_hash_aref(rb_opts, SYMBOL("bdev_type")); if (!NIL_P(rb_bdev_type)) - bdev_type = StringValuePtr(rb_bdev_type); + args.bdev_type = StringValuePtr(rb_bdev_type); rb_bdev_data = rb_hash_aref(rb_opts, SYMBOL("bdev_data")); if (!NIL_P(rb_bdev_data)) - bdev_data = StringValuePtr(rb_bdev_data); + args.bdev_data = StringValuePtr(rb_bdev_data); rb_new_size = rb_hash_aref(rb_opts, SYMBOL("new_size")); if (!NIL_P(rb_bdev_data)) - new_size = NUM2ULL(rb_new_size); + args.new_size = NUM2ULL(rb_new_size); rb_hook_args = rb_hash_aref(rb_opts, SYMBOL("hook_args")); if (!NIL_P(rb_hook_args)) - hook_args = ruby_to_c_string_array(rb_hook_args); + args.hook_args = ruby_to_c_string_array(rb_hook_args); } - Data_Get_Struct(self, struct container_data, data); - container = data->container; + Data_Get_Struct(self, struct container_data, args.data); - new_container = container->clone(container, name, config_path, - flags, bdev_type, bdev_data, new_size, - hook_args); + rb_thread_blocking_region(clone_outside_gil, &args, NULL, NULL); - if (hook_args) - free_c_string_array(hook_args); + if (args.hook_args) + free_c_string_array(args.hook_args); - if (new_container == NULL) + if (args.new_container == NULL) rb_raise(Error, "unable to clone container"); - lxc_container_put(new_container); + lxc_container_put(args.new_container); rb_args[0] = rb_name; rb_args[1] = rb_config_path; return rb_class_new_instance(2, rb_args, Container); } +struct console_outside_gil_args +{ + struct container_data *data; + int tty_num; + int stdin_fd; + int stdout_fd; + int stderr_fd; + int escape; +}; + +static VALUE +console_outside_gil(void *args_void) +{ + struct console_outside_gil_args *args = (struct console_outside_gil_args *)args_void; + return INT2NUM( args->data->container->console(args->data->container, args->tty_num, + args->stdin_fd, args->stdout_fd, + args->stderr_fd, args->escape) ); +} + /* * call-seq: * container.console(opts = {}) @@ -807,29 +871,44 @@ static VALUE container_console(int argc, VALUE *argv, VALUE self) { int ret; - int tty_num = -1, stdin_fd = 0, stdout_fd = 1, stderr_fd = 2, escape = 1; - struct container_data *data; - struct lxc_container *container; + struct console_outside_gil_args args; VALUE rb_opts; + VALUE rb_opt; + + args.tty_num = -1; + args.stdin_fd = 0; + args.stdout_fd = 1; + args.stdout_fd = 2; + args.escape = 1; rb_scan_args(argc, argv, "01", &rb_opts); switch (TYPE(rb_opts)) { case T_HASH: - tty_num = NUM2INT(rb_hash_aref(rb_opts, SYMBOL("tty_num"))); - stdin_fd = NUM2INT(rb_hash_aref(rb_opts, SYMBOL("stdin_fd"))); - stdout_fd = NUM2INT(rb_hash_aref(rb_opts, SYMBOL("stdout_fd"))); - stderr_fd = NUM2INT(rb_hash_aref(rb_opts, SYMBOL("stderr_fd"))); - escape = NUM2INT(rb_hash_aref(rb_opts, SYMBOL("escape"))); + rb_opt = rb_hash_aref(rb_opts, SYMBOL("tty_num")); + if (!NIL_P(rb_opt)) + args.tty_num = NUM2INT(rb_opt); + rb_opt = rb_hash_aref(rb_opts, SYMBOL("stdin_fd")); + if (!NIL_P(rb_opt)) + args.stdin_fd = NUM2INT(rb_opt); + rb_opt = rb_hash_aref(rb_opts, SYMBOL("stdout_fd")); + if (!NIL_P(rb_opt)) + args.stdout_fd = NUM2INT(rb_opt); + rb_opt = rb_hash_aref(rb_opts, SYMBOL("stderr_fd")); + if (!NIL_P(rb_opt)) + args.stderr_fd = NUM2INT(rb_opt); + rb_opt = rb_hash_aref(rb_opts, SYMBOL("escape")); + if (!NIL_P(rb_opt)) + args.escape = NUM2INT(rb_opt); + break; + case T_NIL: break; default: rb_raise(rb_eArgError, "options must be a hash"); } - Data_Get_Struct(self, struct container_data, data); - container = data->container; + Data_Get_Struct(self, struct container_data, args.data); - ret = container->console(container, tty_num, stdin_fd, stdout_fd, stderr_fd, - escape); + ret = NUM2INT(rb_thread_blocking_region(console_outside_gil, &args, NULL, NULL)); if (ret != 0) rb_raise(Error, "unable to access container console"); @@ -863,6 +942,23 @@ container_console_fd(int argc, VALUE *argv, VALUE self) return rb_class_new_instance(1, rb_io_args, rb_cIO); } +/* Used to run container->create outside of GIL */ +struct container_create_outside_gil_args { + struct container_data *data; + char *template; + char *bdevtype; + int flags; + char **args; +}; + +static VALUE +container_create_outside_gil(void *args_void) +{ + struct container_create_outside_gil_args *args = (struct container_create_outside_gil_args *)args_void; + int ret = args->data->container->create(args->data->container, args->template, args->bdevtype, NULL, args->flags, args->args); + return INT2NUM(ret); +} + /* * call-seq: * container.create(template, bdevtype = nil, flags = 0, args = []) @@ -876,28 +972,26 @@ container_console_fd(int argc, VALUE *argv, VALUE self) static VALUE container_create(int argc, VALUE *argv, VALUE self) { - int ret, flags; - char *template; - char *bdevtype; - char **args = { NULL }; - struct container_data *data; - struct lxc_container *container; + int ret; VALUE rb_template, rb_bdevtype, rb_flags, rb_args; + struct container_create_outside_gil_args args; + char ** default_args = { NULL }; + args.args = default_args; rb_scan_args(argc, argv, "13", &rb_template, &rb_bdevtype, &rb_flags, &rb_args); - template = StringValuePtr(rb_template); - bdevtype = NIL_P(rb_bdevtype) ? NULL : StringValuePtr(rb_bdevtype); - flags = NIL_P(rb_flags) ? 0 : NUM2INT(rb_flags); + args.template = StringValuePtr(rb_template); + args.bdevtype = NIL_P(rb_bdevtype) ? NULL : StringValuePtr(rb_bdevtype); + args.flags = NIL_P(rb_flags) ? 0 : NUM2INT(rb_flags); if (!NIL_P(rb_args)) - args = ruby_to_c_string_array(rb_args); + args.args = ruby_to_c_string_array(rb_args); - Data_Get_Struct(self, struct container_data, data); - container = data->container; - ret = container->create(container, template, bdevtype, NULL, flags, args); + Data_Get_Struct(self, struct container_data, args.data); + + ret = NUM2INT(rb_thread_blocking_region(container_create_outside_gil, &args, NULL, NULL)); if (!NIL_P(rb_args)) - free_c_string_array(args); + free_c_string_array(args.args); if (!ret) rb_raise(Error, "unable to create container"); @@ -905,6 +999,14 @@ container_create(int argc, VALUE *argv, VALUE self) return self; } + +static VALUE +destroy_outside_gil(void* data_void) +{ + struct container_data *data = (struct container_data *)data_void; + return INT2NUM( data->container->destroy(data->container) ); +} + /* * call-seq: * container.destroy @@ -919,12 +1021,19 @@ container_destroy(VALUE self) Data_Get_Struct(self, struct container_data, data); - ret = data->container->destroy(data->container); + ret = NUM2INT(rb_thread_blocking_region(destroy_outside_gil, data, NULL, NULL)); if (!ret) rb_raise(Error, "unable to destroy container"); return self; } +static VALUE +freeze_outside_gil(void* data_void) +{ + struct container_data *data = (struct container_data *)data_void; + return INT2NUM( data->container->freeze(data->container) ); +} + /* * call-seq: * container.freeze @@ -939,7 +1048,7 @@ container_freeze(VALUE self) Data_Get_Struct(self, struct container_data, data); - ret = data->container->freeze(data->container); + ret = NUM2INT(rb_thread_blocking_region(freeze_outside_gil, data, NULL, NULL)); if (!ret) rb_raise(Error, "unable to freeze container"); @@ -1146,6 +1255,19 @@ container_ips(int argc, VALUE *argv, VALUE self) return rb_ips; } +struct load_config_outside_gil_args +{ + struct container_data *data; + char *path; +}; + +static VALUE +load_config_outside_gil(void *args_void) +{ + struct load_config_outside_gil_args *args = (struct load_config_outside_gil_args *)args_void; + return INT2NUM( args->data->container->load_config(args->data->container, args->path) ); +} + /* * call-seq: * container.load_config(config_path = nil) @@ -1156,22 +1278,28 @@ static VALUE container_load_config(int argc, VALUE *argv, VALUE self) { int ret; - char *path; - struct container_data *data; VALUE rb_path; + struct load_config_outside_gil_args args; rb_scan_args(argc, argv, "01", &rb_path); - path = NIL_P(rb_path) ? NULL : StringValuePtr(rb_path); + args.path = NIL_P(rb_path) ? NULL : StringValuePtr(rb_path); - Data_Get_Struct(self, struct container_data, data); + Data_Get_Struct(self, struct container_data, args.data); - ret = data->container->load_config(data->container, path); + ret = NUM2INT(rb_thread_blocking_region(load_config_outside_gil, &args, NULL, NULL)); if (!ret) rb_raise(Error, "unable to load configuration file"); return self; } +static VALUE +reboot_outside_gil(void* data_void) +{ + struct container_data *data = (struct container_data *)data_void; + return INT2NUM( data->container->reboot(data->container) ); +} + /* * call-seq: * container.reboot @@ -1186,16 +1314,30 @@ container_reboot(VALUE self) Data_Get_Struct(self, struct container_data, data); - ret = data->container->reboot(data->container); + ret = NUM2INT(rb_thread_blocking_region(reboot_outside_gil, data, NULL, NULL)); if (!ret) rb_raise(Error, "unable to reboot container"); return self; } +struct remove_device_node_outside_gil_args +{ + struct container_data *data; + char *src_path; + char *dest_path; +}; + +static VALUE +remove_device_node_outside_gil(void *args_void) +{ + struct remove_device_node_outside_gil_args *args = (struct remove_device_node_outside_gil_args *)args_void; + return INT2NUM( args->data->container->remove_device_node(args->data->container, args->src_path, args->dest_path) ); +} + /* * call-seq: - * container.remove_device_node(src_path, dst_path = src_path) + * container.remove_device_node(src_path, dest_path = src_path) * * Removes a device node from the container. */ @@ -1203,25 +1345,35 @@ static VALUE container_remove_device_node(int argc, VALUE *argv, VALUE self) { int ret; - char *src_path, *dst_path; - struct lxc_container *container; - struct container_data *data; - VALUE rb_src_path, rb_dst_path; + VALUE rb_src_path, rb_dest_path; + struct remove_device_node_outside_gil_args args; - rb_scan_args(argc, argv, "11", &rb_src_path, &rb_dst_path); - src_path = StringValuePtr(rb_src_path); - dst_path = NIL_P(rb_dst_path) ? NULL : StringValuePtr(rb_dst_path); + rb_scan_args(argc, argv, "11", &rb_src_path, &rb_dest_path); + args.src_path = StringValuePtr(rb_src_path); + args.dest_path = NIL_P(rb_dest_path) ? NULL : StringValuePtr(rb_dest_path); - Data_Get_Struct(self, struct container_data, data); - container = data->container; + Data_Get_Struct(self, struct container_data, args.data); - ret = container->remove_device_node(container, src_path, dst_path); + ret = NUM2INT(rb_thread_blocking_region(remove_device_node_outside_gil, &args, NULL, NULL)); if (!ret) rb_raise(Error, "unable to remove device node"); return self; } +struct rename_outside_gil_args +{ + struct container_data *data; + char *name; +}; + +static VALUE +rename_outside_gil(void* args_void) +{ + struct rename_outside_gil_args *args = (struct rename_outside_gil_args *)args_void; + return INT2NUM( args->data->container->rename(args->data->container, args->name) ); +} + /* * call-seq: * container.rename(new_name) @@ -1233,14 +1385,14 @@ static VALUE container_rename(VALUE self, VALUE rb_name) { int ret; - char *name; - struct container_data *data; VALUE rb_args[2]; + struct rename_outside_gil_args args; - name = StringValuePtr(rb_name); - Data_Get_Struct(self, struct container_data, data); + Data_Get_Struct(self, struct container_data, args.data); - ret = data->container->rename(data->container, name); + args.name = StringValuePtr(rb_name); + + ret = NUM2INT(rb_thread_blocking_region(rename_outside_gil, &args, NULL, NULL)); if (!ret) rb_raise(Error, "unable to rename container"); @@ -1278,20 +1430,32 @@ container_running_config_item(VALUE self, VALUE rb_key) return rb_value; } +struct save_config_outside_gil_args +{ + struct container_data *data; + char *path; +}; + +static VALUE +save_config_outside_gil(void *args_void) +{ + struct save_config_outside_gil_args *args = (struct save_config_outside_gil_args *)args_void; + return INT2NUM( args->data->container->save_config(args->data->container, args->path) ); +} + static VALUE container_save_config(int argc, VALUE *argv, VALUE self) { int ret; - char *path; - struct container_data *data; VALUE rb_path; + struct save_config_outside_gil_args args; rb_scan_args(argc, argv, "01", &rb_path); - path = NIL_P(rb_path) ? NULL : StringValuePtr(rb_path); + args.path = NIL_P(rb_path) ? NULL : StringValuePtr(rb_path); - Data_Get_Struct(self, struct container_data, data); + Data_Get_Struct(self, struct container_data, args.data); - ret = data->container->save_config(data->container, path); + ret = NUM2INT(rb_thread_blocking_region(save_config_outside_gil, &args, NULL, NULL)); if (!ret) rb_raise(Error, "unable to save configuration file"); @@ -1391,6 +1555,19 @@ container_set_config_path(VALUE self, VALUE rb_path) return self; } +struct shutdown_outside_gil_args +{ + struct container_data *data; + int timeout; +}; + +static VALUE +shutdown_outside_gil(void* args_void) +{ + struct shutdown_outside_gil_args *args = (struct shutdown_outside_gil_args *)args_void; + return INT2NUM( args->data->container->shutdown(args->data->container, args->timeout) ); +} + /* * call-seq: * container.shutdown(timeout = -1) @@ -1402,22 +1579,36 @@ container_set_config_path(VALUE self, VALUE rb_path) static VALUE container_shutdown(int argc, VALUE *argv, VALUE self) { - int ret, timeout; - struct container_data *data; + int ret; VALUE rb_timeout; + struct shutdown_outside_gil_args args; rb_scan_args(argc, argv, "01", &rb_timeout); - timeout = NIL_P(rb_timeout) ? -1 : NUM2INT(rb_timeout); - Data_Get_Struct(self, struct container_data, data); + Data_Get_Struct(self, struct container_data, args.data); - ret = data->container->shutdown(data->container, timeout); + args.timeout = NIL_P(rb_timeout) ? -1 : NUM2INT(rb_timeout); + + ret = NUM2INT(rb_thread_blocking_region(shutdown_outside_gil, &args, NULL, NULL)); if (!ret) rb_raise(Error, "unable to shutdown container"); return self; } +struct snapshot_outside_gil_args +{ + struct container_data *data; + char *path; +}; + +static VALUE +snapshot_outside_gil(void* args_void) +{ + struct snapshot_outside_gil_args *args = (struct snapshot_outside_gil_args *)args_void; + return INT2NUM( args->data->container->snapshot(args->data->container, args->path) ); +} + /* * call-seq: * container.snapshot(path = nil) @@ -1428,17 +1619,16 @@ static VALUE container_snapshot(int argc, VALUE *argv, VALUE self) { int ret; - char *path; char new_name[20]; - struct container_data *data; VALUE rb_path; + struct snapshot_outside_gil_args args; rb_scan_args(argc, argv, "01", &rb_path); - path = NIL_P(rb_path) ? NULL : StringValuePtr(rb_path); + args.path = NIL_P(rb_path) ? NULL : StringValuePtr(rb_path); - Data_Get_Struct(self, struct container_data, data); + Data_Get_Struct(self, struct container_data, args.data); - ret = data->container->snapshot(data->container, path); + ret = NUM2INT(rb_thread_blocking_region(snapshot_outside_gil, &args, NULL, NULL)); if (ret < 0) rb_raise(Error, "unable to snapshot container"); @@ -1449,6 +1639,19 @@ container_snapshot(int argc, VALUE *argv, VALUE self) return rb_str_new2(new_name); } +struct snapshot_destroy_outside_gil_args +{ + struct container_data *data; + char *name; +}; + +static VALUE +snapshot_destroy_outside_gil(void* args_void) +{ + struct snapshot_destroy_outside_gil_args *args = (struct snapshot_destroy_outside_gil_args *)args_void; + return INT2NUM( args->data->container->snapshot_destroy(args->data->container, args->name) ); +} + /* * call-seq: * container.snapshot_destroy(name) @@ -1459,20 +1662,32 @@ static VALUE container_snapshot_destroy(VALUE self, VALUE rb_name) { int ret; - char *name; - struct container_data *data; + struct snapshot_destroy_outside_gil_args args; - name = StringValuePtr(rb_name); + Data_Get_Struct(self, struct container_data, args.data); - Data_Get_Struct(self, struct container_data, data); + args.name = StringValuePtr(rb_name); - ret = data->container->snapshot_destroy(data->container, name); + ret = NUM2INT(rb_thread_blocking_region(snapshot_destroy_outside_gil, &args, NULL, NULL)); if (!ret) rb_raise(Error, "unable to destroy snapshot"); return self; } +struct snapshot_list_outside_gil_args +{ + struct container_data *data; + struct lxc_snapshot *snapshots; +}; + +static VALUE +snapshot_list_outside_gil(void* args_void) +{ + struct snapshot_list_outside_gil_args *args = (struct snapshot_list_outside_gil_args *)args_void; + return INT2NUM( args->data->container->snapshot_list(args->data->container, &args->snapshots) ); +} + /* * call-seq: * container.snapshot_list @@ -1483,30 +1698,43 @@ static VALUE container_snapshot_list(VALUE self) { int i, num_snapshots; - struct lxc_snapshot *snapshots; - struct container_data *data; VALUE rb_snapshots; + struct snapshot_list_outside_gil_args args; - Data_Get_Struct(self, struct container_data, data); + Data_Get_Struct(self, struct container_data, args.data); - num_snapshots = data->container->snapshot_list(data->container, &snapshots); + num_snapshots = NUM2INT(rb_thread_blocking_region(snapshot_list_outside_gil, &args, NULL, NULL)); if (num_snapshots < 0) rb_raise(Error, "unable to list snapshots"); rb_snapshots = rb_ary_new2(num_snapshots); for (i = 0; i < num_snapshots; i++) { VALUE attrs = rb_ary_new2(4); - rb_ary_store(attrs, 0, rb_str_new2(snapshots[i].name)); - rb_ary_store(attrs, 1, rb_str_new2(snapshots[i].comment_pathname)); - rb_ary_store(attrs, 2, rb_str_new2(snapshots[i].timestamp)); - rb_ary_store(attrs, 3, rb_str_new2(snapshots[i].lxcpath)); - snapshots[i].free(&snapshots[i]); + rb_ary_store(attrs, 0, rb_str_new2(args.snapshots[i].name)); + rb_ary_store(attrs, 1, rb_str_new2(args.snapshots[i].comment_pathname)); + rb_ary_store(attrs, 2, rb_str_new2(args.snapshots[i].timestamp)); + rb_ary_store(attrs, 3, rb_str_new2(args.snapshots[i].lxcpath)); + args.snapshots[i].free(&args.snapshots[i]); rb_ary_store(rb_snapshots, i, attrs); } return rb_snapshots; } +struct snapshot_restore_outside_gil_args +{ + struct container_data *data; + char *name; + char *new_name; +}; + +static VALUE +snapshot_restore_outside_gil(void* args_void) +{ + struct snapshot_restore_outside_gil_args *args = (struct snapshot_restore_outside_gil_args *)args_void; + return INT2NUM( args->data->container->snapshot_restore(args->data->container, args->name, args->new_name) ); +} + /* * call-seq: * container.snapshot_restore(name, new_name = nil) @@ -1517,23 +1745,42 @@ static VALUE container_snapshot_restore(int argc, VALUE *argv, VALUE self) { int ret; - char *name, *new_name; - struct container_data *data; + struct snapshot_restore_outside_gil_args args; VALUE rb_name, rb_new_name; rb_scan_args(argc, argv, "11", &rb_name, &rb_new_name); - name = StringValuePtr(rb_name); - new_name = NIL_P(rb_new_name) ? NULL : StringValuePtr(rb_new_name); + args.name = StringValuePtr(rb_name); + args.new_name = NIL_P(rb_new_name) ? NULL : StringValuePtr(rb_new_name); - Data_Get_Struct(self, struct container_data, data); + Data_Get_Struct(self, struct container_data, args.data); - ret = data->container->snapshot_restore(data->container, name, new_name); + ret = NUM2INT(rb_thread_blocking_region(snapshot_restore_outside_gil, &args, NULL, NULL)); if (!ret) rb_raise(Error, "unable to restore snapshot"); return self; } + +struct start_outside_gil_args +{ + struct container_data *data; + int use_init; + int daemonize; + int close_fds; + char **args; +}; + +static VALUE +start_outside_gil(void* args_void) +{ + struct start_outside_gil_args *args = (struct start_outside_gil_args *)args_void; + struct lxc_container *container = args->data->container; + container->want_close_all_fds(container, args->close_fds); + container->want_daemonize(container, args->daemonize); + return INT2NUM( container->start(container, args->use_init, args->args) ); +} + /* * call-seq: * container.start(opts = {}) @@ -1548,41 +1795,38 @@ container_snapshot_restore(int argc, VALUE *argv, VALUE self) static VALUE container_start(int argc, VALUE *argv, VALUE self) { - int ret, use_init, daemonize, close_fds; - char **args; - struct container_data *data; + int ret; VALUE rb_use_init, rb_daemonize, rb_close_fds, rb_args, rb_opts; + struct start_outside_gil_args args; - use_init = 0; - daemonize = 1; - close_fds = 0; - args = NULL; + args.use_init = 0; + args.daemonize = 1; + args.close_fds = 0; + args.args = NULL; rb_args = Qnil; rb_scan_args(argc, argv, "01", &rb_opts); if (!NIL_P(rb_opts)) { Check_Type(rb_opts, T_HASH); rb_use_init = rb_hash_aref(rb_opts, SYMBOL("use_init")); - use_init = (rb_use_init != Qnil) && (rb_use_init != Qfalse); + args.use_init = (rb_use_init != Qnil) && (rb_use_init != Qfalse); rb_daemonize = rb_hash_aref(rb_opts, SYMBOL("daemonize")); - daemonize = (rb_daemonize != Qnil) && (rb_daemonize != Qfalse); + args.daemonize = (rb_daemonize != Qnil) && (rb_daemonize != Qfalse); rb_close_fds = rb_hash_aref(rb_opts, SYMBOL("close_fds")); - close_fds = (rb_close_fds != Qnil) && (rb_close_fds != Qfalse); + args.close_fds = (rb_close_fds != Qnil) && (rb_close_fds != Qfalse); rb_args = rb_hash_aref(rb_opts, SYMBOL("args")); - args = NIL_P(rb_args) ? NULL : ruby_to_c_string_array(rb_args); + args.args = NIL_P(rb_args) ? NULL : ruby_to_c_string_array(rb_args); } - Data_Get_Struct(self, struct container_data, data); + Data_Get_Struct(self, struct container_data, args.data); - data->container->want_close_all_fds(data->container, close_fds); - data->container->want_daemonize(data->container, daemonize); - ret = data->container->start(data->container, use_init, args); + ret = NUM2INT(rb_thread_blocking_region(start_outside_gil, &args, NULL, NULL)); if (!NIL_P(rb_args)) - free_c_string_array(args); + free_c_string_array(args.args); if (!ret) rb_raise(Error, "unable to start container"); @@ -1590,6 +1834,13 @@ container_start(int argc, VALUE *argv, VALUE self) return self; } +static VALUE +stop_outside_gil(void* data_void) +{ + struct container_data *data = (struct container_data *)data_void; + return INT2NUM( data->container->stop(data->container) ); +} + /* * call-seq: * container.stop @@ -1604,13 +1855,20 @@ container_stop(VALUE self) Data_Get_Struct(self, struct container_data, data); - ret = data->container->stop(data->container); + ret = NUM2INT(rb_thread_blocking_region(stop_outside_gil, data, NULL, NULL)); if (!ret) rb_raise(Error, "unable to stop container"); return self; } +static VALUE +unfreeze_outside_gil(void* data_void) +{ + struct container_data *data = (struct container_data *)data_void; + return INT2NUM( data->container->unfreeze(data->container) ); +} + /* * call-seq: * container.unfreeze @@ -1625,13 +1883,27 @@ container_unfreeze(VALUE self) Data_Get_Struct(self, struct container_data, data); - ret = data->container->unfreeze(data->container); + ret = NUM2INT(rb_thread_blocking_region(unfreeze_outside_gil, data, NULL, NULL)); if (!ret) - rb_raise(Error, "unable to unfreeze container"); + rb_raise(Error, "unable to unfreeze container: #{lxc_strerror(ret)}"); return self; } +struct wait_outside_gil_args +{ + struct container_data *data; + int timeout; + char *state; +}; + +static VALUE +wait_outside_gil(void* args_void) +{ + struct wait_outside_gil_args *args = (struct wait_outside_gil_args *)args_void; + return INT2NUM( args->data->container->wait(args->data->container, args->state, args->timeout) ); +} + /* * call-seq: * container.wait(state, timeout = -1) @@ -1642,22 +1914,21 @@ container_unfreeze(VALUE self) static VALUE container_wait(int argc, VALUE *argv, VALUE self) { - int ret, timeout; - char *state; - struct container_data *data; + int ret; VALUE rb_state_str, rb_state, rb_timeout; + struct wait_outside_gil_args args; rb_scan_args(argc, argv, "11", &rb_state, &rb_timeout); rb_state_str = rb_funcall(rb_state, rb_intern("to_s"), 0); rb_state_str = rb_funcall(rb_state_str, rb_intern("upcase"), 0); - state = StringValuePtr(rb_state_str); + args.state = StringValuePtr(rb_state_str); - timeout = NIL_P(rb_timeout) ? -1 : NUM2INT(rb_timeout); + args.timeout = NIL_P(rb_timeout) ? -1 : NUM2INT(rb_timeout); - Data_Get_Struct(self, struct container_data, data); + Data_Get_Struct(self, struct container_data, args.data); - ret = data->container->wait(data->container, state, timeout); + ret = NUM2INT( rb_thread_blocking_region(wait_outside_gil, &args, NULL, NULL) ); if (!ret) rb_raise(Error, "error waiting for container"); @@ -1713,7 +1984,7 @@ Init_lxc(void) rb_define_method(Container, "load_config", container_load_config, -1); rb_define_method(Container, "reboot", container_reboot, 0); rb_define_method(Container, "remove_device_node", - container_remove_device_node, 0); + container_remove_device_node, -1); rb_define_method(Container, "rename", container_rename, 1); rb_define_method(Container, "running_config_item", container_running_config_item, 1); diff --git a/test/test_helpers.rb b/test/test_helpers.rb new file mode 100644 index 0000000..829145a --- /dev/null +++ b/test/test_helpers.rb @@ -0,0 +1,27 @@ +require 'lxc' +require 'test/unit/assertions' + +module TestHelpers + def assert_long_running_function_does_not_block_ruby(&block) + r, w = IO.pipe + begin + # Write something after a very short period, but only if Ruby isn't blocked! + t = Thread.new do + sleep(0.001) + # Sleep twice so that if the function is blocking and we wake up just + # after block.call(c) somehow, we will go back to sleep briefly and + # allow B to be written + sleep(0.001) + w.write('A') + end + # Call the function and see if Ruby gets blocked + block.call + w.write('B') + chars = r.read(2) + assert(chars == 'AB', "Expected thread to write before block finished, but it did not. Expected 'AB', got '#{chars}'") + ensure + r.close + w.close + end + end +end diff --git a/test/test_lxc_create_asynchronous.rb b/test/test_lxc_create_asynchronous.rb new file mode 100644 index 0000000..aacefca --- /dev/null +++ b/test/test_lxc_create_asynchronous.rb @@ -0,0 +1,25 @@ +$:.unshift File.expand_path(File.join(File.dirname(__FILE__), 'lib')) + +require 'test/unit' +require 'lxc' +require 'test_helpers' + +class TestLXCCreateAsynchronous < Test::Unit::TestCase + include TestHelpers + + def setup + if Process::Sys::geteuid != 0 + raise 'This test must be ran as root' + end + @name = 'test_async_create' + container = LXC::Container.new(@name) + container.destroy if container.defined? + end + + def test_create_allows_ruby_to_continue + c = LXC::Container.new(@name) + assert_long_running_function_does_not_block_ruby do + c.create('ubuntu') + end + end +end diff --git a/test/test_lxc_created.rb b/test/test_lxc_created.rb index e48beb8..9bdee84 100644 --- a/test/test_lxc_created.rb +++ b/test/test_lxc_created.rb @@ -6,11 +6,15 @@ require 'lxc' class TestLXCCreated < Test::Unit::TestCase def setup if Process::Sys::geteuid != 0 - raise 'This test must be ran as root' + raise 'This test must be run as root' end @name = 'test' @container = LXC::Container.new(@name) @container.create('ubuntu') unless @container.defined? + # Make sure the renamed_test container does not exist, for the rename test + @new_name = "renamed_#{@name}" + new_container = LXC::Container.new(@new_name) + new_container.destroy if new_container.defined? end def test_container_defined @@ -41,9 +45,8 @@ class TestLXCCreated < Test::Unit::TestCase end def test_container_rename - new_name = "renamed_#{@name}" - renamed = @container.rename(new_name) - assert_equal(new_name, renamed.name) + renamed = @container.rename(@new_name) + assert_equal(@new_name, renamed.name) rerenamed = renamed.rename(@name) assert_equal(@name, rerenamed.name) end diff --git a/test/test_lxc_long_running.rb b/test/test_lxc_long_running.rb new file mode 100644 index 0000000..74c777d --- /dev/null +++ b/test/test_lxc_long_running.rb @@ -0,0 +1,228 @@ +$:.unshift File.expand_path(File.join(File.dirname(__FILE__), 'lib')) + +require 'test/unit' +require 'lxc' +require 'test_helpers' + +class TestLXCLongRunning < Test::Unit::TestCase + include TestHelpers + + def setup + if Process::Sys::geteuid != 0 + raise 'This test must be run as root' + end + @name = 'test' + @container = LXC::Container.new(@name) + @container.create('ubuntu') unless @container.defined? + @container.start unless @container.running? + @container.unfreeze + # Destroy leftover snapshots so we don't take up the whole disk + @container.snapshot_list.each do |snapshot, commentfile, timestamp, snapshotfile| + @container.snapshot_destroy(snapshot) + end + end + +# TODO find a way to test unblockness for functions that don't run quite so long +# def test_add_device_node_allows_ruby_to_continue +# assert_long_running_function_does_not_block_ruby do +# @container.add_device_node('/dev/ttyS0') +# end +# end + + def test_attach_wait_true_allows_ruby_to_continue + assert_long_running_function_does_not_block_ruby do + @container.attach(wait: true) do + sleep(2) + end + end + end + + def test_clone_allows_ruby_to_continue + # Ensure the "cloned" VM does not exist + cloned_name = "#{@name}_cloned" + cloned_to_destroy = LXC::Container.new(cloned_name) + cloned_to_destroy.destroy if cloned_to_destroy.defined? + + cloned = nil + @container.stop + begin + assert_long_running_function_does_not_block_ruby do + cloned = @container.clone(cloned_name) + end + ensure + @container.start + end + # Verify that it did something + assert(@container.name == @name, "original container name has changed from '#{@name}' to '#{@container.name}''") + assert(@container.running?, "original container is not running") + assert(cloned.name == cloned_name, "new container name should be '#{cloned_name}', is '#{cloned.name}'") + assert(cloned.init_pid != @container.init_pid, "cloned container init pid is the same as the original! Something ain't right.") + end + +# TODO Unsure how to test this without attaching to the actual tty! Must be a way to obtain a pipe or false tty of some sort. Manual run reveals it works, however. +# def test_console_allows_ruby_to_continue +# assert_long_running_function_does_not_block_ruby do +# @container.console +# end +# end + + def test_destroy_allows_ruby_to_continue + container = LXC::Container.new("test_destroy") + container.create('ubuntu') if !container.defined? + assert_long_running_function_does_not_block_ruby do + container.destroy + end + assert(!container.defined?, "Destroy did not destroy the container!") + end + +# TODO find a way to test unblockness for functions that don't run quite so long +# def test_freeze_allows_ruby_to_continue +# begin +# assert_long_running_function_does_not_block_ruby do +# @container.freeze +# end +# ensure +# @container.unfreeze +# end +# end + +# TODO find a way to test unblockness for functions that don't run quite so long +# def test_load_config_allows_ruby_to_continue +# File.open('/tmp/blah', 'w') do |file| +# file.write("\n\n\n\n") +# end +# begin +# container = LXC::Container.new('blahdeblah') +# assert_long_running_function_does_not_block_ruby do +# container.load_config('/tmp/blah') +# end +# ensure +# File.delete('/tmp/blah') +# end +# end + +# TODO find a way to test unblockness for functions that don't run quite so long +# def test_reboot_allows_ruby_to_continue +# assert_long_running_function_does_not_block_ruby do +# @container.reboot +# end +# end + +# TODO find a way to test unblockness for functions that don't run quite so long +# def test_remove_device_node_allows_ruby_to_continue +# assert_long_running_function_does_not_block_ruby do +# @container.remove_device_node('/dev/ttyS0') +# end +# end + +# TODO find a way to test unblockness for functions that don't run quite so long +# def test_save_config_allows_ruby_to_continue +# File.unlink('/tmp/blah') if File.exist?('/tmp/blah') +# assert_long_running_function_does_not_block_ruby do +# @container.save_config('/tmp/blah') +# end +# assert(File.exist?('/tmp/blah'), "save_config did not save /tmp/blah!") +# end + + def test_shutdown_allows_ruby_to_continue + begin + assert_long_running_function_does_not_block_ruby do + @container.shutdown + end + ensure + @container.start if !@container.running? + end + end + + def test_snapshot_allows_ruby_to_continue + @container.stop + begin + assert_long_running_function_does_not_block_ruby do + @container.snapshot + end + ensure + @container.start + end + end + + def test_snapshot_destroy_allows_ruby_to_continue + @container.stop + begin + snapshot = @container.snapshot + assert_long_running_function_does_not_block_ruby do + @container.snapshot_destroy(snapshot) + end + ensure + @container.start + end + end + +# TODO find a way to test unblockness for functions that don't run quite so long +# def test_snapshot_list_allows_ruby_to_continue +# @container.stop +# begin +# snapshot = @container.snapshot +# assert_long_running_function_does_not_block_ruby do +# assert(@container.snapshot_list > 0, "Snapshot list was empty!" +# end +# ensure +# @container.start +# end +# end + + def test_snapshot_restore_allows_ruby_to_continue + @container.stop + begin + snapshot = @container.snapshot + assert_long_running_function_does_not_block_ruby do + @container.snapshot_restore(snapshot) + end + ensure + @container.start + end + end + + def test_start_allows_ruby_to_continue + @container.stop + assert_long_running_function_does_not_block_ruby do + @container.start + end + assert(@container.running?, "Start did not start container! State = #{@container.state}") + end + +# TODO find a way to test unblockness for functions that don't run quite so long +# def test_stop_allows_ruby_to_continue +# begin +# assert_long_running_function_does_not_block_ruby do +# @container.stop +# end +# assert(!@container.running?, "Stop did not stop container! State = #{@container.state}") +# ensure +# @container.start if !@container.running? +# end +# end + +# TODO find a way to test unblockness for functions that don't run quite so long +# def test_unfreeze_allows_ruby_to_continue +# @container.freeze +# assert_long_running_function_does_not_block_ruby do +# @container.unfreeze +# end +# end + + def test_wait_allows_ruby_to_continue + t = Thread.new do + sleep(0.5) + @container.stop + end + begin + assert_long_running_function_does_not_block_ruby do + @container.wait(:stopped, 2) + end + assert(@container.state == :stopped, "Container never stopped! State is now #{@container.state.inspect}") + ensure + t.join + @container.start if !@container.running? + end + end +end