[Libguestfs] [PATCH] WIP locking/tracing layer split

--- generator/c.ml | 165 +++++++++++++++++++++++++++++++++++++++++++------ src/fuse.c | 14 +++++ src/guestfs-internal.h | 3 + src/handle.c | 29 +++++---- src/inspect-fs.c | 10 +-- src/inspect-icon.c | 7 +++ src/tmpdirs.c | 2 +- 7 files changed, 193 insertions(+), 37 deletions(-) diff --git a/generator/c.ml b/generator/c.ml index ee276dc..113b582 100644 --- a/generator/c.ml +++ b/generator/c.ml @@ -66,6 +66,7 @@ let rec generate_prototype ?(extern = true) ?(static = false) ?(prefix = "") ?(suffix = "") ?handle ?(optarg_proto = Dots) + ?(optarg_name = "") name (ret, args, optargs) = pr "%s" indent; if extern then pr "extern "; @@ -156,7 +157,10 @@ let rec generate_prototype ?(extern = true) ?(static = false) match optarg_proto with | Dots -> pr "..." | VA -> pr "va_list args" - | Argv -> pr "const struct guestfs_%s_argv *optargs" name + | Argv -> pr "const struct guestfs_%s_argv *optargs" + (match optarg_name with + | "" -> name + | _ -> optarg_name) ); (* Was anything output between ()? If not, it's a 'function(void)'. *) @@ -671,13 +675,22 @@ extern GUESTFS_DLL_PUBLIC void *guestfs_next_private (guestfs_h *g, const char * | Some fn -> pr "\n GUESTFS_DEPRECATED_BY (%S);\n" fn | None -> pr ";\n" ); +(* generate_prototype ~single_line:true ~semicolon:false + ~handle:"g" ~prefix:"guestfs__t_" shortname style; + (match deprecated_by with + | Some fn -> pr "\n GUESTFS_DEPRECATED_BY (%S);\n" fn + | None -> pr ";\n" + ); *) if optargs <> [] then ( generate_prototype ~single_line:true ~newline:true ~handle:"g" ~prefix:"guestfs_" ~suffix:"_va" ~optarg_proto:VA ~dll_public:true shortname style; - +(* generate_prototype ~single_line:true ~newline:true ~handle:"g" + ~prefix:"guestfs__t_" ~suffix:"_va" ~optarg_proto:VA + shortname style; + *) pr "\n"; pr "struct guestfs_%s_argv {\n" shortname; pr " uint64_t bitmask;\n"; @@ -703,6 +716,9 @@ extern GUESTFS_DLL_PUBLIC void *guestfs_next_private (guestfs_h *g, const char * ~prefix:"guestfs_" ~suffix:"_argv" ~optarg_proto:Argv ~dll_public:true shortname style; +(* generate_prototype ~single_line:true ~newline:true ~handle:"g" + ~prefix:"guestfs__t_" ~suffix:"_argv" ~optarg_proto:Argv + shortname style; *) ); pr "\n" @@ -791,7 +807,10 @@ and generate_internal_actions_h () = List.iter ( fun { c_name = c_name; style = style } -> generate_prototype ~single_line:true ~newline:true ~handle:"g" - ~prefix:"guestfs__" ~optarg_proto:Argv + ~prefix:"guestfs__t_" ~optarg_proto:Dots + c_name style; + generate_prototype ~single_line:true ~newline:true ~handle:"g" + ~prefix:"guestfs__" ~optarg_proto:Dots c_name style ) non_daemon_functions; @@ -1551,20 +1570,19 @@ and generate_client_actions hash () = ) in - (* For non-daemon functions, generate a wrapper around each function. *) - let generate_non_daemon_wrapper { name = name; c_name = c_name; + let generate_tracing_wrapper { name = name; c_name = c_name; style = ret, _, optargs as style; config_only = config_only } = if optargs = [] then generate_prototype ~extern:false ~semicolon:false ~newline:true ~handle:"g" ~prefix:"guestfs_" - ~dll_public:true - c_name style +(* ~optarg_name:c_name *) + ("_t_" ^ c_name) style else generate_prototype ~extern:false ~semicolon:false ~newline:true ~handle:"g" ~prefix:"guestfs_" ~suffix:"_argv" ~optarg_proto:Argv - ~dll_public:true - c_name style; + ~optarg_name:c_name + ("_t_" ^ c_name) style; pr "{\n"; handle_null_optargs optargs c_name; @@ -1617,11 +1635,101 @@ and generate_client_actions hash () = trace_return name style "r"; ); pr "\n"; + pr " return r;\n"; pr "}\n"; pr "\n" in + (* For non-daemon functions, generate a wrapper around each function. *) + let generate_non_daemon_wrapper ({ name = name; c_name = c_name; + style = ret, _, optargs as style; + config_only = config_only } as arg) = + (* Add tracing, but non-locking layer below public api, we'll use it internally. *) + (* generate_tracing_wrapper { arg with name = ("_t_" ^ c_name) }; *) + generate_tracing_wrapper { arg with name = ("_t_" ^ name) }; + pr "\n"; + + if optargs = [] then + generate_prototype ~extern:false ~semicolon:false ~newline:true + ~handle:"g" ~prefix:"guestfs_" + ~dll_public:true + c_name style + else ( + generate_prototype ~extern:false ~semicolon:false ~newline:true + ~handle:"g" ~prefix:"guestfs_" ~suffix:"_argv" ~optarg_proto:Argv + ~dll_public:true + c_name style + ); + + pr "{\n"; + + handle_null_optargs optargs c_name; + + if optargs = [] then + pr " printf(\"locking in %s\\n\");\n" c_name + else + pr " printf(\"locking in %s_argv\\n\");\n" c_name; + +(* pr " guestfs___per_handle_lock_lock (g);\n"; *) + pr " gl_lock_lock (g->global_lock);\n"; + pr "\n"; + + (match ret with + | RErr | RInt _ | RBool _ -> + pr " int r;\n" + | RInt64 _ -> + pr " int64_t r;\n" + | RConstString _ -> + pr " const char *r;\n" + | RConstOptString _ -> + pr " const char *r;\n" + | RString _ | RBufferOut _ -> + pr " char *r;\n" + | RStringList _ | RHashtable _ -> + pr " char **r;\n" + | RStruct (_, typ) -> + pr " struct guestfs_%s *r;\n" typ + | RStructList (_, typ) -> + pr " struct guestfs_%s_list *r;\n" typ + ); + pr "\n"; + if config_only then ( + pr " if (g->state != CONFIG) {\n"; + pr " error (g, \"%%s: this function can only be called in the config state\",\n"; + pr " \"%s\");\n" c_name; + pr " return -1;\n"; + pr " }\n"; + ); + if optargs = [] then + pr " r = guestfs__t_%s " c_name + else + pr " r = guestfs__t_%s " (c_name ^ "_argv"); + + generate_c_call_args ~handle:"g" ~implicit_size_ptr:"size_r" style; + pr ";\n"; + pr "\n"; + + (* pr " guestfs___per_handle_lock_unlock (g);\n"; *) + pr " gl_lock_unlock (g->global_lock);\n"; + if optargs = [] then + pr " printf(\"unlocked in %s\\n\");\n" c_name + else + pr " printf(\"unlocked in %s_argv\\n\");\n" c_name; + + pr "\n"; + + pr " return r;\n"; + pr "}\n"; + pr "\n" + in + +(* let generate_non_daemon_trace_wrapper x = + pr "WRAPPED!!1"; + generate_non_daemon_wrapper x; + pr "POST WRAP!" + in *) + List.iter ( function | { wrapper = true } as f -> @@ -1949,7 +2057,7 @@ and generate_client_actions_variants () = "; let generate_va_variants { name = name; c_name = c_name; - style = ret, args, optargs as style } = + style = ret, args, optargs as style } locking = assert (optargs <> []); (* checked by caller *) (* Get the name of the last regular argument. *) @@ -1979,15 +2087,23 @@ and generate_client_actions_variants () = sprintf "struct guestfs_%s_list *" typ in (* The regular variable args function, just calls the _va variant. *) - generate_prototype ~extern:false ~semicolon:false ~newline:true - ~handle:"g" ~prefix:"guestfs_" c_name style; + if locking then + generate_prototype ~extern:false ~semicolon:false ~newline:true + ~handle:"g" ~prefix:"guestfs_" c_name style + else + generate_prototype ~extern:false ~semicolon:false ~newline:true + ~handle:"g" ~prefix:"guestfs__t_" c_name style; + pr "{\n"; pr " va_list optargs;\n"; pr "\n"; pr " %sr;\n" rtype; pr "\n"; pr " va_start (optargs, %s);\n" last_arg; - pr " r = guestfs_%s_va " c_name; + if locking then + pr " r = guestfs__t_%s_va " c_name + else + pr " r = guestfs__%s_va " c_name; generate_c_call_args ~handle:"g" ~implicit_size_ptr:"size_r" style; pr ";\n"; pr " va_end (optargs);\n"; @@ -1995,9 +2111,15 @@ and generate_client_actions_variants () = pr " return r;\n"; pr "}\n\n"; - generate_prototype ~extern:false ~semicolon:false ~newline:true - ~handle:"g" ~prefix:"guestfs_" ~suffix:"_va" ~optarg_proto:VA - c_name style; + if locking then + generate_prototype ~extern:false ~semicolon:false ~newline:true + ~handle:"g" ~prefix:"guestfs_" ~suffix:"_va" ~optarg_proto:VA + c_name style + else + generate_prototype ~extern:false ~semicolon:false ~newline:true + ~handle:"g" ~prefix:"guestfs__t_" ~suffix:"_va" ~optarg_proto:VA + c_name style; + pr "{\n"; pr " struct guestfs_%s_argv optargs_s;\n" c_name; pr " struct guestfs_%s_argv *optargs = &optargs_s;\n" c_name; @@ -2045,7 +2167,10 @@ and generate_client_actions_variants () = pr " optargs_s.bitmask |= i_mask;\n"; pr " }\n"; pr "\n"; - pr " return guestfs_%s_argv " c_name; + if locking then + pr " return guestfs_%s_argv " c_name + else + pr " return guestfs__t_%s_argv " c_name; generate_c_call_args ~handle:"g" ~implicit_size_ptr:"size_r" style; pr ";\n"; pr "}\n\n" @@ -2070,9 +2195,11 @@ and generate_client_actions_variants () = function | { style = _, _, [] } -> () | ({ style = _, _, (_::_); once_had_no_optargs = false } as f) -> - generate_va_variants f + generate_va_variants f false; + generate_va_variants f true | ({ style = _, _, (_::_); once_had_no_optargs = true } as f) -> - generate_va_variants f; + generate_va_variants f false; + generate_va_variants f true; generate_back_compat_wrapper f ) all_functions_sorted diff --git a/src/fuse.c b/src/fuse.c index dd63729..020a865 100644 --- a/src/fuse.c +++ b/src/fuse.c @@ -1004,6 +1004,13 @@ guestfs__mount_local (guestfs_h *g, const char *localmountpoint, } int +guestfs__t_mount_local (guestfs_h *g, const char *localmountpoint, + const struct guestfs_mount_local_argv *optargs) +{ + return guestfs__mount_local (g, localmountpoint, optargs); +} + +int guestfs__mount_local_run (guestfs_h *g) { int r, mounted; @@ -1105,6 +1112,13 @@ guestfs__umount_local (guestfs_h *g, return -1; } +int +guestfs__t_umount_local (guestfs_h *g, + const struct guestfs_umount_local_argv *optargs) +{ + return guestfs__umount_local(g, optargs); +} + /* Functions handling the directory cache. * * Note on attribute caching: FUSE can cache filesystem attributes for diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index b99335b..9254d4a 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -32,6 +32,7 @@ #include <libvirt/libvirt.h> #endif +#include "glthread/lock.h" #include "hash.h" #include "guestfs-internal-frontend.h" @@ -493,6 +494,8 @@ struct guestfs_h unsigned int nr_requested_credentials; virConnectCredentialPtr requested_credentials; #endif + + gl_recursive_lock_t global_lock; }; /* Per-filesystem data stored for inspect_os. */ diff --git a/src/handle.c b/src/handle.c index 719bbcd..b68444d 100644 --- a/src/handle.c +++ b/src/handle.c @@ -84,6 +84,9 @@ guestfs_create_flags (unsigned flags, ...) g = calloc (1, sizeof (*g)); if (!g) return NULL; +// guestfs___per_handle_lock_add (g); + gl_lock_init (g->global_lock); + g->state = CONFIG; g->conn = NULL; @@ -167,6 +170,7 @@ guestfs_create_flags (unsigned flags, ...) free (g->path); free (g->hv); free (g->append); + gl_lock_destroy (g->global_lock); free (g); return NULL; } @@ -185,21 +189,21 @@ parse_environment (guestfs_h *g, str = do_getenv (data, "LIBGUESTFS_TRACE"); if (str != NULL && STREQ (str, "1")) - guestfs_set_trace (g, 1); + guestfs__t_set_trace (g, 1); str = do_getenv (data, "LIBGUESTFS_DEBUG"); if (str != NULL && STREQ (str, "1")) - guestfs_set_verbose (g, 1); + guestfs__t_set_verbose (g, 1); str = do_getenv (data, "LIBGUESTFS_TMPDIR"); if (str) { - if (guestfs_set_tmpdir (g, str) == -1) + if (guestfs__t_set_tmpdir (g, str) == -1) return -1; } str = do_getenv (data, "LIBGUESTFS_CACHEDIR"); if (str) { - if (guestfs_set_cachedir (g, str) == -1) + if (guestfs__t_set_cachedir (g, str) == -1) return -1; } @@ -209,20 +213,20 @@ parse_environment (guestfs_h *g, str = do_getenv (data, "LIBGUESTFS_PATH"); if (str) - guestfs_set_path (g, str); + guestfs__t_set_path (g, str); str = do_getenv (data, "LIBGUESTFS_HV"); if (str) - guestfs_set_hv (g, str); + guestfs__t_set_hv (g, str); else { str = do_getenv (data, "LIBGUESTFS_QEMU"); if (str) - guestfs_set_hv (g, str); + guestfs__t_set_hv (g, str); } str = do_getenv (data, "LIBGUESTFS_APPEND"); if (str) - guestfs_set_append (g, str); + guestfs__t_set_append (g, str); str = do_getenv (data, "LIBGUESTFS_MEMSIZE"); if (str) { @@ -230,18 +234,18 @@ parse_environment (guestfs_h *g, error (g, _("non-numeric or too small value for LIBGUESTFS_MEMSIZE")); return -1; } - guestfs_set_memsize (g, memsize); + guestfs__t_set_memsize (g, memsize); } str = do_getenv (data, "LIBGUESTFS_BACKEND"); if (str) { - if (guestfs_set_backend (g, str) == -1) + if (guestfs__t_set_backend (g, str) == -1) return -1; } else { str = do_getenv (data, "LIBGUESTFS_ATTACH_METHOD"); if (str) { - if (guestfs_set_backend (g, str) == -1) + if (guestfs__t_set_backend (g, str) == -1) return -1; } } @@ -255,7 +259,7 @@ parse_environment (guestfs_h *g, return -1; } - if (guestfs_set_backend_settings (g, settings) == -1) + if (guestfs__t_set_backend_settings (g, settings) == -1) return -1; } @@ -374,6 +378,7 @@ guestfs_close (guestfs_h *g) free (g->backend_data); guestfs___free_string_list (g->backend_settings); free (g->append); + gl_lock_destroy (g->global_lock); free (g); } diff --git a/src/inspect-fs.c b/src/inspect-fs.c index c011b5a..03fd0bf 100644 --- a/src/inspect-fs.c +++ b/src/inspect-fs.c @@ -149,12 +149,12 @@ guestfs___check_for_filesystem_on (guestfs_h *g, const char *mountable) guestfs_push_error_handler (g, NULL, NULL); if (vfs_type && STREQ (vfs_type, "ufs")) { /* Hack for the *BSDs. */ /* FreeBSD fs is a variant of ufs called ufs2 ... */ - r = guestfs_mount_vfs (g, "ro,ufstype=ufs2", "ufs", mountable, "/"); + r = guestfs__t_mount_vfs (g, "ro,ufstype=ufs2", "ufs", mountable, "/"); if (r == -1) /* while NetBSD and OpenBSD use another variant labeled 44bsd */ - r = guestfs_mount_vfs (g, "ro,ufstype=44bsd", "ufs", mountable, "/"); + r = guestfs__t_mount_vfs (g, "ro,ufstype=44bsd", "ufs", mountable, "/"); } else { - r = guestfs_mount_ro (g, mountable, "/"); + r = guestfs__t_mount_ro (g, mountable, "/"); } guestfs_pop_error_handler (g); if (r == -1) @@ -366,7 +366,7 @@ guestfs___is_file_nocase (guestfs_h *g, const char *path) p = guestfs___case_sensitive_path_silently (g, path); if (!p) return 0; - r = guestfs_is_file (g, p); + r = guestfs__t_is_file (g, p); return r > 0; } @@ -379,7 +379,7 @@ guestfs___is_dir_nocase (guestfs_h *g, const char *path) p = guestfs___case_sensitive_path_silently (g, path); if (!p) return 0; - r = guestfs_is_dir (g, p); + r = guestfs__t_is_dir (g, p); return r > 0; } diff --git a/src/inspect-icon.c b/src/inspect-icon.c index 94b63a2..f17149b 100644 --- a/src/inspect-icon.c +++ b/src/inspect-icon.c @@ -214,6 +214,13 @@ guestfs__inspect_get_icon (guestfs_h *g, const char *root, size_t *size_r, return r; } +char * +guestfs__t_inspect_get_icon (guestfs_h *g, const char *root, size_t *size_r, + const struct guestfs_inspect_get_icon_argv *optargs) +{ + return guestfs__inspect_get_icon(g, root, size_r, optargs); +} + /* Check that the named file 'filename' is a PNG file and is reasonable. * If it is, download and return it. */ diff --git a/src/tmpdirs.c b/src/tmpdirs.c index 56523aa..63d235e 100644 --- a/src/tmpdirs.c +++ b/src/tmpdirs.c @@ -129,7 +129,7 @@ int guestfs___lazy_make_tmpdir (guestfs_h *g) { if (!g->tmpdir) { - CLEANUP_FREE char *tmpdir = guestfs_get_tmpdir (g); + CLEANUP_FREE char *tmpdir = guestfs__t_get_tmpdir (g); g->tmpdir = safe_asprintf (g, "%s/libguestfsXXXXXX", tmpdir); if (mkdtemp (g->tmpdir) == NULL) { perrorf (g, _("%s: cannot create temporary directory"), g->tmpdir); -- 1.8.5.3