[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[Libguestfs] [PATCH 1/2] generator: Add symbol versioning.



From: "Richard W.M. Jones" <rjones redhat com>

---
 generator/actions.ml |  1 +
 generator/c.ml       | 96 ++++++++++++++++++++++++++++++++++++++++++++++------
 generator/checks.ml  | 10 ++++++
 generator/types.ml   |  2 ++
 src/Makefile.am      |  1 +
 src/compat.c         | 25 ++++++++++++++
 src/guestfs.pod      | 26 ++++++++++++++
 7 files changed, 151 insertions(+), 10 deletions(-)
 create mode 100644 src/compat.c

diff --git a/generator/actions.ml b/generator/actions.ml
index 71aee37..e1db3db 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -32,6 +32,7 @@ let defaults = { name = ""; style = RErr, [], []; proc_nr = None;
                  progress = false; camel_name = "";
                  cancellable = false; config_only = false;
                  once_had_no_optargs = false; blocking = true;
+                 symbol_version = None;
                  c_name = ""; c_function = ""; c_optarg_prefix = "";
                  non_c_aliases = [] }
 
diff --git a/generator/c.ml b/generator/c.ml
index ac8fd5e..68f5f46 100644
--- a/generator/c.ml
+++ b/generator/c.ml
@@ -1069,14 +1069,36 @@ trace_send_line (guestfs_h *g)
   (* 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 } =
+                                    config_only = config_only;
+                                    symbol_version = symbol_version } =
+    (* GCC symver won't let us just use the external name of the
+     * function if the function has multiple versions.  Instead we
+     * have to give the function a unique, non-exported name
+     * ('latest_guestfs_...').
+     *)
+    let prefix = match symbol_version with
+      | None -> "guestfs_"
+      | Some _ -> "latest_guestfs_" in
+    if prefix <> "guestfs_" then (
+      if optargs = [] then
+        generate_prototype ~extern:true ~single_line:true ~newline:true
+          ~handle:"g" ~prefix
+          c_name style
+      else
+        generate_prototype ~extern:true ~single_line:true ~newline:true
+          ~handle:"g" ~prefix
+          ~suffix:"_argv" ~optarg_proto:Argv
+          c_name style
+    );
+
     if optargs = [] then
       generate_prototype ~extern:false ~semicolon:false ~newline:true
-        ~handle:"g" ~prefix:"guestfs_"
+        ~handle:"g" ~prefix
         c_name style
     else
       generate_prototype ~extern:false ~semicolon:false ~newline:true
-        ~handle:"g" ~prefix:"guestfs_" ~suffix:"_argv" ~optarg_proto:Argv
+        ~handle:"g" ~prefix
+        ~suffix:"_argv" ~optarg_proto:Argv
         c_name style;
     pr "{\n";
 
@@ -1140,20 +1162,43 @@ trace_send_line (guestfs_h *g)
 
   (* Client-side stubs for each function. *)
   let generate_daemon_stub { name = name; c_name = c_name;
-                             style = ret, args, optargs as style } =
+                             style = ret, args, optargs as style;
+                             symbol_version = symbol_version } =
     let errcode =
       match errcode_of_ret ret with
       | `CannotReturnError -> assert false
       | (`ErrorIsMinusOne | `ErrorIsNULL) as e -> e in
 
+    (* GCC symver won't let us just use the external name of the
+     * function if the function has multiple versions.  Instead we
+     * have to give the function a unique, non-exported name
+     * ('latest_guestfs_...').
+     *)
+    let prefix = match symbol_version with
+      | None -> "guestfs_"
+      | Some _ -> "latest_guestfs_" in
+    if prefix <> "guestfs_" then (
+      if optargs = [] then
+        generate_prototype ~extern:true ~single_line:true ~newline:true
+          ~handle:"g" ~prefix
+          c_name style
+      else
+        generate_prototype ~extern:true ~single_line:true ~newline:true
+          ~handle:"g" ~prefix
+          ~suffix:"_argv" ~optarg_proto:Argv
+          c_name style
+    );
+
     (* Generate the action stub. *)
     if optargs = [] then
       generate_prototype ~extern:false ~semicolon:false ~newline:true
-        ~handle:"g" ~prefix:"guestfs_" c_name style
+        ~handle:"g" ~prefix
+        c_name style
     else
       generate_prototype ~extern:false ~semicolon:false ~newline:true
-        ~handle:"g" ~prefix:"guestfs_" ~suffix:"_argv"
-        ~optarg_proto:Argv c_name style;
+        ~handle:"g" ~prefix
+        ~suffix:"_argv" ~optarg_proto:Argv
+        c_name style;
 
     pr "{\n";
 
@@ -1578,7 +1623,12 @@ trace_send_line (guestfs_h *g)
     | ({ style = _, _, (_::_); once_had_no_optargs = true } as f) ->
       generate_va_variants f;
       generate_back_compat_wrapper f
-  ) all_functions_sorted
+  ) all_functions_sorted;
+
+  pr "/* Include backwards compatibility code for old ABIs. */\n";
+  pr "#include \"compat.c\"\n";
+  pr "\n";
+  pr "/* EOF */\n"
 
 (* Generate the linker script which controls the visibility of
  * symbols in the public ABI and ensures no other symbols get
@@ -1646,14 +1696,40 @@ and generate_linker_script () =
     ) in
   let globals = List.sort compare (globals @ functions @ structs) in
 
-  pr "{\n";
+  pr "GUESTFS_0.0 {\n";
   pr "    global:\n";
   List.iter (pr "        %s;\n") globals;
   pr "\n";
 
   pr "    local:\n";
   pr "        *;\n";
-  pr "};\n"
+  pr "};\n";
+
+  (* Explicitly versioned symbols. *)
+  let h = Hashtbl.create 13 in
+  List.iter (
+    function
+    | { name = name; symbol_version = Some ver } ->
+      let names = try Hashtbl.find h ver with Not_found -> [] in
+      Hashtbl.replace h ver (name :: names)
+    | { symbol_version = None } -> ()
+  ) all_functions;
+  let vers = Hashtbl.fold (fun k _ ks -> k :: ks) h [] in
+  let vers = List.sort compare vers in
+  let prev = ref "GUESTFS_0.0" in
+  List.iter (
+    fun ver ->
+      let names = Hashtbl.find h ver in
+      let names = List.sort compare names in
+
+      pr "\n";
+      pr "%s {\n" ver;
+      pr "    global:\n";
+      List.iter (pr "        guestfs_%s;\n") names;
+      pr "} %s;\n" !prev;
+
+      prev := ver
+  ) vers
 
 and generate_max_proc_nr () =
   pr "%d\n" max_proc_nr
diff --git a/generator/checks.ml b/generator/checks.ml
index 2cccf26..f813fbf 100644
--- a/generator/checks.ml
+++ b/generator/checks.ml
@@ -243,6 +243,16 @@ let () =
     | { blocking = true } -> ()
   ) daemon_functions;
 
+  (* Check symbol version string is sane. *)
+  List.iter (
+    function
+    | { name = name; symbol_version = Some ver } ->
+      let len = String.length ver in
+      if len < 12 || String.sub ver 0 10 <> "GUESTFS_1." then
+        failwithf "%s: invalid symbol_version (%s)" name ver
+    | { symbol_version = None } -> ()
+  ) all_functions;
+
   (* Non-fish functions must have correct camel_name. *)
   List.iter (
     fun { name = name; camel_name = camel_name } ->
diff --git a/generator/types.ml b/generator/types.ml
index cff3c6d..df019e8 100644
--- a/generator/types.ml
+++ b/generator/types.ml
@@ -399,6 +399,8 @@ type action = {
                                      set flags in the handle are marked
                                      non-blocking so that we don't add
                                      machinery in various bindings. *)
+  symbol_version : string option; (* C symbol version.
+                                     See guestfs(3)/SYMBOL VERSIONING *)
 
   (* "Internal" data attached by the generator at various stages.  This
    * doesn't need to (and shouldn't) be set when defining actions.
diff --git a/src/Makefile.am b/src/Makefile.am
index 5f80bc1..ce2398f 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -45,6 +45,7 @@ EXTRA_DIST = \
 	libguestfs.3 \
 	libguestfs.pc libguestfs.pc.in \
 	guestfs.pod \
+	compat.c \
 	api-support/added \
 	api-support/README \
 	api-support/update-from-tarballs.sh
diff --git a/src/compat.c b/src/compat.c
new file mode 100644
index 0000000..119eb7c
--- /dev/null
+++ b/src/compat.c
@@ -0,0 +1,25 @@
+/* libguestfs
+ * Copyright (C) 2012 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/* This is used for symbol versioning.  See guestfs(3)/SYMBOL VERSIONING.
+ *
+ * XXX Symbol versioning only seems to work if all the related
+ * functions are compiled into a single file, so this file is
+ * #included directly into src/actions.c, instead of being compiled as
+ * a separate unit.
+ */
diff --git a/src/guestfs.pod b/src/guestfs.pod
index 6b119dc..8e3baea 100644
--- a/src/guestfs.pod
+++ b/src/guestfs.pod
@@ -3481,6 +3481,32 @@ into the appliance.
 Debugging messages are never translated, since they are intended for
 the programmers.
 
+=head2 SYMBOL VERSIONING
+
+The generator supports symbol versioning.  This is used
+B<as a last resort only> when we need to modify an API and we
+cannot possibly make the change ABI compatible.  Using symbol
+versioning allows us to get older applications to transparently use a
+compatibility function (preserving ABI) while newly compiled
+applications get the new API.
+
+First, familiarize yourself with symbol versioning by reading the
+relevant sections of the GNU ld documentation and this document by
+Ulrich Drepper: L<http://www.akkadia.org/drepper/dsohowto.pdf>
+
+The modified API should have a C<symbol_version> added.  This has the
+form C<GUESTFS_1.X> where C<1.X> is the first stable version of
+libguestfs where the new, incompatible API will appear.
+
+Next edit C<src/compat.c> and add the relevant C<.symver> directives
+so that old libraries call a translation function that is backwards
+compatible with the old ABI, and to make the new symbol the default
+for newly compiled code.  There are examples in this file.
+
+Finally check that old binaries do not crash and still return the same
+data.  It's a good idea to add a C<debug> call to the translation
+function so you can be sure it is being called.
+
 =head2 SOURCE CODE SUBDIRECTORIES
 
 =over 4
-- 
1.7.11.4


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]