3:36 p.m.
v1 was posted here:
https://www.redhat.com/archives/libguestfs/2017-July/msg00098.html
This series now depends on two small patches which I posted separately:
https://www.redhat.com/archives/libguestfs/2017-July/msg00207.html
https://www.redhat.com/archives/libguestfs/2017-July/msg00209.html
v1 -> v2:
- Previously changes to generator/daemon.ml were made incrementally
through the patch series. Now these changes are moved into the
first patch. This wasn't quite straightforward because I also had
to move the static functions into a separate file so that bisection
wouldn't break.
- Related to previous change, move ocaml_exn_to_reply_with_error to
daemon/daemon-c.c, and rename it
guestfs_int_daemon_exn_to_reply_with_error.
- Move separate patches implementing optgroups and
CommandFlagFoldStdoutOnStderr into the first patch.
- daemon/Makefile.am: Deduplicate BUILT_SOURCES & generator_built vars.
- daemon/chroot.mli: In a comment, fix misspelled parmeter -> parameter.
- daemon/utils.ml: Use stringify_args.
- Add a chomp function to Std_utils [patch posted separately]
and use it in daemon/utils.ml.
- Add Sysroot.sysroot_path function which works like the C function
and use it throughout.
- Modify Chroot.create with optional ?chroot parameter, so you don't
need to call Sysroot followed by Chroot in two steps.
- get_blkid_tag: Add device name to error message as in C equiv.
- get_blkid_tag: Use String.chomp instead of String.trimr, since
that's closer to what the C code does.
- Replace O_CLOEXEC with Unix.set_close_on_exec. We'll need to fix
this properly at some point, especially if we make the daemon
multithreaded.
- Simplify (fun dev -> "/dev/" ^ dev) -> ((^) "/dev/") as
suggested.
- Factored the functions in daemon/is.ml about as far as is possible.
- daemon/parted.ml: Use triml before sscanf.
- daemon/filearch.ml: Remove comment about "caller must free".
- daemon/filearch.ml: Use Unix_utils.Mkdtemp instead of making by hand,
and clean up afterwards.
- daemon/ldm.ml: Factor the two functions as far as possible.
- daemon/lvm.ml: Readd the comment above lvs_has_S_opt which was
accidentally dropped during conversion.
- daemon/lvm.ml: Optimize handling of prefix in convert_lvm_output.
- daemon/optgroups.c: Remove unnecessary <stdio.h>, <stdlib.h> additions.
- daemon/optgroups.c: Use optgroup_names so we don't need to check
for non-retired groups.
- daemon/btrfs.ml: Rewrite ‘with_mounted’ helper to use Mkdtemp.
- daemon/btrfs.ml: Filter empty lines.
- daemon/listfs.ml: Use StringSet to simplify filtering the
devices.
- daemon/parted.ml: Fix print_partition_table and add a comment
so it's clear this version only works in -m mode.
- daemon/md.ml: Filter empty lines.
- Reran the tests.
Requested, but not done for various reasons:
- Combining Utils, Sysroot, Chroot modules into Daemon. Not possible
because of the order of initialization of the modules (Daemon must
come after everything else, whereas the others must come before
everything else). It would be possible to combine Sysroot and
Chroot together completely , but since they are separate modules
with different purposes there didn't seem much point. However I
did modify the Chroot.create ?chroot function -- see above.
- Writing C bindings to command* functions: too complex, and the OCaml
replacements are better anyway.
- Reimplement udev_settle{,_file} as wrappers around C functions.
I tried to make this change but it is unexpectedly complex, because
it would mean that daemon/utils.ml contains a C function which can
only be satisfied by linking to the daemon, but daemon_utils_tests
is a standalone program which cannot be linked to the daemon. So
we'd either need to create another OCaml module, or split up the C
parts of the daemon into "library bits" and "daemon bits".
- Removing GUESTFSD_EXT_CMD. This is still being used by OpenSUSE, so
we'll have to think of a better mechanism for it. Something like
‘guestfsd --dump-commands’ may work better.
- Replace compare with subtraction in daemon/devsparts.ml. I find the
compare version easier to understand.
- In parted.ml, didn't replace sscanf " %x", since my reading is that
OCaml sscanf works the same way as C sscanf with regard to spaces,
ie. it will eat any amount of leading whitespace.
- Generating:
external available : unit -> bool = "guestfs_int_daemon_optgroup..."
etc. It's hard to generate these functions and have them added to the
correct modules (since the OCaml modules are hand-written, and it's
awkward to include generated bits in them).
Other notes:
- daemon must be linked to -ldl -lm because the OCaml runtime
(libasmrun.so) depends on both.
- Structs must be repeated in *.mli files because you can't use OCaml
‘include’ to pick up single structs from another module. However
the structs are still type checked by the compiler, so we cannot
write incorrect / untyped code this way.
Rich.