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

[Libguestfs] [PATCH v3 4/5] generator: Add symbol versioning.



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

---
 generator/actions.ml  |   1 +
 generator/c.ml        | 132 +++++++++++++++++++++++++++++++++++++++++++++-----
 generator/checks.ml   |  18 +++++++
 generator/structs.ml  |   4 +-
 generator/structs.mli |   2 +
 generator/types.ml    |   2 +
 po/POTFILES           |   1 +
 src/Makefile.am       |   4 ++
 src/compat.c          |  26 ++++++++++
 src/guestfs.pod       |  26 ++++++++++
 10 files changed, 204 insertions(+), 12 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 54f35b5..5e75be1 100644
--- a/generator/c.ml
+++ b/generator/c.ml
@@ -710,7 +710,8 @@ and generate_client_free_structs () =
   pr "\n";
 
   List.iter (
-    fun { s_name = typ } ->
+    function
+    | { s_name = typ; s_symbol_version = None } ->
       pr "GUESTFS_DLL_PUBLIC void\n";
       pr "guestfs_free_%s (struct guestfs_%s *x)\n" typ typ;
       pr "{\n";
@@ -727,7 +728,38 @@ and generate_client_free_structs () =
       pr "}\n";
       pr "\n";
 
-  ) structs
+    | { s_name = typ; s_symbol_version = Some v } ->
+      let suffix = "__" ^ replace_char v '.' '_' in
+
+      pr "extern GUESTFS_DLL_PUBLIC void guestfs_free_%s%s (struct guestfs_%s *x);\n"
+        typ suffix typ;
+      pr "\n";
+      pr "GUESTFS_DLL_PUBLIC void\n";
+      pr "guestfs_free_%s%s (struct guestfs_%s *x)\n" typ suffix typ;
+      pr "{\n";
+      pr "  xdr_free ((xdrproc_t) xdr_guestfs_int_%s, (char *) x);\n" typ;
+      pr "  free (x);\n";
+      pr "}\n";
+      pr "\n";
+
+      pr "extern GUESTFS_DLL_PUBLIC void guestfs_free_%s_list%s (struct guestfs_%s_list *x);\n"
+        typ suffix typ;
+      pr "\n";
+      pr "GUESTFS_DLL_PUBLIC void\n";
+      pr "guestfs_free_%s_list%s (struct guestfs_%s_list *x)\n" typ suffix typ;
+      pr "{\n";
+      pr "  xdr_free ((xdrproc_t) xdr_guestfs_int_%s_list, (char *) x);\n" typ;
+      pr "  free (x);\n";
+      pr "}\n";
+      pr "\n";
+
+  ) structs;
+
+  pr "/* Include backwards compatibility code for old ABIs. */\n";
+  pr "#define COMPAT_STRUCTS 1\n";
+  pr "#include \"compat.c\"\n";
+  pr "\n";
+  pr "/* EOF */\n"
 
 (* Generate the client-side dispatch stubs. *)
 and generate_client_actions () =
@@ -1108,18 +1140,53 @@ trace_send_line (guestfs_h *g)
     )
   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
+   * ('guestfs_...__<VERSION>').
+   *)
+  let suffix_for_symbol_versions c_name style = function
+    | None -> ""
+    | Some v ->
+      let suffix = "__" ^ replace_char v '.' '_' in
+
+      pr "/* This symbol has multiple versions, resolved by the linker\n";
+      pr " * at run time.  This is the latest version used for new code.\n";
+      pr " * See src/compat.c for old compatibility versions.\n";
+      pr " */\n";
+      let _, _, optargs = style in
+      if optargs = [] then
+        generate_prototype ~extern:true ~single_line:true ~newline:true
+          ~handle:"g" ~prefix:"guestfs_" ~suffix
+          ~dll_public:true
+          c_name style
+      else
+        generate_prototype ~extern:true ~single_line:true ~newline:true
+          ~handle:"g" ~prefix:"guestfs_" ~suffix:("_argv" ^ suffix)
+          ~optarg_proto:Argv
+          ~dll_public:true
+          c_name style;
+      pr "\n";
+
+      suffix
+  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 } =
+                                    config_only = config_only;
+                                    symbol_version = symbol_version } =
+    let suffix = suffix_for_symbol_versions c_name style symbol_version in
+
     if optargs = [] then
       generate_prototype ~extern:false ~semicolon:false ~newline:true
-        ~handle:"g" ~prefix:"guestfs_"
+        ~handle:"g" ~prefix:"guestfs_" ~suffix
         ~dll_public:true
         c_name style
     else
       generate_prototype ~extern:false ~semicolon:false ~newline:true
-        ~handle:"g" ~prefix:"guestfs_" ~suffix:"_argv" ~optarg_proto:Argv
+        ~handle:"g" ~prefix:"guestfs_" ~suffix:("_argv" ^ suffix)
+        ~optarg_proto:Argv
         ~dll_public:true
         c_name style;
     pr "{\n";
@@ -1184,21 +1251,24 @@ 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
 
+    let suffix = suffix_for_symbol_versions c_name style symbol_version in
+
     (* Generate the action stub. *)
     if optargs = [] then
       generate_prototype ~extern:false ~semicolon:false ~newline:true
-        ~handle:"g" ~prefix:"guestfs_"
+        ~handle:"g" ~prefix:"guestfs_" ~suffix
         ~dll_public:true
         c_name style
     else
       generate_prototype ~extern:false ~semicolon:false ~newline:true
-        ~handle:"g" ~prefix:"guestfs_" ~suffix:"_argv"
+        ~handle:"g" ~prefix:"guestfs_" ~suffix:("_argv" ^ suffix)
         ~optarg_proto:Argv
         ~dll_public:true
         c_name style;
@@ -1599,7 +1669,13 @@ 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 "#define COMPAT_ACTIONS 1\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
@@ -1667,14 +1743,48 @@ 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
+    | { symbol_version = None } -> ()
+    | { name = name; symbol_version = Some ver } ->
+      let names = try Hashtbl.find h ver with Not_found -> [] in
+      Hashtbl.replace h ver (name :: names)
+  ) all_functions;
+  List.iter (
+    function
+    | { s_symbol_version = None } -> ()
+    | { s_name = name; s_symbol_version = Some ver } ->
+      let names = try Hashtbl.find h ver with Not_found -> [] in
+      Hashtbl.replace h ver
+        (sprintf "free_%s" name :: sprintf "free_%s_list" name :: names)
+  ) Structs.structs;
+  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..9ecb2a3 100644
--- a/generator/checks.ml
+++ b/generator/checks.ml
@@ -20,6 +20,7 @@
 
 open Types
 open Utils
+open Structs
 open Actions
 
 (* Check function names etc. for consistency. *)
@@ -243,6 +244,23 @@ let () =
     | { blocking = true } -> ()
   ) daemon_functions;
 
+  (* Check symbol version string is sane. *)
+  let symbol_version_is_sane name = function
+    | None -> ()
+    | 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
+  in
+  List.iter (
+    fun { name = name; symbol_version = symbol_version } ->
+      symbol_version_is_sane name symbol_version
+  ) all_functions;
+  List.iter (
+    fun { s_name = name; s_symbol_version = symbol_version } ->
+      symbol_version_is_sane name symbol_version
+  ) structs;
+
   (* Non-fish functions must have correct camel_name. *)
   List.iter (
     fun { name = name; camel_name = camel_name } ->
diff --git a/generator/structs.ml b/generator/structs.ml
index 4dcb2c9..e378224 100644
--- a/generator/structs.ml
+++ b/generator/structs.ml
@@ -26,6 +26,7 @@ type struc = {
   s_name : string;
   s_cols : cols;
   s_camel_name : string;
+  s_symbol_version : string option;
   s_unused : unit; (* Silences warning 23 when using 'defaults with ...' *)
 }
 
@@ -94,7 +95,8 @@ let lvm_lv_cols = [
   "modules", FString;
 ]
 
-let defaults = { s_name = ""; s_cols = []; s_camel_name = ""; s_unused = () }
+let defaults = { s_name = ""; s_cols = []; s_camel_name = "";
+                 s_symbol_version = None; s_unused = () }
 
 (* Names and fields in all structures (in RStruct and RStructList)
  * that we support.
diff --git a/generator/structs.mli b/generator/structs.mli
index f0c2ba6..38bee6a 100644
--- a/generator/structs.mli
+++ b/generator/structs.mli
@@ -27,6 +27,8 @@ type struc = {
   s_name : string;                      (** Regular name. *)
   s_cols : cols;                        (** Columns. *)
   s_camel_name : string;                (** Camel-cased name. *)
+  s_symbol_version : string option;     (** C symbol version.
+                                            See guestfs(3)/SYMBOL VERSIONING *)
   s_unused : unit;
 }
 
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/po/POTFILES b/po/POTFILES
index 3f49a05..8cc71fe 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -219,6 +219,7 @@ src/actions.c
 src/appliance.c
 src/bindtests.c
 src/command.c
+src/compat.c
 src/dbdump.c
 src/errnostring-gperf.c
 src/errnostring.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 082c122..e6e6c83 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -46,6 +46,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
@@ -152,6 +153,9 @@ libguestfs_la_SOURCES = \
 	proto.c \
 	libguestfs.syms
 
+# These files #include "compat.c".  Express that dependency.
+actions.c free-structs.c: compat.c
+
 libguestfs_la_LIBADD = \
 	$(PCRE_LIBS) $(MAGIC_LIBS) \
 	$(LIBVIRT_LIBS) $(LIBXML2_LIBS) \
diff --git a/src/compat.c b/src/compat.c
new file mode 100644
index 0000000..27ffd05
--- /dev/null
+++ b/src/compat.c
@@ -0,0 +1,26 @@
+/* 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 and src/free-structs.c,
+ * instead of being compiled as a separate unit.  The cpp symbols
+ * COMPAT_ACTIONS or COMPAT_STRUCTS are defined as appropriate.
+ */
diff --git a/src/guestfs.pod b/src/guestfs.pod
index 60664e7..04d8b6b 100644
--- a/src/guestfs.pod
+++ b/src/guestfs.pod
@@ -3499,6 +3499,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]