[Libguestfs] [PATCH 2/6] daemon: Deduplicate some utility functions.
Richard W.M. Jones
rjones at redhat.com
Thu Aug 3 17:13:47 UTC 2017
The following functions were previously reimplemented in OCaml. This
commit replaces them with calls to the C functions:
- is_root_device
- prog_exists
- udev_settle
plus the internal get_verbose_flag function.
However note that we cannot do this for every utility function. In
particular the C function must not call any reply* functions.
---
daemon/Makefile.am | 25 ++++++++++---
daemon/daemon-c.c | 7 ----
daemon/daemon-c.h | 1 -
daemon/daemon-utils-tests-stubs.c | 32 +++++++++++++++++
daemon/daemon.ml | 5 +--
daemon/dummy.c | 2 --
daemon/utils-c.c | 75 +++++++++++++++++++++++++++++++++++++++
daemon/utils.ml | 39 +++-----------------
daemon/utils.mli | 7 ++--
docs/C_SOURCE_FILES | 4 ++-
10 files changed, 139 insertions(+), 58 deletions(-)
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 5bfe409a4..63b28f52d 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -174,6 +174,7 @@ guestfsd_SOURCES = \
umask.c \
upload.c \
utils.c \
+ utils-c.c \
utimens.c \
utsname.c \
uuids.c \
@@ -297,7 +298,8 @@ OCAMLPACKAGES = \
-I $(top_srcdir)/common/mlutils \
-I $(top_builddir)/common/utils/.libs \
-I $(top_srcdir)/common/mlpcre \
- -I $(top_builddir)/common/mlpcre/.libs
+ -I $(top_builddir)/common/mlpcre/.libs \
+ -I $(top_builddir)/gnulib/lib/.libs
OCAMLFLAGS = $(OCAML_FLAGS) $(OCAML_WARN_FLAGS)
@@ -350,12 +352,25 @@ TESTS = \
check-valgrind:
$(MAKE) VG="@VG@" check
-daemon_utils_tests_SOURCES = dummy.c
+daemon_utils_tests_SOURCES = \
+ command.c \
+ daemon-utils-tests-stubs.c \
+ utils.c \
+ utils-c.c
daemon_utils_tests_CPPFLAGS = \
-I. \
-I$(top_builddir) \
-I$(shell $(OCAMLC) -where) \
- -I$(top_srcdir)/lib
+ -I$(top_srcdir)/gnulib/lib \
+ -I$(top_builddir)/gnulib/lib \
+ -I$(top_srcdir)/common/errnostring \
+ -I$(top_builddir)/common/errnostring \
+ -I$(top_srcdir)/common/protocol \
+ -I$(top_builddir)/common/protocol \
+ -I$(top_srcdir)/common/utils \
+ -I$(top_builddir)/common/utils \
+ -I$(top_srcdir)/lib \
+ -I$(top_builddir)/lib
daemon_utils_tests_BOBJECTS = \
utils.cmo \
daemon_utils_tests.cmo
@@ -377,7 +392,9 @@ daemon_utils_tests_DEPENDENCIES = \
$(daemon_utils_tests_THEOBJECTS) \
$(top_srcdir)/ocaml-link.sh
daemon_utils_tests_LINK = \
- $(top_srcdir)/ocaml-link.sh -- \
+ $(top_srcdir)/ocaml-link.sh \
+ -cclib '-lutils -lgnu' \
+ -- \
$(OCAMLFIND) $(BEST) $(OCAMLFLAGS) $(OCAMLLINKFLAGS) \
$(OCAMLPACKAGES) \
$(daemon_utils_tests_THEOBJECTS) -o $@
diff --git a/daemon/daemon-c.c b/daemon/daemon-c.c
index 3ecaed4ca..3bfa06393 100644
--- a/daemon/daemon-c.c
+++ b/daemon/daemon-c.c
@@ -76,13 +76,6 @@ guestfs_int_daemon_exn_to_reply_with_error (const char *func, value exn)
func, exn_name);
}
-/* NB: This is a "noalloc" call. */
-value
-guestfs_int_daemon_get_verbose_flag (value unitv)
-{
- return Val_bool (verbose);
-}
-
/* Implement String (Mountable, _) parameter. */
value
guestfs_int_daemon_copy_mountable (const mountable_t *mountable)
diff --git a/daemon/daemon-c.h b/daemon/daemon-c.h
index 1b9f102ff..1fcfd707c 100644
--- a/daemon/daemon-c.h
+++ b/daemon/daemon-c.h
@@ -27,7 +27,6 @@
#include <caml/mlvalues.h>
-extern value guestfs_int_daemon_get_verbose_flag (value unitv);
extern void guestfs_int_daemon_exn_to_reply_with_error (const char *func, value exn);
extern value guestfs_int_daemon_copy_mountable (const mountable_t *mountable);
extern char **guestfs_int_daemon_return_string_list (value retv);
diff --git a/daemon/daemon-utils-tests-stubs.c b/daemon/daemon-utils-tests-stubs.c
new file mode 100644
index 000000000..47329a976
--- /dev/null
+++ b/daemon/daemon-utils-tests-stubs.c
@@ -0,0 +1,32 @@
+/* guestfsd
+ * Copyright (C) 2017 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.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "daemon.h"
+
+/* This stubs out some functions that we want to link to the unit
+ * tests, but don't want to actually pull in plus dependencies.
+ */
+
+char *device_name_translation (const char *device) { abort (); }
+void reply_with_error_errno (int err, const char *fs, ...) { abort (); }
+void reply_with_perror_errno (int err, const char *fs, ...) { abort (); }
diff --git a/daemon/daemon.ml b/daemon/daemon.ml
index bf486344f..1f8a6d87d 100644
--- a/daemon/daemon.ml
+++ b/daemon/daemon.ml
@@ -18,9 +18,6 @@
open Printf
-external get_verbose_flag : unit -> bool =
- "guestfs_int_daemon_get_verbose_flag" "noalloc"
-
(* When guestfsd starts up, early on (after parsing the command line
* but not much else), it calls 'caml_startup' which runs all
* initialization code in the OCaml modules, including this one.
@@ -32,7 +29,7 @@ let () =
(* Connect the guestfsd [-v] (verbose) flag into 'verbose ()'
* used in OCaml code to print debugging messages.
*)
- if get_verbose_flag () then (
+ if Utils.get_verbose_flag () then (
Std_utils.set_verbose ();
eprintf "OCaml daemon loaded\n%!"
);
diff --git a/daemon/dummy.c b/daemon/dummy.c
deleted file mode 100644
index ebab6198c..000000000
--- a/daemon/dummy.c
+++ /dev/null
@@ -1,2 +0,0 @@
-/* Dummy source, to be used for OCaml-based tools with no C sources. */
-enum { foo = 1 };
diff --git a/daemon/utils-c.c b/daemon/utils-c.c
new file mode 100644
index 000000000..22b0d57c6
--- /dev/null
+++ b/daemon/utils-c.c
@@ -0,0 +1,75 @@
+/* guestfsd
+ * Copyright (C) 2017 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.
+ */
+
+/**
+ * Bindings for utility functions.
+ *
+ * Note that functions called from OCaml code B<must never> call
+ * any of the C<reply*> functions.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <caml/alloc.h>
+#include <caml/mlvalues.h>
+#include <caml/memory.h>
+#include <caml/unixsupport.h>
+
+#include "daemon.h"
+
+#pragma GCC diagnostic ignored "-Wmissing-prototypes"
+
+/* NB: This is a "noalloc" call. */
+value
+guestfs_int_daemon_get_verbose_flag (value unitv)
+{
+ return Val_bool (verbose);
+}
+
+/* NB: This is a "noalloc" call. */
+value
+guestfs_int_daemon_is_root_device (value device)
+{
+ return Val_bool (is_root_device (String_val (device)));
+}
+
+/* NB: This is a "noalloc" call. */
+value
+guestfs_int_daemon_prog_exists (value prog)
+{
+ return Val_bool (prog_exists (String_val (prog)));
+}
+
+/* NB: This is a "noalloc" call. */
+value
+guestfs_int_daemon_udev_settle (value optfilenamev, value unitv)
+{
+ const char *file;
+
+ if (optfilenamev == Val_int (0))
+ file = NULL;
+ else
+ file = String_val (Field (optfilenamev, 0));
+
+ udev_settle_file (file);
+
+ return Val_unit;
+}
diff --git a/daemon/utils.ml b/daemon/utils.ml
index b459a2314..9e9e68ab4 100644
--- a/daemon/utils.ml
+++ b/daemon/utils.ml
@@ -21,9 +21,10 @@ open Printf
open Std_utils
-let prog_exists prog =
- try ignore (which prog); true
- with Executable_not_found _ -> false
+external get_verbose_flag : unit -> bool = "guestfs_int_daemon_get_verbose_flag" "noalloc"
+external is_root_device : string -> bool = "guestfs_int_daemon_is_root_device" "noalloc"
+external prog_exists : string -> bool = "guestfs_int_daemon_prog_exists" "noalloc"
+external udev_settle : ?filename:string -> unit -> unit = "guestfs_int_daemon_udev_settle" "noalloc"
let commandr ?(fold_stdout_on_stderr = false) prog args =
if verbose () then
@@ -99,38 +100,6 @@ let command ?fold_stdout_on_stderr prog args =
failwithf "%s exited with status %d: %s" prog r stderr;
stdout
-let udev_settle ?filename () =
- let args = ref [] in
- if verbose () then
- push_back args "--debug";
- push_back args "settle";
- (match filename with
- | None -> ()
- | Some filename ->
- push_back args "-E";
- push_back args filename
- );
- let args = !args in
- let r, _, err = commandr "udevadm" args in
- if r <> 0 then
- eprintf "udevadm settle: %s\n" err
-
-let root_device = lazy ((stat "/").st_dev)
-
-let is_root_device_stat statbuf =
- statbuf.st_rdev = Lazy.force root_device
-
-let is_root_device device =
- udev_settle ~filename:device ();
- try
- let statbuf = stat device in
- is_root_device_stat statbuf
- with
- Unix_error (err, func, arg) ->
- eprintf "is_root_device: %s: %s: %s: %s\n"
- device func arg (error_message err);
- false
-
(* XXX This function is copied from C, but is misconceived. It
* cannot by design work for devices like /dev/md0. It would be
* better if it checked for the existence of devices and partitions
diff --git a/daemon/utils.mli b/daemon/utils.mli
index 16569f018..8807b864b 100644
--- a/daemon/utils.mli
+++ b/daemon/utils.mli
@@ -37,10 +37,6 @@ val udev_settle : ?filename:string -> unit -> unit
val is_root_device : string -> bool
(** Return true if this is the root (appliance) device. *)
-val is_root_device_stat : Unix.stats -> bool
-(** As for {!is_root_device} but operates on a statbuf instead of
- a device name. *)
-
val split_device_partition : string -> string * int
(** Split a device name like [/dev/sda1] into a device name and
partition number, eg. ["sda", 1].
@@ -85,3 +81,6 @@ val commandr : ?fold_stdout_on_stderr:bool -> string -> string list -> (int * st
val is_small_file : string -> bool
(** Return true if the path is a small regular file. *)
+
+(**/**)
+val get_verbose_flag : unit -> bool
diff --git a/docs/C_SOURCE_FILES b/docs/C_SOURCE_FILES
index e47469a6a..d02c0bcf7 100644
--- a/docs/C_SOURCE_FILES
+++ b/docs/C_SOURCE_FILES
@@ -85,6 +85,7 @@ daemon/cpio.c
daemon/cpmv.c
daemon/daemon-c.c
daemon/daemon-c.h
+daemon/daemon-utils-tests-stubs.c
daemon/daemon.h
daemon/dd.c
daemon/debug-bmap.c
@@ -97,7 +98,6 @@ daemon/dispatch.c
daemon/dmesg.c
daemon/dropcaches.c
daemon/du.c
-daemon/dummy.c
daemon/echo-daemon.c
daemon/ext2.c
daemon/fallocate.c
@@ -180,6 +180,8 @@ daemon/truncate.c
daemon/tsk.c
daemon/umask.c
daemon/upload.c
+daemon/utils-c.c
+daemon/utils.c
daemon/utimens.c
daemon/utsname.c
daemon/uuids.c
--
2.13.1
More information about the Libguestfs
mailing list