Bug 623808 - need a way to create PV objects in liblvm without touching disks
need a way to create PV objects in liblvm without touching disks
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: lvm2 (Show other bugs)
6.1
All Linux
medium Severity medium
: rc
: ---
Assigned To: Petr Rockai
Corey Marthaler
: Reopened
Depends On:
Blocks: 623811
  Show dependency treegraph
 
Reported: 2010-08-12 16:27 EDT by Mike Snitzer
Modified: 2011-12-06 11:52 EST (History)
9 users (show)

See Also:
Fixed In Version: lvm2-2.02.86-1.el6
Doc Type: Bug Fix
Doc Text:
Before, it was not possible to create a PV object with all properties calculated (e.g. the PE start value) without a need to write the PV label on disk while using LVM2 library (lvm2app). This has been changed so that the PV label is written out later in the process as part of the lvm_vg_write call making it possible to calculate all PV properties and query them without actually writing the PV label on disk.
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-12-06 11:52:38 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)


External Trackers
Tracker ID Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2011:1522 normal SHIPPED_LIVE lvm2 bug fix and enhancement update 2011-12-05 19:50:10 EST

  None (edit)
Description Mike Snitzer 2010-08-12 16:27:01 EDT
Description of problem:
Anaconda needs to know the pe_start for each PV it will create (before it creates it).  This will enable Anaconda to get away from hardcoding (and assuming) a particular pe_start.  Bug 623458 illustrates that anaconda is sensitive to any variation in the pe_start.

Additional info:
Anaconda requests storage configuration up front and then later runs various commands to actually provision the storage as configured.  This opens the possibility of inconsistencies if anaconda's assumptions about the tools it consumes are incorrect.

For LVM, anaconda predicts the amount of free space that each PV will contribute to its parent VG by:
1) knowing the size of the device
2) using a specific extent size
3) assuming a particular pe_start
Comment 1 Alasdair Kergon 2010-08-12 18:23:30 EDT
Note that at the command line level it is the vgcreate and vgextend commands that set pe_start (not pvcreate).

And a reminder of the rule that all library functionality must also be available through the LVM command line.

So we're perhaps talking about enhancing the way --test can be used to provide this information and incorporating --test into the library.
Comment 2 Mike Snitzer 2010-08-12 18:37:40 EDT
(In reply to comment #1)
> Note that at the command line level it is the vgcreate and vgextend commands
> that set pe_start (not pvcreate).

pe_start is established via pvcreate.  vgcreate can do the implicit PV creation but pvcreate does lay down the pe_start without having to add the PV to a VG.
Comment 3 Alasdair Kergon 2010-08-12 18:50:22 EDT
You're correct, thanks.  So it's pvcreate.  That simplifies the problem a little.  I'm having dreams about building 'pvs' into 'pvcreate' etc....
I.e. allowing tools to output selected fields relating to the objects they have manipulated.  (Particularly useful with --test, but may be workable more generally.)
Comment 4 Dave Wysochanski 2010-10-21 11:48:27 EDT
liblvm (lvm2app) will provide access to pe_start via the new generic property functions submitted upstream.  Some discussion still happening but likely to be checked in soon.
Comment 5 Petr Rockai 2011-01-03 10:02:00 EST
The existing liblvm code can provide the information, but it can't create the required object (PV, in this case) without writing labels. We need to defer PV creation to the VG write/commit time, which will make it possible to create PVs the same way as LVs are created: create the structures (which makes it possible to query them) and decide later whether to write the changes to disk or not.

This requires somewhat extensive changes to current code: split pv_write in format_handler into two (adjust PV fields and actually write the label), move the actual label writes into vg_write and keep around a list of writes to do.
Comment 6 Petr Rockai 2011-01-19 11:29:47 EST
This is turning out to be a bit too tricky, partly due to MDA wiping and partly due to lvmcache interference. One catch is, *none* of our tests fails if I simply comment out MDA wiping, so there's clearly no testing whatsoever done to verify it works. Meaning we first need to design tests to cover this area, if we want to know that we aren't breaking things.

Moreover, Peter Rajnoha is working on the same area of code, but his changes won't make it 6.1. So I am un-acking this, and bumping to 6.2, where it is more realistic to achieve correctly and without compromising stability. Luckily, the changes done by Peter should make this a lot more feasible.
Comment 7 RHEL Product and Program Management 2011-01-19 11:45:23 EST
Development Management has reviewed and declined this request.  You may appeal
this decision by reopening this request.
Comment 9 Petr Rockai 2011-04-27 10:08:27 EDT
This is now halfway done and blocked on the review of this patch: http://www.redhat.com/archives/lvm-devel/2011-April/msg00056.html
Comment 10 Petr Rockai 2011-06-02 16:33:56 EDT
The "hard" patches have been now merged (hard meaning they were potentially disruptive for normal LVM used, unrelated to liblvm). The liblvm part of the change should be relatively easy now.
Comment 11 Corey Marthaler 2011-06-02 17:22:30 EDT
QE reviewed this BZ for QA_ACK but was unable to ack due to a lack of
requirements or description of how the new feature is supposed to work or be
tested.

Please see
https://wiki.test.redhat.com/ClusterStorage/WhyNoAck
Comment 12 Petr Rockai 2011-06-04 18:00:24 EDT
Turns out that no further changes are needed to facilitate this. I have added a test to our upstream test suite for the functionality. For completeness, I am reproducing here the C program that demonstrates how to use the requested feature:

#include "lvm2app.h"
#include "assert.h"

int main(int argc, char *argv[])
{
	lvm_t handle;
	vg_t vg = NULL;
	pv_t pv;
	struct lvm_property_value v;

	handle = lvm_init(NULL);
        assert(handle);

	vg = lvm_vg_create(handle, argv[1]);
        assert(vg);

	if (lvm_vg_extend(vg, argv[2]))
		abort();

	pv = lvm_pv_from_name(vg, argv[2]);
	assert(pv);

        v = lvm_pv_get_property(pv, "pe_start");
        assert(v.is_valid);
	fprintf(stderr, "pe_start = %d\n", (int)v.value.integer);
        assert(v.value.integer == 2048 * 512);

        lvm_vg_close(vg);
	lvm_quit(handle);
        return 0;
}

(of course, the assertions are appropriate for testing, not for production code)

The above code, according to the requirements set forth in this bugzilla, should give the correct "pe_start" value for a given volume group, even if the volume group does not physically exist, but has only been created in memory.
Comment 13 Petr Rockai 2011-06-04 18:02:34 EDT
(gosh, sorry for the misformatted code... let me try again)

#include "lvm2app.h"
#include "assert.h"

int main(int argc, char *argv[])
{
	lvm_t handle;
	vg_t vg = NULL;
	pv_t pv;
	struct lvm_property_value v;

	handle = lvm_init(NULL);
	assert(handle);

	vg = lvm_vg_create(handle, argv[1]);
	assert(vg);

	if (lvm_vg_extend(vg, argv[2]))
		abort();

	pv = lvm_pv_from_name(vg, argv[2]);
	assert(pv);

	v = lvm_pv_get_property(pv, "pe_start");
	assert(v.is_valid);
	fprintf(stderr, "pe_start = %d\n", (int)v.value.integer);
	assert(v.value.integer == 2048 * 512);

	lvm_vg_close(vg);
	lvm_quit(handle);
	return 0;
}
Comment 14 Corey Marthaler 2011-06-16 18:00:36 EDT
Adding QA ack for 6.2. 

Based on comment #12, a test for this API feature already exists in the lvm devel test suite. 

This bug will be marked verified (SanityOnly) once lvm devel shows unit test results and final 6.2 regression testing has been completed.
Comment 16 Corey Marthaler 2011-09-07 11:44:15 EDT
Petr - Can you please add the output from the test run of the test in comment #13, so that I can mark this verified.
Comment 17 Petr Rockai 2011-09-12 08:18:43 EDT
Running api/pe_start.sh ...                      @TESTDIR=/srv/build/lvm2/cvs-clean/default/test/LVMTEST21976.NusqXjqwP8
@PREFIX=LVMTEST21976

aux prepare_devs 2
+ aux prepare_devs 2
## preparing loop device....ok (/dev/loop1)
## preparing 2 devices...ok
aux apitest pe_start test_vg $dev1
+ aux apitest pe_start test_vg /srv/build/lvm2/cvs-clean/default/test/LVMTEST21976.NusqXjqwP8/dev/mapper/LVMTEST21976pv1
  No physical volume label read from /srv/build/lvm2/cvs-clean/default/test/LVMTEST21976.NusqXjqwP8/dev/mapper/LVMTEST21976pv1
pe_start = 1048576
not vgs test_vg
+ not vgs test_vg
  Volume group "test_vg" not found
not pvs $dev1                               
+ not pvs /srv/build/lvm2/cvs-clean/default/test/LVMTEST21976.NusqXjqwP8/dev/mapper/LVMTEST21976pv1
  No physical volume label read from /srv/build/lvm2/cvs-clean/default/test/LVMTEST21976.NusqXjqwP8/dev/mapper/LVMTEST21976pv1                        
  Failed to read physical volume "/srv/build/lvm2/cvs-clean/default/test/LVMTEST21976.NusqXjqwP8/dev/mapper/LVMTEST21976pv1"                          
aux teardown                                
+ aux teardown                              
## teardown......ok                         
passed.    0:00                             

The test itself, pe_test.c, reads:

#undef NDEBUG

#include "lvm2app.h"
#include "assert.h"

int main(int argc, char *argv[])
{
        lvm_t handle;
        vg_t vg = NULL;
        pv_t pv;
        struct lvm_property_value v;

        handle = lvm_init(NULL);
        assert(handle);

        vg = lvm_vg_create(handle, argv[1]);
        assert(vg);

        if (lvm_vg_extend(vg, argv[2]))
                abort();

        pv = lvm_pv_from_name(vg, argv[2]);
        assert(pv);

        v = lvm_pv_get_property(pv, "pe_start");
        assert(v.is_valid);
        fprintf(stderr, "pe_start = %d\n", (int)v.value.integer);
        assert(v.value.integer == 2048 * 512);

        lvm_vg_close(vg);
        lvm_quit(handle);
        return 0;
}
Comment 18 Corey Marthaler 2011-09-12 12:06:10 EDT
Marking verified based on comment #17.
Comment 19 Peter Rajnoha 2011-10-26 10:54:27 EDT
    Technical note added. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    New Contents:
Before, it was not possible to create a PV object with all properties calculated (e.g. the PE start value) without a need to write the PV label on disk while using LVM2 library (lvm2app). This has been changed so that the PV label is written out later in the process as part of the lvm_vg_write call making it possible to calculate all PV properties and query them without actually writing the PV label on disk.
Comment 20 errata-xmlrpc 2011-12-06 11:52:38 EST
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2011-1522.html

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