Make blivet aware of internal LVs #149
Conversation
vpodzime
force-pushed
the
master-lvm_cache
branch
from
d5a9b7d
to
a4e439e
Compare
|
I think this is generally a good approach. The internal LVs are represented, but because they aren't in the devicetree they do not complicate things on its level. |
vpodzime
force-pushed
the
master-lvm_cache
branch
from
a4e439e
to
e682124
Compare
vpodzime
changed the title
|
Pushed a version that should be close to the final revision of this change. I did some manual testing on a VM with quite nice setup (4 LVs + ~20 internal LVs) and all seems to work as expected. |
vpodzime
force-pushed
the
master-lvm_cache
branch
from
e682124
to
a36fdce
Compare
|
Changed order of the last two patches and improved the second one. Should be ready for a review now. |
|
Have you run pylint on the tree with your changes applied? |
Yes, but I only run it from time to time because on my Fedora 21 I get quite many false positives and pylint runs for a long time. I'll definitely run it and take care of all real issues before I merge this. The review of the idea, approach and implementation strategy is what the first required step, I think. |
Just ignore the unsupported "sub-LVs" and consider cached LVs normal LVs.
Those that are related to each other should be next to each other and titled.
Internal LVs are not going to be referenced by the DeviceTree instance and are going to have no parents. Their "parent" LVs are going to be referenced as 'self.parent_lv' instead of 'self.parents[0]' because all the other devices use the opposite logic for parent-children relations -- children are consisting of or built on top of parents (whereas "parent" LVs are consisting of their internal LVs). This way internal LVs can easily override these methods.
vpodzime
force-pushed
the
master-lvm_cache
branch
from
a36fdce
to
e316dc6
Compare
|
New version with the suggestions and comments addressed. Internal LVs now have their Also rebased to current master. |
vpodzime
force-pushed
the
master-lvm_cache
branch
from
e316dc6
to
b9800db
Compare
| @@ -32,7 +32,7 @@ | |||
|
|
|||
| from .. import errors | |||
| from .. import util | |||
| from ..formats import getFormat | |||
| from ..formats import getFormat, DeviceFormat | |||
|
Looks good to me aside from the one unused import. |
Also make the LVMLogicalVolumeDevice class ready for managing internal LVs.
vpodzime
force-pushed
the
master-lvm_cache
branch
from
b9800db
to
7d1938f
Compare
Instead of calculating static values for number of copies, log size and metadata size of a given LV, we should use internal LVs assigned to their "parent" (normal) LVs and calculate such values dynamically from those internal LVs.
These are both valid types for the self.parents attribute.
Thin pools and thin LVs are also LVs so they should be included in the result. Also don't use the to-be-deprecated-soon getDevicesByType() method.
Thin/cache pools have internal data/metadata LVs the name of which can be queried. Let's use this functionality to help determining the parent LV if name matching fails.
Internal metadata LVs of thin pools can be resized so our representation should also allow it.
vpodzime
force-pushed
the
master-lvm_cache
branch
from
7d1938f
to
86748b3
Compare
Make blivet aware of internal LVs
This is really more a request for comments than a real pull request as I'd like to know your opinion on the chosen approach. Making blivet aware of internal LVs is one of the crucial steps towards creation and manipulation of thin/cache pools' metadata volumes and other things.
The internal LVs are not part of the DeviceTree, they are only referenced by their "parent" LVs as they should not be manipulated with directly and because there may be many of them (tens on quite simple setups). However, these LVs are still just LVs like any other as far as things like name, UUID, size, etc. go.