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

Maros Zatko mzatko at redhat.com
Fri Jun 27 14:55:34 UTC 2014


---
 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




More information about the Libguestfs mailing list