Libguestfs - Jul 2019 - Re: [PATCH libnbd] lib: Use symbol versions.

This patch adds support for symbol versions.  It is based on what
libvirt does.

The generated syms file looks like:

LIBNBD_1.0 {
  global:
    nbd_...;
    nbd_...;
  local: *;
};

In a future stable 1.2 release, new symbols would go into a new
section which would look like this:

LIBNBD_1.2 {
  global:
    nbd_new_symbol;
    nbd_another_new_symbol;
  local: *;
} LIBNBD_1.0;

In my testing the ‘local:’ label is needed.  For some reason libvirt
doesn’t use it.

The change appears to be working in as much as I can see the symbol
versions appearing, and the test/example programs continue to run.

$ nm -D --with-symbol-versions lib/.libs/libnbd.so.0 | grep LIBNBD
0000000000000000 A LIBNBD_1.0@@LIBNBD_1.0
0000000000005470 T nbd_add_meta_context@@LIBNBD_1.0
00000000000085d0 T nbd_aio_block_status@@LIBNBD_1.0
0000000000008710 T nbd_aio_block_status_callback@@LIBNBD_1.0
0000000000008110 T nbd_aio_cache@@LIBNBD_1.0
0000000000008230 T nbd_aio_cache_callback@@LIBNBD_1.0
00000000000089e0 T nbd_aio_command_completed@@LIBNBD_1.0
0000000000006f20 T nbd_aio_connect@@LIBNBD_1.0
0000000000007320 T nbd_aio_connect_command@@LIBNBD_1.0
[etc]

$ nm -D --with-symbol-versions examples/.libs/glib-main-loop | grep LIBNBD
                 U nbd_aio_connect_command@LIBNBD_1.0
                 U nbd_aio_get_direction@LIBNBD_1.0
                 U nbd_aio_get_fd@LIBNBD_1.0
                 U nbd_aio_in_flight@LIBNBD_1.0
                 U nbd_aio_is_ready@LIBNBD_1.0
                 U nbd_aio_notify_read@LIBNBD_1.0
                 U nbd_aio_notify_write@LIBNBD_1.0
                 U nbd_aio_peek_command_completed@LIBNBD_1.0
                 U nbd_aio_pread_callback@LIBNBD_1.0
                 U nbd_aio_pwrite_callback@LIBNBD_1.0
                 U nbd_close@LIBNBD_1.0
                 U nbd_create@LIBNBD_1.0
                 U nbd_get_debug@LIBNBD_1.0
                 U nbd_get_error@LIBNBD_1.0

Rich.

This changes the libnbd.syms file (the linker script) to use symbol
versions.  The only version currently used is ‘LIBNBD_1.0’ for a
notional stable 1.0 release in the future.  It is expected that future
stable releases will use ‘LIBNBD_1.2’ etc.

Note this only deals with new symbols, not with the question of how to
deal with symbols which we decide to change incompatibly, if that
issue ever arises.
---
 generator/generator | 72 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 60 insertions(+), 12 deletions(-)

diff --git a/generator/generator b/generator/generator
index 60e0ab5..cc3d12d 100755
--- a/generator/generator
+++ b/generator/generator
@@ -840,6 +840,11 @@ type call = {
    * setting this to false.
    *)
   may_set_error : bool;
+  (* The first stable version that the symbol appeared in, for
+   * example (1, 2) if the symbol was added in development cycle
+   * 1.1.x and thus was the first stable version was 1.2
+   *)
+  first_version : int * int;
 }
 and arg  | ArrayAndLen of arg * string (* array + number of entries *)
@@ -884,7 +889,8 @@ and permitted_state  let default_call = { args = []; ret =
RErr;
                      shortdesc = ""; longdesc = "";
                      permitted_states = [];
-                     is_locked = true; may_set_error = true }
+                     is_locked = true; may_set_error = true;
+                     first_version = (1, 0) }
 
 (* Calls.
  *
@@ -2318,6 +2324,20 @@ let rec filter_map f = function
       | Some y -> y :: filter_map f xs
       | None -> filter_map f xs
 
+(* group_by [1, "foo"; 2, "bar"; 2, "baz"; 2,
"biz"; 3, "boo"; 4, "fizz"]
+ * - : (int * string list) list + * [(1, ["foo"]); (2,
["bar"; "baz"; "biz"]); (3, ["boo"]);
(4, ["fizz"])]
+ *)
+let rec group_by = function
+| [] -> []
+| [day, x] -> [day, [x]]
+| (day1, x1) :: (day2, x2) :: rest when day1 = day2 ->
+   let rest = group_by ((day2, x2) :: rest) in
+   let day, xs = List.hd rest in
+   (day, x1 :: xs) :: List.tl rest
+| (day, x) :: rest ->
+   (day, [x]) :: group_by rest
+
 let chan = ref Pervasives.stdout
 let pr fs = ksprintf (fun str -> output_string !chan str) fs
 
@@ -3067,22 +3087,50 @@ let ()      | name, { ret = RUInt; may_set_error = true
} ->
        failwithf "%s: if ret is RUInt, may_set_error must be false"
name
     | _ -> ()
+  ) handle_calls;
+
+  (* First stable version must be 1.x where x is even. *)
+  List.iter (
+    fun (name, { first_version = (major, minor) }) ->
+      if major <> 1 then
+        failwithf "%s: first_version must be 1.x" name;
+      if minor mod 2 <> 0 then
+        failwithf "%s: first_version must refer to a stable release"
name
   ) handle_calls
 
 let generate_lib_libnbd_syms ()    generate_header HashStyle;
 
-  pr "{\n";
-  pr "  global:\n";
-  pr "    nbd_create;\n";
-  pr "    nbd_close;\n";
-  pr "    nbd_get_errno;\n";
-  pr "    nbd_get_error;\n";
-  List.iter (fun (name, _) -> pr "    nbd_%s;\n" name)
handle_calls;
-  pr "\n";
-  pr "  # Everything else is hidden.\n";
-  pr "  local: *;\n";
-  pr "};\n"
+  (* Sort and group the calls by first_version, and emit them in order. *)
+  let cmp (_, {first_version = a}) (_, {first_version = b}) = compare a b in
+  let calls = List.sort cmp handle_calls in
+  let extract ((_, {first_version}) as call) = first_version, call in
+  let calls = List.map extract calls in
+  let calls = group_by calls in
+
+  let prev = ref None in
+  List.iter (
+    fun ((major, minor), calls) ->
+      pr "LIBNBD_%d.%d {\n" major minor;
+      pr "  global:\n";
+      if (major, minor) = (1, 0) then (
+        pr "    nbd_create;\n";
+        pr "    nbd_close;\n";
+        pr "    nbd_get_errno;\n";
+        pr "    nbd_get_error;\n"
+      );
+      List.iter (fun (name, _) -> pr "    nbd_%s;\n" name) calls;
+      pr "  # Everything else is hidden.\n";
+      pr "  local: *;\n";
+      pr "}";
+      (match !prev with
+       | None -> ()
+       | Some (old_major, old_minor) ->
+          pr " LIBNBD_%d.%d" old_minor old_minor
+      );
+      pr ";\n";
+      prev := Some (major, minor)
+  ) calls
 
 let rec name_of_arg = function
 | ArrayAndLen (arg, n) -> name_of_arg arg @ [n]
-- 
2.22.0

On Sat, Jul 27, 2019 at 02:19:47PM +0100, Richard W.M. Jones
wrote:
> This patch adds support for symbol versions.  It is based on what
> libvirt does.
> 
> The generated syms file looks like:
> 
> LIBNBD_1.0 {
>   global:
>     nbd_...;
>     nbd_...;
>   local: *;
> };
> 
> In a future stable 1.2 release, new symbols would go into a new
> section which would look like this:
> 
> LIBNBD_1.2 {
>   global:
>     nbd_new_symbol;
>     nbd_another_new_symbol;
>   local: *;
> } LIBNBD_1.0;
> 
> In my testing the ‘local:’ label is needed.  For some reason libvirt
> doesn’t use it.
We do use it - just note that src/libvirt_public.syms is *not* what
is used to link. We combine that file with libvirt_private.syms too,
and when combining, add the "local: *" bit to the result.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Mon, Jul 29, 2019 at 09:04:33AM +0100, Daniel P. Berrangé
wrote:
> On Sat, Jul 27, 2019 at 02:19:47PM +0100, Richard W.M. Jones wrote:
> > This patch adds support for symbol versions.  It is based on what
> > libvirt does.
> > 
> > The generated syms file looks like:
> > 
> > LIBNBD_1.0 {
> >   global:
> >     nbd_...;
> >     nbd_...;
> >   local: *;
> > };
> > 
> > In a future stable 1.2 release, new symbols would go into a new
> > section which would look like this:
> > 
> > LIBNBD_1.2 {
> >   global:
> >     nbd_new_symbol;
> >     nbd_another_new_symbol;
> >   local: *;
> > } LIBNBD_1.0;
> > 
> > In my testing the ‘local:’ label is needed.  For some reason libvirt
> > doesn’t use it.
> 
> We do use it - just note that src/libvirt_public.syms is *not* what
> is used to link. We combine that file with libvirt_private.syms too,
> and when combining, add the "local: *" bit to the result.
Got it, thanks.  I adjusted the output to look more like
src/libvirt.syms and pushed it.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html