Bug 1291650

+++ This bug was initially created as a clone of Bug #1278878 +++

Description of problem:
Anaconda can create disks images with the autopart --type=thinp feature (https://github.com/rhinstaller/pykickstart/blob/master/docs/kickstart-docs.rst#autopart) which will basically use an thin LV for the rootfs.
guestfish should be able to cope with this kind of layout by default, currently it can not.

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


How reproducible:


Steps to Reproduce:
1.
2.
3.

Actual results:


Expected results:


Additional info:

--- Additional comment from Richard W.M. Jones on 2015-11-10 11:06:41 CET ---

Looks like udev isn't creating the expected /dev/VG/LV device.
I couldn't reproduce this by creating a thinp disk image
(not OS) directly, so I'll have to try installing CentOS.

Also since this is an older copy of libguestfs
(version=1.30.0fedora=22,release=1.fc22,libvirt) you might
want to try updating to the latest version (1.30.4 in F22).

--- Additional comment from Richard W.M. Jones on 2015-11-10 11:28:43 CET ---

Could you share your kickstart file?  I'm having trouble getting
the 'autopart --type=thinp' command to do anything (the --type=thinp
option seems to just be ignored).

--- Additional comment from Fabian Deutsch on 2015-11-10 11:53:24 CET ---

(In reply to Richard W.M. Jones from comment #1)
> Looks like udev isn't creating the expected /dev/VG/LV device.
> I couldn't reproduce this by creating a thinp disk image
> (not OS) directly, so I'll have to try installing CentOS.

Can I somehow also see this debug output from guestfish? So to see what it is running to bring up LVM?

> Also since this is an older copy of libguestfs
> (version=1.30.0fedora=22,release=1.fc22,libvirt) you might
> want to try updating to the latest version (1.30.4 in F22).

I'll try.

(In reply to Richard W.M. Jones from comment #2)
> Could you share your kickstart file?  I'm having trouble getting
> the 'autopart --type=thinp' command to do anything (the --type=thinp
> option seems to just be ignored).

Sure, you find the KS used for installing here:

https://gerrit.ovirt.org/gitweb?p=ovirt-appliance.git;a=blob;f=node-appliance/ovirt-node-appliance-auto-installation.ks.in;h=d1d00aeb578fda0d120360f834865946b2da009a;hb=HEAD

And for reference:

   1 lang en_US.UTF-8
   2 keyboard us
   3 timezone --utc Etc/UTC
   4 auth --enableshadow --passalgo=sha512
   5 selinux --permissive
   6 network --bootproto=dhcp
   7 firstboot --reconfig
   8 
   9 rootpw --lock
  10 user --name=node --lock
  11 
  12 clearpart --all --initlabel # --disklabel=gpt
  13 bootloader --timeout=1
  14 autopart --type=thinp --fstype=ext4
  15 
  16 liveimg --url=@SQUASHFS_URL@
  17 
  18 poweroff

--- Additional comment from Richard W.M. Jones on 2015-11-10 12:01:04 CET ---

It's running this script:
https://github.com/libguestfs/libguestfs/blob/master/appliance/init#L126

The output is the debug attachment (comment 0) but the commands
that are run are not printed.

I'll try that CentOS install again.

--- Additional comment from Fabian Deutsch on 2015-11-10 12:05:25 CET ---

Right, looking at the debug output I see:

  LV     VG     Attr       LSize Pool   Origin Data%  Meta%  Move Log Cpy%Sync Convert
  pool00 centos twi-aotz-- 7.05g               32.18  14.21                           
  root   centos Vwi-a-tz-- 7.04g pool00        32.27                                  
  swap   centos -wi-a----- 1.00g             

So the root LV is active, but it seems that guestfs is trying to do something with pool00 - and I don't know what it wnats to do with it and why.

--- Additional comment from Fabian Deutsch on 2015-11-10 12:05:59 CET ---

[fabiand@tee tests (node-tests)]$ guestfish --version
guestfish 1.30.4fedora=22,release=1.fc22,libvirt
[fabiand@tee tests (node-tests)]$ guestfish -ia ../ovirt-node-appliance-auto-installation.qcow2
libguestfs: Fehler: internal_parse_mountable: internal_parse_mountable_stub: /dev/centos/pool00: No such file or directory

The updated guestfish does not solve the problem.

--- Additional comment from Richard W.M. Jones on 2015-11-10 12:38 CET ---

I was able to reproduce this with 1.31.24
kernel 4.2.5-300.fc23.x86_64
systemd-222-8.fc23.x86_64

(In reply to Fabian Deutsch from comment #5)
> Right, looking at the debug output I see:
> 
>   LV     VG     Attr       LSize Pool   Origin Data%  Meta%  Move Log
> Cpy%Sync Convert
>   pool00 centos twi-aotz-- 7.05g               32.18  14.21                 
> 
>   root   centos Vwi-a-tz-- 7.04g pool00        32.27                        
> 
>   swap   centos -wi-a----- 1.00g             
> 
> So the root LV is active, but it seems that guestfs is trying to do
> something with pool00 - and I don't know what it wnats to do with it and why.

The LVM code in libguestfs predates pools and thin volumes.  If you
add the trace option (-x) [see attachment] it becomes a bit more obvious
what's going on here:

  libguestfs: trace: lvs
  guestfsd: main_loop: proc 198 (vfs_type) took 0.00 seconds
  guestfsd: main_loop: new request, len 0x28
  lvm lvs -o vg_name,lv_name --noheadings --separator /
  /run/lvm/lvmetad.socket: connect failed: No such file or directory
  WARNING: Failed to connect to lvmetad. Falling back to internal scanning.
  libguestfs: trace: lvs = ["/dev/centos/home", "/dev/centos/pool00", "/dev/centos/root", "/dev/centos/swap"]

Then it runs guestfs_vfs_type() on each of those LVs until it gets to:

  libguestfs: trace: vfs_type "/dev/centos/pool00"
  guestfsd: main_loop: proc 47 (umount_all) took 0.00 seconds
  guestfsd: main_loop: new request, len 0x40
  /dev/centos/pool00: No such file or directory

Diagnosis: The pool shouldn't be returned as an LV.

--- Additional comment from Richard W.M. Jones on 2015-11-10 12:43 CET ---

Reproducer:

(1) Download and uncompress the attached test case.  (The test
case was prepared by hand because libguestfs cannot currently
create thin-provisioned LVs).

(2) Open the file with guestfish:

$ guestfish -a thinp
><fs> run

(3) Run the lvs command, which returns both LVs and the pool
(thin_data):

><fs> lvs
/dev/VG/thin
/dev/VG/thin_data

(4) Run a command such as vfs-type on the pool:

><fs> vfs-type /dev/VG/thin_data
libguestfs: error: vfs_type: vfs_type_stub: /dev/VG/thin_data: No such file or directory

--- Additional comment from Fabian Deutsch on 2015-11-10 12:55:20 CET ---

$ lvs -o +lv_role -S "lv_role = public"

can be used to only show "public" LVs.

Or: teach guestfish about thin vols

--- Additional comment from Richard W.M. Jones on 2015-11-10 13:07:02 CET ---

A patch like this possibly:

https://www.redhat.com/archives/libguestfs/2015-November/msg00111.html

It needs a lot more testing first.

--- Additional comment from Richard W.M. Jones on 2015-11-10 13:31:59 CET ---

That patch solves part of the problem.  The next problem is the
lvs-full API.  This API runs the following command and parses
the output:

lvm lvs -o lv_name,lv_uuid,lv_attr,lv_major,lv_minor,lv_kernel_major,lv_kernel_minor,lv_size,seg_count,origin,snap_percent,copy_percent,move_pv,lv_tags,mirror_log,modules --unbuffered --noheadings --nosuffix --separator , --units b

Unfortunately when you have a thin pool, the last element ('modules')
contains a ',' breaking our parser.  It's easier to see this if I
change the separator to ':' and show the output:

lvm lvs -o lv_name,lv_uuid,lv_attr,lv_major,lv_minor,lv_kernel_major,lv_kernel_minor,lv_size,seg_count,origin,snap_percent,copy_percent,move_pv,lv_tags,mirror_log,modules --unbuffered --noheadings --nosuffix --separator : --units b
  thin_data:HcczzT-KO65-yWfX-9W7m-gZSj-WER6-LxxyzZ:twi-aotz--:-1:-1:253:2:5368709120:1:::::::thin-pool
  thin:3Akdai-LCup-wTQY-Cfys-YDhq-JT10-mlhpAQ:Vwi-a-tz--:-1:-1:253:4:1073741824:1:::::::thin,thin-pool

Here is another patch to fix the separator char:

https://www.redhat.com/archives/libguestfs/2015-November/msg00113.html

The final question is should be filter the output of lvs-full
in the same was as lvs?  I think the answer is _no_ for three
reasons:

(1) Since lvs-full returns the LV attributes, the caller may
do the filtering themselves.

(2) lvs-full doesn't return a /dev/VG/LV path.  So callers can't
(mis?)use the API by immediately running stuff like vfs-type on
the direct output of lvs-full.

(3) Current users of the API -- at least within libguestfs -- don't
rely on being able to immediately access all the LVs returned by
lvs-full.

--- Additional comment from Fabian Deutsch on 2015-11-10 14:52:57 CET ---

(In reply to Richard W.M. Jones from comment #11)

…

> The final question is should be filter the output of lvs-full
> in the same was as lvs?

I'd say yes. Because looking at the pool itself doesn't yield any insightful informations.

--- Additional comment from Fabian Deutsch on 2015-11-12 08:34:50 CET ---

What is the path forward here?

Do you want to bring those patches into rawhide?

Or should osme more testing done before that?

--- Additional comment from Richard W.M. Jones on 2015-11-12 09:48:29 CET ---

It's waiting for patch review on the mailing list, so if you
want to review and/or test that would be helpful.

Full LVM thinp support, including the ability to create the layouts
from scratch, is a bit more work.

--- Additional comment from Richard W.M. Jones on 2015-11-12 09:49:31 CET ---

Also, these patches (and more) are included in my libguestfs fork
here:

https://github.com/rwmjones/libguestfs (master branch)

so that may be easier to test.

--- Additional comment from Richard W.M. Jones on 2015-11-13 20:00:59 CET ---

The basic fix is included in 1.31.26.




--- Additional comment from Richard W.M. Jones on 2015-12-10 19:08:16 CET ---

As Pino says, we're rebasing anyway.  The only reason
to clone the bug would be in case this needs special
attention from QA, in which case clone it and explain
to QA how the feature needs to be tested.

--- Additional comment from Fabian Deutsch on 2015-12-14 12:16:05 CET ---

Will the rbease also happen in Fedora 23?

--- Additional comment from Richard W.M. Jones on 2015-12-14 12:58:09 CET ---

It's not planned.  Please test the fix first, and if it works,
clone the bug for Fedora 23.

--- Additional comment from Fabian Deutsch on 2015-12-14 20:17:58 CET ---

I tried building it in copr - it fails because of a failed supermin dependency.
If I can get a build then I can happily test it.

--- Additional comment from Richard W.M. Jones on 2015-12-14 20:32:55 CET ---

I don't know what the copr failure is, but you can grab the
source of libguestfs 1.30 from this branch:
https://github.com/libguestfs/libguestfs/tree/stable-1.30
compile it and use the ./run script to run a local copy.  Read
the README file first.

--- Additional comment from Fabian Deutsch on 2015-12-15 12:49:32 CET ---

So, your branch, Richard, does work with the thin LVM layout:

[fabiand@tee libguestfs (master)]$ ./run guestfish -ia ~/work/ovirt/gerrit/ngn/ovirt-node-ng-auto-installation.raw 

Welcome to guestfish, the guest filesystem shell for
editing virtual machine filesystems and disk images.

Type: 'help' for help on commands
      'man' to read the manual
      'quit' to quit the shell

Operating system: CentOS Linux release 7.2.1511 (Core) 
/dev/centos/root mounted on /
/dev/sda2 mounted on /boot

><fs> cat /etc/os-release
NAME="CentOS Linux"
VERSION="7 (Core)"
ID="centos"
ID_LIKE="rhel fedora"
VERSION_ID="7"
PRETTY_NAME="CentOS Linux 7 (Core)"
ANSI_COLOR="0;31"
CPE_NAME="cpe:/o:centos:centos:7"
HOME_URL="https://www.centos.org/"
BUG_REPORT_URL="https://bugs.centos.org/"

CENTOS_MANTISBT_PROJECT="CentOS-7"
CENTOS_MANTISBT_PROJECT_VERSION="7"
REDHAT_SUPPORT_PRODUCT="centos"
REDHAT_SUPPORT_PRODUCT_VERSION="7"


><fs> sh "lvm lvs"
  LV     VG     Attr       LSize  Pool   Origin Data%  Meta%  Move Log Cpy%Sync Convert
  pool00 centos twi-aotz-- 14.52g               12.93  6.10                            
  root   centos Vwi-aotz-- 14.50g pool00        12.94                                  
  swap   centos -wi-a-----  2.05g                                                      

><fs> sh "findmnt /"
TARGET SOURCE                  FSTYPE OPTIONS
/      /dev/mapper/centos-root ext4   rw,relatime,stripe=16,data=ordered

><fs> 


Now testing the stable branch

--- Additional comment from Fabian Deutsch on 2015-12-15 12:56:52 CET ---

The stable branch from u/s also seems to work fine:

[fabiand@tee libguestfs ((1.30.5))]$ ./run guestfish -ia ~/work/ovirt/gerrit/ngn/ovirt-node-ng-auto-installation.raw 

Welcome to guestfish, the guest filesystem shell for
editing virtual machine filesystems and disk images.

Type: 'help' for help on commands
      'man' to read the manual
      'quit' to quit the shell

Operating system: CentOS Linux release 7.2.1511 (Core) 
/dev/centos/root mounted on /
/dev/sda2 mounted on /boot

><fs> cat /etc/os-release 
NAME="CentOS Linux"
VERSION="7 (Core)"
ID="centos"
ID_LIKE="rhel fedora"
VERSION_ID="7"
PRETTY_NAME="CentOS Linux 7 (Core)"
ANSI_COLOR="0;31"
CPE_NAME="cpe:/o:centos:centos:7"
HOME_URL="https://www.centos.org/"
BUG_REPORT_URL="https://bugs.centos.org/"

CENTOS_MANTISBT_PROJECT="CentOS-7"
CENTOS_MANTISBT_PROJECT_VERSION="7"
REDHAT_SUPPORT_PRODUCT="centos"
REDHAT_SUPPORT_PRODUCT_VERSION="7"


><fs> sh "findmnt /"
TARGET SOURCE                  FSTYPE OPTIONS
/      /dev/mapper/centos-root ext4   rw,relatime,stripe=16,data=ordered

><fs> sh "lvm lvs"
  LV     VG     Attr       LSize  Pool   Origin Data%  Meta%  Move Log Cpy%Sync Convert
  pool00 centos twi-aotz-- 14.52g               12.93  6.10                            
  root   centos Vwi-aotz-- 14.50g pool00        12.94                                  
  swap   centos -wi-a-----  2.05g                                                      

><fs>

--- Additional comment from Fabian Deutsch on 2015-12-15 12:57:17 CET ---

Kudos for the nice way to test guestfish.