[pve-devel] [PATCH Storage] Newly created LVM volumes are now created with a number of stripes equal to the number of physical volumes in a volume group.

[Martin Lablans]

Of course it would be preferable to leave this in the admin's hand via 
system-wide LVM configuration. This would also give Tom the flexibility 
for his setup. However, I don't know a way to achieve striping in LVM 
without using the -i option. raid_stripe_all_devices=1 does not achieve 
that result, cf. https://serverfault.com/questions/849088

If someone knows how to configure striping in LVM, then I'll be happy to 
write a few lines for the Proxmox documentation or wiki. If not, using 
-i seems to be the only way to achieve striping.

Basically, we are discussing to change the current behaviour from "use 1 
stripe" to "use <number of pvs> stripes". The latter option seems to 
make a lot more sense to me, although I can see that there are special 
setups requiring exactly one stripe.

I see three options:
1) We find a way to configure that using LVM; I'll contribute documentation
2) We add a "-stripes" option to pct create, with options
	0	==> old behaviour (no -i)
	n>0	==> n stripes
	auto	==> number of pvs
3) Like 2), but define it as an option of the PVE storage 
(/etc/pve/storage.cfg)

Option 3) would probably be beyond my humble Perl coding skills, but I 
would happily contribute to 1) or 2)

What do you think?

Martin

Am 27.07.2017 um 11:54 schrieb Wolfgang Bumiller:
> LVM seems to do a sane thing by default here, and can be configured to
> do what you're doing via lvm.conf AFAICT.
> From lvcreate(1):
> | In order to stripe across all PVs of the VG if the -i argument is
> | omitted, set raid_stripe_all_devices=1 in the allocation section of
> | lvm.conf (5)
>
> And the default behavior:
> | RAID 0 will stripe across 2 devices, RAID 4/5 across 3 PVs, RAID 6
> | across 5 PVs and RAID 10 across 4 PVs in the volume group if the -i
> | argument is omitted.
>
>
> I'd argue this should be left to the user to configure via lvm.conf
> then.
>
> In any case, two comments on the patch itself:
> You should disable line-wrapping in your mail client when sending
> patches as it cannot be applied via `git am` currently.
> And I think I'd prefer adding the "pv_count" option to `lvm_vgs()`
> instead of adding a separate function for it.
>
>
> On Wed, Jul 26, 2017 at 08:47:03PM +0200, Martin Lablans wrote:
>> Signed-off-by: Martin Lablans <code at martin.lablans.de>
>> ---
>>  PVE/Storage/LVMPlugin.pm | 19 ++++++++++++++++++-
>>  1 file changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/PVE/Storage/LVMPlugin.pm b/PVE/Storage/LVMPlugin.pm
>> index 9633e4c..cfb18e4 100644
>> --- a/PVE/Storage/LVMPlugin.pm
>> +++ b/PVE/Storage/LVMPlugin.pm
>> @@ -285,13 +285,30 @@ sub alloc_image {
>>      $name = lvm_find_free_diskname(lvm_list_volumes($vg), $vg, $storeid,
>> $vmid)
>>  	if !$name;
>>
>> -    my $cmd = ['/sbin/lvcreate', '-aly', '--addtag', "pve-vm-$vmid",
>> '--size', "${size}k", '--name', $name, $vg];
>> +    my $num_pvs = lvm_find_number_of_pv($vg);
>> +
>> +    my $cmd = ['/sbin/lvcreate', '-aly', '-i', $num_pvs, '--addtag',
>> "pve-vm-$vmid", '--size', "${size}k", '--name', $name, $vg];
>>
>>      run_command($cmd, errmsg => "lvcreate '$vg/pve-vm-$vmid' error");
>>
>>      return $name;
>>  }
>>
>> +sub lvm_find_number_of_pv {
>> +    my ($vg) = @_;
>> +
>> +    my $cmd = ['/sbin/vgs', $vg, '--noheadings', '--unbuffered',
>> '--nosuffix', '--options', 'pv_count'];
>> +
>> +    my $ret;
>> +
>> +    run_command($cmd, errmsg => "lvm_find_number_of_pv: Cannot determine
>> number of PVs for VG $vg", outfunc => sub {
>> +        my $line = shift;
>> +        $ret = trim($line);
>> +    });
>> +
>> +    return $ret;
>> +}
>> +
>>  sub free_image {
>>      my ($class, $storeid, $scfg, $volname, $isBase) = @_;
>