Bug 1090013

Summary: [RFE] implement vgreduce --move command
Product: [Fedora] Fedora Reporter: Jan Safranek <jsafrane>
Component: lvm2Assignee: Ondrej Kozina <okozina>
Status: ASSIGNED --- QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: rawhideCC: agk, anaconda-maint-list, bcl, bmarzins, bmr, dlehman, dwysocha, heinzm, jbrassow, jonathan, lvm-team, msnitzer, okozina, prajnoha, vtrefny, zkabelac
Target Milestone: ---Keywords: FutureFeature
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Enhancement
Doc Text:
Story Points: ---
Clone Of:
: 1181121 (view as bug list) Environment:
Last Closed: 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: 1181121    
Attachments:
Description Flags
reproducer none

Description Jan Safranek 2014-04-22 11:36:33 UTC
Created attachment 888455 [details]
reproducer

I have a VG with PVs that is unused, i.e. no LV uses it (sdc4 below):

$ pvs
PV         VG   Fmt  Attr PSize  PFree
/dev/sdc4  test lvm2 a--  92.00m 92.00m
/dev/sdc5  test lvm2 a--  92.00m 80.00m

When I want to remove the PV from VG using blivet, I get:

  File "/usr/lib/python2.7/site-packages/blivet/__init__.py", line 313, in doIt
    self.devicetree.processActions()
  File "/usr/lib/python2.7/site-packages/blivet/devicetree.py", line 290, in processActions
    action.execute()
  File "/usr/lib/python2.7/site-packages/blivet/deviceaction.py", line 749, in execute
    self.container.remove(self.device)
  File "/usr/lib/python2.7/site-packages/blivet/devices.py", line 2247, in remove
    self._remove(member)
  File "/usr/lib/python2.7/site-packages/blivet/devices.py", line 2453, in _remove
    lvm.pvmove(member.path)
  File "/usr/lib/python2.7/site-packages/blivet/devicelibs/lvm.py", line 262, in pvmove
    raise LVMError("pvmove failed for %s->%s: %s" % (source, dest, msg))

The reason is broken (?) pvmove. blivet calls 'pvmove /dev/sdc4' to free the device. pvmove detects that it has nothing to do (there are no data to move) and treats it as error and returns 5 instead of 0.

$ pvmove /dev/sdc4
  No data to move for test
$ echo $?
5


Version-Release number of selected component (if applicable):
python-blivet-0.49-1.fc21.noarch

How reproducible:
always

Steps to Reproduce:
1. vgcreate test /dev/sdc{4,5}
2. lvcreate -L 10M -n lv1 /dev/test /dev/sdc5
3. run attached reproducer
   (it just performs ActionRemoveMember(/dev/test, /dev/sdc4))

Actual results:
traceback

Expected results:
VG is reduced

Comment 1 David Lehman 2014-04-22 16:07:45 UTC
Unfortunately that return value is a generic ECMD_FAILED error that covers much more than this specific situation, so we cannot just treat it as a success and move on. We will probably have to run pvs before running pvmove to see if there are any extents to move.

Comment 2 David Lehman 2014-12-02 20:26:30 UTC
It seems like having no data to move would be considered a succcess (as in "nothing to do").

Comment 3 Jaroslav Reznik 2015-03-03 17:00:39 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 22 development cycle.
Changing version to '22'.

More information and reason for this action is here:
https://fedoraproject.org/wiki/Fedora_Program_Management/HouseKeeping/Fedora22

Comment 4 Ondrej Kozina 2015-05-21 11:59:01 UTC
Altering pvmove return codes doesn't look like a way to go. Could we add a new feature, something like:

vgreduce vg00 --pvmove /dev/sda4

that would:
a) try to initiate pvmove if necessary
b) wait for eventual pvmove to finish
c) perform vgreduce on a blank PV?

the command (w/ --pvmove option) would return error if:
a) pvmove was required to perform on /dev/sda4 (there were some allocated extents) but it couldn't be initiated. (i.e.: not enough space to accommodate extents from /dev/sda4 PV on other PVs)
b) pvmove failed for any reason
c) subsequent vgreduce failed for any reason

I'm aware there are catches hidden inside. Basically and most notably it would in fact perform 2 different lvm2 commands and had to decide who to do on failure...

Comment 5 Ondrej Kozina 2015-06-09 13:24:07 UTC
Another suggestion by anaconda team would be to add new cmd line option telling pvmove not to fail when there's nothing to move

Comment 6 Ondrej Kozina 2015-09-11 11:32:56 UTC
We settled on pvmove --vgreduce approach.