lvm: Enhancements for LVM2 RAID support

[mvollmer]

• lvm: Enhancements for LVM2 RAID support libblockdev#730.
• lvm: Expose logical volume substructure libblockdev#740

This is part of supporting LVM2 RAID in Cockpit: cockpit-project/cockpit#17226

[blivetghoul]

Can one of the admins verify this patch?

[tbzatek]

I know this is pretty much WIP, just a couple of notes that caught my eye.

Generally it looks good so far!

[tbzatek]

I'm wondering whether it's worth adding a note that this concerns LVM RAID scenarios only. Might be perhaps confusing to newcomers trying to find their way through the interface with so many puzzling properties.

[mvollmer]

Is this true, however? Maybe other operations like pvmove or lvconvert also use this. AFAIK, "Copy%" predates raid support in LVM2. But shrug. I'll document it as only applying toRAID, and make sure in the code that others have always 1.0 as the value.

[tbzatek]

I have no idea honestly :-) @vojtechtrefny?

[vojtechtrefny]

Yes, the copy_sync/sync_percent LV property is also used for pvmove.

[mvollmer]

I know this is pretty much WIP, just a couple of notes that caught my eye.

Thanks a lot for having a look! Yes, there is lots of refactoring to be done. There are now three methods that take lists of PVs, and they should all share code for turning them into a strv for libblockdev.

[mvollmer]

• What about CreateThinPoolVolume and CreateVDOVolume? They can very well be some RAID, but because they already use --type for themselves,

Oh, they don't. I have to experiment with this.

[mvollmer]

• What about CreateThinPoolVolume and CreateVDOVolume? They can very well be some RAID, but because they already use --type for themselves,

Oh, they don't. I have to experiment with this.

Yes, they do. lvcreate --thinpool foo bar creates a new thin pool, as expected, but lvcreate --thinpool foo bar --type raid1 just fails: "Command does not accept option: --thinpool thin."

Maybe we should ask lvm2 to make this work before we support raid thin pools.

[tbzatek]

Couple more minor things spotted, nothing major. Still looks good otherwise.

[vojtechtrefny]

• What about CreateThinPoolVolume and CreateVDOVolume? They can very well be some RAID, but because they already use --type for themselves,

Oh, they don't. I have to experiment with this.

Yes, they do. lvcreate --thinpool foo bar creates a new thin pool, as expected, but lvcreate --thinpool foo bar --type raid1 just fails: "Command does not accept option: --thinpool thin."

Maybe we should ask lvm2 to make this work before we support raid thin pools.

For RAID thin pools we need to manually create the two RAID LVs for metadata and data and then use lvconvert --type thinpool to create the thin pool from them. --thinpool and --type can't be used together because --thinpool is just an alias for --type thinpool.

[mvollmer]

• The API probably also needs to allow to specify the size of a new LV as "%PVS". That could be another option that requires a Features flag, so maybe that's already reason enough to use options instead of additional functions.

I no longer think this will happen. The experience when using "%PVS" for anything but "100%PVS" is not good: "50%PVS" does not give you something that is half the size of "100%PVS":

# lsblk /dev/sdd /dev/sde /dev/sdf
NAME MAJ:MIN RM  SIZE RO TYPE MOUNTPOINTS
sdd    8:48   0  500M  0 disk 
sde    8:64   0  500M  0 disk 
sdf    8:80   0  500M  0 disk 

# vgcreate vgroup0 /dev/sdd /dev/sde /dev/sdf
 Volume group "vgroup0" successfully created

# lvcreate vgroup0 -n lvol0 --type raid5 -l "100%PVS"
  Using default stripesize 64.00 KiB.
  Logical volume "lvol0" created.

# lvs vgroup0
  LV    VG      Attr       LSize   Pool Origin Data%  Meta%  Move Log Cpy%Sync Convert
  lvol0 vgroup0 rwi-a-r--- 984.00m                                    100.00          

# lvremove vgroup0/lvol0
  Logical volume "lvol0" successfully removed

# lvcreate vgroup0 -n lvol0 --type raid5 -l "50%PVS"
  Using default stripesize 64.00 KiB.
  Logical volume "lvol0" created.

# lvs vgroup0
  LV    VG      Attr       LSize   Pool Origin Data%  Meta%  Move Log Cpy%Sync Convert
  lvol0 vgroup0 rwi-a-r--- 744.00m                                    100.00          

A raid5 on "100%PVS" is 984 megs, but with "50%PVS" it is 744 megs instead of the expected 490ish megs.

This is because "%PVS" doesn't work well with layouts that require much more space on PVs than they provide on the resulting LV. "100%PVS" in the case above is asking for a LV of size 1500ish megs, but lvcreate flips on a special mode and makes one that is as large as it can be, which turns out to be 980 megs. "50%PVS" asks for half of 1500 = 750, which is possible, so we get that.

So exposing a "%PVS" slider with its current semantics doesn't make sense. And if we have to change LVM2 anyway, we should change it to report the maximum possible size in a dry-run or test mode.

[mvollmer]

• Should we have *CreateFooWithLayout functions?

I'd say "yes". I at least don't know how this all will work for pools, with all their meta data etc. Maybe we'll expose lvconvert in the API? So I wouldn't worry about that yet at all.

[tbzatek]

Jenkins, ok to test.

[tbzatek]

Thanks for the fixes. This looks good to me in its current form.

@mvollmer: do you plan any further changes to this? There are some design questions from you above, I don't have much more to say to that.

As we're close to the udisks-2.10.0 release, the API in this pull request will get public and fozen. If you think it might still need another iteration, we may postpone this to 2.11.0.

[mvollmer]

As we're close to the udisks-2.10.0 release, the API in this pull request will get public and fozen.

I am happy with that.

Thanks a lot!