Libguestfs - Jan 2018 - Re: [PATCH] daemon: build also without Hivex.OPEN_UNSAFE (RHBZ#1493048)

Do a configure check for the OPEN_UNSAFE flag in the OCaml binding of
Hivex, using it only when available.  This makes it possible to use
hivex < 1.3.14 to build libguestfs (the daemon, actually).

Amend the building documentation accordingly.
---
 .gitignore                 |  1 +
 configure.ac               |  1 +
 daemon/Makefile.am         |  2 ++
 daemon/config_daemon.ml.in | 20 ++++++++++++++++++++
 daemon/config_daemon.mli   | 19 +++++++++++++++++++
 daemon/inspect_utils.ml    |  2 +-
 docs/guestfs-building.pod  |  2 +-
 m4/guestfs-ocaml.m4        | 19 +++++++++++++++++++
 8 files changed, 64 insertions(+), 2 deletions(-)
 create mode 100644 daemon/config_daemon.ml.in
 create mode 100644 daemon/config_daemon.mli

diff --git a/.gitignore b/.gitignore
index 8276afb26..59d7278be 100644
--- a/.gitignore
+++ b/.gitignore
@@ -186,6 +186,7 @@ Makefile.in
 /daemon/actions.h
 /daemon/callbacks.ml
 /daemon/caml-stubs.c
+/daemon/config_daemon.ml
 /daemon/daemon_utils_tests
 /daemon/dispatch.c
 /daemon/guestfsd
diff --git a/configure.ac b/configure.ac
index daf9abf6f..fbba5ab65 100644
--- a/configure.ac
+++ b/configure.ac
@@ -247,6 +247,7 @@ AC_CONFIG_FILES([Makefile
                  common/windows/Makefile
                  csharp/Makefile
                  customize/Makefile
+                 daemon/config_daemon.ml
                  daemon/Makefile
                  df/Makefile
                  dib/Makefile
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 994bcd61a..4ac4e38d8 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -250,6 +250,7 @@ SOURCES_MLI = \
 	btrfs.mli \
 	callbacks.mli \
 	chroot.mli \
+	config_daemon.mli \
 	daemon.mli \
 	devsparts.mli \
 	file.mli \
@@ -279,6 +280,7 @@ SOURCES_MLI = \
 	utils.mli
 
 SOURCES_ML = \
+	config_daemon.ml \
 	utils.ml \
 	structs.ml \
 	optgroups.ml \
diff --git a/daemon/config_daemon.ml.in b/daemon/config_daemon.ml.in
new file mode 100644
index 000000000..e3ee1fd3f
--- /dev/null
+++ b/daemon/config_daemon.ml.in
@@ -0,0 +1,20 @@
+(* guestfsd
+ * @configure_input@
+ * Copyright (C) 2018 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *)
+
+let hivex_flag_unsafe = [@HIVEX_OPEN_UNSAFE_FLAG@]
diff --git a/daemon/config_daemon.mli b/daemon/config_daemon.mli
new file mode 100644
index 000000000..1d358b7fd
--- /dev/null
+++ b/daemon/config_daemon.mli
@@ -0,0 +1,19 @@
+(* guestfsd
+ * Copyright (C) 2018 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *)
+
+val hivex_flag_unsafe : Hivex.open_flag list
diff --git a/daemon/inspect_utils.ml b/daemon/inspect_utils.ml
index 5127bf30f..b2ce072d6 100644
--- a/daemon/inspect_utils.ml
+++ b/daemon/inspect_utils.ml
@@ -176,7 +176,7 @@ let parse_version_from_major_minor str data    )
 
 let with_hive hive_filename f -  let flags = [ Hivex.OPEN_UNSAFE ] in
+  let flags = Config_daemon.hivex_flag_unsafe in
   let flags = if verbose () then Hivex.OPEN_VERBOSE :: flags else flags in
   let h = Hivex.open_file hive_filename flags in
   protect ~f:(fun () -> f h (Hivex.root h)) ~finally:(fun () ->
Hivex.close h)
diff --git a/docs/guestfs-building.pod b/docs/guestfs-building.pod
index e77e47454..de5d77050 100644
--- a/docs/guestfs-building.pod
+++ b/docs/guestfs-building.pod
@@ -178,7 +178,7 @@ I<Required>.
 I<Required> if compiling from git.
 Optional if compiling from tarball.
 
-=item hivex E<ge> 1.3.14
+=item hivex
 
 =item ocaml-hivex
 
diff --git a/m4/guestfs-ocaml.m4 b/m4/guestfs-ocaml.m4
index e72b5ad39..5ceeaf976 100644
--- a/m4/guestfs-ocaml.m4
+++ b/m4/guestfs-ocaml.m4
@@ -79,6 +79,7 @@ else
 fi
 AC_SUBST([OCAMLDEP_ONE_LINE])
 
+have_Hivex_OPEN_UNSAFE=no
 if test "x$enable_daemon" = "xyes"; then
     OCAML_PKG_hivex=no
     AC_CHECK_OCAML_PKG(hivex)
@@ -86,6 +87,18 @@ if test "x$enable_daemon" = "xyes"; then
         AC_MSG_ERROR([the OCaml module 'hivex' is required])
     fi
 
+    # Check if Hivex has 'OPEN_UNSAFE' flag.
+    AC_MSG_CHECKING([for Hivex.OPEN_UNSAFE])
+    rm -f conftest.ml
+    echo 'let s = Hivex.OPEN_UNSAFE' > conftest.ml
+    if $OCAMLFIND ocamlc -package hivex -c conftest.ml >&5 2>&5 ;
then
+        AC_MSG_RESULT([yes])
+        have_Hivex_OPEN_UNSAFE=yes
+    else
+        AC_MSG_RESULT([no])
+        have_Hivex_OPEN_UNSAFE=no
+    fi
+
     dnl Check which OCaml runtime to link the daemon again.
     dnl We can't use AC_CHECK_LIB here unfortunately because
     dnl the other symbols are resolved by OCaml itself.
@@ -182,6 +195,12 @@ AC_SUBST([OCAML_BYTES_COMPAT_CMO])
 AC_SUBST([OCAML_BYTES_COMPAT_ML])
 AM_CONDITIONAL([HAVE_BYTES_COMPAT_ML],
 	       [test "x$OCAML_BYTES_COMPAT_ML" != "x"])
+AS_IF([test "x$have_Hivex_OPEN_UNSAFE" = "xno"],[
+    HIVEX_OPEN_UNSAFE_FLAG=""
+],[
+    HIVEX_OPEN_UNSAFE_FLAG=" Hivex.OPEN_UNSAFE "
+])
+AC_SUBST([HIVEX_OPEN_UNSAFE_FLAG])
 
 dnl Flags we want to pass to every OCaml compiler call.
 OCAML_WARN_ERROR="-warn-error CDEFLMPSUVYZX+52-3"
-- 
2.14.3

On Tue, Jan 30, 2018 at 06:24:19PM +0100, Pino Toscano
wrote:
> --- a/docs/guestfs-building.pod
> +++ b/docs/guestfs-building.pod
> @@ -178,7 +178,7 @@ I<Required>.
>  I<Required> if compiling from git.
>  Optional if compiling from tarball.
>  
> -=item hivex E<ge> 1.3.14
> +=item hivex
It really works with any version of hivex now?

I appreciate the thought and effort in this patch, but maybe
it's easier to get people to upgrade hivex to a more recent
version.

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

On Tuesday, 30 January 2018 18:29:13 CET Richard W.M. Jones
wrote:
> On Tue, Jan 30, 2018 at 06:24:19PM +0100, Pino Toscano wrote:
> > --- a/docs/guestfs-building.pod
> > +++ b/docs/guestfs-building.pod
> > @@ -178,7 +178,7 @@ I<Required>.
> >  I<Required> if compiling from git.
> >  Optional if compiling from tarball.
> >  
> > -=item hivex E<ge> 1.3.14
> > +=item hivex
> 
> It really works with any version of hivex now?
I cannot really test it on any version", although my patch worked with
hivex 1.3.8 and 1.3.13.  I can restore the previous version though,
which was 1.2.7 before 64f49df747c0937d9433eb11d4191d92a4af748c.
> I appreciate the thought and effort in this patch, but maybe
> it's easier to get people to upgrade hivex to a more recent
> version.
In case of a C library is easy; OTOH deploying a new OCaml module, and
make it available to ocamlfind does not seem an easy task.

-- 
Pino Toscano

On Tue, Jan 30, 2018 at 06:24:19PM +0100, Pino Toscano
wrote:
> Do a configure check for the OPEN_UNSAFE flag in the OCaml binding of
> Hivex, using it only when available.  This makes it possible to use
> hivex < 1.3.14 to build libguestfs (the daemon, actually).
> 
> Amend the building documentation accordingly.
> ---
>  .gitignore                 |  1 +
>  configure.ac               |  1 +
>  daemon/Makefile.am         |  2 ++
>  daemon/config_daemon.ml.in | 20 ++++++++++++++++++++
>  daemon/config_daemon.mli   | 19 +++++++++++++++++++
I guess this should be daemon_config.ml etc for consistency
with guestfs_config.ml used elsewhere.
> diff --git a/daemon/config_daemon.mli b/daemon/config_daemon.mli
> new file mode 100644
> index 000000000..1d358b7fd
> --- /dev/null
> +++ b/daemon/config_daemon.mli
> @@ -0,0 +1,19 @@
> +(* guestfsd
> + * Copyright (C) 2018 Red Hat Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + *)
> +
> +val hivex_flag_unsafe : Hivex.open_flag list
I can see why you made this a list, because it helps with
text inclusion, but it's still the wrong type.  

I think rewriting the macro so:
> +AS_IF([test "x$have_Hivex_OPEN_UNSAFE" = "xno"],[
> +    HIVEX_OPEN_UNSAFE_FLAG=""
this becomes None
> +],[
> +    HIVEX_OPEN_UNSAFE_FLAG=" Hivex.OPEN_UNSAFE "
and this becomes ‘Some Hivex.OPEN_UNSAFE’ allows you to
use an option type.

ACK if these changes are made.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top