11:39 a.m.
On Sat, Jan 26, 2019 at 01:19:59PM +0100, Tomáš Golembiovský wrote:
Allow multiple alternative directory names for distributions (or
distribution familiy) when installing Linux guest tools packages.
Original naming required that there is a separate directory for every
version of a distribution (e.g. fc28, fc29, ...). This is inconvenient
when users want to keep just a single version of the package for the
distribution.
For each distribution one can have either a common directory (e.g.
fedora) or a versioned directory (fedora28). This can also be combined.
I.e. one can have both `fedora` and `fedora28` in which case `fedora28`
will be used when converting Fedora 28 guest wheres `fedora` will be
used when converting guests with any other Fedora version.
To have better names for unversioned directories the original names
were changed this way:
fc -> fedora
el -> rhel
lp -> suse
The original directory names are kept for backward compatibility and are
aliased to new names as described below. When both new and old name are
present on file system the new name takes precedence.
fc28 -> fedora
el6 -> rhel6
el7 -> rhel7
lp151 -> suse
Can we use an actual existing scheme like libosinfo ID?
Signed-off-by: Tomáš Golembiovský <tgolembi(a)redhat.com>
---
v2v/windows_virtio.ml | 65 +++++++++++++++++++++++++------------------
1 file changed, 38 insertions(+), 27 deletions(-)
diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml
index 94c4774b7..cc33d9502 100644
--- a/v2v/windows_virtio.ml
+++ b/v2v/windows_virtio.ml
@@ -186,14 +186,18 @@ let rec install_drivers ((g, _) as reg) inspect rcaps =
and install_linux_tools g inspect =
let os =
match inspect.i_distro with
- | "fedora" -> Some "fc28"
+ | "fedora" -> Some [
+ (sprintf "fedora%d" inspect.i_major_version); "fedora";
"fc28"]
| "rhel" | "centos" | "scientificlinux" |
"redhat-based"
- | "oraclelinux" ->
- (match inspect.i_major_version with
- | 6 -> Some "el6"
- | 7 -> Some "el7"
- | _ -> None)
- | "sles" | "suse-based" | "opensuse" -> Some
"lp151"
+ | "oraclelinux" -> Some (
+ [(sprintf "rhel%d" inspect.i_major_version)]
As a general rule in functional languages function application
[ie. calling a function] binds tightest. So:
f 1 + 2 means: (f 1) + 2
You don't need the parentheses around the sprintf statement above or
later in the patch.
+ @ (match inspect.i_major_version with
+ | 6 -> ["el6"]
+ | 7 -> ["el7"]
+ | _ -> [])
+ @ ["rhel"])
You could also rewrite the whole awkward list building non-
functionally. It would end up like this which seems a whole lot more
readable to me:
let r = ref [] in
List.push_back r (sprintf "rhel%d" inspect.i_major_version);
List.push_back r (match ...);
List.push_back r "rhel";
Some r
+ | "sles" | "suse-based" |
"opensuse" -> Some [
+ (sprintf "fedora%d" inspect.i_major_version); "suse";
"lp151"]
But this is OK, except drop the extra parens around sprintf.
+ with Not_found ->
+ missing()
Needs a space between missing and ‘()’ (and the same later on).
I wonder if this code would be simpler if we started to use the
‘return’ statement:
https://github.com/libguestfs/libguestfs/blob/b26bd778b58b78eb59503413fd5...
Anyway, looks reasonable. I keep thinking it needs a test though.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/