Pino Toscano
2020-Jan-09 15:07 UTC
[Libguestfs] [PATCH v2 0/4] Switch augeas APIs to OCaml
This reimplements the augeas APIs using ocaml-augeas (dropping all the C code). The behaviour seems unchanged, although I may have not tested all the various corner cases. Pino Toscano (4): daemon: fix/enhance error reporting of Augeas exceptions Revert "Revert "daemon: implement OptString for OCaml APIs"" daemon: move augeas APIs to OCaml daemon: drop usage of C augeas library .gitignore | 1 + daemon/Makefile.am | 7 +- daemon/aug.ml | 143 +++++++++++ daemon/aug_utils.ml | 42 +++ daemon/aug_utils.mli | 32 +++ daemon/augeas.c | 493 ------------------------------------ daemon/cleanups.c | 11 - daemon/daemon-c.c | 29 ++- daemon/daemon-c.h | 1 + daemon/daemon.h | 35 --- daemon/lvm-filter.c | 2 - daemon/mount.c | 3 +- daemon/mount_utils.ml | 2 +- docs/C_SOURCE_FILES | 1 - generator/actions_augeas.ml | 15 ++ generator/actions_core.ml | 2 + generator/daemon.ml | 18 +- po/POTFILES | 1 - 18 files changed, 288 insertions(+), 550 deletions(-) create mode 100644 daemon/aug.ml create mode 100644 daemon/aug_utils.ml create mode 100644 daemon/aug_utils.mli delete mode 100644 daemon/augeas.c -- 2.24.1
Pino Toscano
2020-Jan-09 15:07 UTC
[Libguestfs] [PATCH v2 1/4] daemon: fix/enhance error reporting of Augeas exceptions
The current code was broken, as the field 1 of the exception value is
the error code (int), not an error string, and thus it would have
crashed. This did not happen in practice, as all the usage of
ocaml-augeas were only in the inspection code with ad-hoc exception
catching blocks.
Other than fixing the aforementioned issue, enhance the error reporting
to be as close as possible to what the current AUGEAS_ERROR() macro
does: error message, error minor message (if available), error details
(if available).
---
daemon/daemon-c.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/daemon/daemon-c.c b/daemon/daemon-c.c
index 533bf7ce7..62e7daf2c 100644
--- a/daemon/daemon-c.c
+++ b/daemon/daemon-c.c
@@ -65,8 +65,15 @@ guestfs_int_daemon_exn_to_reply_with_error (const char *func,
value exn)
reply_with_error ("%s", String_val (Field (exn, 1)));
else if (STREQ (exn_name, "Invalid_argument"))
reply_with_error ("invalid argument: %s", String_val (Field (exn,
1)));
- else if (STREQ (exn_name, "Augeas.Error"))
- reply_with_error ("augeas error: %s", String_val (Field (exn,
1)));
+ else if (STREQ (exn_name, "Augeas.Error")) {
+ const char *message = String_val (Field (exn, 3));
+ const char *minor = String_val (Field (exn, 4));
+ const char *details = String_val (Field (exn, 5));
+ reply_with_error ("augeas error: %s%s%s%s%s",
+ message,
+ minor ? ": " : "", minor ? minor :
"",
+ details ? ": " : "", details ?
details : "");
+}
else if (STREQ (exn_name, "PCRE.Error")) {
value pair = Field (exn, 1);
reply_with_error ("PCRE error: %s (PCRE error code: %d)",
--
2.24.1
Pino Toscano
2020-Jan-09 15:07 UTC
[Libguestfs] [PATCH v2 2/4] Revert "Revert "daemon: implement OptString for OCaml APIs""
Val_optstring will be used by the next commit. Unfortunately, this
breaks bisection when building with -Werror.
This reverts commit ca8f8afcc5b5b77be69f5b353ed8aef3fae1883d.
---
generator/daemon.ml | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/generator/daemon.ml b/generator/daemon.ml
index b67c4d20b..418ef2fde 100644
--- a/generator/daemon.ml
+++ b/generator/daemon.ml
@@ -600,6 +600,22 @@ let generate_daemon_caml_stubs () #include
\"actions.h\"
#include \"daemon-c.h\"
+static CAMLprim value
+Val_optstring (const char *s)
+{
+ CAMLparam0 ();
+ CAMLlocal2 (optv, v);
+
+ if (s) { /* Return Some val */
+ v = caml_copy_string (s);
+ optv = caml_alloc (1, 0);
+ Field (optv, 0) = v;
+ } else /* Return None */
+ optv = Val_int (0);
+
+ CAMLreturn (optv);
+}
+
";
(* Implement code for returning structs and struct lists. *)
@@ -797,7 +813,7 @@ let generate_daemon_caml_stubs () | String
((Mountable|Mountable_or_Path), n) ->
pr "guestfs_int_daemon_copy_mountable (%s)" n
| String _ -> assert false
- | OptString _ -> assert false
+ | OptString n -> pr "Val_optstring (%s)" n
| StringList _ -> assert false
| BufferIn _ -> assert false
| Pointer _ -> assert false
--
2.24.1
Pino Toscano
2020-Jan-09 15:07 UTC
[Libguestfs] [PATCH v2 3/4] daemon: move augeas APIs to OCaml
Rewrite all the augeas APIs to OCaml using ocaml-augeas (already in use
in the daemon), removing their C code. The APIs are in a "Aug"
module,
as "Augeas" is the module exposed by ocaml-augeas already.
The handling of the singleton augeas handle is in a smaller separate
"Aug_utils" module, so other modules can access it; this is done
because
the interfaces of modules with OCaml APIs are automatically generated by
the generator, so there is no way to expose stuff.
In addition, make sure that the C umount-all still closes the augeas
handle: register the augeas handle finalization as OCaml callback, and
create a C function to invoke it. This can be dropped once umount-all
is rewritten to OCaml in the future.
---
.gitignore | 1 +
daemon/Makefile.am | 5 +
daemon/aug.ml | 143 +++++++++++++
daemon/aug_utils.ml | 42 ++++
daemon/aug_utils.mli | 32 +++
daemon/augeas.c | 402 ------------------------------------
daemon/daemon-c.c | 18 ++
daemon/daemon-c.h | 1 +
daemon/daemon.h | 1 -
daemon/mount.c | 3 +-
daemon/mount_utils.ml | 2 +-
generator/actions_augeas.ml | 15 ++
generator/actions_core.ml | 2 +
13 files changed, 262 insertions(+), 405 deletions(-)
create mode 100644 daemon/aug.ml
create mode 100644 daemon/aug_utils.ml
create mode 100644 daemon/aug_utils.mli
diff --git a/.gitignore b/.gitignore
index 1abb1c8b7..1692b898b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -143,6 +143,7 @@ Makefile.in
/customize/virt-customize.1
/daemon/.depend
/daemon/actions.h
+/daemon/aug.mli
/daemon/blkid.mli
/daemon/btrfs.mli
/daemon/callbacks.ml
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 25948dbe9..220b934a3 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -36,6 +36,7 @@ BUILT_SOURCES = \
generator_built = \
$(BUILT_SOURCES) \
+ aug.ml \
blkid.mli \
btrfs.mli \
callbacks.ml \
@@ -265,6 +266,8 @@ guestfsd_CFLAGS = \
# library and then linked to the daemon. See
# https://caml.inria.fr/pub/docs/manual-ocaml/intfc.html
SOURCES_MLI = \
+ aug.mli \
+ aug_utils.mli \
blkid.mli \
btrfs.mli \
callbacks.mli \
@@ -308,6 +311,8 @@ SOURCES_ML = \
sysroot.ml \
mountable.ml \
chroot.ml \
+ aug_utils.ml \
+ aug.ml \
blkid.ml \
btrfs.ml \
devsparts.ml \
diff --git a/daemon/aug.ml b/daemon/aug.ml
new file mode 100644
index 000000000..921f4c66c
--- /dev/null
+++ b/daemon/aug.ml
@@ -0,0 +1,143 @@
+(* libguestfs - the guestfsd daemon
+ * Copyright (C) 2009-2019 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.
+ *)
+
+open Printf
+
+open Std_utils
+
+open Aug_utils
+
+include Structs
+
+(* We need to rewrite the root path so it is based at /sysroot. *)
+let aug_init root flags + aug_finalize ();
+
+ let flag_map = [
+ Augeas.AugSaveBackup;
+ Augeas.AugSaveNewFile;
+ Augeas.AugTypeCheck;
+ Augeas.AugNoStdinc;
+ Augeas.AugSaveNoop;
+ Augeas.AugNoLoad;
+ Augeas.AugNoModlAutoload;
+ Augeas.AugEnableSpan;
+ Augeas.AugNoErrClose;
+ Augeas.AugTraceModuleLoading;
+ ] in
+ let flags_list = ref [] in
+ List.iteri (
+ fun i flag ->
+ if flags land (1 lsl i) > 0 then
+ List.push_back flags_list flag
+ ) flag_map;
+
+ set_aug (Augeas.create (Sysroot.sysroot_path root) None !flags_list)
+
+let aug_close () + (* Error out if no handle was initialized. *)
+ ignore (get_aug ());
+ aug_finalize ()
+
+let aug_defvar name expr + let aug = get_aug () in
+ let r = Augeas.defvar aug name expr in
+ match r with
+ | Some v -> v
+ | None -> 0
+
+let aug_defnode name expr value + let aug = get_aug () in
+ let r, created = Augeas.defnode aug name expr (Some value) in
+ { i = Int32.of_int r; b = if created then Int32.one else Int32.zero }
+
+let aug_get path + let aug = get_aug () in
+ let r = Augeas.get aug path in
+ match r with
+ | None -> failwith "aug_get got no results"
+ | Some s -> s
+
+let aug_set path value + let aug = get_aug () in
+ Augeas.set aug path (Some value)
+
+let aug_clear path + let aug = get_aug () in
+ Augeas.set aug path None
+
+let aug_insert path label before + let aug = get_aug () in
+ Augeas.insert ~before aug path label
+
+let aug_rm path + let aug = get_aug () in
+ Augeas.rm aug path
+
+let aug_mv src dest + let aug = get_aug () in
+ Augeas.mv aug src dest
+
+let aug_match path + let aug = get_aug () in
+ Augeas.matches aug path
+
+let aug_save () + let aug = get_aug () in
+ Augeas.save aug
+
+let aug_load () + let aug = get_aug () in
+ Augeas.load aug
+
+(* Simpler version of aug-match, which also sorts the output. *)
+let aug_ls path + let aug = get_aug () in
+
+ (* Note that path might also be a previously defined variable
+ * (defined with aug_defvar). See RHBZ#580016.
+ *)
+ let len = String.length path in
+ if len > 0 then (
+ match String.get path (len - 1) with
+ | '/' | ']' | '*' ->
+ invalid_arg "don't use aug-ls with a path that ends with / ]
*"
+ | _ -> ()
+ );
+
+ let expr + if path = "/" then path
+ else sprintf "%s/*" path in
+
+ List.sort compare (Augeas.matches aug expr)
+
+let aug_setm base sub value + let aug = get_aug () in
+ Augeas.setm aug base sub (Some value)
+
+let aug_label path + let aug = get_aug () in
+ let r = Augeas.label aug path in
+ match r with
+ | None -> failwith "no matching nodes found"
+ | Some s -> s
+
+let aug_transform ?(remove = false) lens file + let aug = get_aug () in
+ let mode = if remove then Augeas.Exclude else Augeas.Include in
+ Augeas.transform aug lens file mode
diff --git a/daemon/aug_utils.ml b/daemon/aug_utils.ml
new file mode 100644
index 000000000..200333870
--- /dev/null
+++ b/daemon/aug_utils.ml
@@ -0,0 +1,42 @@
+(* libguestfs - the guestfsd daemon
+ * Copyright (C) 2009-2019 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.
+ *)
+
+(* The Augeas handle. We maintain a single handle per daemon, which
+ * is all that is necessary and reduces the complexity of the API
+ * considerably.
+ *)
+let aug_handle = ref None
+
+(* Clean up the augeas handle on daemon exit. *)
+let aug_finalize () + match !aug_handle with
+ | None -> ()
+ | Some aug ->
+ Augeas.close aug;
+ aug_handle := None
+let () + Callback.register "Aug_utils.aug_finalize" aug_finalize;
+ at_exit aug_finalize
+
+let get_aug () + match !aug_handle with
+ | None -> failwith "you must call 'aug-init' first to
initialize Augeas"
+ | Some aug -> aug
+
+let set_aug handle + aug_handle := Some handle
diff --git a/daemon/aug_utils.mli b/daemon/aug_utils.mli
new file mode 100644
index 000000000..b2b7b5714
--- /dev/null
+++ b/daemon/aug_utils.mli
@@ -0,0 +1,32 @@
+(* libguestfs - the guestfsd daemon
+ * Copyright (C) 2019 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.
+ *)
+
+(** This module handles the singleton augeas handle. *)
+
+val aug_finalize : unit -> unit
+(** Close the augeas handle, and unset it.
+
+ Nothing is done if the handle is not open. *)
+
+val get_aug : unit -> Augeas.t
+(** Get the augeas handle.
+
+ This raises [Failure] when the handle is not open. *)
+
+val set_aug : Augeas.t -> unit
+(** Set the augeas handle. *)
diff --git a/daemon/augeas.c b/daemon/augeas.c
index b34222ab5..d0c6f19a2 100644
--- a/daemon/augeas.c
+++ b/daemon/augeas.c
@@ -47,12 +47,6 @@
int augeas_version;
-/* The Augeas handle. We maintain a single handle per daemon, which
- * is all that is necessary and reduces the complexity of the API
- * considerably.
- */
-static augeas *aug = NULL;
-
void
aug_read_version (void)
{
@@ -95,399 +89,3 @@ aug_read_version (void)
augeas_version = (major << 16) | (minor << 8) | patch;
}
-
-/* Clean up the augeas handle on daemon exit. */
-void aug_finalize (void) __attribute__((destructor));
-void
-aug_finalize (void)
-{
- if (aug) {
- aug_close (aug);
- aug = NULL;
- }
-}
-
-#define NEED_AUG(errcode) \
- do { \
- if (!aug) { \
- reply_with_error ("%s: you must call 'aug-init' first to
initialize Augeas", __func__); \
- return (errcode); \
- } \
- } \
- while (0)
-
-/* We need to rewrite the root path so it is based at /sysroot. */
-int
-do_aug_init (const char *root, int flags)
-{
- CLEANUP_FREE char *buf = NULL;
-
- if (aug) {
- aug_close (aug);
- aug = NULL;
- }
-
- buf = sysroot_path (root);
- if (!buf) {
- reply_with_perror ("malloc");
- return -1;
- }
-
- /* Pass AUG_NO_ERR_CLOSE so we can display detailed errors. */
- aug = aug_init (buf, NULL, flags | AUG_NO_ERR_CLOSE);
-
- if (!aug) {
- reply_with_error ("augeas initialization failed");
- return -1;
- }
-
- if (aug_error (aug) != AUG_NOERROR) {
- AUGEAS_ERROR ("aug_init: %s (flags %d)", root, flags);
- aug_close (aug);
- aug = NULL;
- return -1;
- }
-
- return 0;
-}
-
-int
-do_aug_close (void)
-{
- NEED_AUG(-1);
-
- aug_close (aug);
- aug = NULL;
-
- return 0;
-}
-
-int
-do_aug_defvar (const char *name, const char *expr)
-{
- int r;
-
- NEED_AUG (-1);
-
- r = aug_defvar (aug, name, expr);
- if (r == -1) {
- AUGEAS_ERROR ("aug_defvar: %s: %s", name, expr);
- return -1;
- }
- return r;
-}
-
-guestfs_int_int_bool *
-do_aug_defnode (const char *name, const char *expr, const char *val)
-{
- guestfs_int_int_bool *r;
- int i, created;
-
- NEED_AUG (NULL);
-
- i = aug_defnode (aug, name, expr, val, &created);
- if (i == -1) {
- AUGEAS_ERROR ("aug_defnode: %s: %s: %s", name, expr, val);
- return NULL;
- }
-
- r = malloc (sizeof *r);
- if (r == NULL) {
- reply_with_perror ("malloc");
- return NULL;
- }
-
- r->i = i;
- r->b = created;
-
- return r;
-}
-
-char *
-do_aug_get (const char *path)
-{
- const char *value = NULL;
- char *v;
- int r;
-
- NEED_AUG (NULL);
-
- r = aug_get (aug, path, &value);
- if (r == 0) {
- reply_with_error ("no matching node");
- return NULL;
- }
- if (r != 1) {
- AUGEAS_ERROR ("aug_get: %s", path);
- return NULL;
- }
-
- /* value can still be NULL here, eg. try with path == "/augeas".
- * I don't understand this case, and it seems to contradict the
- * documentation.
- */
- if (value == NULL) {
- reply_with_error ("Augeas returned NULL match");
- return NULL;
- }
-
- /* The value is an internal Augeas string, so we must copy it. GC FTW. */
- v = strdup (value);
- if (v == NULL) {
- reply_with_perror ("strdup");
- return NULL;
- }
-
- return v; /* Caller frees. */
-}
-
-int
-do_aug_set (const char *path, const char *val)
-{
- int r;
-
- NEED_AUG (-1);
-
- r = aug_set (aug, path, val);
- if (r == -1) {
- AUGEAS_ERROR ("aug_set: %s: %s", path, val);
- return -1;
- }
-
- return 0;
-}
-
-int
-do_aug_clear (const char *path)
-{
- int r;
-
- NEED_AUG (-1);
-
- r = aug_set (aug, path, NULL);
- if (r == -1) {
- AUGEAS_ERROR ("aug_clear: %s", path);
- return -1;
- }
-
- return 0;
-}
-
-int
-do_aug_insert (const char *path, const char *label, int before)
-{
- int r;
-
- NEED_AUG (-1);
-
- r = aug_insert (aug, path, label, before);
- if (r == -1) {
- AUGEAS_ERROR ("aug_insert: %s: %s [before=%d]", path, label,
before);
- return -1;
- }
-
- return 0;
-}
-
-int
-do_aug_rm (const char *path)
-{
- int r;
-
- NEED_AUG (-1);
-
- r = aug_rm (aug, path);
- if (r == -1) {
- AUGEAS_ERROR ("aug_rm: %s", path);
- return -1;
- }
-
- return r;
-}
-
-int
-do_aug_mv (const char *src, const char *dest)
-{
- int r;
-
- NEED_AUG (-1);
-
- r = aug_mv (aug, src, dest);
- if (r == -1) {
- AUGEAS_ERROR ("aug_mv: %s: %s", src, dest);
- return -1;
- }
-
- return 0;
-}
-
-char **
-do_aug_match (const char *path)
-{
- char **matches = NULL;
- void *vp;
- int r;
-
- NEED_AUG (NULL);
-
- r = aug_match (aug, path, &matches);
- if (r == -1) {
- AUGEAS_ERROR ("aug_match: %s", path);
- return NULL;
- }
-
- /* This returns an array of length r, which we must extend
- * and add a terminating NULL.
- */
- vp = realloc (matches, sizeof (char *) * (r+1));
- if (vp == NULL) {
- reply_with_perror ("realloc");
- free (vp);
- return NULL;
- }
- matches = vp;
- matches[r] = NULL;
-
- return matches; /* Caller frees. */
-}
-
-int
-do_aug_save (void)
-{
- NEED_AUG (-1);
-
- if (aug_save (aug) == -1) {
- AUGEAS_ERROR ("aug_save");
- return -1;
- }
-
- return 0;
-}
-
-int
-do_aug_load (void)
-{
- NEED_AUG (-1);
-
- if (aug_load (aug) == -1) {
- AUGEAS_ERROR ("aug_load");
- return -1;
- }
-
- return 0;
-}
-
-/* Simpler version of aug-match, which also sorts the output. */
-char **
-do_aug_ls (const char *path)
-{
- char **matches;
- size_t len;
-
- NEED_AUG (NULL);
-
- /* Note that path might also be a previously defined variable
- * (defined with aug_defvar). See RHBZ#580016.
- */
-
- len = strlen (path);
-
- if (len > 1 &&
- (path[len-1] == '/' || path[len-1] == ']' || path[len-1]
== '*')) {
- reply_with_error ("don't use aug-ls with a path that ends with / ]
*");
- return NULL;
- }
-
- if (STREQ (path, "/"))
- matches = do_aug_match ("/*");
- else {
- char *buf = NULL;
-
- if (asprintf (&buf, "%s/*", path) == -1) {
- reply_with_perror ("asprintf");
- return NULL;
- }
-
- matches = do_aug_match (buf);
- free (buf);
- }
-
- if (matches == NULL)
- return NULL; /* do_aug_match has already sent the error */
-
- sort_strings (matches, guestfs_int_count_strings ((void *) matches));
- return matches; /* Caller frees. */
-}
-
-int
-do_aug_setm (const char *base, const char *sub, const char *val)
-{
- int r;
-
- NEED_AUG (-1);
-
- r = aug_setm (aug, base, sub, val);
- if (r == -1) {
- AUGEAS_ERROR ("aug_setm: %s: %s: %s", base, sub ? sub :
"(null)", val);
- return -1;
- }
-
- return r;
-}
-
-char *
-do_aug_label (const char *augpath)
-{
- int r;
- const char *label;
- char *ret;
-
- NEED_AUG (NULL);
-
- r = aug_label (aug, augpath, &label);
- if (r == -1) {
- AUGEAS_ERROR ("aug_label: %s", augpath);
- return NULL;
- }
- if (r == 0) {
- reply_with_error ("no matching nodes found");
- return NULL;
- }
-
- if (label == NULL) {
- reply_with_error ("internal error: expected label != NULL (r =
%d)", r);
- return NULL;
- }
-
- /* 'label' points to an interior field in the Augeas handle, so
- * we must return a copy.
- */
- ret = strdup (label);
- if (ret == NULL) {
- reply_with_perror ("strdup");
- return NULL;
- }
-
- return ret; /* caller frees */
-}
-
-/* Takes optional arguments, consult optargs_bitmask. */
-int
-do_aug_transform (const char *lens, const char *file, int remove)
-{
- int r;
- int excl = 0; /* add by default */
-
- NEED_AUG (-1);
-
- if (optargs_bitmask & GUESTFS_AUG_TRANSFORM_REMOVE_BITMASK)
- excl = remove;
-
- r = aug_transform (aug, lens, file, excl);
- if (r == -1) {
- AUGEAS_ERROR ("aug_transform: %s: %s: %s", lens, file, excl ?
"excl" : "incl");
- return -1;
- }
-
- return r;
-}
diff --git a/daemon/daemon-c.c b/daemon/daemon-c.c
index 62e7daf2c..e6a3c25ca 100644
--- a/daemon/daemon-c.c
+++ b/daemon/daemon-c.c
@@ -22,6 +22,7 @@
#include <stdlib.h>
#include <caml/alloc.h>
+#include <caml/callback.h>
#include <caml/mlvalues.h>
#include <caml/memory.h>
#include <caml/unixsupport.h>
@@ -261,3 +262,20 @@ guestfs_int_daemon_return_hashtable_string_mountable (value
retv)
return take_stringsbuf (&ret); /* caller frees */
}
+
+/* Call the OCaml function ‘Aug_utils.aug_finalize’. */
+void
+guestfs_int_aug_finalize (void)
+{
+ static value *cb = NULL;
+ CAMLparam0 ();
+ CAMLlocalN (args, 1);
+
+ if (cb == NULL)
+ cb = caml_named_value ("Aug_utils.aug_finalize");
+
+ args[0] = Val_unit;
+ caml_callbackN_exn (*cb, 1, args);
+
+ CAMLreturn0;
+}
diff --git a/daemon/daemon-c.h b/daemon/daemon-c.h
index 9b7085bce..6091b9215 100644
--- a/daemon/daemon-c.h
+++ b/daemon/daemon-c.h
@@ -35,5 +35,6 @@ extern char **guestfs_int_daemon_return_string_mountable_list
(value retv);
extern char **guestfs_int_daemon_return_hashtable_string_string (value retv);
extern char **guestfs_int_daemon_return_hashtable_mountable_string (value
retv);
extern char **guestfs_int_daemon_return_hashtable_string_mountable (value
retv);
+extern void guestfs_int_aug_finalize (void);
#endif /* GUESTFSD_DAEMON_C_H */
diff --git a/daemon/daemon.h b/daemon/daemon.h
index 170fb2537..72f1b1a4b 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -255,7 +255,6 @@ extern void wipe_device_before_mkfs (const char *device);
/* augeas.c */
extern void aug_read_version (void);
-extern void aug_finalize (void);
/* The version of augeas, saved as:
* (MAJOR << 16) | (MINOR << 8) | PATCH
diff --git a/daemon/mount.c b/daemon/mount.c
index 61ce64449..f59eb2877 100644
--- a/daemon/mount.c
+++ b/daemon/mount.c
@@ -30,6 +30,7 @@
#include "daemon.h"
#include "actions.h"
+#include "daemon-c.h"
#define MAX_ARGS 64
@@ -267,7 +268,7 @@ do_umount_all (void)
* function, and since the umount will definitely fail if any
* handles are open, we may as well close them.
*/
- aug_finalize ();
+ guestfs_int_aug_finalize ();
hivex_finalize ();
journal_finalize ();
diff --git a/daemon/mount_utils.ml b/daemon/mount_utils.ml
index c1e0b60ee..eff237b07 100644
--- a/daemon/mount_utils.ml
+++ b/daemon/mount_utils.ml
@@ -36,8 +36,8 @@ let rec umount_all () * function, and since the umount
will definitely fail if any
* handles are open, we may as well close them.
*)
+ Aug_utils.aug_finalize ();
(* XXX
- aug_finalize ();
hivex_finalize ();
journal_finalize ();
*)
diff --git a/generator/actions_augeas.ml b/generator/actions_augeas.ml
index bb0fe4db0..c3744a341 100644
--- a/generator/actions_augeas.ml
+++ b/generator/actions_augeas.ml
@@ -26,6 +26,7 @@ let daemon_functions = [
{ defaults with
name = "aug_init"; added = (0, 0, 7);
style = RErr, [String (Pathname, "root"); Int "flags"],
[];
+ impl = OCaml "Aug.aug_init";
tests = [
InitBasicFS, Always, TestResultString (
[["mkdir"; "/etc"];
@@ -90,6 +91,7 @@ To find out more about Augeas, see
L<http://augeas.net/>." };
{ defaults with
name = "aug_close"; added = (0, 0, 7);
style = RErr, [], [];
+ impl = OCaml "Aug.aug_close";
shortdesc = "close the current Augeas handle";
longdesc = "\
Close the current Augeas handle and free up any resources
@@ -100,6 +102,7 @@ Augeas functions." };
{ defaults with
name = "aug_defvar"; added = (0, 0, 7);
style = RInt "nrnodes", [String (PlainString, "name");
OptString "expr"], [];
+ impl = OCaml "Aug.aug_defvar";
shortdesc = "define an Augeas variable";
longdesc = "\
Defines an Augeas variable C<name> whose value is the result
@@ -112,6 +115,7 @@ C<0> if C<expr> evaluates to something which is
not a nodeset." };
{ defaults with
name = "aug_defnode"; added = (0, 0, 7);
style = RStruct ("nrnodescreated", "int_bool"), [String
(PlainString, "name"); String (PlainString, "expr"); String
(PlainString, "val")], [];
+ impl = OCaml "Aug.aug_defnode";
shortdesc = "define an Augeas node";
longdesc = "\
Defines a variable C<name> whose value is the result of
@@ -128,6 +132,7 @@ if a node was created." };
{ defaults with
name = "aug_get"; added = (0, 0, 7);
style = RString (RPlainString, "val"), [String (PlainString,
"augpath")], [];
+ impl = OCaml "Aug.aug_get";
shortdesc = "look up the value of an Augeas path";
longdesc = "\
Look up the value associated with C<path>. If C<path>
@@ -136,6 +141,7 @@ matches exactly one node, the C<value> is
returned." };
{ defaults with
name = "aug_set"; added = (0, 0, 7);
style = RErr, [String (PlainString, "augpath"); String
(PlainString, "val")], [];
+ impl = OCaml "Aug.aug_set";
tests = [
InitBasicFS, Always, TestResultString (
[["mkdir"; "/etc"];
@@ -156,6 +162,7 @@ C<guestfs_aug_clear> call." };
{ defaults with
name = "aug_insert"; added = (0, 0, 7);
style = RErr, [String (PlainString, "augpath"); String
(PlainString, "label"); Bool "before"], [];
+ impl = OCaml "Aug.aug_insert";
tests = [
InitBasicFS, Always, TestResultString (
[["mkdir"; "/etc"];
@@ -182,6 +189,7 @@ with a bracketed index C<[N]>." };
{ defaults with
name = "aug_rm"; added = (0, 0, 7);
style = RInt "nrnodes", [String (PlainString,
"augpath")], [];
+ impl = OCaml "Aug.aug_rm";
shortdesc = "remove an Augeas path";
longdesc = "\
Remove C<path> and all of its children.
@@ -191,6 +199,7 @@ On success this returns the number of entries which were
removed." };
{ defaults with
name = "aug_mv"; added = (0, 0, 7);
style = RErr, [String (PlainString, "src"); String (PlainString,
"dest")], [];
+ impl = OCaml "Aug.aug_mv";
shortdesc = "move Augeas node";
longdesc = "\
Move the node C<src> to C<dest>. C<src> must match exactly
@@ -199,6 +208,7 @@ one node. C<dest> is overwritten if it exists."
};
{ defaults with
name = "aug_match"; added = (0, 0, 7);
style = RStringList (RPlainString, "matches"), [String
(PlainString, "augpath")], [];
+ impl = OCaml "Aug.aug_match";
shortdesc = "return Augeas nodes which match augpath";
longdesc = "\
Returns a list of paths which match the path expression C<path>.
@@ -208,6 +218,7 @@ exactly one node in the current tree." };
{ defaults with
name = "aug_save"; added = (0, 0, 7);
style = RErr, [], [];
+ impl = OCaml "Aug.aug_save";
shortdesc = "write all pending Augeas changes to disk";
longdesc = "\
This writes all pending changes to disk.
@@ -218,6 +229,7 @@ how files are saved." };
{ defaults with
name = "aug_load"; added = (0, 0, 7);
style = RErr, [], [];
+ impl = OCaml "Aug.aug_load";
shortdesc = "load files into the tree";
longdesc = "\
Load files into the tree.
@@ -228,6 +240,7 @@ details." };
{ defaults with
name = "aug_ls"; added = (0, 0, 8);
style = RStringList (RPlainString, "matches"), [String
(PlainString, "augpath")], [];
+ impl = OCaml "Aug.aug_ls";
tests = [
InitBasicFS, Always, TestResult (
[["mkdir"; "/etc"];
@@ -244,6 +257,7 @@ C<path/*> and sorting the resulting nodes into
alphabetical order." };
{ defaults with
name = "aug_clear"; added = (1, 3, 4);
style = RErr, [String (PlainString, "augpath")], [];
+ impl = OCaml "Aug.aug_clear";
shortdesc = "clear Augeas path";
longdesc = "\
Set the value associated with C<path> to C<NULL>. This
@@ -252,6 +266,7 @@ is the same as the L<augtool(1)> C<clear>
command." };
{ defaults with
name = "aug_transform"; added = (1, 35, 2);
style = RErr, [String (PlainString, "lens"); String (PlainString,
"file")], [ OBool "remove"];
+ impl = OCaml "Aug.aug_transform";
shortdesc = "add/remove an Augeas lens transformation";
longdesc = "\
Add an Augeas transformation for the specified C<lens> so it can
diff --git a/generator/actions_core.ml b/generator/actions_core.ml
index cb7e8dcd0..c48845f84 100644
--- a/generator/actions_core.ml
+++ b/generator/actions_core.ml
@@ -8562,6 +8562,7 @@ See also
C<guestfs_journal_get_data_threshold>." };
{ defaults with
name = "aug_setm"; added = (1, 23, 14);
style = RInt "nodes", [String (PlainString, "base");
OptString "sub"; String (PlainString, "val")], [];
+ impl = OCaml "Aug.aug_setm";
tests = [
InitBasicFS, Always, TestResultString (
[["mkdir"; "/etc"];
@@ -8584,6 +8585,7 @@ This returns the number of nodes modified." };
{ defaults with
name = "aug_label"; added = (1, 23, 14);
style = RString (RPlainString, "label"), [String (PlainString,
"augpath")], [];
+ impl = OCaml "Aug.aug_label";
tests = [
InitBasicFS, Always, TestResultString (
[["mkdir"; "/etc"];
--
2.24.1
Pino Toscano
2020-Jan-09 15:07 UTC
[Libguestfs] [PATCH v2 4/4] daemon: drop usage of C augeas library
Since all the usage of augeas is done in OCaml parts (via ocaml-augeas),
drop all the C code using the C augeas library.
---
daemon/Makefile.am | 2 -
daemon/augeas.c | 91 ---------------------------------------------
daemon/cleanups.c | 11 ------
daemon/daemon.h | 34 -----------------
daemon/lvm-filter.c | 2 -
docs/C_SOURCE_FILES | 1 -
po/POTFILES | 1 -
7 files changed, 142 deletions(-)
delete mode 100644 daemon/augeas.c
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 220b934a3..f20dc8584 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -85,7 +85,6 @@ guestfsd_SOURCES = \
acl.c \
actions.h \
available.c \
- augeas.c \
base64.c \
blkdiscard.c \
blkid.c \
@@ -256,7 +255,6 @@ guestfsd_CPPFLAGS = \
guestfsd_CFLAGS = \
$(WARN_CFLAGS) $(WERROR_CFLAGS) \
$(RPC_CFLAGS) \
- $(AUGEAS_CFLAGS) \
$(HIVEX_CFLAGS) \
$(SD_JOURNAL_CFLAGS) \
$(JANSSON_CFLAGS) \
diff --git a/daemon/augeas.c b/daemon/augeas.c
deleted file mode 100644
index d0c6f19a2..000000000
--- a/daemon/augeas.c
+++ /dev/null
@@ -1,91 +0,0 @@
-/* libguestfs - the guestfsd daemon
- * Copyright (C) 2009 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 <string.h>
-#include <unistd.h>
-
-#include <augeas.h>
-
-#include "daemon.h"
-#include "actions.h"
-#include "optgroups.h"
-
-#define FPRINTF_AUGEAS_ERROR(aug,fs,...) \
- do { \
- const int code = aug_error (aug); \
- if (code == AUG_ENOMEM) \
- reply_with_error (fs ": augeas out of memory", ##__VA_ARGS__);
\
- else { \
- const char *aug_err_message = aug_error_message (aug); \
- const char *aug_err_minor = aug_error_minor_message (aug); \
- const char *aug_err_details = aug_error_details (aug); \
- fprintf (stderr, fs ": %s%s%s%s%s", ##__VA_ARGS__,
\
- aug_err_message, \
- aug_err_minor ? ": " : "", aug_err_minor ?
aug_err_minor : "", \
- aug_err_details ? ": " : "", aug_err_details ?
aug_err_details : ""); \
- } \
- } while (0)
-
-int augeas_version;
-
-void
-aug_read_version (void)
-{
- CLEANUP_AUG_CLOSE augeas *ah = NULL;
- int r;
- const char *str;
- int major = 0, minor = 0, patch = 0;
-
- if (augeas_version != 0)
- return;
-
- /* Optimization: do not load the files nor the lenses, since we are
- * only interested in the version.
- */
- ah = aug_init ("/", NULL, AUG_NO_ERR_CLOSE | AUG_NO_LOAD |
AUG_NO_STDINC);
- if (!ah) {
- FPRINTF_AUGEAS_ERROR (ah, "augeas initialization failed");
- return;
- }
-
- if (aug_error (ah) != AUG_NOERROR) {
- FPRINTF_AUGEAS_ERROR (ah, "aug_init");
- return;
- }
-
- r = aug_get (ah, "/augeas/version", &str);
- if (r != 1) {
- FPRINTF_AUGEAS_ERROR (ah, "aug_get");
- return;
- }
-
- r = sscanf (str, "%d.%d.%d", &major, &minor, &patch);
- if (r != 2 && r != 3) {
- fprintf (stderr, "cannot match the version string in
'%s'\n", str);
- return;
- }
-
- if (verbose)
- fprintf (stderr, "augeas version: %d.%d.%d\n", major, minor,
patch);
-
- augeas_version = (major << 16) | (minor << 8) | patch;
-}
diff --git a/daemon/cleanups.c b/daemon/cleanups.c
index b4767178a..0a22d38ef 100644
--- a/daemon/cleanups.c
+++ b/daemon/cleanups.c
@@ -22,19 +22,8 @@
#include <stdlib.h>
#include <unistd.h>
-#include <augeas.h>
-
#include "daemon.h"
-void
-cleanup_aug_close (void *ptr)
-{
- augeas *aug = * (augeas **) ptr;
-
- if (aug != NULL)
- aug_close (aug);
-}
-
void
cleanup_free_stringsbuf (void *ptr)
{
diff --git a/daemon/daemon.h b/daemon/daemon.h
index 72f1b1a4b..41246c40a 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -100,14 +100,11 @@ extern void cleanup_free_mountable (mountable_t
*mountable);
/* These functions are used internally by the CLEANUP_* macros.
* Don't call them directly.
*/
-extern void cleanup_aug_close (void *ptr);
extern void cleanup_free_stringsbuf (void *ptr);
#ifdef HAVE_ATTRIBUTE_CLEANUP
-#define CLEANUP_AUG_CLOSE __attribute__((cleanup(cleanup_aug_close)))
#define CLEANUP_FREE_STRINGSBUF
__attribute__((cleanup(cleanup_free_stringsbuf)))
#else
-#define CLEANUP_AUG_CLOSE
#define CLEANUP_FREE_STRINGSBUF
#endif
@@ -253,20 +250,6 @@ extern int lv_canonical (const char *device, char **ret);
/* zero.c */
extern void wipe_device_before_mkfs (const char *device);
-/* augeas.c */
-extern void aug_read_version (void);
-
-/* The version of augeas, saved as:
- * (MAJOR << 16) | (MINOR << 8) | PATCH
- */
-extern int augeas_version;
-static inline int
-augeas_is_version (int major, int minor, int patch)
-{
- aug_read_version (); /* Lazy version reading. */
- return augeas_version >= ((major << 16) | (minor << 8) |
patch);
-}
-
/* hivex.c */
extern void hivex_finalize (void);
@@ -373,21 +356,4 @@ extern int upload_to_fd (int fd, const char *filename);
} \
while (0)
-/* Calls reply_with_error, but includes the Augeas error details. */
-#define AUGEAS_ERROR(fs,...) \
- do { \
- const int code = aug_error (aug); \
- if (code == AUG_ENOMEM) \
- reply_with_error (fs ": augeas out of memory", ##__VA_ARGS__);
\
- else { \
- const char *message = aug_error_message (aug); \
- const char *minor = aug_error_minor_message (aug); \
- const char *details = aug_error_details (aug); \
- reply_with_error (fs ": %s%s%s%s%s", ##__VA_ARGS__,
\
- message, \
- minor ? ": " : "", minor ? minor
: "", \
- details ? ": " : "", details ?
details : ""); \
- } \
- } while (0)
-
#endif /* GUESTFSD_DAEMON_H */
diff --git a/daemon/lvm-filter.c b/daemon/lvm-filter.c
index c6dd35156..bd9600a26 100644
--- a/daemon/lvm-filter.c
+++ b/daemon/lvm-filter.c
@@ -28,8 +28,6 @@
#include <sys/stat.h>
#include <sys/wait.h>
-#include <augeas.h>
-
#include "c-ctype.h"
#include "ignore-value.h"
diff --git a/docs/C_SOURCE_FILES b/docs/C_SOURCE_FILES
index 26446d4de..8b084ec32 100644
--- a/docs/C_SOURCE_FILES
+++ b/docs/C_SOURCE_FILES
@@ -66,7 +66,6 @@ customize/perl_edit-c.c
daemon/9p.c
daemon/acl.c
daemon/actions.h
-daemon/augeas.c
daemon/available.c
daemon/base64.c
daemon/blkdiscard.c
diff --git a/po/POTFILES b/po/POTFILES
index fa826bc18..2c157ec46 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -49,7 +49,6 @@ customize/crypt-c.c
customize/perl_edit-c.c
daemon/9p.c
daemon/acl.c
-daemon/augeas.c
daemon/available.c
daemon/base64.c
daemon/blkdiscard.c
--
2.24.1
Richard W.M. Jones
2020-Jan-10 22:06 UTC
Re: [Libguestfs] [PATCH v2 4/4] daemon: drop usage of C augeas library
I didn't check every binding in great detail, but the intent seems right and the overall shape of the code is fine. We should find out soon enough if there are bugs. ACK series. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v