On Tuesday, 3 January 2017 11:18:55 CET Cédric Bosdonnat wrote:
There is already a libosinfo reading function located in src folder to get the iso informations. Provide a similar but more generic function to be used in ocaml tools. ---
I'd rather avoid duplicating the logic used for reading the osinfo-db, especially that a) it was touched recently to support the newer osinfo-db, the newer internal libosinfo-db, and the old internal libosinfo-db b) we might support more from the osinfo-db spec in the future (e.g. OSINFO_LOCAL_DIR and OSINFO_USER_DIR), especially since they are declared "MUST" A way to do this could be the following (at least IMHO): 1) adapt the current C code (src/osinfo.c) to use a callback (with void *opaque) to parse each XML file -- in short, adding a new parameter to read_osinfo_db_xml 2) separate osinfo.c in two parts: 2.a) what reads the osinfo-db and invokes a callback 2.b) the libguestfs DB (osinfo_db, osinfo_db_size) with read_osinfo_db_xml as callback to fill it 3) expose (2.a) to OCaml, with an implementation similar to what Rich did recently for the visit API -- see cat/visit.c, mllib/visit-c.c, and mllib/visit.ml 4) build & use (2.b) + (3) in virt-builder This way we would have just the OCaml callback to read the XML file, and just collecting the list of short-id's.
+ fun os_file -> + let file_path = distro_path // os_file in + let xml = read_whole_file file_path in + let doc = Xml.parse_memory xml in
Hmm I've seen this pattern already, and IMHO it's time to change it: the Xml module needs also a parse_file method to parse an existing file, instead of keeping it all in memory. Other users of this new parse_file are in v2v/input_ova.ml, and v2v/test-harness/v2v_test_harness.ml.
+let osinfo_read_db filter = + let path = try + Sys.getenv "OSINFO_SYSTEM_DIR" + with Not_found -> "/usr/share/osinfo" in + + try osinfo_db_read_three_levels (path // "os") filter with + | _ -> + try osinfo_db_read_three_levels (osinfopath // "os") filter with + | _ -> + try osinfo_db_read_flat (osinfopath // "oses") filter with + | _ -> []
Note that this chain of try..with is too "coarse", and swallows also errors which needs to be considered fatal (I/O errors, XML issues).
diff --git a/mllib/osinfo.mli b/mllib/osinfo.mli new file mode 100644 index 000000000..4b881408e --- /dev/null +++ b/mllib/osinfo.mli @@ -0,0 +1,21 @@ +(* virt-builder + * Copyright (C) 2016 - SUSE 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 osinfo_read_db : (Xml.xpathctx -> 'a) -> 'a list +(** [osinfo_read_db filter] runs the [filter] function on the + XPath context of every osinfo os description XML file. *)
diff --git a/mllib/osinfopath.ml b/mllib/osinfopath.ml new file mode 100644 index 000000000..a58b8d60f --- /dev/null +++ b/mllib/osinfopath.ml @@ -0,0 +1 @@ +let osinfopath = "/usr/local/share/libosinfo/db"
Note that a generated file should not be in the patch, and needs to be added to the top level .gitignore. Thanks, -- Pino Toscano