Bug 1661038

Summary: virt-inspector fails with "error: int_of_string" on a Linux image when /etc/fstab contains a partionless device
Product: [Community] Virtualization Tools Reporter: baydemir
Component: libguestfsAssignee: Richard W.M. Jones <rjones>
Status: CLOSED UPSTREAM QA Contact:
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: unspecifiedCC: baydemir, mxie, ptoscano, rjones, tburke
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
: 1672951 (view as bug list) Environment:
Last Closed: 2019-01-14 16:39:26 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1672951, 1674457, 1679966, 1714747    
Attachments:
Description Flags
Output from 'virt-inspector -v'
none
Output from 'libguestfs-test-tool'
none
bz1661038.img (xz compressed)
none
bug-guest
none
guest-reproduce none

Description baydemir 2018-12-19 22:23:41 UTC
Created attachment 1515776 [details]
Output from 'virt-inspector -v'

Description of problem:

guestfish fails to correctly parse (some?) partitionless devices listed in /etc/fstab within a Linux image and unexpectedly exits with an error.

---

Version-Release number of selected component (if applicable):

$ guestfish --version
guestfish 1.38.2rhel=7,release=12.el7_6.1,libvirt

---

How reproducible:

Consistently.

---

Steps to Reproduce:

1. Build a qcow2 image for a Linux OS where '/etc/fstab' specifies where a device such as '/dev/vdc' should be mounted.

2. Run 'virt-inspector' on the image. Or run 'guestfish -a <image>', and from within the 'guestfish' shell, run 'launch' followed by 'inspect_os'.

As of this writing, the following are bootable images for Fedora 24 and Ubuntu 12.04 that illustrate the issue:

* https://platform.swampinabox.org/platform-images/condor-fedora-24-64-master-2016080801.qcow2.gz
* https://platform.swampinabox.org/platform-images/condor-ubuntu-12.04-64-master-2016102702.qcow2.gz

---

Actual results:

$ virt-inspector condor-fedora-24-64-master-2016080801.qcow2 
libguestfs: error: inspect_os: int_of_string
virt-inspector: no operating system could be detected inside this disk image.

---

Expected results:

Valid information.

---

Additional info:

In the 'libguestfs' sources, 'daemon/inspect_fs_unix_fstab.ml' appears to contain a bug. The definition of 're_xdev' allows for partionless devices. The use of this regular expression within the function 'resolve_fstab_device' assumes that the string being matched does specify a partition:

    else if PCRE.matches re_xdev spec then (
      ...
      and part = int_of_string (PCRE.sub 3)
      ...
    )

See the attachment 'virt-inspector-verbose.txt' for the output from 'virt-inspector -v'. The following section appears to be relevant:

    check_fstab_entry: augeas path: /files/etc/fstab/5
    check_fstab_entry: spec=/dev/vdc
    check_fstab_entry: mp=/mnt/out
    resolve_fstab_device: /dev/vdc matched xdev
    ocaml_exn: 'inspect_os' raised 'Failure' exception
    guestfsd: error: int_of_string
    guestfsd: => inspect_os (0x1e0) took 1.14 secs
    libguestfs: error: inspect_os: int_of_string

Comment 1 baydemir 2018-12-19 22:24:20 UTC
Created attachment 1515777 [details]
Output from 'libguestfs-test-tool'

Comment 2 baydemir 2018-12-19 22:45:30 UTC
For the purposes of reproducing the bug, a colleague informs me that we don't need an image with a complete OS installed on it. We need only enough files and directories to trigger the inspection code:

$ mkdir -p input/etc input/bin input/root input/mnt
$ cat >>input/etc/fstab <<EOF
> /dev/vda / auto defaults 0 0
> EOF
$ virt-make-fs --format=qcow2 input input.qcow2
$ virt-inspector -a input.qcow2 
libguestfs: error: inspect_os: int_of_string
virt-inspector: no operating system could be detected inside this disk image.

This may be because the file is not a disk image, or is not a virtual machine
image, or because the OS type is not understood by libguestfs.

NOTE for Red Hat Enterprise Linux 6 users: for Windows guest support you must
install the separate libguestfs-winsupport package.

If you feel this is an error, please file a bug report including as much
information about the disk image as possible.

$

Comment 3 Richard W.M. Jones 2018-12-20 08:29:51 UTC
To reproduce these bugs you can look at the dummy disk images in the libguestfs
sources (under test-data/phony-guests).  However what does /etc/fstab contain
exactly?

Comment 4 baydemir 2018-12-20 14:39:58 UTC
Below are the /etc/fstab files from the Fedora and Ubuntu images I linked to earlier.

---Fedora

#
# /etc/fstab
# Created by anaconda on Mon Aug  8 21:33:45 2016
#
# Accessible filesystems, by reference, are maintained under '/dev/disk'
# See man pages fstab(5), findfs(8), mount(8) and/or blkid(8) for more info
#
/dev/mapper/vg0-lv_root /                       ext4    defaults        1 1
UUID=4c029e08-3e60-4227-8a5b-de2c500daf08 /boot                   ext4    defaults        1 2
/dev/mapper/vg0-lv_swap swap                    swap    defaults        0 0
/dev/vdb	/mnt/in		auto	defaults	0 0
/dev/vdc	/mnt/out	auto	defaults	0 0

---Ubuntu

# /etc/fstab: static file system information.
#
# Use 'blkid' to print the universally unique identifier for a
# device; this may be used with UUID= as a more robust way to name devices
# that works even if disks are added and removed. See fstab(5).
#
# <file system> <mount point>   <type>  <options>       <dump>  <pass>
proc            /proc           proc    nodev,noexec,nosuid 0       0
/dev/mapper/vg0-root /               ext4    errors=remount-ro 0       1
# /boot was on /dev/vda1 during installation
UUID=681c21f3-25fc-4a8a-a5b0-e7c52027534f /boot           ext2    defaults        0       2
/dev/mapper/vg0-swap_1 none            swap    sw              0       0
/dev/vdb	/mnt/in		auto	defaults,nobootwait	0 0
/dev/vdc	/mnt/out	auto	defaults,nobootwait	0 0

Comment 5 Richard W.M. Jones 2018-12-20 14:59:26 UTC
Created attachment 1515889 [details]
bz1661038.img (xz compressed)

Yes this is a bug in inspection.  Attached is a small reproducer
derived from our Fedora phony image.

Comment 7 mxie@redhat.com 2018-12-25 13:59:11 UTC
Created attachment 1516657 [details]
bug-guest

Comment 8 mxie@redhat.com 2018-12-25 13:59:32 UTC
Created attachment 1516658 [details]
guest-reproduce

Comment 9 Pino Toscano 2019-01-14 16:30:27 UTC
Hi!

(In reply to baydemir from comment #0)
> Steps to Reproduce:
> 
> 1. Build a qcow2 image for a Linux OS where '/etc/fstab' specifies where a
> device such as '/dev/vdc' should be mounted.
> 
> 2. Run 'virt-inspector' on the image. Or run 'guestfish -a <image>', and
> from within the 'guestfish' shell, run 'launch' followed by 'inspect_os'.
> 
> As of this writing, the following are bootable images for Fedora 24 and
> Ubuntu 12.04 that illustrate the issue:
> 
> *
> https://platform.swampinabox.org/platform-images/condor-fedora-24-64-master-
> 2016080801.qcow2.gz
> *
> https://platform.swampinabox.org/platform-images/condor-ubuntu-12.04-64-
> master-2016102702.qcow2.gz

Thanks for the pointers to these images, they helped to verify the bug with non-test images.

> Additional info:
> 
> In the 'libguestfs' sources, 'daemon/inspect_fs_unix_fstab.ml' appears to
> contain a bug. The definition of 're_xdev' allows for partionless devices.
> The use of this regular expression within the function
> 'resolve_fstab_device' assumes that the string being matched does specify a
> partition:
> 
>     else if PCRE.matches re_xdev spec then (
>       ...
>       and part = int_of_string (PCRE.sub 3)
>       ...
>     )

To be more precise: the OCaml code above assumes that the partition number is not an empty string, while re_xdev is:
  let re_xdev = PCRE.compile "^/dev/(h|s|v|xv)d([a-z]+)(\\d*)$"
which indeed can result in an empty string for the 3rd match.

This is a regression in the new OCaml code, compared to the old C one (in libguestfs < 1.38).
The old code just took the result of the 3rd match as string, using it as-is without trying to convert it as integer.

Sent a simple patch to fix this:
https://www.redhat.com/archives/libguestfs/2019-January/msg00103.html

Comment 10 Pino Toscano 2019-01-14 16:39:26 UTC
Fixed upstream with
https://github.com/libguestfs/libguestfs/commit/cf6b527824b2a8dc6e8bc65e38ebdceb227e6db1
which is in libguestfs >= 1.39.15.