[pve-devel] [PATCH storage 2/8] add API for LVM management

[Thomas Lamprecht]

Am 07/30/2018 um 10:26 AM schrieb Dominik Csapak:
> currently only list and create,
> the list is in a format so that we can use it in an extjs tree
> 

not really looked at the code itself, just a higher level note:
Are we sure we want to pack more storage management in here?
AFAICT, pve-storage was only the interface to already setup storages,
local or remote. But it did not handle the creation, management of them
(at their level), more or less it only handled mounting them and
handling PVE interactions with them.

AFAIK, that was one of the reasons the pveceph stuff got put into
manager, not here. You always need to do node-level stuff, and thus a
storage interface library may not be the best place putting this,
although the code naturally can and should use the methods and helper
provided by pve-storage.
I mean you already made a precedence by putting the disk management
here... If we add further such stuff in here, which may be the case,
having a separate package could be nice - especially if the spoken-off
Proxmox Storage Management comes to life. Now, this may not be worth it
yet, but it's worth to think if this should be put into manager, like
pveceph is, else this feels a bit spread out and IMO, our storage
interface library just feels like it may not be the best place.
This is no hard feeling, so if we define this is just fine then OK,
else now would be the best time to move this.

besides that, nice effort here!

cheers,
Thomas