[Libguestfs] [PATCH v11 1/6] builder: add simple OCaml osinfo-db reader

[Richard W.M. Jones]

On Thu, Oct 05, 2017 at 04:58:25PM +0200, Cédric Bosdonnat wrote:
> +let rec iterate_db fn =
> +  let locations = ref [] in
> +
> +  (* (1) Try the shared osinfo directory, using either the
> +   * $OSINFO_SYSTEM_DIR envvar or its default value.
> +   *)
> +  let () =
> +    let dir =
> +      try Sys.getenv "OSINFO_SYSTEM_DIR"
> +      with Not_found -> "/usr/share/osinfo" in
> +    push_back locations ((dir // "os"), read_osinfo_db_three_levels)
> +  in
> +
> +  (* (2) Try the libosinfo directory, using the newer three-directory
> +   * layout ($LIBOSINFO_DB_PATH / "os" / $group-ID / [file.xml]).
> +   *)
> +  let () =
> +    let path = Osinfo_config.libosinfo_db_path // "os" in
> +    push_back locations (path, read_osinfo_db_three_levels)
> +  in
> +
> +  (* (3) Try the libosinfo directory, using the old flat directory
> +   * layout ($LIBOSINFO_DB_PATH / "oses" / [file.xml]).
> +   *)
> +  let () =
> +    let path = Osinfo_config.libosinfo_db_path // "oses" in
> +    push_back locations (path, read_osinfo_db_flat)
> +  in
> +
> +  let rec loop = function
> +    | (path, f) :: paths ->
> +      if is_directory path then f fn path
> +      (* This is not an error: RHBZ#948324. *)
> +      else loop paths
> +    | [] -> ()
> +  in
> +
> +  loop !locations

This is just a stylistic thing but it seems as if this could be
written more simply as something like:

  let locations =
    let dir =
      try Sys.getenv "OSINFO_SYSTEM_DIR"
      with Not_found -> "/usr/share/osinfo" in
    ((dir // "os"), read_osinfo_db_three_levels) @
    ... same for the other two cases ...

You might or might not need parens around the (let dir = ...) @

The rest of the commit seems fine.

Rich.

> +and read_osinfo_db_three_levels fn path =
> +  debug "osinfo: loading 3-level-directories database from %s" path;
> +  let entries = Array.to_list (Sys.readdir path) in
> +  let entries = List.map ((//) path) entries in
> +  (* Iterate only on directories. *)
> +  let entries = List.filter is_directory entries in
> +  List.iter (read_osinfo_db_directory fn) entries
> +
> +and read_osinfo_db_flat fn path =
> +  debug "osinfo: loading flat database from %s" path;
> +  read_osinfo_db_directory fn path
> +
> +and read_osinfo_db_directory fn path =
> +  let entries = Array.to_list (Sys.readdir path) in
> +  let entries = List.filter (fun x -> Filename.check_suffix x ".xml") entries in
> +  let entries = List.map ((//) path) entries in
> +  let entries = List.filter is_regular_file entries in
> +  List.iter fn entries
> +
> diff --git a/builder/osinfo.mli b/builder/osinfo.mli
> new file mode 100644
> index 000000000..949d776a9
> --- /dev/null
> +++ b/builder/osinfo.mli
> @@ -0,0 +1,22 @@
> +(* virt-builder
> + * 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.
> + *)
> +
> +val iterate_db : (string -> unit) -> unit
> +(** [iterate_db fun] iterates over the osinfo-db/libosinfo database
> +    of OS definitions, invoking the specified [fun] on each XML file.
> + *)
> -- 
> 2.13.2
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs

-- 
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