Tomáš Golembiovský
2019-Feb-08 10:44 UTC
[Libguestfs] [PATCH v2 0/3] allow alternative guest tools directories for distributions
First patch just fixes installing guest tools from directory that was broken. Second patch revamps how virt-v2v chooses from which directory install guest tools on Linux. Details in commit message. v2: - included comments from Pino and Rich - added test Tomáš Golembiovský (3): v2v: fix path to source when copying files from guest tools directory v2v: allow alternative directories for distributions v2v: tests: test paths for installation of linux guest tools v2v/Makefile.am | 1 + v2v/test-v2v-copy-guest-tools.sh | 87 ++++++++++++++++++++++++++++++++ v2v/windows_virtio.ml | 82 ++++++++++++++++++------------ 3 files changed, 137 insertions(+), 33 deletions(-) create mode 100755 v2v/test-v2v-copy-guest-tools.sh -- 2.20.1
Tomáš Golembiovský
2019-Feb-08 10:44 UTC
[Libguestfs] [PATCH v2 1/3] v2v: fix path to source when copying files from guest tools directory
The debug message was slightly changed too to better match the similar message for ISO case. It refers to the root directory instead of the specific subdirectory inside guest tools. Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> --- v2v/windows_virtio.ml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml index 92bf3ec60..a2b59d1ec 100644 --- a/v2v/windows_virtio.ml +++ b/v2v/windows_virtio.ml @@ -308,10 +308,11 @@ and copy_drivers g inspect driverdir and copy_from_virtio_win g inspect srcdir destdir filter missing let ret = ref [] in if is_directory virtio_win then ( - let dir = virtio_win // srcdir in - debug "windows: copy_from_virtio_win: guest tools source directory %s" dir; + debug "windows: copy_from_virtio_win: guest tools source directory %s" + virtio_win; - if not (is_directory srcdir) then missing () + let dir = virtio_win // srcdir in + if not (is_directory dir) then missing () else ( let cmd = sprintf "cd %s && find -L -type f" (quote dir) in let paths = external_command cmd in -- 2.20.1
Tomáš Golembiovský
2019-Feb-08 10:44 UTC
[Libguestfs] [PATCH v2 2/3] v2v: allow alternative directories for distributions
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 Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> --- v2v/windows_virtio.ml | 79 +++++++++++++++++++++++++------------------ 1 file changed, 47 insertions(+), 32 deletions(-) diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml index a2b59d1ec..9ef4904be 100644 --- a/v2v/windows_virtio.ml +++ b/v2v/windows_virtio.ml @@ -184,32 +184,38 @@ let rec install_drivers ((g, _) as reg) inspect rcaps ) and install_linux_tools g inspect - let os + let oses match inspect.i_distro with - | "fedora" -> Some "fc28" + | "fedora" -> [ + 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" - | _ -> None in - - match os with - | None -> + let r = ref [] in + List.push_back r (sprintf "rhel%d" inspect.i_major_version); + List.push_back_list r (match inspect.i_major_version with + | 6 -> ["el6"] + | 7 -> ["el7"] + | _ -> []); + List.push_back r "rhel"; + !r + | "sles" | "suse-based" | "opensuse" -> [ + sprintf "suse%d" inspect.i_major_version; "suse"; "lp151"] + | _ -> [] in + + match oses with + | [] -> warning (f_"don't know how to install guest tools on %s-%d") inspect.i_distro inspect.i_major_version - | Some os -> - let src_path = "linux" // os in + | oses -> + let src_paths = List.map ((//) "linux") oses in let dst_path = "/var/tmp" in - debug "locating packages in %s" src_path; + debug "locating packages in: %s" (String.concat ", " src_paths); let packages - copy_from_virtio_win g inspect src_path dst_path + copy_from_virtio_win g inspect src_paths dst_path (fun _ _ -> true) (fun () -> - warning (f_"guest tools directory ‘%s’ is missing from the virtio-win directory or ISO.\n\nGuest tools are only provided in the RHV Guest Tools ISO, so this can happen if you are using the version of virtio-win which contains just the virtio drivers. In this case only virtio drivers can be installed in the guest, and installation of Guest Tools will be skipped.") - src_path) in + warning (f_"none of the guest tools directories ‘[%s]’ was found on the virtio-win directory or ISO.\n\nGuest tools are only provided in the oVirt/RHV Guest Tools ISO, so this can happen if you are using the version of virtio-win which contains just the virtio drivers. In this case only virtio drivers can be installed in the guest, and installation of Guest Tools will be skipped.") + (String.concat ", " src_paths)) in debug "done copying %d files" (List.length packages); let packages = List.map ((//) dst_path) packages in try @@ -290,30 +296,37 @@ and ddb_regedits inspect drv_name drv_pciid * been copied. *) and copy_drivers g inspect driverdir - [] <> copy_from_virtio_win g inspect "/" driverdir + [] <> copy_from_virtio_win g inspect ["/"] driverdir virtio_iso_path_matches_guest_os (fun () -> error (f_"root directory ‘/’ is missing from the virtio-win directory or ISO.\n\nThis should not happen and may indicate that virtio-win or virt-v2v is broken in some way. Please report this as a bug with a full debug log.")) -(* Copy all files from virtio_win directory/ISO located in [srcdir] - * subdirectory and all its subdirectories to the [destdir]. The directory - * hierarchy is not preserved, meaning all files will be directly in [destdir]. - * The file list is filtered based on [filter] function. +(* Find first existing direcotry from [scrdirs] list located in virtio_win + * directory/ISO and copy all files from that directory and all its + * subdirectories to the [destdir]. The directory hierarchy is not preserved, + * meaning all files will be directly in [destdir]. The file list is filtered + * based on [filter] function. * - * If [srcdir] is missing from the ISO then [missing ()] is called + * If none of the directories in [srcdirs] exists [missing ()] is called * which might give a warning or error. * + * Note that the call may succeed whithout copying any file at all. This may + * happen when the source subdirectory exists but is empty or when [filter] + * function is too strict to allow any of the files. + * * Returns list of copied files. *) -and copy_from_virtio_win g inspect srcdir destdir filter missing +and copy_from_virtio_win g inspect srcdirs destdir filter missing + if srcdirs == [] then + invalid_arg "windows: copy_from_virtio_win: no source directories"; let ret = ref [] in if is_directory virtio_win then ( debug "windows: copy_from_virtio_win: guest tools source directory %s" virtio_win; - let dir = virtio_win // srcdir in - if not (is_directory dir) then missing () - else ( + try + let srcdirs = List.map ((//) virtio_win) srcdirs in + let dir = List.find is_directory srcdirs in let cmd = sprintf "cd %s && find -L -type f" (quote dir) in let paths = external_command cmd in List.iter ( @@ -329,7 +342,8 @@ and copy_from_virtio_win g inspect srcdir destdir filter missing List.push_front target_name ret ) ) paths - ) + with Not_found -> + missing () ) else if is_regular_file virtio_win then ( debug "windows: copy_from_virtio_win: guest tools source ISO %s" virtio_win; @@ -340,9 +354,9 @@ and copy_from_virtio_win g inspect srcdir destdir filter missing g2#launch (); let vio_root = "/" in g2#mount_ro "/dev/sda" vio_root; - let srcdir = vio_root ^ "/" ^ srcdir in - if not (g2#is_dir srcdir) then missing () - else ( + let srcdirs = List.map ((^) (vio_root ^ "/")) srcdirs in + try + let srcdir = List.find g2#is_dir srcdirs in let paths = g2#find srcdir in Array.iter ( fun path -> @@ -358,7 +372,8 @@ and copy_from_virtio_win g inspect srcdir destdir filter missing List.push_front target_name ret ) ) paths; - ); + with Not_found -> + missing (); g2#close() with Guestfs.Error msg -> error (f_"%s: cannot open virtio-win ISO file: %s") virtio_win msg -- 2.20.1
Tomáš Golembiovský
2019-Feb-08 10:44 UTC
[Libguestfs] [PATCH v2 3/3] v2v: tests: test paths for installation of linux guest tools
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> --- v2v/Makefile.am | 1 + v2v/test-v2v-copy-guest-tools.sh | 87 ++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100755 v2v/test-v2v-copy-guest-tools.sh diff --git a/v2v/Makefile.am b/v2v/Makefile.am index 2312812fb..84667e11d 100644 --- a/v2v/Makefile.am +++ b/v2v/Makefile.am @@ -456,6 +456,7 @@ TESTS += \ if HAVE_LIBVIRT TESTS += \ test-v2v-cdrom.sh \ + test-v2v-copy-guest-tools.sh \ test-v2v-floppy.sh \ test-v2v-in-place.sh \ test-v2v-mac.sh \ diff --git a/v2v/test-v2v-copy-guest-tools.sh b/v2v/test-v2v-copy-guest-tools.sh new file mode 100755 index 000000000..e9c1e5b90 --- /dev/null +++ b/v2v/test-v2v-copy-guest-tools.sh @@ -0,0 +1,87 @@ +#!/bin/bash - +# libguestfs virt-v2v test script +# Copyright (C) 2019 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 copying of guest tools into guest + +set -e + +$TEST_FUNCTIONS +slow_test +skip_if_skipped +skip_unless_arch x86_64 + +guestname="fedora-29" + +d=test-v2v-copy-guest-tools.d +disk="$guestname.img" +xml="$guestname.xml" +libvirt_uri="test://$abs_builddir/$xml" + +rm -f "$disk" "$xml" +rm -rf $d +mkdir $d + +# Build a guest (using virt-builder). +virt-builder "$guestname" --quiet -o "$disk" --selinux-relabel + +# Create some minimal test metadata. +cat > "$xml" <<EOF +<node> + <domain type='test'> + <name>$guestname</name> + <memory>1048576</memory> + <os> + <type>hvm</type> + <boot dev='hd'/> + </os> + <devices> + <disk type='file' device='disk'> + <driver name='qemu' type='raw'/> + <source file='$abs_builddir/$disk'/> + <target dev='vda' bus='virtio'/> + </disk> + </devices> + </domain> +</node> +EOF + +export VIRTIO_WIN="$d/fake-tools" + +rm -rf $d +mkdir -p $d +mkdir -p $VIRTIO_WIN/linux/fedora{,27,29} +# has: fedora, fedora27, fedora29; should use: fedora29 +$VG virt-v2v --debug-gc -v -x \ + -i libvirt -ic "$libvirt_uri" $guestname \ + -o local -os $d > $d/log 2>&1 +grep '^locating packages in: linux/fedora29, linux/fedora, linux/fc28$' $d/log +grep "^cd 'test-v2v-copy-guest-tools.d/fake-tools/linux/fedora29' && " $d/log + +rm -rf $d +mkdir -p $d +mkdir -p $VIRTIO_WIN/linux/fedora{,27} +# has: fedora, fedora27; should use: fedora +$VG virt-v2v --debug-gc -v -x \ + -i libvirt -ic "$libvirt_uri" $guestname \ + -o local -os $d > $d/log 2>&1 +grep '^locating packages in: linux/fedora29, linux/fedora, linux/fc28$' $d/log +grep "^cd 'test-v2v-copy-guest-tools.d/fake-tools/linux/fedora' && " $d/log + +# Finished +rm -r $d +rm -f "$disk" "$xml" -- 2.20.1
Pino Toscano
2019-Mar-21 09:36 UTC
Re: [Libguestfs] [PATCH v2 1/3] v2v: fix path to source when copying files from guest tools directory
On Friday, 8 February 2019 11:44:41 CET Tomáš Golembiovský wrote:> The debug message was slightly changed too to better match the similar > message for ISO case. It refers to the root directory instead of the > specific subdirectory inside guest tools. > > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> > ---LGTM. (it was already ACKed in v1 of this series...) -- Pino Toscano
Pino Toscano
2019-Mar-21 09:52 UTC
Re: [Libguestfs] [PATCH v2 2/3] v2v: allow alternative directories for distributions
On Friday, 8 February 2019 11:44:42 CET 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 > > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> > ---Mostly LGTM -- few notes below.> diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml > index a2b59d1ec..9ef4904be 100644 > --- a/v2v/windows_virtio.ml > +++ b/v2v/windows_virtio.ml > @@ -184,32 +184,38 @@ let rec install_drivers ((g, _) as reg) inspect rcaps > ) > > and install_linux_tools g inspect > - let os > + let oses > match inspect.i_distro with > - | "fedora" -> Some "fc28" > + | "fedora" -> [ > + sprintf "fedora%d" inspect.i_major_version; "fedora"; "fc28"]The indentation here is funky... I'd put the whole array in its own line.> | "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" > - | _ -> None in > - > - match os with > - | None -> > + let r = ref [] in > + List.push_back r (sprintf "rhel%d" inspect.i_major_version); > + List.push_back_list r (match inspect.i_major_version with > + | 6 -> ["el6"] > + | 7 -> ["el7"]Why not just 'sprintf "el%d" inspect.i_major_version'?> + | _ -> []); > + List.push_back r "rhel"; > + !r > + | "sles" | "suse-based" | "opensuse" -> [ > + sprintf "suse%d" inspect.i_major_version; "suse"; "lp151"]Funky indentation here too.> @@ -340,9 +354,9 @@ and copy_from_virtio_win g inspect srcdir destdir filter missing > g2#launch (); > let vio_root = "/" in > g2#mount_ro "/dev/sda" vio_root; > - let srcdir = vio_root ^ "/" ^ srcdir in > - if not (g2#is_dir srcdir) then missing () > - else ( > + let srcdirs = List.map ((^) (vio_root ^ "/")) srcdirs inPaths in the appliance are always with '/', so you cannot use (^) to join appliance paths (as (^) expands to the path separator of the OS where virt-v2v runs). In practice it is not a problem, since so far libguestfs runs on Unix platforms only. However, it would be nice to get it correct, so there is less potential confusion between host paths, and appliance paths. -- Pino Toscano
Pino Toscano
2019-Mar-21 10:57 UTC
Re: [Libguestfs] [PATCH v2 3/3] v2v: tests: test paths for installation of linux guest tools
On Friday, 8 February 2019 11:44:43 CET Tomáš Golembiovský wrote:> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> > --- > v2v/Makefile.am | 1 + > v2v/test-v2v-copy-guest-tools.sh | 87 ++++++++++++++++++++++++++++++++ > 2 files changed, 88 insertions(+) > create mode 100755 v2v/test-v2v-copy-guest-tools.sh > > diff --git a/v2v/Makefile.am b/v2v/Makefile.am > index 2312812fb..84667e11d 100644 > --- a/v2v/Makefile.am > +++ b/v2v/Makefile.am > @@ -456,6 +456,7 @@ TESTS += \ > if HAVE_LIBVIRT > TESTS += \ > test-v2v-cdrom.sh \ > + test-v2v-copy-guest-tools.sh \ > test-v2v-floppy.sh \ > test-v2v-in-place.sh \ > test-v2v-mac.sh \ > diff --git a/v2v/test-v2v-copy-guest-tools.sh b/v2v/test-v2v-copy-guest-tools.sh > new file mode 100755 > index 000000000..e9c1e5b90 > --- /dev/null > +++ b/v2v/test-v2v-copy-guest-tools.sh > @@ -0,0 +1,87 @@ > +#!/bin/bash - > +# libguestfs virt-v2v test script > +# Copyright (C) 2019 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 copying of guest tools into guest > + > +set -e > + > +$TEST_FUNCTIONS > +slow_test > +skip_if_skipped > +skip_unless_arch x86_64This arch skip line is not needed, because of...> +guestname="fedora-29"... this: skip_unless_virt_builder_guest "$guestname"> +d=test-v2v-copy-guest-tools.d > +disk="$guestname.img" > +xml="$guestname.xml" > +libvirt_uri="test://$abs_builddir/$xml" > + > +rm -f "$disk" "$xml" > +rm -rf $d > +mkdir $d > + > +# Build a guest (using virt-builder). > +virt-builder "$guestname" --quiet -o "$disk" --selinux-relabelNo need for --selinux-relabel, since the guest is not run anyway.> + > +# Create some minimal test metadata. > +cat > "$xml" <<EOF > +<node> > + <domain type='test'> > + <name>$guestname</name> > + <memory>1048576</memory> > + <os> > + <type>hvm</type> > + <boot dev='hd'/> > + </os> > + <devices> > + <disk type='file' device='disk'> > + <driver name='qemu' type='raw'/> > + <source file='$abs_builddir/$disk'/> > + <target dev='vda' bus='virtio'/> > + </disk> > + </devices> > + </domain> > +</node> > +EOFI think using -i disk should be enough, no need to resort to a libvirt XML.> + > +export VIRTIO_WIN="$d/fake-tools" > + > +rm -rf $d > +mkdir -p $d > +mkdir -p $VIRTIO_WIN/linux/fedora{,27,29} > +# has: fedora, fedora27, fedora29; should use: fedora29 > +$VG virt-v2v --debug-gc -v -x \ > + -i libvirt -ic "$libvirt_uri" $guestname \ > + -o local -os $d > $d/log 2>&1Considering the output is not used, most probably -o null is a better choice. Adding --no-copy also makes sure to skip that null copying.> +grep '^locating packages in: linux/fedora29, linux/fedora, linux/fc28$' $d/log > +grep "^cd 'test-v2v-copy-guest-tools.d/fake-tools/linux/fedora29' && " $d/log > + > +rm -rf $d > +mkdir -p $d > +mkdir -p $VIRTIO_WIN/linux/fedora{,27} > +# has: fedora, fedora27; should use: fedora > +$VG virt-v2v --debug-gc -v -x \ > + -i libvirt -ic "$libvirt_uri" $guestname \ > + -o local -os $d > $d/log 2>&1 > +grep '^locating packages in: linux/fedora29, linux/fedora, linux/fc28$' $d/log > +grep "^cd 'test-v2v-copy-guest-tools.d/fake-tools/linux/fedora' && " $d/log > + > +# Finished > +rm -r $d > +rm -f "$disk" "$xml" >-- Pino Toscano
Seemingly Similar Threads
- [PATCH 2/2] v2v: allow alternative directories for distributions
- Re: [PATCH 2/2] v2v: allow alternative directories for distributions
- Re: [PATCH v2 2/3] v2v: allow alternative directories for distributions
- [PATCH v3 3/3] v2v: linux: install QEMU-GA (RHBZ#1619665)
- [PATCH v4 3/3] v2v: linux: install QEMU-GA (RHBZ#1619665)