[PATCH] inspector: --xpath: Copy node to new document (RHBZ#1281577).
by Richard W.M. Jones
'virt-inspector --xpath' can segfault.
When run under valgrind, it shows this error:
==2254== Invalid free() / delete / delete[] / realloc()
==2254== at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==2254== by 0x53BA198: xmlFreeNodeList (tree.c:3690)
==2254== by 0x53B9F65: xmlFreeDoc (tree.c:1247)
==2254== by 0x405BFA: do_xpath (inspector.c:808)
==2254== by 0x405BFA: main (inspector.c:250)
==2254== Address 0x1030a037 is 311 bytes inside a block of size 1,048 alloc'd
==2254== at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==2254== by 0x545DE86: xmlDictAddString.isra.0 (dict.c:270)
==2254== by 0x545E961: xmlDictLookup (dict.c:923)
==2254== by 0x539C6DC: xmlDetectSAX2 (parser.c:1067)
==2254== by 0x53B0B92: xmlParseDocument (parser.c:10725)
==2254== by 0x53B1276: xmlDoRead (parser.c:15295)
==2254== by 0x40587D: do_xpath (inspector.c:772)
==2254== by 0x40587D: main (inspector.c:250)
The cause appears to be that when copying the matching node(s) found
by the xpath expression, we have to copy them into the new document
(using xmlDocCopyNode instead of xmlCopyNode).
This bug has existed since this functionality was originally added in
commit d1ee71782ace98a11c5aabaf1f9fd5f601e08367.
---
inspector/inspector.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/inspector/inspector.c b/inspector/inspector.c
index d0013ed..dd5be44 100644
--- a/inspector/inspector.c
+++ b/inspector/inspector.c
@@ -811,7 +811,7 @@ do_xpath (const char *query)
guestfs_int_program_name);
exit (EXIT_FAILURE);
}
- wrnode = xmlCopyNode (nodes->nodeTab[i], 1);
+ wrnode = xmlDocCopyNode (nodes->nodeTab[i], wrdoc, 1);
if (wrnode == NULL) {
fprintf (stderr, _("%s: xmlCopyNode failed\n"),
guestfs_int_program_name);
--
2.5.0
8 years, 6 months
[PATCH 1/2] dib: Make the interface between cmdline.ml and dib.ml explicit.
by Richard W.M. Jones
---
dib/Makefile.am | 5 ++-
dib/cmdline.ml | 49 +++++++++++++++++++++---
dib/cmdline.mli | 51 +++++++++++++++++++++++++
dib/dib.ml | 113 ++++++++++++++++++++++++++++++--------------------------
4 files changed, 158 insertions(+), 60 deletions(-)
create mode 100644 dib/cmdline.mli
diff --git a/dib/Makefile.am b/dib/Makefile.am
index 0786d64..ad1fd6a 100644
--- a/dib/Makefile.am
+++ b/dib/Makefile.am
@@ -18,11 +18,14 @@
include $(top_srcdir)/subdir-rules.mk
EXTRA_DIST = \
- $(SOURCES_ML) $(SOURCES_C) \
+ $(SOURCES_MLI) $(SOURCES_ML) $(SOURCES_C) \
virt-dib.pod
CLEANFILES = *~ *.annot *.cmi *.cmo *.cmx *.cmxa *.o virt-dib
+SOURCES_MLI = \
+ cmdline.mli
+
SOURCES_ML = \
utils.ml \
cmdline.ml \
diff --git a/dib/cmdline.ml b/dib/cmdline.ml
index 4aa6a53..3a97366 100644
--- a/dib/cmdline.ml
+++ b/dib/cmdline.ml
@@ -25,7 +25,37 @@ open Utils
open Printf
-let parse_args () =
+type cmdline = {
+ debug : int;
+ basepath : string;
+ elements : string list;
+ excluded_elements : string list;
+ element_paths : string list;
+ excluded_scripts : string list;
+ use_base : bool;
+ drive : string option;
+ image_name : string;
+ fs_type : string;
+ size : int64;
+ root_label : string option;
+ install_type : string;
+ image_cache : string option;
+ compressed : bool;
+ qemu_img_options : string option;
+ mkfs_options : string option;
+ is_ramdisk : bool;
+ ramdisk_element : string;
+ extra_packages : string list;
+ memsize : int option;
+ network : bool;
+ smp : int option;
+ delete_on_failure : bool;
+ formats : string list;
+ arch : string;
+ envvars : string list;
+}
+
+let parse_cmdline () =
let usage_msg =
sprintf (f_"\
%s: run diskimage-builder elements to generate images
@@ -220,8 +250,15 @@ read the man page virt-dib(1).
if elements = [] then
error (f_"at least one distribution root element must be specified");
- debug, basepath, elements, excluded_elements, element_paths,
- excluded_scripts, use_base, drive,
- image_name, fs_type, size, root_label, install_type, image_cache, compressed,
- qemu_img_options, mkfs_options, is_ramdisk, ramdisk_element, extra_packages,
- memsize, network, smp, delete_on_failure, formats, arch, envvars
+ { debug = debug; basepath = basepath; elements = elements;
+ excluded_elements = excluded_elements; element_paths = element_paths;
+ excluded_scripts = excluded_scripts; use_base = use_base; drive = drive;
+ image_name = image_name; fs_type = fs_type; size = size;
+ root_label = root_label; install_type = install_type;
+ image_cache = image_cache; compressed = compressed;
+ qemu_img_options = qemu_img_options; mkfs_options = mkfs_options;
+ is_ramdisk = is_ramdisk; ramdisk_element = ramdisk_element;
+ extra_packages = extra_packages; memsize = memsize; network = network;
+ smp = smp; delete_on_failure = delete_on_failure;
+ formats = formats; arch = arch; envvars = envvars;
+ }
diff --git a/dib/cmdline.mli b/dib/cmdline.mli
new file mode 100644
index 0000000..0a1aa9d
--- /dev/null
+++ b/dib/cmdline.mli
@@ -0,0 +1,51 @@
+(* virt-dib
+ * Copyright (C) 2015 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.
+ *)
+
+(** Command line argument parsing. *)
+
+type cmdline = {
+ debug : int;
+ basepath : string;
+ elements : string list;
+ excluded_elements : string list;
+ element_paths : string list;
+ excluded_scripts : string list;
+ use_base : bool;
+ drive : string option;
+ image_name : string;
+ fs_type : string;
+ size : int64;
+ root_label : string option;
+ install_type : string;
+ image_cache : string option;
+ compressed : bool;
+ qemu_img_options : string option;
+ mkfs_options : string option;
+ is_ramdisk : bool;
+ ramdisk_element : string;
+ extra_packages : string list;
+ memsize : int option;
+ network : bool;
+ smp : int option;
+ delete_on_failure : bool;
+ formats : string list;
+ arch : string;
+ envvars : string list;
+}
+
+val parse_cmdline : unit -> cmdline
diff --git a/dib/dib.ml b/dib/dib.ml
index fdb5857..4a0c9ee 100644
--- a/dib/dib.ml
+++ b/dib/dib.ml
@@ -432,28 +432,24 @@ let run_install_packages ~debug ~blockdev ~log_file
out
let main () =
- let debug, basepath, elements, excluded_elements, element_paths,
- excluded_scripts, use_base, drive,
- image_name, fs_type, size, root_label, install_type, image_cache, compressed,
- qemu_img_options, mkfs_options, is_ramdisk, ramdisk_element, extra_packages,
- memsize, network, smp, delete_on_failure, formats, arch, envvars =
- parse_args () in
+ let cmdline = parse_cmdline () in
+ let debug = cmdline.debug in
(* Check that the specified base directory of diskimage-builder
* has the "die" script in it, so we know the directory is the
* right one (hopefully so, at least).
*)
- if not (Sys.file_exists (basepath // "die")) then
+ if not (Sys.file_exists (cmdline.basepath // "die")) then
error (f_"the specified base path is not the diskimage-builder library");
(* Check for required tools. *)
require_tool "uuidgen";
- if List.mem "qcow2" formats then
+ if List.mem "qcow2" cmdline.formats then
require_tool "qemu-img";
- if List.mem "vhd" formats then
+ if List.mem "vhd" cmdline.formats then
require_tool "vhd-util";
- let image_basename = Filename.basename image_name in
+ let image_basename = Filename.basename cmdline.image_name in
let image_basename_d = image_basename ^ ".d" in
let tmpdir = Mkdtemp.temp_dir "dib." "" in
@@ -465,15 +461,19 @@ let main () =
let extradatatmpdir = tmpdir // "extra-data" in
do_mkdir extradatatmpdir;
do_mkdir (auxtmpdir // "out" // image_basename_d);
- let elements = if use_base then ["base"] @ elements else elements in
- let elements = if is_ramdisk then [ramdisk_element] @ elements else elements in
+ let elements =
+ if cmdline.use_base then ["base"] @ cmdline.elements
+ else cmdline.elements in
+ let elements =
+ if cmdline.is_ramdisk then [cmdline.ramdisk_element] @ elements
+ else elements in
message (f_"Elements: %s") (String.concat " " elements);
if debug >= 1 then (
printf "tmpdir: %s\n" tmpdir;
- printf "element paths: %s\n" (String.concat ":" element_paths);
+ printf "element paths: %s\n" (String.concat ":" cmdline.element_paths);
);
- let loaded_elements = load_elements ~debug element_paths in
+ let loaded_elements = load_elements ~debug cmdline.element_paths in
if debug >= 1 then (
printf "loaded elements:\n";
Hashtbl.iter (
@@ -488,11 +488,11 @@ let main () =
);
let all_elements = load_dependencies elements loaded_elements in
let all_elements = exclude_elements all_elements
- (excluded_elements @ builtin_elements_blacklist) in
+ (cmdline.excluded_elements @ builtin_elements_blacklist) in
message (f_"Expanded elements: %s") (String.concat " " (StringSet.elements all_elements));
- let envvars = read_envvars envvars in
+ let envvars = read_envvars cmdline.envvars in
message (f_"Carried environment variables: %s") (String.concat " " (List.map fst envvars));
if debug >= 1 then (
printf "carried over envvars:\n";
@@ -515,7 +515,7 @@ let main () =
message (f_"Preparing auxiliary data");
copy_elements all_elements loaded_elements
- (excluded_scripts @ builtin_scripts_blacklist) hookstmpdir;
+ (cmdline.excluded_scripts @ builtin_scripts_blacklist) hookstmpdir;
(* Re-read the hook scripts from the hooks dir, as d-i-b (and we too)
* has basically copied over anything found in elements.
@@ -525,24 +525,24 @@ let main () =
let log_file = "/tmp/aux/perm/" ^ (log_filename ()) in
let arch =
- match arch with
+ match cmdline.arch with
| "" -> current_arch ()
| arch -> arch in
let root_label =
- match root_label with
+ match cmdline.root_label with
| None ->
(* XFS has a limit of 12 characters for filesystem labels.
* Not changing the default for other filesystems to maintain
* backwards compatibility.
*)
- (match fs_type with
+ (match cmdline.fs_type with
| "xfs" -> "img-rootfs"
| _ -> "cloudimg-rootfs")
| Some label -> label in
let image_cache =
- match image_cache with
+ match cmdline.image_cache with
| None -> Sys.getenv "HOME" // ".cache" // "image-create"
| Some dir -> dir in
do_mkdir image_cache;
@@ -553,29 +553,32 @@ let main () =
function
| "qcow2" | "raw" | "vhd" -> true
| _ -> false
- ) formats in
+ ) cmdline.formats in
let formats_img_nonraw = List.filter ((<>) "raw") formats_img in
prepare_aux ~envvars ~dib_args ~dib_vars ~log_file ~out_name:image_basename
- ~rootfs_uuid ~arch ~network ~root_label ~install_type ~debug
- ~extra_packages
- auxtmpdir all_elements;
+ ~rootfs_uuid ~arch ~network:cmdline.network ~root_label
+ ~install_type:cmdline.install_type ~debug
+ ~extra_packages:cmdline.extra_packages
+ auxtmpdir all_elements;
- let delete_output_file = ref delete_on_failure in
+ let delete_output_file = ref cmdline.delete_on_failure in
let delete_file () =
if !delete_output_file then (
List.iter (
fun fmt ->
- try Unix.unlink (output_filename image_name fmt) with _ -> ()
- ) formats
+ try Unix.unlink (output_filename cmdline.image_name fmt) with _ -> ()
+ ) cmdline.formats
)
in
at_exit delete_file;
prepare_external ~dib_args ~dib_vars ~out_name:image_basename ~root_label
- ~rootfs_uuid ~image_cache ~arch ~network ~debug
- tmpdir basepath hookstmpdir extradatatmpdir (auxtmpdir // "fake-bin")
- all_elements element_paths;
+ ~rootfs_uuid ~image_cache ~arch ~network:cmdline.network
+ ~debug
+ tmpdir cmdline.basepath hookstmpdir extradatatmpdir
+ (auxtmpdir // "fake-bin")
+ all_elements cmdline.element_paths;
let run_hook_host hook =
try
@@ -623,13 +626,14 @@ let main () =
message (f_"Opening the disks");
- let is_ramdisk_build = is_ramdisk || StringSet.mem "ironic-agent" all_elements in
+ let is_ramdisk_build =
+ cmdline.is_ramdisk || StringSet.mem "ironic-agent" all_elements in
let g, tmpdisk, tmpdiskfmt, drive_partition =
let g = open_guestfs () in
- may g#set_memsize memsize;
- may g#set_smp smp;
- g#set_network network;
+ may g#set_memsize cmdline.memsize;
+ may g#set_smp cmdline.smp;
+ g#set_network cmdline.network;
(* Make sure to turn SELinux off to avoid awkward interactions
* between the appliance kernel and applications/libraries interacting
@@ -643,17 +647,19 @@ let main () =
(* If "raw" is among the selected outputs, use it as main backing
* disk, otherwise create a temporary disk.
*)
- if not is_ramdisk_build && List.mem "raw" formats_img then image_name
- else Filename.temp_file ~temp_dir:tmpdir "image." "" in
+ if not is_ramdisk_build && List.mem "raw" formats_img then
+ cmdline.image_name
+ else
+ Filename.temp_file ~temp_dir:tmpdir "image." "" in
let fn = output_filename fn fmt in
(* Produce the output image. *)
- g#disk_create fn fmt size;
+ g#disk_create fn fmt cmdline.size;
g#add_drive ~readonly:false ~format:fmt fn;
(* Helper drive for elements and binaries. *)
g#add_drive_scratch (unit_GB 5);
- (match drive with
+ (match cmdline.drive with
| None ->
g#add_drive_scratch (unit_GB 5)
| Some drive ->
@@ -667,12 +673,12 @@ let main () =
g#mount "/dev/sdb" "/";
copy_in g auxtmpdir "/";
- copy_in g basepath "/lib";
+ copy_in g cmdline.basepath "/lib";
g#umount "/";
(* Prepare the /aux/perm partition. *)
let drive_partition =
- match drive with
+ match cmdline.drive with
| None ->
g#mkfs "ext2" "/dev/sdc";
"/dev/sdc"
@@ -758,11 +764,11 @@ let main () =
(* Create and mount the target filesystem. *)
let mkfs_options =
- match mkfs_options with
+ match cmdline.mkfs_options with
| None -> []
| Some o -> [ o ] in
let mkfs_options =
- (match fs_type with
+ (match cmdline.fs_type with
| "ext4" ->
(* Very conservative to handle images being resized a lot
* Without -J option specified, default journal size will be set to 32M
@@ -770,10 +776,10 @@ let main () =
*)
[ "-i"; "4096"; "-J"; "size=64" ]
| _ -> []
- ) @ mkfs_options @ [ "-t"; fs_type; blockdev ] in
+ ) @ mkfs_options @ [ "-t"; cmdline.fs_type; blockdev ] in
ignore (g#debug "sh" (Array.of_list ([ "mkfs" ] @ mkfs_options)));
g#set_label blockdev root_label;
- (match fs_type with
+ (match cmdline.fs_type with
| x when String.is_prefix x "ext" -> g#set_uuid blockdev rootfs_uuid
| _ -> ());
g#mount blockdev "/";
@@ -805,8 +811,9 @@ let main () =
run_hook_in "pre-install.d";
- if extra_packages <> [] then
- ignore (run_install_packages ~debug ~blockdev ~log_file g extra_packages);
+ if cmdline.extra_packages <> [] then
+ ignore (run_install_packages ~debug ~blockdev ~log_file g
+ cmdline.extra_packages);
run_hook_in "install.d";
@@ -832,8 +839,8 @@ let main () =
if g#ls out_dir <> [||] then (
message (f_"Extracting data out of the image");
- do_mkdir (image_name ^ ".d");
- g#copy_out out_dir (Filename.dirname image_name);
+ do_mkdir (cmdline.image_name ^ ".d");
+ g#copy_out out_dir (Filename.dirname cmdline.image_name);
);
(* Unmount everything, and remount only the root to cleanup
@@ -849,7 +856,7 @@ let main () =
List.iter (
fun fmt ->
- let fn = output_filename image_name fmt in
+ let fn = output_filename cmdline.image_name fmt in
match fmt with
| "tar" ->
message (f_"Compressing the image as tar");
@@ -875,17 +882,17 @@ let main () =
if not is_ramdisk_build then (
List.iter (
fun fmt ->
- let fn = output_filename image_name fmt in
+ let fn = output_filename cmdline.image_name fmt in
message (f_"Converting to %s") fmt;
match fmt with
| "qcow2" ->
let cmd =
sprintf "qemu-img convert%s -f %s %s -O %s%s %s"
- (if compressed then " -c" else "")
+ (if cmdline.compressed then " -c" else "")
tmpdiskfmt
(quote tmpdisk)
fmt
- (match qemu_img_options with
+ (match cmdline.qemu_img_options with
| None -> ""
| Some opt -> " -o " ^ quote opt)
(quote (qemu_input_filename fn)) in
--
2.5.0
8 years, 6 months
[PATCH] sparsify: Make the interface between cmdline.ml and sparsify.ml explicit.
by Richard W.M. Jones
We could go a bit further here and push the cmdline struct
into Copying.run and In_place.run.
---
sparsify/Makefile.am | 5 ++++-
sparsify/cmdline.ml | 23 +++++++++++++++++++----
sparsify/cmdline.mli | 36 ++++++++++++++++++++++++++++++++++++
sparsify/sparsify.ml | 13 +++++++------
4 files changed, 66 insertions(+), 11 deletions(-)
create mode 100644 sparsify/cmdline.mli
diff --git a/sparsify/Makefile.am b/sparsify/Makefile.am
index 33f418b..d99f311 100644
--- a/sparsify/Makefile.am
+++ b/sparsify/Makefile.am
@@ -18,13 +18,16 @@
include $(top_srcdir)/subdir-rules.mk
EXTRA_DIST = \
- $(SOURCES_ML) $(SOURCES_C) \
+ $(SOURCES_MLI) $(SOURCES_ML) $(SOURCES_C) \
virt-sparsify.pod \
test-virt-sparsify.sh \
test-virt-sparsify-in-place.sh
CLEANFILES = *~ *.annot *.cmi *.cmo *.cmx *.cmxa *.o virt-sparsify
+SOURCES_MLI = \
+ cmdline.mli
+
SOURCES_ML = \
utils.ml \
cmdline.ml \
diff --git a/sparsify/cmdline.ml b/sparsify/cmdline.ml
index 868456f..8f2b721 100644
--- a/sparsify/cmdline.ml
+++ b/sparsify/cmdline.ml
@@ -25,9 +25,18 @@ open Common_utils
open Utils
-type mode_t =
-| Mode_copying of string * check_t * bool * string option * string option *
- string option
+type cmdline = {
+ indisk : string;
+ format : string option;
+ ignores : string list;
+ machine_readable : bool;
+ zeroes : string list;
+ mode : mode_t;
+}
+
+and mode_t =
+| Mode_copying of
+ string * check_t * bool * string option * string option * string option
| Mode_in_place
and check_t = [`Ignore|`Continue|`Warn|`Fail]
@@ -175,4 +184,10 @@ read the man page virt-sparsify(1).
else
Mode_in_place in
- indisk, format, ignores, machine_readable, zeroes, mode
+ { indisk = indisk;
+ format = format;
+ ignores = ignores;
+ machine_readable = machine_readable;
+ zeroes = zeroes;
+ mode = mode;
+ }
diff --git a/sparsify/cmdline.mli b/sparsify/cmdline.mli
new file mode 100644
index 0000000..706ecc9
--- /dev/null
+++ b/sparsify/cmdline.mli
@@ -0,0 +1,36 @@
+(* virt-sparsify
+ * Copyright (C) 2011-2015 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.
+ *)
+
+(** Command line argument parsing. *)
+
+type cmdline = {
+ indisk : string;
+ format : string option;
+ ignores : string list;
+ machine_readable : bool;
+ zeroes : string list;
+ mode : mode_t;
+}
+
+and mode_t =
+| Mode_copying of
+ string * check_t * bool * string option * string option * string option
+| Mode_in_place
+and check_t = [`Ignore|`Continue|`Warn|`Fail]
+
+val parse_cmdline : unit -> cmdline
diff --git a/sparsify/sparsify.ml b/sparsify/sparsify.ml
index 30e3020..b40dbf4 100644
--- a/sparsify/sparsify.ml
+++ b/sparsify/sparsify.ml
@@ -30,15 +30,16 @@ module G = Guestfs
let () = Random.self_init ()
let rec main () =
- let indisk, format, ignores, machine_readable, zeroes, mode =
- parse_cmdline () in
+ let cmdline = parse_cmdline () in
- (match mode with
+ (match cmdline.mode with
| Mode_copying (outdisk, check_tmpdir, compress, convert, option, tmp) ->
- Copying.run indisk outdisk check_tmpdir compress convert
- format ignores machine_readable option tmp zeroes
+ Copying.run cmdline.indisk outdisk check_tmpdir compress convert
+ cmdline.format cmdline.ignores cmdline.machine_readable
+ option tmp cmdline.zeroes
| Mode_in_place ->
- In_place.run indisk format ignores machine_readable zeroes
+ In_place.run cmdline.indisk cmdline.format cmdline.ignores
+ cmdline.machine_readable cmdline.zeroes
)
let () = run_main_and_handle_errors main
--
2.5.0
8 years, 6 months
[PATCH 0/4]: mllib: Add 'may' function, and refactoring.
by Richard W.M. Jones
The 'may' function is a higher-order function (HOF) that replaces:
match x with
| None -> ()
| Some x -> f x
with:
may f x
The idea comes from lablgtk (OCaml Gtk bindings) where it is widely
used.
If this change is clearer than previous code, then this could be used
in many more places. However I previously steered clear from using
HOFs like this because they can be quite confusing for newcomers to
functional programming.
Rich.
8 years, 6 months
[PATCH] builder: Make the interface between cmdline.ml and builder.ml explicit.
by Richard W.M. Jones
---
builder/Makefile.am | 1 +
builder/builder.ml | 69 +++++++++++++++++++++++++----------------------------
builder/cmdline.ml | 37 ++++++++++++++++++++++++----
builder/cmdline.mli | 44 ++++++++++++++++++++++++++++++++++
4 files changed, 110 insertions(+), 41 deletions(-)
create mode 100644 builder/cmdline.mli
diff --git a/builder/Makefile.am b/builder/Makefile.am
index 6742822..993cc7b 100644
--- a/builder/Makefile.am
+++ b/builder/Makefile.am
@@ -53,6 +53,7 @@ CLEANFILES = \
SOURCES_MLI = \
cache.mli \
+ cmdline.mli \
downloader.mli \
checksums.mli \
index.mli \
diff --git a/builder/builder.ml b/builder/builder.ml
index b0fef48..957bc37 100644
--- a/builder/builder.ml
+++ b/builder/builder.ml
@@ -1,5 +1,5 @@
(* virt-builder
- * Copyright (C) 2013 Red Hat Inc.
+ * Copyright (C) 2013-2015 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
@@ -77,11 +77,7 @@ let remove_duplicates index =
let main () =
(* Command line argument parsing - see cmdline.ml. *)
- let mode, arg,
- arch, attach, cache, check_signature, curl,
- delete_on_failure, format, gpg, list_format, memsize,
- network, ops, output, size, smp, sources, sync =
- parse_cmdline () in
+ let cmdline = parse_cmdline () in
(* If debugging, echo the command line arguments and the sources. *)
if verbose () then (
@@ -91,29 +87,29 @@ let main () =
iteri (
fun i (source, fingerprint) ->
printf "source[%d] = (%S, %S)\n" i source fingerprint
- ) sources
+ ) cmdline.sources
);
(* Handle some modes here, some later on. *)
let mode =
- match mode with
+ match cmdline.mode with
| `Get_kernel -> (* --get-kernel is really a different program ... *)
let cmd =
sprintf "virt-get-kernel%s%s%s%s --add %s"
(if verbose () then " --verbose" else "")
(if trace () then " -x" else "")
- (match format with
+ (match cmdline.format with
| None -> ""
| Some format -> sprintf " --format %s" (quote format))
- (match output with
+ (match cmdline.output with
| None -> ""
| Some output -> sprintf " --output %s" (quote output))
- (quote arg) in
+ (quote cmdline.arg) in
if verbose () then printf "%s\n%!" cmd;
exit (Sys.command cmd)
| `Delete_cache -> (* --delete-cache *)
- (match cache with
+ (match cmdline.cache with
| Some cachedir ->
message (f_"Deleting: %s") cachedir;
Cache.clean_cachedir cachedir;
@@ -129,16 +125,16 @@ let main () =
(* Check that gpg is installed. Optional as long as the user
* disables all signature checks.
*)
- let cmd = sprintf "%s --help >/dev/null 2>&1" gpg in
+ let cmd = sprintf "%s --help >/dev/null 2>&1" cmdline.gpg in
if Sys.command cmd <> 0 then (
- if check_signature then
+ if cmdline.check_signature then
error (f_"gpg is not installed (or does not work)\nYou should install gpg, or use --gpg option, or use --no-check-signature.")
else if verbose () then
warning (f_"gpg program is not available")
);
(* Check that curl works. *)
- let cmd = sprintf "%s --help >/dev/null 2>&1" curl in
+ let cmd = sprintf "%s --help >/dev/null 2>&1" cmdline.curl in
if Sys.command cmd <> 0 then
error (f_"curl is not installed (or does not work)");
@@ -149,7 +145,7 @@ let main () =
(* Create the cache. *)
let cache =
- match cache with
+ match cmdline.cache with
| None -> None
| Some dir ->
try Some (Cache.create ~directory:dir)
@@ -160,7 +156,7 @@ let main () =
in
(* Download the sources. *)
- let downloader = Downloader.create ~curl ~cache in
+ let downloader = Downloader.create ~curl:cmdline.curl ~cache in
let repos = Sources.read_sources () in
let sources = List.map (
fun (source, fingerprint) ->
@@ -170,15 +166,16 @@ let main () =
proxy = Downloader.SystemProxy;
format = Sources.FormatNative;
}
- ) sources in
+ ) cmdline.sources in
let sources = List.append sources repos in
let index : Index.index =
List.concat (
List.map (
fun source ->
let sigchecker =
- Sigchecker.create ~gpg ~check_signature
- ~gpgkey:source.Sources.gpgkey in
+ Sigchecker.create ~gpg:cmdline.gpg
+ ~check_signature:cmdline.check_signature
+ ~gpgkey:source.Sources.gpgkey in
match source.Sources.format with
| Sources.FormatNative ->
Index_parser.get_index ~downloader ~sigchecker source
@@ -192,7 +189,7 @@ let main () =
let mode =
match mode with
| `List -> (* --list *)
- List_entries.list_entries ~list_format ~sources index;
+ List_entries.list_entries ~list_format:cmdline.list_format ~sources index;
exit 0
| `Print_cache -> (* --print-cache *)
@@ -220,7 +217,7 @@ let main () =
fun (name,
{ Index.revision = revision; file_uri = file_uri;
proxy = proxy }) ->
- let template = name, arch, revision in
+ let template = name, cmdline.arch, revision in
message (f_"Downloading: %s") file_uri;
let progress_bar = not (quiet ()) in
ignore (Downloader.download downloader ~template ~progress_bar
@@ -240,18 +237,18 @@ let main () =
fun (name, { Index.aliases = aliases }) ->
match aliases with
| None -> false
- | Some l -> List.mem arg l
+ | Some l -> List.mem cmdline.arg l
) index in
fst item
- with Not_found -> arg in
+ with Not_found -> cmdline.arg in
let item =
try List.find (
fun (name, { Index.arch = a }) ->
- name = arg && arch = normalize_arch a
+ name = arg && cmdline.arch = normalize_arch a
) index
with Not_found ->
error (f_"cannot find os-version '%s' with architecture '%s'.\nUse --list to list available guest types.")
- arg arch in
+ arg cmdline.arch in
let entry = snd item in
let sigchecker = entry.Index.sigchecker in
@@ -278,7 +275,7 @@ let main () =
let template, delete_on_exit =
let { Index.revision = revision; file_uri = file_uri;
proxy = proxy } = entry in
- let template = arg, arch, revision in
+ let template = arg, cmdline.arch, revision in
message (f_"Downloading: %s") file_uri;
let progress_bar = not (quiet ()) in
Downloader.download downloader ~template ~progress_bar ~proxy
@@ -328,7 +325,7 @@ let main () =
(* Planner: Goal. *)
let output_filename, output_format =
- match output, format with
+ match cmdline.output, cmdline.format with
| None, None -> sprintf "%s.img" arg, "raw"
| None, Some "raw" -> sprintf "%s.img" arg, "raw"
| None, Some format -> sprintf "%s.%s" arg format, format
@@ -353,7 +350,7 @@ let main () =
let { Index.size = original_image_size } = entry in
let size =
- match size with
+ match cmdline.size with
| Some size -> size
(* --size parameter missing, output to file: use original image size *)
| None when not output_is_block_dev -> original_image_size
@@ -526,7 +523,7 @@ let main () =
* if it's block device, or if --no-delete-on-failure is set.
*)
let delete_output_file =
- ref (delete_on_failure && not output_is_block_dev) in
+ ref (cmdline.delete_on_failure && not output_is_block_dev) in
let delete_file () =
if !delete_output_file then
try unlink output_filename with _ -> ()
@@ -626,9 +623,9 @@ let main () =
let g =
let g = open_guestfs () in
- may g#set_memsize memsize;
- may g#set_smp smp;
- g#set_network network;
+ may g#set_memsize cmdline.memsize;
+ may g#set_smp cmdline.smp;
+ g#set_network cmdline.network;
(* Make sure to turn SELinux off to avoid awkward interactions
* between the appliance kernel and applications/libraries interacting
@@ -643,7 +640,7 @@ let main () =
List.iter (
fun (format, file) ->
g#add_drive_opts ?format ~readonly:true file;
- ) attach;
+ ) cmdline.attach;
g#launch ();
@@ -667,7 +664,7 @@ let main () =
error (f_"no guest operating systems or multiboot OS found in this disk image\nThis is a failure of the source repository. Use -v for more information.")
in
- Customize_run.run g root ops;
+ Customize_run.run g root cmdline.ops;
(* Collect some stats about the final output file.
* Notes:
@@ -721,7 +718,7 @@ let main () =
* and therefore bypasses the host cache). In general you should not
* use cache=none.
*)
- if sync then
+ if cmdline.sync then
Fsync.file output_filename;
(* Now that we've finished the build, don't delete the output file on
diff --git a/builder/cmdline.ml b/builder/cmdline.ml
index 1d0d3ba..e4a45c3 100644
--- a/builder/cmdline.ml
+++ b/builder/cmdline.ml
@@ -1,5 +1,5 @@
(* virt-builder
- * Copyright (C) 2013 Red Hat Inc.
+ * Copyright (C) 2013-2015 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
@@ -30,6 +30,29 @@ module G = Guestfs
open Unix
open Printf
+type cmdline = {
+ mode : [ `Cache_all | `Delete_cache | `Get_kernel | `Install | `List
+ | `Notes | `Print_cache ];
+ arg : string;
+ arch : string;
+ attach : (string option * string) list;
+ cache : string option;
+ check_signature : bool;
+ curl : string;
+ delete_on_failure : bool;
+ format : string option;
+ gpg : string;
+ list_format : [`Short|`Long|`Json];
+ memsize : int option;
+ network : bool;
+ ops : Customize_cmdline.ops;
+ output : string option;
+ size : int64 option;
+ smp : int option;
+ sources : (string * string) list;
+ sync : bool;
+}
+
let parse_cmdline () =
let mode = ref `Install in
let list_mode () = mode := `List in
@@ -293,7 +316,11 @@ read the man page virt-builder(1).
{ ops with ops = ops.ops @ [ `RootPassword pw ] }
) in
- mode, arg,
- arch, attach, cache, check_signature, curl,
- delete_on_failure, format, gpg, list_format, memsize,
- network, ops, output, size, smp, sources, sync
+ { mode = mode; arg = arg;
+ arch = arch; attach = attach; cache = cache;
+ check_signature = check_signature; curl = curl;
+ delete_on_failure = delete_on_failure; format = format;
+ gpg = gpg; list_format = list_format; memsize = memsize;
+ network = network; ops = ops; output = output;
+ size = size; smp = smp; sources = sources; sync = sync;
+ }
diff --git a/builder/cmdline.mli b/builder/cmdline.mli
new file mode 100644
index 0000000..35e7c7e
--- /dev/null
+++ b/builder/cmdline.mli
@@ -0,0 +1,44 @@
+(* virt-builder
+ * Copyright (C) 2013-2015 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.
+ *)
+
+(** Command line argument parsing. *)
+
+type cmdline = {
+ mode : [ `Cache_all | `Delete_cache | `Get_kernel | `Install | `List
+ | `Notes | `Print_cache ];
+ arg : string;
+ arch : string;
+ attach : (string option * string) list;
+ cache : string option;
+ check_signature : bool;
+ curl : string;
+ delete_on_failure : bool;
+ format : string option;
+ gpg : string;
+ list_format : [`Short|`Long|`Json];
+ memsize : int option;
+ network : bool;
+ ops : Customize_cmdline.ops;
+ output : string option;
+ size : int64 option;
+ smp : int option;
+ sources : (string * string) list;
+ sync : bool;
+}
+
+val parse_cmdline : unit -> cmdline
--
2.5.0
8 years, 6 months
[PATCH] v2v: Make the interface between cmdline.ml and v2v.ml
by Richard W.M. Jones
I'm interested to hear opinions on whether this makes the code
clearer, or not.
This is virt-v2v, but many other virt-* tools work the same way, and
analogous changes could be made.
Currently when command line argument parsing is done in 'cmdline.ml'
the list of parsed parameters is passed to the main program in a very
long tuple. Each parameter is strongly typed, but not named (so for
example two 'bool' flags on the command line might be swapped
accidentally).
This patch uses a struct to pass the parameters instead, so they are
both strongly typed *and* named - two parameters with the same type
could not be swapped by accident.
Also the type of each parameter is now defined in the struct 'type'
definition, which also appears in a 'cmdline.mli' file.
In addition, in the main program it should be clearer where a
particular variable comes from, ie. you have to write
'cmdline.output_alloc' instead of 'output_alloc', which may make it
clearer that it comes from the command line '-oa' parameter, thus
making the code easier to understand.
Rich.
8 years, 6 months
[PATCH] customize, dib, resize, sysprep: Use 'may' pattern in various places.
by Richard W.M. Jones
---
customize/customize_main.ml | 5 ++---
dib/dib.ml | 5 ++---
resize/resize.ml | 13 ++-----------
sysprep/sysprep_operation.ml | 5 +----
4 files changed, 7 insertions(+), 21 deletions(-)
diff --git a/customize/customize_main.ml b/customize/customize_main.ml
index e161e82..13d40bc 100644
--- a/customize/customize_main.ml
+++ b/customize/customize_main.ml
@@ -166,9 +166,8 @@ read the man page virt-customize(1).
(* Connect to libguestfs. *)
let g =
let g = open_guestfs () in
-
- (match memsize with None -> () | Some memsize -> g#set_memsize memsize);
- (match smp with None -> () | Some smp -> g#set_smp smp);
+ may g#set_memsize memsize;
+ may g#set_smp smp;
g#set_network network;
(* Make sure to turn SELinux off to avoid awkward interactions
* between the appliance kernel and applications/libraries interacting
diff --git a/dib/dib.ml b/dib/dib.ml
index 1ae8876..fdb5857 100644
--- a/dib/dib.ml
+++ b/dib/dib.ml
@@ -627,9 +627,8 @@ let main () =
let g, tmpdisk, tmpdiskfmt, drive_partition =
let g = open_guestfs () in
-
- (match memsize with None -> () | Some memsize -> g#set_memsize memsize);
- (match smp with None -> () | Some smp -> g#set_smp smp);
+ may g#set_memsize memsize;
+ may g#set_smp smp;
g#set_network network;
(* Make sure to turn SELinux off to avoid awkward interactions
diff --git a/resize/resize.ml b/resize/resize.ml
index b2802c7..ff10fca 100644
--- a/resize/resize.ml
+++ b/resize/resize.ml
@@ -1210,17 +1210,8 @@ read the man page virt-resize(1).
if p.p_bootable then
g#part_set_bootable "/dev/sdb" p.p_target_partnum true;
- (match p.p_label with
- | Some label ->
- g#part_set_name "/dev/sdb" p.p_target_partnum label;
- | None -> ()
- );
-
- (match p.p_guid with
- | Some guid ->
- g#part_set_gpt_guid "/dev/sdb" p.p_target_partnum guid;
- | None -> ()
- );
+ may (g#part_set_name "/dev/sdb" p.p_target_partnum) p.p_label;
+ may (g#part_set_gpt_guid "/dev/sdb" p.p_target_partnum) p.p_guid;
match parttype, p.p_id with
| GPT, GPT_Type gpt_type ->
diff --git a/sysprep/sysprep_operation.ml b/sysprep/sysprep_operation.ml
index af2004e..057c8c5 100644
--- a/sysprep/sysprep_operation.ml
+++ b/sysprep/sysprep_operation.ml
@@ -186,10 +186,7 @@ let dump_pod () =
if op.enabled_by_default then printf "*\n";
printf "\n";
printf "%s.\n\n" op.heading;
- (match op.pod_description with
- | None -> ()
- | Some description -> printf "%s\n\n" description
- );
+ may (printf "%s\n\n") op.pod_description;
(match op.pod_notes with
| None -> ()
| Some notes ->
--
2.5.0
8 years, 6 months
[PATCH] daemon: lvm: Only return public LVs from guestfs_lvs API (RHBZ#1278878).
by Richard W.M. Jones
When a disk image uses LVM thinp (thin provisioning), the guestfs_lvs
API would return the thinp pools. This confused other APIs because
thinp pools don't have corresponding /dev/VG/LV device nodes.
Filter the LVs that are returned using "lv_role=public".
Thanks: Fabian Deutsch
---
daemon/lvm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/daemon/lvm.c b/daemon/lvm.c
index d989986..6e201e3 100644
--- a/daemon/lvm.c
+++ b/daemon/lvm.c
@@ -148,7 +148,9 @@ do_lvs (void)
r = command (&out, &err,
str_lvm, "lvs",
- "-o", "vg_name,lv_name", "--noheadings",
+ "-o", "vg_name,lv_name",
+ "-S", "lv_role=public",
+ "--noheadings",
"--separator", "/", NULL);
if (r == -1) {
reply_with_error ("%s", err);
--
2.5.0
8 years, 6 months