Libguestfs - Aug 2016 - [PATCH v2 4/7] v2v: add /boot/grub/grub.cfg as Grub2 config

Hi,

this series implements the first bits in v2v to convert Debian/Ubuntu
(and derived) guests.  The series does not complete the support (see
known issues below), but all the patches here should be fit for review
and inclusion.  The series does not enable the conversion, yet.

Known issues:

* there is no grubby nor Bootloader::Tools Perl module available in
  Debian, so there is no way to know what is the default kernel;
  parsing the Grub2 configuration is not exactly an easy task either

* currently tested with simple local guest images, hence needs testing
  with real guests on libvirt/VMware/Xen/VirtualBox/etc (which have
  various tools to be removed/tweaked)

* is tweaking the "enterprise-linux" module the correct/wanted way?
  should it be renamed to just "linux" then?

* surely something else I'm missing

Changes from v1:
- rebased on master
- dropped patch #8, will be solved in a different way
- adapted patch #4 to the Linux_bootloaders module

Thanks,


Pino Toscano (7):
  v2v: refactor Linux.remove
  v2v: add basic support for the "deb" package manager
  v2v: linux: identify Debian-based distros as `Debian_family
  v2v: add /boot/grub/grub.cfg as Grub2 config
  v2v: linux: check also kernel config for modules
  v2v: linux: check the kernel package name for Debian
  v2v: linux: adapt initrd name for Debian

 v2v/convert_linux.ml     | 34 ++++++++++++++++++++++----
 v2v/linux.ml             | 62 ++++++++++++++++++++++++++++++++++++++----------
 v2v/linux_bootloaders.ml |  1 +
 3 files changed, 79 insertions(+), 18 deletions(-)

-- 
2.7.4

Move the actual job in an helper function, so the common bits (like the
check of the size of 'packages' and the reload of Augeas) can be done
for all the package manager implementations.

This should be code motion with no behaviour change.
---
 v2v/linux.ml | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/v2v/linux.ml b/v2v/linux.ml
index 46cb3ba..ed639c1 100644
--- a/v2v/linux.ml
+++ b/v2v/linux.ml
@@ -37,22 +37,25 @@ and augeas_reload g    g#aug_load ();
   debug_augeas_errors g
 
-let remove g inspect packages +let rec remove g inspect packages    if packages
<> [] then (
-    let package_format = inspect.i_package_format in
-    match package_format with
-    | "rpm" ->
-      let cmd = [ "rpm"; "-e" ] @ packages in
-      let cmd = Array.of_list cmd in
-      ignore (g#command cmd);
+    do_remove g inspect packages;
+    (* Reload Augeas in case anything changed. *)
+    augeas_reload g
+  )
 
-      (* Reload Augeas in case anything changed. *)
-      augeas_reload g
+and do_remove g inspect packages +  assert (List.length packages > 0);
+  let package_format = inspect.i_package_format in
+  match package_format with
+  | "rpm" ->
+    let cmd = [ "rpm"; "-e" ] @ packages in
+    let cmd = Array.of_list cmd in
+    ignore (g#command cmd)
 
-    | format ->
-      error (f_"don't know how to remove packages using %s: packages:
%s")
-        format (String.concat " " packages)
-  )
+  | format ->
+    error (f_"don't know how to remove packages using %s: packages:
%s")
+      format (String.concat " " packages)
 
 let file_list_of_package (g : Guestfs.guestfs) inspect app    let
package_format = inspect.i_package_format in
-- 
2.7.4

Implement the 'remove', 'file_list_of_package', and
'file_owner' methods
of the Linux module for the "deb" package manager (dpkg basically, on
Debian and derived distributions).

Also allow it for the main conversion code.
---
 v2v/convert_linux.ml |  2 +-
 v2v/linux.ml         | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
index 7829612..efac0da 100644
--- a/v2v/convert_linux.ml
+++ b/v2v/convert_linux.ml
@@ -79,7 +79,7 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect
source rcaps      | "sles" | "suse-based" |
"opensuse" -> `SUSE_family
     | _ -> assert false in
 
-  assert (inspect.i_package_format = "rpm");
+  assert (inspect.i_package_format = "rpm" ||
inspect.i_package_format = "deb");
 
   (* We use Augeas for inspection and conversion, so initialize it early. *)
   Linux.augeas_init g;
diff --git a/v2v/linux.ml b/v2v/linux.ml
index ed639c1..d449e10 100644
--- a/v2v/linux.ml
+++ b/v2v/linux.ml
@@ -48,6 +48,10 @@ and do_remove g inspect packages    assert (List.length
packages > 0);
   let package_format = inspect.i_package_format in
   match package_format with
+  | "deb" ->
+    let cmd = [ "dpkg"; "--purge" ] @ packages in
+    let cmd = Array.of_list cmd in
+    ignore (g#command cmd);
   | "rpm" ->
     let cmd = [ "rpm"; "-e" ] @ packages in
     let cmd = Array.of_list cmd in
@@ -61,6 +65,12 @@ let file_list_of_package (g : Guestfs.guestfs) inspect app   
let package_format = inspect.i_package_format in
 
   match package_format with
+  | "deb" ->
+    let cmd = [| "dpkg"; "-L"; app.G.app2_name |] in
+    debug "%s" (String.concat " " (Array.to_list cmd));
+    let files = g#command_lines cmd in
+    let files = Array.to_list files in
+    List.sort compare files
   | "rpm" ->
     (* Since RPM allows multiple packages installed with the same
      * name, always check the full ENVR here (RHBZ#1161250).
@@ -98,6 +108,29 @@ let file_list_of_package (g : Guestfs.guestfs) inspect app 
let rec file_owner (g : G.guestfs) inspect path    let package_format =
inspect.i_package_format in
   match package_format with
+  | "deb" ->
+      (* With dpkg usually the directories are owned by all the packages
+       * that install anything in them.  Also with multiarch the same
+       * package is allowed (although with different architectures).
+       * This function returns only one package in all the cases.
+       *)
+      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.
-- 
2.7.4

Identify these distributions, so it is possible to add specific code
for them.

This does not allow them as supported distributions, yet.
---
 v2v/convert_linux.ml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
index efac0da..ce9a1e3 100644
--- a/v2v/convert_linux.ml
+++ b/v2v/convert_linux.ml
@@ -77,6 +77,7 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect
source rcaps      | "rhel" | "centos" |
"scientificlinux" | "redhat-based"
     | "oraclelinux" -> `RHEL_family
     | "sles" | "suse-based" | "opensuse" ->
`SUSE_family
+    | "debian" | "ubuntu" | "linuxmint" ->
`Debian_family
     | _ -> assert false in
 
   assert (inspect.i_package_format = "rpm" ||
inspect.i_package_format = "deb");
-- 
2.7.4

This is the location of the Grub2 configuration in Debian-based systems.
---
 v2v/linux_bootloaders.ml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/v2v/linux_bootloaders.ml b/v2v/linux_bootloaders.ml
index d12edf8..74c40c2 100644
--- a/v2v/linux_bootloaders.ml
+++ b/v2v/linux_bootloaders.ml
@@ -299,6 +299,7 @@ let detect_bootloader (g : G.guestfs) inspect    let
config_file, typ      let locations = [
       "/boot/grub2/grub.cfg", Grub2;
+      "/boot/grub/grub.cfg", Grub2;
       "/boot/grub/menu.lst", Grub1;
       "/boot/grub/grub.conf", Grub1;
     ] in
-- 
2.7.4

When checking whether a kernel supports virtio or it is Xen-based, it is
assumed that the feature has the kernel module for it; this will fail if
the feature is built-in in the kernel, misrepresenting it.

The solution is to check the kernel configuration (/boot/config-$kver)
whether the feature is built-in, in case there is no module available.

This fixes the virtio detection on Ubuntu kernels, where virtio is
built in and not as module.
---
 v2v/convert_linux.ml | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
index ce9a1e3..6f96073 100644
--- a/v2v/convert_linux.ml
+++ b/v2v/convert_linux.ml
@@ -95,6 +95,21 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect
source rcaps    (* What kernel/kernel-like packages are installed on the current
guest? *)
   let installed_kernels : kernel_info list      let rex_ko = Str.regexp
".*\\.k?o\\(\\.xz\\)?$" in
+    let check_config version feature +      let prefix = "^CONFIG_" ^
String.uppercase_ascii feature ^ "=" in
+      let lines = g#grep ~extended:true prefix ("/boot/config-" ^
version) in
+      let lines = Array.to_list lines in
+      match lines with
+      | [] -> false
+      | line :: _ ->
+        let kind = snd (String.split "=" line) in
+        (match kind with
+        | "m" (* Theoretically this should not be needed, since the
module
+               * would be found. *)
+        | "y" -> true
+        | _ -> false
+        )
+    in
     let rex_ko_extract = Str.regexp
".*/\\([^/]+\\)\\.k?o\\(\\.xz\\)?$" in
     let rex_initrd = Str.regexp
"^initr\\(d\\|amfs\\)-.*\\(\\.img\\)?$" in
     filter_map (
@@ -193,8 +208,11 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps               ) modules in
              assert (List.length modules > 0);
 
-             let supports_virtio = List.mem "virtio_net" modules in
-             let is_xen_kernel = List.mem "xennet" modules in
+             let kernel_supports what +               List.mem what modules ||
check_config version what in
+
+             let supports_virtio = kernel_supports "virtio_net" in
+             let is_xen_kernel = kernel_supports "xennet" in
 
              (* If the package name is like "kernel-debug", then
it's
               * a debug kernel.
-- 
2.7.4

On Debian-based systems, the kernel packages are named like
"linux-image-$kver", so check for them.
---
 v2v/convert_linux.ml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
index 6f96073..464ad49 100644
--- a/v2v/convert_linux.ml
+++ b/v2v/convert_linux.ml
@@ -115,7 +115,8 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect
source rcaps      filter_map (
       function
       | { G.app2_name = name } as app
-          when name = "kernel" || String.is_prefix name
"kernel-" ->
+          when name = "kernel" || String.is_prefix name
"kernel-"
+               || String.is_prefix name "linux-image-" ->
         (try
            (* For each kernel, list the files directly owned by the kernel. *)
            let files = Linux.file_list_of_package g inspect app in
-- 
2.7.4

The name of the initrd image on Debian-based systems is different from
what used on Fedora/RHEL/SUSE and derived; set a different regexp to
avoid making the current regexp even more complex.
---
 v2v/convert_linux.ml | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
index 464ad49..9585a23 100644
--- a/v2v/convert_linux.ml
+++ b/v2v/convert_linux.ml
@@ -111,7 +111,11 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps          )
     in
     let rex_ko_extract = Str.regexp
".*/\\([^/]+\\)\\.k?o\\(\\.xz\\)?$" in
-    let rex_initrd = Str.regexp
"^initr\\(d\\|amfs\\)-.*\\(\\.img\\)?$" in
+    let rex_initrd +      if family = `Debian_family then
+        Str.regexp "^initrd.img-.*$"
+      else
+        Str.regexp "^initr\\(d\\|amfs\\)-.*\\(\\.img\\)?$" in
     filter_map (
       function
       | { G.app2_name = name } as app
-- 
2.7.4

On Fri, Aug 26, 2016 at 11:02:06AM +0200, Pino Toscano
wrote:
> This is the location of the Grub2 configuration in Debian-based systems.
> ---
>  v2v/linux_bootloaders.ml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/v2v/linux_bootloaders.ml b/v2v/linux_bootloaders.ml
> index d12edf8..74c40c2 100644
> --- a/v2v/linux_bootloaders.ml
> +++ b/v2v/linux_bootloaders.ml
> @@ -299,6 +299,7 @@ let detect_bootloader (g : G.guestfs) inspect >   
let config_file, typ >      let locations = [
>        "/boot/grub2/grub.cfg", Grub2;
> +      "/boot/grub/grub.cfg", Grub2;
>        "/boot/grub/menu.lst", Grub1;
>        "/boot/grub/grub.conf", Grub1;
>      ] in
ACK patches 1 - 4.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

On Fri, Aug 26, 2016 at 11:02:07AM +0200, Pino Toscano
wrote:
> When checking whether a kernel supports virtio or it is Xen-based, it is
> assumed that the feature has the kernel module for it; this will fail if
> the feature is built-in in the kernel, misrepresenting it.
> 
> The solution is to check the kernel configuration (/boot/config-$kver)
> whether the feature is built-in, in case there is no module available.
> 
> This fixes the virtio detection on Ubuntu kernels, where virtio is
> built in and not as module.
Should we only check the config file?  Seems like a waste of time to
download the module names and check those, and I suppose in some
corner case might even be misleading.

Also the code as it stands makes the assumption that the name of the
CONFIG_* option is the same as the kernel module, which is not true
for some things we care about (virtio-scsi, I think?).

Rich.
>  v2v/convert_linux.ml | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
> index ce9a1e3..6f96073 100644
> --- a/v2v/convert_linux.ml
> +++ b/v2v/convert_linux.ml
> @@ -95,6 +95,21 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps >    (* What kernel/kernel-like packages are installed
on the current guest? *)
>    let installed_kernels : kernel_info list >      let rex_ko =
Str.regexp ".*\\.k?o\\(\\.xz\\)?$" in
> +    let check_config version feature > +      let prefix =
"^CONFIG_" ^ String.uppercase_ascii feature ^ "=" in
> +      let lines = g#grep ~extended:true prefix ("/boot/config-"
^ version) in
> +      let lines = Array.to_list lines in
> +      match lines with
> +      | [] -> false
> +      | line :: _ ->
> +        let kind = snd (String.split "=" line) in
> +        (match kind with
> +        | "m" (* Theoretically this should not be needed, since
the module
> +               * would be found. *)
> +        | "y" -> true
> +        | _ -> false
> +        )
> +    in
>      let rex_ko_extract = Str.regexp
".*/\\([^/]+\\)\\.k?o\\(\\.xz\\)?$" in
>      let rex_initrd = Str.regexp
"^initr\\(d\\|amfs\\)-.*\\(\\.img\\)?$" in
>      filter_map (
> @@ -193,8 +208,11 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps >               ) modules in
>               assert (List.length modules > 0);
>  
> -             let supports_virtio = List.mem "virtio_net" modules
in
> -             let is_xen_kernel = List.mem "xennet" modules in
> +             let kernel_supports what > +               List.mem what
modules || check_config version what in
> +
> +             let supports_virtio = kernel_supports "virtio_net"
in
> +             let is_xen_kernel = kernel_supports "xennet" in
>  
>               (* If the package name is like "kernel-debug", then
it's
>                * a debug kernel.
> -- 
> 2.7.4
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

On Fri, Aug 26, 2016 at 11:02:08AM +0200, Pino Toscano
wrote:
> On Debian-based systems, the kernel packages are named like
> "linux-image-$kver", so check for them.
> ---
>  v2v/convert_linux.ml | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
> index 6f96073..464ad49 100644
> --- a/v2v/convert_linux.ml
> +++ b/v2v/convert_linux.ml
> @@ -115,7 +115,8 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps >      filter_map (
>        function
>        | { G.app2_name = name } as app
> -          when name = "kernel" || String.is_prefix name
"kernel-" ->
> +          when name = "kernel" || String.is_prefix name
"kernel-"
> +               || String.is_prefix name "linux-image-" ->
>          (try
>             (* For each kernel, list the files directly owned by the
kernel. *)
>             let files = Linux.file_list_of_package g inspect app in
> -- 
ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

On Fri, Aug 26, 2016 at 11:02:09AM +0200, Pino Toscano
wrote:
> The name of the initrd image on Debian-based systems is different from
> what used on Fedora/RHEL/SUSE and derived; set a different regexp to
> avoid making the current regexp even more complex.
> ---
>  v2v/convert_linux.ml | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
> index 464ad49..9585a23 100644
> --- a/v2v/convert_linux.ml
> +++ b/v2v/convert_linux.ml
> @@ -111,7 +111,11 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps >          )
>      in
>      let rex_ko_extract = Str.regexp
".*/\\([^/]+\\)\\.k?o\\(\\.xz\\)?$" in
> -    let rex_initrd = Str.regexp
"^initr\\(d\\|amfs\\)-.*\\(\\.img\\)?$" in
> +    let rex_initrd > +      if family = `Debian_family then
> +        Str.regexp "^initrd.img-.*$"
> +      else
> +        Str.regexp "^initr\\(d\\|amfs\\)-.*\\(\\.img\\)?$" in
>      filter_map (
>        function
>        | { G.app2_name = name } as app
> -- 
ACK.

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/