Bug 1278878 - guestfish should be able to handle LVM thin layouts
Summary: guestfish should be able to handle LVM thin layouts
Keywords:
Status: CLOSED UPSTREAM
Alias: None
Product: Virtualization Tools
Classification: Community
Component: libguestfs
Version: unspecified
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Richard W.M. Jones
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks: 1290755 1291650
TreeView+ depends on / blocked
 
Reported: 2015-11-06 15:43 UTC by Fabian Deutsch
Modified: 2016-01-21 18:08 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 1290755 1291650 (view as bug list)
Environment:
Last Closed: 2015-11-13 19:00:59 UTC
Embargoed:


Attachments (Terms of Use)
Logfile with the failure (61.36 KB, text/plain)
2015-11-06 15:43 UTC, Fabian Deutsch
no flags Details
log with tracing enabled (77.39 KB, text/plain)
2015-11-10 11:38 UTC, Richard W.M. Jones
no flags Details
thinp.xz (1.49 MB, application/x-xz)
2015-11-10 11:43 UTC, Richard W.M. Jones
no flags Details

Description Fabian Deutsch 2015-11-06 15:43:40 UTC
Created attachment 1090718 [details]
Logfile with the failure

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:

Comment 1 Richard W.M. Jones 2015-11-10 10:06:41 UTC
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).

Comment 2 Richard W.M. Jones 2015-11-10 10:28:43 UTC
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).

Comment 3 Fabian Deutsch 2015-11-10 10:53:24 UTC
(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

Comment 4 Richard W.M. Jones 2015-11-10 11:01:04 UTC
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.

Comment 5 Fabian Deutsch 2015-11-10 11:05:25 UTC
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.

Comment 6 Fabian Deutsch 2015-11-10 11:05:59 UTC
[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.

Comment 7 Richard W.M. Jones 2015-11-10 11:38:19 UTC
Created attachment 1092162 [details]
log with tracing enabled

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.

Comment 8 Richard W.M. Jones 2015-11-10 11:43:50 UTC
Created attachment 1092163 [details]
thinp.xz

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

Comment 9 Fabian Deutsch 2015-11-10 11:55:20 UTC
$ lvs -o +lv_role -S "lv_role = public"

can be used to only show "public" LVs.

Or: teach guestfish about thin vols

Comment 10 Richard W.M. Jones 2015-11-10 12:07:02 UTC
A patch like this possibly:

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

It needs a lot more testing first.

Comment 11 Richard W.M. Jones 2015-11-10 12:31:59 UTC
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.

Comment 12 Fabian Deutsch 2015-11-10 13:52:57 UTC
(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.

Comment 13 Fabian Deutsch 2015-11-12 07:34:50 UTC
What is the path forward here?

Do you want to bring those patches into rawhide?

Or should osme more testing done before that?

Comment 14 Richard W.M. Jones 2015-11-12 08:48:29 UTC
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.

Comment 15 Richard W.M. Jones 2015-11-12 08:49:31 UTC
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.

Comment 16 Richard W.M. Jones 2015-11-13 19:00:59 UTC
The basic fix is included in 1.31.26.

Comment 20 Fabian Deutsch 2015-12-14 11:16:05 UTC
Will the rbease also happen in Fedora 23?

Comment 21 Richard W.M. Jones 2015-12-14 11:58:09 UTC
It's not planned.  Please test the fix first, and if it works,
clone the bug for Fedora 23.

Comment 22 Fabian Deutsch 2015-12-14 19:17:58 UTC
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.

Comment 23 Richard W.M. Jones 2015-12-14 19:32:55 UTC
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.

Comment 24 Fabian Deutsch 2015-12-15 11:49:32 UTC
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

Comment 25 Fabian Deutsch 2015-12-15 11:56:52 UTC
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>

Comment 26 Fabian Deutsch 2015-12-15 11:57:17 UTC
Kudos for the nice way to test guestfish.


Note You need to log in before you can comment on or make changes to this bug.