9:04 a.m.
v1 was here:
https://www.redhat.com/archives/libguestfs/2017-October/msg00183.html
v2:
- Adds back the Debian test, but simplified.
- Adds tests on Fedora & Debian.
Rich.
9:04 a.m.
Linux.file_owner is not used by any other function, so remove it.
Linux.is_file_owned is only used when removing kmod-xenpv on old RHEL
releases, and so is only required to work for RPM.
The old file_owner/is_file_owned functions were completely broken for
the RPM case. This replaces them with a simpler, working
implementation of is_file_owned only.
Thanks: Ming Xie for finding and reporting the original bug.
---
v2v/linux.ml | 61 ++++++++++++++++++++++++++---------------------------------
v2v/linux.mli | 3 ---
2 files changed, 27 insertions(+), 37 deletions(-)
diff --git a/v2v/linux.ml b/v2v/linux.ml
index bc4af1ad2..c97f03f05 100644
--- a/v2v/linux.ml
+++ b/v2v/linux.ml
@@ -99,7 +99,7 @@ let file_list_of_package (g : Guestfs.guestfs) inspect app =
error (f_"don’t know how to get list of files from package using %s")
format
-let rec file_owner (g : G.guestfs) { i_package_format = package_format } path =
+let is_file_owned (g : G.guestfs) { i_package_format = package_format } path =
match package_format with
| "deb" ->
(* With dpkg usually the directories are owned by all the packages
@@ -109,48 +109,41 @@ let rec file_owner (g : G.guestfs) { i_package_format =
package_format } path =
*)
let cmd = [| "dpkg"; "-S"; path |] in
debug "%s" (String.concat " " (Array.to_list cmd));
- let lines =
- try g#command_lines cmd
- with Guestfs.Error msg as exn ->
- if String.find msg "no path found matching pattern" >= 0 then
- raise Not_found
- else
- raise exn in
- if Array.length lines = 0 then
- error (f_"internal error: file_owner: dpkg command returned no
output");
- let line = lines.(0) in
- let line =
- try String.sub line 0 (String.rindex line ':')
- with Invalid_argument _ ->
- error (f_"internal error: file_owner: invalid dpkg output: ‘%s’")
- line in
- fst (String.split "," line)
-
- | "rpm" ->
- (* Although it is possible in RPM for multiple packages to own
- * a file, this deliberately only returns one package.
- *)
- let cmd = [| "rpm"; "-qf"; "--qf";
"%{NAME}\\n"; path |] in
- debug "%s" (String.concat " " (Array.to_list cmd));
(try
- let pkgs = g#command_lines cmd in
- pkgs.(0)
+ let lines = g#command_lines cmd in
+ if Array.length lines = 0 then
+ error (f_"internal error: is_file_owned: dpkg command returned no
output");
+ (* Just check the output looks something like "pkg: filename". *)
+ if String.find lines.(0) ": " >= 0 then
+ true
+ else
+ error (f_"internal error: is_file_owned: unexpected output from dpkg
command: %s")
+ lines.(0)
with Guestfs.Error msg as exn ->
- if String.find msg "is not owned" >= 0 then
- raise Not_found
+ if String.find msg "no path found matching pattern" >= 0 then
+ false
else
raise exn
- | Invalid_argument _ (* pkgs.(0) raises index out of bounds *) ->
- error (f_"internal error: file_owner: rpm command returned no
output")
)
+ | "rpm" ->
+ (* Run rpm -qf and print a magic string if the file is owned.
+ * If not owned, rpm will print "... is not owned by any package"
+ * and exit with an error. Unfortunately the string is sent to
+ * stdout, so here we ignore the exit status of rpm and just
+ * look for one of the two strings.
+ *)
+ let magic = "FILE_OWNED_TEST" in
+ let cmd = sprintf "rpm -qf --qf %s %s 2>&1 ||:"
+ (quote (magic ^ "\n")) (quote path) in
+ let r = g#sh cmd in
+ if String.find r magic >= 0 then true
+ else if String.find r "is not owned" >= 0 then false
+ else failwithf "RPM file owned test failed: %s" r
+
| format ->
error (f_"don’t know how to find file owner using %s") format
-and is_file_owned g inspect path =
- try ignore (file_owner g inspect path); true
- with Not_found -> false
-
let is_package_manager_save_file filename =
(* Recognized suffixes of package managers. *)
let suffixes = [ ".dpkg-old"; ".dpkg-new"; ".rpmsave";
".rpmnew"; ] in
diff --git a/v2v/linux.mli b/v2v/linux.mli
index 705073644..08146a460 100644
--- a/v2v/linux.mli
+++ b/v2v/linux.mli
@@ -29,9 +29,6 @@ val remove : Guestfs.guestfs -> Types.inspect -> string list ->
unit
val file_list_of_package : Guestfs.guestfs -> Types.inspect -> Guestfs.application2
-> string list
(** Return list of files owned by package. *)
-val file_owner : Guestfs.guestfs -> Types.inspect -> string -> string
-(** Return the name of the package that owns a file. *)
-
val is_file_owned : Guestfs.guestfs -> Types.inspect -> string -> bool
(** Returns true if the file is owned by an installed package. *)
--
2.13.2
9:04 a.m.
New subject: [PATCH v2 2/2] v2v: Add a slow test of the Linux.is_file_owned function.
The previous commit revealed this function was very broken. This adds
a slow test so that we can detect if it is broken or gets broken in
future. It tests Fedora and Debian guests.
---
.gitignore | 2 ++
po/POTFILES-ml | 1 +
v2v/Makefile.am | 57 ++++++++++++++++++++++++++++++++++--
v2v/v2v-slow-unit-tests.sh | 42 +++++++++++++++++++++++++++
v2v/v2v_slow_unit_tests.ml | 72 ++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 172 insertions(+), 2 deletions(-)
diff --git a/.gitignore b/.gitignore
index 36a193054..823b9e050 100644
--- a/.gitignore
+++ b/.gitignore
@@ -655,6 +655,8 @@ Makefile.in
/v2v/test-v2v-conversion-of-*.sh
/v2v/uefi.ml
/v2v/uefi.mli
+/v2v/v2v-slow-unit-tests-*.img
+/v2v/v2v_slow_unit_tests
/v2v/v2v_unit_tests
/v2v/virt-v2v
/v2v/virt-v2v.1
diff --git a/po/POTFILES-ml b/po/POTFILES-ml
index dbf151ea1..478f71b9b 100644
--- a/po/POTFILES-ml
+++ b/po/POTFILES-ml
@@ -163,6 +163,7 @@ v2v/types.ml
v2v/uefi.ml
v2v/utils.ml
v2v/v2v.ml
+v2v/v2v_slow_unit_tests.ml
v2v/v2v_unit_tests.ml
v2v/vCenter.ml
v2v/windows.ml
diff --git a/v2v/Makefile.am b/v2v/Makefile.am
index 6d90faf00..51f345250 100644
--- a/v2v/Makefile.am
+++ b/v2v/Makefile.am
@@ -21,6 +21,8 @@ EXTRA_DIST = \
$(SOURCES_MLI) $(SOURCES_ML) $(SOURCES_C) \
copy_to_local.ml \
copy_to_local.mli \
+ v2v_slow_unit_tests.ml \
+ v2v-slow-unit-tests.sh \
v2v_unit_tests.ml \
virt-v2v.pod \
virt-v2v-copy-to-local.pod
@@ -305,7 +307,9 @@ TESTS += \
endif
if HAVE_OCAML_PKG_OUNIT
-TESTS += v2v_unit_tests
+TESTS += \
+ v2v_unit_tests \
+ v2v-slow-unit-tests.sh
endif
if ENABLE_APPLIANCE
@@ -360,6 +364,10 @@ SLOW_TESTS = \
$(real_guests_scripts) \
test-v2v-trim.sh
+if HAVE_OCAML_PKG_OUNIT
+SLOW_TESTS += v2v-slow-unit-tests.sh
+endif
+
check-slow:
$(MAKE) check TESTS="$(SLOW_TESTS)" SLOW=1
@@ -483,7 +491,9 @@ EXTRA_DIST += \
# Unit tests.
check_PROGRAMS =
if HAVE_OCAML_PKG_OUNIT
-check_PROGRAMS += v2v_unit_tests
+check_PROGRAMS += \
+ v2v_unit_tests \
+ v2v_slow_unit_tests
endif
v2v_unit_tests_BOBJECTS = \
@@ -528,6 +538,49 @@ v2v_unit_tests_LINK = \
$(OCAMLLINKFLAGS) \
$(v2v_unit_tests_THEOBJECTS) -o $@
+v2v_slow_unit_tests_BOBJECTS = \
+ types.cmo \
+ uefi.cmo \
+ utils.cmo \
+ DOM.cmo \
+ create_ovf.cmo \
+ windows.cmo \
+ windows_virtio.cmo \
+ linux.cmo \
+ parse_vmx.cmo \
+ inspect_source.cmo \
+ v2v_slow_unit_tests.cmo
+v2v_slow_unit_tests_XOBJECTS = $(v2v_slow_unit_tests_BOBJECTS:.cmo=.cmx)
+
+v2v_slow_unit_tests_SOURCES = $(virt_v2v_SOURCES)
+v2v_slow_unit_tests_CPPFLAGS = $(virt_v2v_CPPFLAGS)
+v2v_slow_unit_tests_CFLAGS = $(virt_v2v_CFLAGS)
+
+if !HAVE_OCAMLOPT
+# Can't call this v2v_slow_unit_tests_OBJECTS because automake gets confused.
+v2v_slow_unit_tests_THEOBJECTS = $(v2v_slow_unit_tests_BOBJECTS)
+v2v_slow_unit_tests.cmo: OCAMLPACKAGES += -package oUnit
+else
+v2v_slow_unit_tests_THEOBJECTS = $(v2v_slow_unit_tests_XOBJECTS)
+v2v_slow_unit_tests.cmx: OCAMLPACKAGES += -package oUnit
+endif
+
+v2v_slow_unit_tests_DEPENDENCIES = \
+ $(v2v_slow_unit_tests_THEOBJECTS) \
+ ../common/mlstdutils/mlstdutils.$(MLARCHIVE) \
+ ../common/mlxml/mlxml.$(MLARCHIVE) \
+ ../common/mlgettext/mlgettext.$(MLARCHIVE) \
+ ../common/mlpcre/mlpcre.$(MLARCHIVE) \
+ ../common/mlutils/mlcutils.$(MLARCHIVE) \
+ ../common/mltools/mltools.$(MLARCHIVE) \
+ $(top_srcdir)/ocaml-link.sh
+v2v_slow_unit_tests_LINK = \
+ $(top_srcdir)/ocaml-link.sh -cclib '$(OCAMLCLIBS)' -- \
+ $(OCAMLFIND) $(BEST) $(OCAMLFLAGS) \
+ $(OCAMLPACKAGES) -package oUnit \
+ $(OCAMLLINKFLAGS) \
+ $(v2v_slow_unit_tests_THEOBJECTS) -o $@
+
# Dependencies.
.depend: *.mli *.ml
$(top_builddir)/ocaml-dep.sh $^
diff --git a/v2v/v2v-slow-unit-tests.sh b/v2v/v2v-slow-unit-tests.sh
new file mode 100755
index 000000000..1847211db
--- /dev/null
+++ b/v2v/v2v-slow-unit-tests.sh
@@ -0,0 +1,42 @@
+#!/bin/bash -
+# libguestfs virt-v2v test script
+# 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.
+
+# Wrapper around v2v_slow_unit_tests (OCaml test program).
+
+set -e
+
+$TEST_FUNCTIONS
+slow_test
+skip_if_skipped
+
+rm -f v2v-slow-unit-tests-*.img
+
+# These images are used for testing.
+virt-builder fedora-26 \
+ -o v2v-slow-unit-tests-fedora-26.img \
+ --mkdir /tmp/notowned \
+ --touch /tmp/notowned2
+virt-builder debian-9 \
+ -o v2v-slow-unit-tests-debian-9.img \
+ --mkdir /tmp/notowned \
+ --touch /tmp/notowned2
+
+# Run the actual tests.
+./v2v_slow_unit_tests
+
+rm v2v-slow-unit-tests-*.img
diff --git a/v2v/v2v_slow_unit_tests.ml b/v2v/v2v_slow_unit_tests.ml
new file mode 100644
index 000000000..c27cba634
--- /dev/null
+++ b/v2v/v2v_slow_unit_tests.ml
@@ -0,0 +1,72 @@
+(* virt-v2v
+ * 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.
+ *)
+
+(* Test individual virt-v2v utility functions for tests which
+ * are long running.
+ *)
+
+open Printf
+
+open OUnit2
+
+open Std_utils
+open Tools_utils
+
+open Types
+
+let get_g_i filename =
+ let g = open_guestfs () in
+ g#add_drive filename ~readonly:true;
+ g#launch ();
+ let inspect = Inspect_source.inspect_source SingleRoot g in
+ g, inspect
+
+let fedora_g, fedora_i = get_g_i "v2v-slow-unit-tests-fedora-26.img"
+let debian_g, debian_i = get_g_i "v2v-slow-unit-tests-debian-9.img"
+
+let test_is_file_owned ctx =
+ let test g i filename expected =
+ let printer = string_of_bool in
+ let msg =
+ sprintf "Linux.is_file_owned %S (expecting %b)" filename expected in
+ assert_equal ~printer ~msg
+ expected (Linux.is_file_owned g i filename)
+ in
+
+ test fedora_g fedora_i "/etc/passwd" true;
+ (* /etc/passwd is not in fact owned in Debian. *)
+ test debian_g debian_i "/etc/debian_version" true;
+ test fedora_g fedora_i "/var/log" true;
+ test debian_g debian_i "/var/log" true;
+
+ test fedora_g fedora_i "/tmp/notowned" false;
+ test debian_g debian_i "/tmp/notowned" false;
+ test fedora_g fedora_i "/tmp/notowned2" false;
+ test debian_g debian_i "/tmp/notowned2" false;
+
+ ()
+
+(* Suites declaration. *)
+let suite =
+ "virt-v2v" >:::
+ [
+ "Linux.is_file_owned" >:: test_is_file_owned;
+ ]
+
+let () =
+ run_test_tt_main suite
--
2.13.2
2401
days inactive
2401
days old
2 comments
1 participants
participants (1)
-
Richard W.M. Jones