[Libguestfs] [PATCH 03/27] daemon: Reimplement ‘file’ API in OCaml.

[Pino Toscano]

On Friday, 14 July 2017 15:39:11 CEST Richard W.M. Jones wrote:
> diff --git a/daemon/file.c b/daemon/file.c
> index 84874dc6f..ee79eb507 100644
> --- a/daemon/file.c
> +++ b/daemon/file.c
> @@ -30,7 +30,6 @@
>  #include "actions.h"
>  #include "optgroups.h"
>  
> -GUESTFSD_EXT_CMD(str_file, file);

When migrating to OCaml, these extra sections in the ELF are not added
anymore.  I read they were added to help packagers (IIRC it started
from the SUSE guys), so they could easily notice the binaries used by
the daemon, and add the proper dependencies.

Do we still want to keep this possibility, somehow?  If not, I'd just
do a wholesale removal of GUESTFSD_EXT_CMD.

> +  if not is_dev then (
> +    let sysroot = Sysroot.sysroot () in
> +    let chroot = Chroot.create sysroot ~name:(sprintf "file: %s" path) in

I notice this pattern done every time, and IMHO it could be simplified:
in utils.ml(i), add something like:

  let create_chroot ?name () =
    Chroot.create (Sysroot.sysroot ()) ?name

this way it can be used like:

  let chroot = create_chroot ~name:(sprintf "file: %s" path) in

> +
> +    let statbuf = Chroot.f chroot lstat path in

Hm is chroot needed for this?  The current C implementation does not
use CHROOT_IN/OUT, and it does not even resolve symlinks, so it should
be safe.

-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20170719/cdff8356/attachment.sig>